Fix was not finishing span when it was dropped

romtsn · romtsn · commit 971b171f9ce9 · 2023-11-29T10:37:23.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -94,6 +94,7 @@ val transaction = Sentry.startTransaction("name", "op", TransactionOptions().app
     - This prevents issues when using the SDK on older AGP versions (< 4.x.x)
 - Reduce main thread work on init ([#3036](https://github.com/getsentry/sentry-java/pull/3036))
 - Move Integrations registration to background on init ([#3043](https://github.com/getsentry/sentry-java/pull/3043))
+- Fix `SentryOkHttpInterceptor.BeforeSpanCallback` was not finishing span when it was dropped ([#2958](https://github.com/getsentry/sentry-java/pull/2958))
 
 ## 6.34.0
 
diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt
@@ -41,7 +41,8 @@ class SentryOkHttpInterceptor(
 ) : Interceptor by io.sentry.okhttp.SentryOkHttpInterceptor(
     hub,
     { span, request, response ->
-        beforeSpan?.execute(span, request, response)
+        beforeSpan ?: return@SentryOkHttpInterceptor span
+        beforeSpan.execute(span, request, response)
     },
     captureFailedRequests,
     failedRequestStatusCodes,
diff --git a/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt b/sentry-okhttp/src/main/java/io/sentry/okhttp/SentryOkHttpInterceptor.kt
@@ -169,11 +169,10 @@ public open class SentryOkHttpInterceptor(
             if (result == null) {
                 // span is dropped
                 span.spanContext.sampled = false
-            } else {
-                // The SentryOkHttpEventListener will finish the span itself if used for this call
-                if (!isFromEventListener) {
-                    span.finish()
-                }
+            }
+            // The SentryOkHttpEventListener will finish the span itself if used for this call
+            if (!isFromEventListener) {
+                span.finish()
             }
         } else {
             // The SentryOkHttpEventListener will finish the span itself if used for this call
diff --git a/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-okhttp/src/test/java/io/sentry/okhttp/SentryOkHttpInterceptorTest.kt
@@ -371,6 +371,7 @@ class SentryOkHttpInterceptorTest {
         )
         sut.newCall(getRequest()).execute()
         val httpClientSpan = fixture.sentryTracer.children.first()
+        assertTrue(httpClientSpan.isFinished)
         assertNotNull(httpClientSpan.spanContext.sampled) {
             assertFalse(it)
         }

Original file line number	Diff line number	Diff line change
`@@ -371,6 +371,7 @@ class SentryOkHttpInterceptorTest {`
`371`	`371`	`)`
`372`	`372`	`sut.newCall(getRequest()).execute()`
`373`	`373`	`val httpClientSpan = fixture.sentryTracer.children.first()`
	`374`	`+ assertTrue(httpClientSpan.isFinished)`
`374`	`375`	`assertNotNull(httpClientSpan.spanContext.sampled) {`
`375`	`376`	`assertFalse(it)`
`376`	`377`	`}`