Skip to content

Commit 8cf8f57

Browse files
adinaueradinauer
authored
Merge branch 'main' into feat/improved-graphql-server-support
2 parents ab741eb + f86fb22 commit 8cf8f57

File tree

23 files changed

+244
-49
lines changed

23 files changed

+244
-49
lines changed

‎.github/workflows/agp-matrix.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ jobs:
3535
uses: actions/checkout@v3
3636

3737
- name: Setup Gradle
38-
uses: gradle/gradle-build-action@915a66c096a03101667f9df2e56c9efef558b165 # pin@v2
38+
uses: gradle/gradle-build-action@a4cf152f482c7ca97ef56ead29bf08bcd953284c # pin@v2
3939

4040
- name: Setup Java Version
4141
uses: actions/setup-java@v3

‎.github/workflows/enforce-license-compliance.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ jobs:
1111
runs-on: ubuntu-latest
1212
steps:
1313
- name: Setup Gradle
14-
uses: gradle/gradle-build-action@915a66c096a03101667f9df2e56c9efef558b165 # pin@v2
14+
uses: gradle/gradle-build-action@a4cf152f482c7ca97ef56ead29bf08bcd953284c # pin@v2
1515

1616
- name: Set up Java
1717
uses: actions/setup-java@v3

‎CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,19 @@
44

55
### Features
66

7+
- Add HTTP response code to Spring WebFlux transactions ([#2870](https://github.com/getsentry/sentry-java/pull/2870))
8+
- Add `sampled` to Dynamic Sampling Context ([#2869](https://github.com/getsentry/sentry-java/pull/2869))
79
- Improve server side GraphQL support for spring-graphql and Nextflix DGS ([#2856](https://github.com/getsentry/sentry-java/pull/2856))
810
- If you have already been using `SentryDataFetcherExceptionHandler` that still works but has been deprecated. Please use `SentryGenericDataFetcherExceptionHandler` combined with `SentryInstrumentation` instead for better error reporting.
911
- More exceptions and errors caught and reported to Sentry by also looking at the `ExecutionResult` (more specifically its `errors`)
1012
- More details for Sentry events: query, variables and response (where possible)
1113
- Breadcrumbs for operation (query, mutation, subscription), data fetchers and data loaders (Spring only)
1214
- Better hub propagation by using `GraphQLContext`
1315

16+
### Fixes
17+
18+
- Propagate OkHttp status to parent spans ([#2872](https://github.com/getsentry/sentry-java/pull/2872))
19+
1420
## 6.27.0
1521

1622
### Features

‎sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req
6363
breadcrumb.setData("status_code", response.code)
6464
callRootSpan?.setData(PROTOCOL_KEY, response.protocol.name)
6565
callRootSpan?.setData(SpanDataConvention.HTTP_STATUS_CODE_KEY, response.code)
66-
callRootSpan?.status = SpanStatus.fromHttpStatusCode(response.code)
6766
}
6867

6968
fun setProtocol(protocolName: String?) {
@@ -98,24 +97,20 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req
9897
/** Starts a span, if the callRootSpan is not null. */
9998
fun startSpan(event: String) {
10099
// Find the parent of the span being created. E.g. secureConnect is child of connect
101-
val parentSpan = when (event) {
102-
// PROXY_SELECT, DNS, CONNECT and CONNECTION are not children of one another
103-
SECURE_CONNECT_EVENT -> eventSpans[CONNECT_EVENT]
104-
REQUEST_HEADERS_EVENT -> eventSpans[CONNECTION_EVENT]
105-
REQUEST_BODY_EVENT -> eventSpans[CONNECTION_EVENT]
106-
RESPONSE_HEADERS_EVENT -> eventSpans[CONNECTION_EVENT]
107-
RESPONSE_BODY_EVENT -> eventSpans[CONNECTION_EVENT]
108-
else -> callRootSpan
109-
} ?: callRootSpan
100+
val parentSpan = findParentSpan(event)
110101
val span = parentSpan?.startChild("http.client.$event") ?: return
111102
span.spanContext.origin = TRACE_ORIGIN
112103
eventSpans[event] = span
113104
}
114105

115-
/** Finishes a previously started span, and runs [beforeFinish] on it and on the call root span. */
106+
/** Finishes a previously started span, and runs [beforeFinish] on it, on its parent and on the call root span. */
116107
fun finishSpan(event: String, beforeFinish: ((span: ISpan) -> Unit)? = null) {
117108
val span = eventSpans[event] ?: return
109+
val parentSpan = findParentSpan(event)
118110
beforeFinish?.invoke(span)
111+
if (parentSpan != null && parentSpan != callRootSpan) {
112+
beforeFinish?.invoke(parentSpan)
113+
}
119114
callRootSpan?.let { beforeFinish?.invoke(it) }
120115
span.finish()
121116
}
@@ -125,7 +120,10 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req
125120
callRootSpan ?: return
126121

127122
// We forcefully finish all spans, even if they should already have been finished through finishSpan()
128-
eventSpans.values.filter { !it.isFinished }.forEach { it.finish(SpanStatus.DEADLINE_EXCEEDED) }
123+
eventSpans.values.filter { !it.isFinished }.forEach {
124+
// If a status was set on the span, we use that, otherwise we set its status as error.
125+
it.finish(it.status ?: SpanStatus.INTERNAL_ERROR)
126+
}
129127
beforeFinish?.invoke(callRootSpan)
130128
callRootSpan.finish()
131129

@@ -137,4 +135,14 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req
137135
hub.addBreadcrumb(breadcrumb, hint)
138136
return
139137
}
138+
139+
private fun findParentSpan(event: String): ISpan? = when (event) {
140+
// PROXY_SELECT, DNS, CONNECT and CONNECTION are not children of one another
141+
SECURE_CONNECT_EVENT -> eventSpans[CONNECT_EVENT]
142+
REQUEST_HEADERS_EVENT -> eventSpans[CONNECTION_EVENT]
143+
REQUEST_BODY_EVENT -> eventSpans[CONNECTION_EVENT]
144+
RESPONSE_HEADERS_EVENT -> eventSpans[CONNECTION_EVENT]
145+
RESPONSE_BODY_EVENT -> eventSpans[CONNECTION_EVENT]
146+
else -> callRootSpan
147+
} ?: callRootSpan
140148
}

‎sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,10 @@ class SentryOkHttpEventListener(
313313
okHttpEvent.setResponse(response)
314314
okHttpEvent.finishSpan(RESPONSE_HEADERS_EVENT) {
315315
it.setData(SpanDataConvention.HTTP_STATUS_CODE_KEY, response.code)
316-
it.status = SpanStatus.fromHttpStatusCode(response.code)
316+
// Let's not override the status of a span that was set
317+
if (it.status == null) {
318+
it.status = SpanStatus.fromHttpStatusCode(response.code)
319+
}
317320
}
318321
}
319322

‎sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import okhttp3.mockwebserver.MockResponse
1919
import okhttp3.mockwebserver.MockWebServer
2020
import okhttp3.mockwebserver.SocketPolicy
2121
import org.mockito.kotlin.any
22+
import org.mockito.kotlin.anyOrNull
2223
import org.mockito.kotlin.eq
2324
import org.mockito.kotlin.mock
2425
import org.mockito.kotlin.spy
@@ -249,7 +250,7 @@ class SentryOkHttpEventListenerTest {
249250

250251
@Test
251252
fun `propagate all calls to the event listener passed in the ctor`() {
252-
val sut = fixture.getSut(eventListener = fixture.mockEventListener, httpStatusCode = 500)
253+
val sut = fixture.getSut(eventListener = fixture.mockEventListener)
253254
val listener = fixture.sentryOkHttpEventListener
254255
val request = postRequest(body = "requestBody")
255256
val call = sut.newCall(request)
@@ -260,7 +261,7 @@ class SentryOkHttpEventListenerTest {
260261

261262
@Test
262263
fun `propagate all calls to the event listener factory passed in the ctor`() {
263-
val sut = fixture.getSut(eventListenerFactory = fixture.mockEventListenerFactory, httpStatusCode = 500)
264+
val sut = fixture.getSut(eventListenerFactory = fixture.mockEventListenerFactory)
264265
val listener = fixture.sentryOkHttpEventListener
265266
val request = postRequest(body = "requestBody")
266267
val call = sut.newCall(request)
@@ -272,7 +273,7 @@ class SentryOkHttpEventListenerTest {
272273
@Test
273274
fun `propagate all calls to the SentryOkHttpEventListener passed in the ctor`() {
274275
val originalListener = spy(SentryOkHttpEventListener(fixture.hub, fixture.mockEventListener))
275-
val sut = fixture.getSut(eventListener = originalListener, httpStatusCode = 500)
276+
val sut = fixture.getSut(eventListener = originalListener)
276277
val listener = fixture.sentryOkHttpEventListener
277278
val request = postRequest(body = "requestBody")
278279
val call = sut.newCall(request)
@@ -284,7 +285,7 @@ class SentryOkHttpEventListenerTest {
284285
@Test
285286
fun `propagate all calls to the SentryOkHttpEventListener factory passed in the ctor`() {
286287
val originalListener = spy(SentryOkHttpEventListener(fixture.hub, fixture.mockEventListener))
287-
val sut = fixture.getSut(eventListenerFactory = { originalListener }, httpStatusCode = 500)
288+
val sut = fixture.getSut(eventListenerFactory = { originalListener })
288289
val listener = fixture.sentryOkHttpEventListener
289290
val request = postRequest(body = "requestBody")
290291
val call = sut.newCall(request)
@@ -305,6 +306,49 @@ class SentryOkHttpEventListenerTest {
305306
assertEquals(9, fixture.sentryTracer.children.size)
306307
}
307308

309+
@Test
310+
fun `status propagates to parent span and call root span`() {
311+
val sut = fixture.getSut(httpStatusCode = 500)
312+
val request = getRequest()
313+
val call = sut.newCall(request)
314+
val response = call.execute()
315+
val okHttpEvent = SentryOkHttpEventListener.eventMap[call]
316+
val callSpan = okHttpEvent?.callRootSpan
317+
val responseHeaderSpan =
318+
fixture.sentryTracer.children.firstOrNull { it.operation == "http.client.response_headers" }
319+
val connectionSpan = fixture.sentryTracer.children.firstOrNull { it.operation == "http.client.connection" }
320+
response.close()
321+
assertNotNull(callSpan)
322+
assertNotNull(responseHeaderSpan)
323+
assertNotNull(connectionSpan)
324+
assertEquals(SpanStatus.fromHttpStatusCode(500), callSpan.status)
325+
assertEquals(SpanStatus.fromHttpStatusCode(500), responseHeaderSpan.status)
326+
assertEquals(SpanStatus.fromHttpStatusCode(500), connectionSpan.status)
327+
}
328+
329+
@Test
330+
fun `call root span status is not overridden if not null`() {
331+
val mockListener = mock<EventListener>()
332+
lateinit var call: Call
333+
whenever(mockListener.connectStart(any(), anyOrNull(), anyOrNull())).then {
334+
val okHttpEvent = SentryOkHttpEventListener.eventMap[call]
335+
val callSpan = okHttpEvent?.callRootSpan
336+
assertNotNull(callSpan)
337+
assertNull(callSpan.status)
338+
callSpan.status = SpanStatus.UNKNOWN
339+
it
340+
}
341+
val sut = fixture.getSut(eventListener = mockListener)
342+
val request = getRequest()
343+
call = sut.newCall(request)
344+
val response = call.execute()
345+
val okHttpEvent = SentryOkHttpEventListener.eventMap[call]
346+
val callSpan = okHttpEvent?.callRootSpan
347+
assertNotNull(callSpan)
348+
response.close()
349+
assertEquals(SpanStatus.UNKNOWN, callSpan.status)
350+
}
351+
308352
private fun verifyDelegation(
309353
listener: SentryOkHttpEventListener,
310354
originalListener: EventListener,

‎sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import io.sentry.SentryTracer
88
import io.sentry.Span
99
import io.sentry.SpanDataConvention
1010
import io.sentry.SpanOptions
11+
import io.sentry.SpanStatus
1112
import io.sentry.TracesSamplingDecision
1213
import io.sentry.TransactionContext
1314
import io.sentry.TypeCheckHint
@@ -242,6 +243,28 @@ class SentryOkHttpEventTest {
242243
)
243244
}
244245

246+
@Test
247+
fun `when finishEvent, all running spans are finished with internal_error status`() {
248+
val sut = fixture.getSut()
249+
sut.startSpan("span")
250+
val spans = sut.getEventSpans()
251+
assertNull(spans["span"]!!.status)
252+
sut.finishEvent()
253+
assertEquals(SpanStatus.INTERNAL_ERROR, spans["span"]!!.status)
254+
}
255+
256+
@Test
257+
fun `when finishEvent, does not override running spans status if set`() {
258+
val sut = fixture.getSut()
259+
sut.startSpan("span")
260+
val spans = sut.getEventSpans()
261+
assertNull(spans["span"]!!.status)
262+
spans["span"]!!.status = SpanStatus.OK
263+
assertEquals(SpanStatus.OK, spans["span"]!!.status)
264+
sut.finishEvent()
265+
assertEquals(SpanStatus.OK, spans["span"]!!.status)
266+
}
267+
245268
@Test
246269
fun `setResponse set protocol and code in the breadcrumb and root span, and response in the hint`() {
247270
val sut = fixture.getSut()
@@ -443,6 +466,32 @@ class SentryOkHttpEventTest {
443466
assertEquals(rootSpan.spanId, responseBodySpan?.parentSpanId)
444467
}
445468

469+
@Test
470+
fun `finishSpan beforeFinish is called on span, parent and call root span`() {
471+
val sut = fixture.getSut()
472+
sut.startSpan(CONNECTION_EVENT)
473+
sut.startSpan(REQUEST_HEADERS_EVENT)
474+
sut.startSpan("random event")
475+
sut.finishSpan(REQUEST_HEADERS_EVENT) { it.status = SpanStatus.INTERNAL_ERROR }
476+
sut.finishSpan("random event") { it.status = SpanStatus.DEADLINE_EXCEEDED }
477+
sut.finishSpan(CONNECTION_EVENT)
478+
val spans = sut.getEventSpans()
479+
val connectionSpan = spans[CONNECTION_EVENT] as Span?
480+
val requestHeadersSpan = spans[REQUEST_HEADERS_EVENT] as Span?
481+
val randomEventSpan = spans["random event"] as Span?
482+
assertNotNull(connectionSpan)
483+
assertNotNull(requestHeadersSpan)
484+
assertNotNull(randomEventSpan)
485+
// requestHeadersSpan was finished with INTERNAL_ERROR
486+
assertEquals(SpanStatus.INTERNAL_ERROR, requestHeadersSpan.status)
487+
// randomEventSpan was finished with DEADLINE_EXCEEDED
488+
assertEquals(SpanStatus.DEADLINE_EXCEEDED, randomEventSpan.status)
489+
// requestHeadersSpan was finished with INTERNAL_ERROR, and it propagates to its parent
490+
assertEquals(SpanStatus.INTERNAL_ERROR, connectionSpan.status)
491+
// random event was finished last with DEADLINE_EXCEEDED, and it propagates to root call
492+
assertEquals(SpanStatus.DEADLINE_EXCEEDED, sut.callRootSpan!!.status)
493+
}
494+
446495
/** Retrieve all the spans started in the event using reflection. */
447496
private fun SentryOkHttpEvent.getEventSpans() = getProperty<MutableMap<String, ISpan>>("eventSpans")
448497
}

‎sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
@Open
1515
public class SentryProperties extends SentryOptions {
1616

17-
/** Weather to use Git commit id as a release. */
17+
/** Whether to use Git commit id as a release. */
1818
private boolean useGitCommitIdAsRelease = true;
1919

2020
/** Report all or only uncaught web exceptions. */

‎sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryProperties.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
@Open
1515
public class SentryProperties extends SentryOptions {
1616

17-
/** Weather to use Git commit id as a release. */
17+
/** Whether to use Git commit id as a release. */
1818
private boolean useGitCommitIdAsRelease = true;
1919

2020
/** Report all or only uncaught web exceptions. */

‎sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,17 @@ private void finishTransaction(ServerWebExchange exchange, ITransaction transact
110110
transaction.setName(transactionName, TransactionNameSource.ROUTE);
111111
transaction.setOperation(TRANSACTION_OP);
112112
}
113-
if (transaction.getStatus() == null) {
114-
final @Nullable ServerHttpResponse response = exchange.getResponse();
115-
if (response != null) {
116-
final @Nullable HttpStatusCode statusCode = response.getStatusCode();
117-
if (statusCode != null) {
113+
final @Nullable ServerHttpResponse response = exchange.getResponse();
114+
if (response != null) {
115+
final @Nullable HttpStatusCode statusCode = response.getStatusCode();
116+
if (statusCode != null) {
117+
transaction
118+
.getContexts()
119+
.withResponse(
120+
(sentryResponse) -> {
121+
sentryResponse.setStatusCode(statusCode.value());
122+
});
123+
if (transaction.getStatus() == null) {
118124
transaction.setStatus(SpanStatus.fromHttpStatusCode(statusCode.value()));
119125
}
120126
}

‎sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ class SentryWebFluxTracingFilterTest {
115115
assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.OK)
116116
assertThat(it.contexts.trace!!.operation).isEqualTo("http.server")
117117
assertThat(it.contexts.trace!!.origin).isEqualTo("auto.spring_jakarta.webflux")
118+
assertThat(it.contexts.response!!.statusCode).isEqualTo(200)
118119
},
119120
anyOrNull<TraceContext>(),
120121
anyOrNull(),
@@ -133,6 +134,7 @@ class SentryWebFluxTracingFilterTest {
133134
verify(fixture.hub).captureTransaction(
134135
check {
135136
assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.INTERNAL_ERROR)
137+
assertThat(it.contexts.response!!.statusCode).isEqualTo(500)
136138
},
137139
anyOrNull<TraceContext>(),
138140
anyOrNull(),

‎sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -144,11 +144,17 @@ private void finishTransaction(ServerWebExchange exchange, ITransaction transact
144144
transaction.setName(transactionName, TransactionNameSource.ROUTE);
145145
transaction.setOperation(TRANSACTION_OP);
146146
}
147-
if (transaction.getStatus() == null) {
148-
final @Nullable ServerHttpResponse response = exchange.getResponse();
149-
if (response != null) {
150-
final @Nullable Integer rawStatusCode = response.getRawStatusCode();
151-
if (rawStatusCode != null) {
147+
final @Nullable ServerHttpResponse response = exchange.getResponse();
148+
if (response != null) {
149+
final @Nullable Integer rawStatusCode = response.getRawStatusCode();
150+
if (rawStatusCode != null) {
151+
transaction
152+
.getContexts()
153+
.withResponse(
154+
(sentryResponse) -> {
155+
sentryResponse.setStatusCode(rawStatusCode);
156+
});
157+
if (transaction.getStatus() == null) {
152158
transaction.setStatus(SpanStatus.fromHttpStatusCode(rawStatusCode));
153159
}
154160
}

‎sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ class SentryWebFluxTracingFilterTest {
116116
assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.OK)
117117
assertThat(it.contexts.trace!!.operation).isEqualTo("http.server")
118118
assertThat(it.contexts.trace!!.origin).isEqualTo("auto.spring.webflux")
119+
assertThat(it.contexts.response!!.statusCode).isEqualTo(200)
119120
},
120121
anyOrNull<TraceContext>(),
121122
anyOrNull(),
@@ -134,6 +135,7 @@ class SentryWebFluxTracingFilterTest {
134135
verify(fixture.hub).captureTransaction(
135136
check {
136137
assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.INTERNAL_ERROR)
138+
assertThat(it.contexts.response!!.statusCode).isEqualTo(500)
137139
},
138140
anyOrNull<TraceContext>(),
139141
anyOrNull(),

0 commit comments

Comments
 (0)