Feat: OkHttp callback for Customising the Span (#1478)

Fixes #1365

CHANGELOG.md

@@ -1,5 +1,6 @@
 # Unreleased
 
+* Feat: OkHttp callback for Customising the Span (#1478)
 * Feat: Add breadcrumb in Spring RestTemplate integration (#1481)
 * Fix: Cloning Stack (#1483)
 

sentry-android-okhttp/api/sentry-android-okhttp.api

@@ -8,8 +8,12 @@ public final class io/sentry/android/okhttp/BuildConfig {
 
 public final class io/sentry/android/okhttp/SentryOkHttpInterceptor : okhttp3/Interceptor {
 	public fun <init> ()V
-	public fun <init> (Lio/sentry/IHub;)V
-	public synthetic fun <init> (Lio/sentry/IHub;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public fun <init> (Lio/sentry/IHub;Lio/sentry/android/okhttp/SentryOkHttpInterceptor$BeforeSpanCallback;)V
+	public synthetic fun <init> (Lio/sentry/IHub;Lio/sentry/android/okhttp/SentryOkHttpInterceptor$BeforeSpanCallback;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public fun intercept (Lokhttp3/Interceptor$Chain;)Lokhttp3/Response;
 }
 
+public abstract interface class io/sentry/android/okhttp/SentryOkHttpInterceptor$BeforeSpanCallback {
+	public abstract fun execute (Lio/sentry/ISpan;Lokhttp3/Request;Lokhttp3/Response;)Lio/sentry/ISpan;
+}
+

sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt

@@ -3,13 +3,16 @@ package io.sentry.android.okhttp
 import io.sentry.Breadcrumb
 import io.sentry.HubAdapter
 import io.sentry.IHub
+import io.sentry.ISpan
 import io.sentry.SpanStatus
 import java.io.IOException
 import okhttp3.Interceptor
+import okhttp3.Request
 import okhttp3.Response
 
 class SentryOkHttpInterceptor(
-    private val hub: IHub = HubAdapter.getInstance()
+    private val hub: IHub = HubAdapter.getInstance(),
+    private val beforeSpan: BeforeSpanCallback? = null
 ) : Interceptor {
 
     override fun intercept(chain: Interceptor.Chain): Response {
@@ -19,7 +22,7 @@ class SentryOkHttpInterceptor(
         val method = request.method
 
         // read transaction from the bound scope
-        val span = hub.span?.startChild("http.client", "$method $url")
+        var span = hub.span?.startChild("http.client", "$method $url")
 
         var response: Response? = null
 
@@ -39,7 +42,12 @@ class SentryOkHttpInterceptor(
             }
             throw e
         } finally {
-            span?.finish()
+            if (span != null) {
+                if (beforeSpan != null) {
+                    span = beforeSpan.execute(span, request, response)
+                }
+                span?.finish()
+            }
             val breadcrumb = Breadcrumb.http(request.url.toString(), request.method, code)
             request.body?.contentLength().ifHasValidLength {
                 breadcrumb.setData("requestBodySize", it)
@@ -56,4 +64,18 @@ class SentryOkHttpInterceptor(
             fn.invoke(this)
         }
     }
+
+    /**
+     * The BeforeSpan callback
+     */
+    interface BeforeSpanCallback {
+        /**
+         * Mutates or drops span before being added
+         *
+         * @param span the span to mutate or drop
+         * @param request the HTTP request executed by okHttp
+         * @param response the HTTP response received by okHttp
+         */
+        fun execute(span: ISpan, request: Request, response: Response?): ISpan?
+    }
 }

sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt

@@ -7,6 +7,7 @@ import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.whenever
 import io.sentry.Breadcrumb
 import io.sentry.IHub
+import io.sentry.ISpan
 import io.sentry.SentryOptions
 import io.sentry.SentryTraceHeader
 import io.sentry.SentryTracer
@@ -15,6 +16,7 @@ import io.sentry.TransactionContext
 import java.io.IOException
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
@@ -24,6 +26,7 @@ import okhttp3.MediaType.Companion.toMediaType
 import okhttp3.OkHttpClient
 import okhttp3.Request
 import okhttp3.RequestBody.Companion.toRequestBody
+import okhttp3.Response
 import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.MockWebServer
 import okhttp3.mockwebserver.SocketPolicy
@@ -32,7 +35,7 @@ class SentryOkHttpInterceptorTest {
 
     class Fixture {
         val hub = mock<IHub>()
-        val interceptor = SentryOkHttpInterceptor(hub)
+        var interceptor = SentryOkHttpInterceptor(hub)
         val server = MockWebServer()
         val sentryTracer = SentryTracer(TransactionContext("name", "op"), hub)
 
@@ -44,7 +47,8 @@ class SentryOkHttpInterceptorTest {
             isSpanActive: Boolean = true,
             httpStatusCode: Int = 201,
             responseBody: String = "success",
-            socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN
+            socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN,
+            beforeSpan: SentryOkHttpInterceptor.BeforeSpanCallback? = null
         ): OkHttpClient {
             if (isSpanActive) {
                 whenever(hub.span).thenReturn(sentryTracer)
@@ -54,6 +58,9 @@ class SentryOkHttpInterceptorTest {
                     .setSocketPolicy(socketPolicy)
                     .setResponseCode(httpStatusCode))
             server.start()
+            if (beforeSpan != null) {
+                interceptor = SentryOkHttpInterceptor(hub, beforeSpan)
+            }
             return OkHttpClient.Builder().addInterceptor(interceptor).build()
         }
     }
@@ -98,6 +105,7 @@ class SentryOkHttpInterceptorTest {
         assertEquals("http.client", httpClientSpan.operation)
         assertEquals("GET ${request.url}", httpClientSpan.description)
         assertEquals(SpanStatus.OK, httpClientSpan.status)
+        assertTrue(httpClientSpan.isFinished)
     }
 
     @Test
@@ -159,4 +167,46 @@ class SentryOkHttpInterceptorTest {
         assertEquals(SpanStatus.INTERNAL_ERROR, httpClientSpan.status)
         assertTrue(httpClientSpan.throwable is IOException)
     }
+
+    @Test
+    fun `customizer modifies span`() {
+        val sut = fixture.getSut(beforeSpan = object : SentryOkHttpInterceptor.BeforeSpanCallback {
+            override fun execute(span: ISpan, request: Request, response: Response?): ISpan {
+                span.description = "overwritten description"
+                return span
+            }
+        })
+        val request = getRequest()
+        sut.newCall(request).execute()
+        assertEquals(1, fixture.sentryTracer.children.size)
+        val httpClientSpan = fixture.sentryTracer.children.first()
+        assertEquals("overwritten description", httpClientSpan.description)
+    }
+
+    @Test
+    fun `customizer receives request and response`() {
+        var request: Request? = null
+        val sut = fixture.getSut(beforeSpan = object : SentryOkHttpInterceptor.BeforeSpanCallback {
+            override fun execute(span: ISpan, req: Request, res: Response?): ISpan {
+            assertEquals(request!!.url, req.url)
+            assertEquals(request!!.method, req.method)
+            assertNotNull(res) {
+                assertEquals(201, it.code)
+            }
+            return span
+        } })
+        request = getRequest()
+        sut.newCall(request).execute()
+    }
+
+    @Test
+    fun `customizer can drop the span`() {
+        val sut = fixture.getSut(beforeSpan = object : SentryOkHttpInterceptor.BeforeSpanCallback {
+            override fun execute(span: ISpan, request: Request, response: Response?): ISpan? {
+            return null
+        } })
+        sut.newCall(getRequest()).execute()
+        val httpClientSpan = fixture.sentryTracer.children.first()
+        assertFalse(httpClientSpan.isFinished)
+    }
 }