From d9e665896765e3a71e7c2da5369fd959b8b877fa Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Tue, 11 Apr 2023 17:55:37 +0200 Subject: [PATCH 01/10] added SentryNetworkCallEventListener with spans for each callback made SentryOkHttpInterceptor work nicely with SentryNetworkCallEventListener --- .../okhttp/SentryNetworkCallEventListener.kt | 345 ++++++++++++++++++ .../android/okhttp/SentryOkHttpInterceptor.kt | 50 ++- 2 files changed, 378 insertions(+), 17 deletions(-) create mode 100644 sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryNetworkCallEventListener.kt diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryNetworkCallEventListener.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryNetworkCallEventListener.kt new file mode 100644 index 0000000000..c77cc06350 --- /dev/null +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryNetworkCallEventListener.kt @@ -0,0 +1,345 @@ +package io.sentry.android.okhttp + +import android.util.Log +import io.sentry.Breadcrumb +import io.sentry.Hint +import io.sentry.HubAdapter +import io.sentry.IHub +import io.sentry.ISpan +import io.sentry.SpanStatus +import io.sentry.TypeCheckHint +import okhttp3.Call +import okhttp3.Connection +import okhttp3.EventListener +import okhttp3.Handshake +import okhttp3.HttpUrl +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import java.io.IOException +import java.net.InetAddress +import java.net.InetSocketAddress +import java.net.Proxy +import java.net.URL + +/** + * Logs network performance event metrics to Sentry + * + * Usage - add instance of [SentryNetworkCallEventListener] in [OkHttpClient.eventListenerFactory] + */ +class SentryNetworkCallEventListener( + private val hub: IHub = HubAdapter.getInstance() +) : EventListener() { + + companion object { + private const val CALL_EVENT = "call" + private const val PROXY_SELECT_EVENT = "proxySelect" + private const val DNS_EVENT = "dns" + private const val SECURE_CONNECT_EVENT = "secureConnect" + private const val CONNECT_EVENT = "connect" + private const val CONNECTION_EVENT = "connection" + private const val REQUEST_HEADERS_EVENT = "requestHeaders" + private const val REQUEST_BODY_EVENT = "requestBody" + private const val RESPONSE_HEADERS_EVENT = "responseHeaders" + private const val RESPONSE_BODY_EVENT = "responseBody" + + internal val eventMap: MutableMap = HashMap() + } + + //region Callback functions + + override fun callStart(call: Call) { + eventMap[call] = SentryNetworkCallEvent(hub, call) + } + + override fun proxySelectStart(call: Call, url: HttpUrl) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.startSpan(PROXY_SELECT_EVENT) + } + + override fun proxySelectEnd( + call: Call, + url: HttpUrl, + proxies: List + ) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.finishSpan(PROXY_SELECT_EVENT) { + if (proxies.isNotEmpty()) { + it.setData("proxies", proxies.joinToString { proxy -> proxy.toString() }) + } + } + } + + override fun dnsStart(call: Call, domainName: String) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.startSpan(DNS_EVENT) + } + + override fun dnsEnd( + call: Call, + domainName: String, + inetAddressList: List + ) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.finishSpan(DNS_EVENT) { + it.setData("domainName", domainName) + if (inetAddressList.isNotEmpty()) { + it.setData("dns", inetAddressList.joinToString { address -> address.toString() }) + } + } + } + + override fun connectStart( + call: Call, + inetSocketAddress: InetSocketAddress, + proxy: Proxy + ) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.startSpan(CONNECT_EVENT) + } + + override fun secureConnectStart(call: Call) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.startSpan(SECURE_CONNECT_EVENT) + } + + override fun secureConnectEnd(call: Call, handshake: Handshake?) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.finishSpan(SECURE_CONNECT_EVENT) + } + + override fun connectEnd( + call: Call, + inetSocketAddress: InetSocketAddress, + proxy: Proxy, + protocol: Protocol? + ) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.setProtocol(protocol?.name) + networkEvent.finishSpan(CONNECT_EVENT) + } + + override fun connectFailed( + call: Call, + inetSocketAddress: InetSocketAddress, + proxy: Proxy, + protocol: Protocol?, + ioe: IOException + ) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.setProtocol(protocol?.name) + networkEvent.setError(ioe.message) + networkEvent.finishSpan(CONNECT_EVENT) { + it.throwable = ioe + it.status = SpanStatus.INTERNAL_ERROR + } + } + + override fun connectionAcquired(call: Call, connection: Connection) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.startSpan(CONNECTION_EVENT) + } + + override fun connectionReleased(call: Call, connection: Connection) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.finishSpan(CONNECTION_EVENT) + } + + override fun requestHeadersStart(call: Call) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.startSpan(REQUEST_HEADERS_EVENT) + } + + override fun requestHeadersEnd(call: Call, request: Request) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.finishSpan(REQUEST_HEADERS_EVENT) + } + + override fun requestBodyStart(call: Call) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.startSpan(REQUEST_BODY_EVENT) + } + + override fun requestBodyEnd(call: Call, byteCount: Long) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.finishSpan(REQUEST_BODY_EVENT) { + if (byteCount > 0) { + it.setData("request_body_size", byteCount) + } + } + networkEvent.setRequestBodyLength(byteCount) + } + + override fun requestFailed(call: Call, ioe: IOException) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.setError(ioe.message) + networkEvent.finishSpan(REQUEST_HEADERS_EVENT) { + if (!it.isFinished) { + it.status = SpanStatus.INTERNAL_ERROR + it.throwable = ioe + } + } + networkEvent.finishSpan(REQUEST_BODY_EVENT) { + it.status = SpanStatus.INTERNAL_ERROR + it.throwable = ioe + } + } + + override fun responseHeadersStart(call: Call) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.startSpan(RESPONSE_HEADERS_EVENT) + } + + override fun responseHeadersEnd(call: Call, response: Response) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.setResponse(response) + networkEvent.finishSpan(RESPONSE_HEADERS_EVENT) { + it.setData("status_code", response.code) + } + } + + override fun responseBodyStart(call: Call) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.startSpan(RESPONSE_BODY_EVENT) + } + + override fun responseBodyEnd(call: Call, byteCount: Long) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.setResponseBodyLength(byteCount) + networkEvent.finishSpan(RESPONSE_BODY_EVENT) { + if (byteCount > 0) { + it.setData("response_body_size", byteCount) + } + } + } + + override fun responseFailed(call: Call, ioe: IOException) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.setError(ioe.message) + networkEvent.finishSpan(RESPONSE_HEADERS_EVENT) { + if (!it.isFinished) { + it.status = SpanStatus.INTERNAL_ERROR + it.throwable = ioe + } + } + networkEvent.finishSpan(RESPONSE_BODY_EVENT) { + it.status = SpanStatus.INTERNAL_ERROR + it.throwable = ioe + } + } + + override fun callEnd(call: Call) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.finishSpan(CALL_EVENT) + } + + override fun callFailed(call: Call, ioe: IOException) { + val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return + networkEvent.setError(ioe.message) + networkEvent.finishSpan(CALL_EVENT) { + it.status = SpanStatus.INTERNAL_ERROR + it.throwable = ioe + } + } + + //endregion + + internal class SentryNetworkCallEvent(private val hub: IHub, private val call: Call) { + private val eventSpans: MutableMap = HashMap() + private val breadcrumb: Breadcrumb + internal val rootSpan: ISpan? + private var response: Response? = null + + init { + val url: String = call.request().url.toString() + val trimmedUrl: String = trimUrl(url) + val host: String = call.request().url.host + val encodedPath: String = call.request().url.encodedPath + val method: String = call.request().method + val requestBodyLength: Long? = call.request().body?.contentLength() + + rootSpan = hub.span?.startChild("http.client", "$method $url") + + breadcrumb = Breadcrumb.http(url, method) + // todo check PII - remove magic strings - check breadcrumb and span keys + breadcrumb.setData("URL", url) + breadcrumb.setData("Filtered URL", trimmedUrl) + breadcrumb.setData("Host", host) + breadcrumb.setData("Path", encodedPath) + breadcrumb.setData("Method", method) + breadcrumb.setData("Success", true) + if (requestBodyLength != null && requestBodyLength > -1) { + breadcrumb.setData("RequestBody Length", requestBodyLength) + } + } + + private fun trimUrl(url: String): String { + val uuidRegex = Regex("[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}") + val trimmedUrl = url.replace(uuidRegex, "*") + if (URL(trimmedUrl).query == null) { + return trimmedUrl + } + return trimmedUrl.replace(URL(trimmedUrl).query, "").replace("?", "") + } + + fun setResponse(response: Response) { + this.response = response + breadcrumb.setData("protocol", response.protocol.name) + breadcrumb.setData("status_code", response.code) + } + + fun setProtocol(protocolName: String?) { + if (protocolName != null) { + breadcrumb.setData("protocol", protocolName) + } + } + + fun setRequestBodyLength(byteCount: Long) { + if (byteCount > -1) { + breadcrumb.setData("request_body_size", byteCount) + } + } + + fun setResponseBodyLength(byteCount: Long) { + if (byteCount > -1) { + breadcrumb.setData("response_body_size", byteCount) + } + } + + fun setError(errorMessage: String?) { + breadcrumb.setData("Success", false) + if (errorMessage != null) { + breadcrumb.setData("Error Message", errorMessage) + } + } + + fun startSpan(event: String) { + val span = rootSpan?.startChild("http.client", event) ?: return + Log.e("BBB", "BBB - start $event") + eventSpans[event] = span + } + + fun finishSpan(event: String, beforeFinish: ((span: ISpan) -> Unit)? = null) { + if (event == CALL_EVENT) { + if (rootSpan == null) { + eventMap.remove(call) + return + } + eventSpans.values.filter { !it.isFinished }.forEach { it.finish(SpanStatus.DEADLINE_EXCEEDED) } + beforeFinish?.invoke(rootSpan) + rootSpan.finish() + + val hint = Hint() + hint.set(TypeCheckHint.OKHTTP_REQUEST, call.request()) + response?.let { hint.set(TypeCheckHint.OKHTTP_RESPONSE, it) } + + hub.addBreadcrumb(breadcrumb, hint) + return + } + val span = eventSpans[event] ?: return + Log.e("BBB", "BBB - finish $event") + beforeFinish?.invoke(span) + span.finish() + } + } +} 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 index bbe6364375..efa5f880b3 100644 --- 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 @@ -67,7 +67,16 @@ class SentryOkHttpInterceptor( val method = request.method // read transaction from the bound scope - val span = hub.span?.startChild("http.client", "$method $url") + val span: ISpan? + val isFromEventListener: Boolean + + if (SentryNetworkCallEventListener.eventMap.containsKey(chain.call())) { + span = SentryNetworkCallEventListener.eventMap[chain.call()]?.rootSpan + isFromEventListener = true + } else { + span = hub.span?.startChild("http.client", "$method $url") + isFromEventListener = false + } urlDetails.applyToSpan(span) var response: Response? = null @@ -105,28 +114,31 @@ class SentryOkHttpInterceptor( } throw e } finally { - finishSpan(span, request, response) + finishSpan(span, request, response, isFromEventListener) - val breadcrumb = Breadcrumb.http(request.url.toString(), request.method, code) - request.body?.contentLength().ifHasValidLength { - breadcrumb.setData("request_body_size", it) - } + // The SentryNetworkCallEventListener will send the breadcrumb itself if used for this call + if (!isFromEventListener) { + val breadcrumb = Breadcrumb.http(request.url.toString(), request.method, code) + request.body?.contentLength().ifHasValidLength { + breadcrumb.setData("request_body_size", it) + } + + val hint = Hint() + .also { it.set(OKHTTP_REQUEST, request) } + response?.let { + it.body?.contentLength().ifHasValidLength { responseBodySize -> + breadcrumb.setData("response_body_size", responseBodySize) + } - val hint = Hint() - .also { it.set(OKHTTP_REQUEST, request) } - response?.let { - it.body?.contentLength().ifHasValidLength { responseBodySize -> - breadcrumb.setData("response_body_size", responseBodySize) + hint[OKHTTP_RESPONSE] = it } - hint[OKHTTP_RESPONSE] = it + hub.addBreadcrumb(breadcrumb, hint) } - - hub.addBreadcrumb(breadcrumb, hint) } } - private fun finishSpan(span: ISpan?, request: Request, response: Response?) { + private fun finishSpan(span: ISpan?, request: Request, response: Response?, isFromEventListener: Boolean) { if (span != null) { if (beforeSpan != null) { val result = beforeSpan.execute(span, request, response) @@ -134,10 +146,14 @@ class SentryOkHttpInterceptor( // span is dropped span.spanContext.sampled = false } else { - span.finish() + if (!isFromEventListener) { + span.finish() + } } } else { - span.finish() + if (!isFromEventListener) { + span.finish() + } } } } From 28946feb905f26acd8d30572fc745756993cda50 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 17 Apr 2023 11:47:22 +0200 Subject: [PATCH 02/10] added SentryOkHttpEventListener added SentryOkHttpEvent added tests --- .../api/sentry-android-okhttp.api | 34 ++ sentry-android-okhttp/build.gradle.kts | 1 + .../okhttp/SentryNetworkCallEventListener.kt | 345 ---------------- .../android/okhttp/SentryOkHttpEvent.kt | 138 +++++++ .../okhttp/SentryOkHttpEventListener.kt | 248 +++++++++++ .../android/okhttp/SentryOkHttpInterceptor.kt | 65 +-- .../okhttp/SentryOkHttpEventListenerTest.kt | 222 ++++++++++ .../android/okhttp/SentryOkHttpEventTest.kt | 390 ++++++++++++++++++ .../okhttp/SentryOkHttpInterceptorTest.kt | 11 + 9 files changed, 1080 insertions(+), 374 deletions(-) delete mode 100644 sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryNetworkCallEventListener.kt create mode 100644 sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt create mode 100644 sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt create mode 100644 sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt create mode 100644 sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt diff --git a/sentry-android-okhttp/api/sentry-android-okhttp.api b/sentry-android-okhttp/api/sentry-android-okhttp.api index 897755948a..8dc1a8e55f 100644 --- a/sentry-android-okhttp/api/sentry-android-okhttp.api +++ b/sentry-android-okhttp/api/sentry-android-okhttp.api @@ -6,6 +6,40 @@ public final class io/sentry/android/okhttp/BuildConfig { public fun ()V } +public final class io/sentry/android/okhttp/SentryOkHttpEventListener : okhttp3/EventListener { + public static final field Companion Lio/sentry/android/okhttp/SentryOkHttpEventListener$Companion; + public fun ()V + public fun (Lio/sentry/IHub;)V + public synthetic fun (Lio/sentry/IHub;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun callEnd (Lokhttp3/Call;)V + public fun callFailed (Lokhttp3/Call;Ljava/io/IOException;)V + public fun callStart (Lokhttp3/Call;)V + public fun connectEnd (Lokhttp3/Call;Ljava/net/InetSocketAddress;Ljava/net/Proxy;Lokhttp3/Protocol;)V + public fun connectFailed (Lokhttp3/Call;Ljava/net/InetSocketAddress;Ljava/net/Proxy;Lokhttp3/Protocol;Ljava/io/IOException;)V + public fun connectStart (Lokhttp3/Call;Ljava/net/InetSocketAddress;Ljava/net/Proxy;)V + public fun connectionAcquired (Lokhttp3/Call;Lokhttp3/Connection;)V + public fun connectionReleased (Lokhttp3/Call;Lokhttp3/Connection;)V + public fun dnsEnd (Lokhttp3/Call;Ljava/lang/String;Ljava/util/List;)V + public fun dnsStart (Lokhttp3/Call;Ljava/lang/String;)V + public fun proxySelectEnd (Lokhttp3/Call;Lokhttp3/HttpUrl;Ljava/util/List;)V + public fun proxySelectStart (Lokhttp3/Call;Lokhttp3/HttpUrl;)V + public fun requestBodyEnd (Lokhttp3/Call;J)V + public fun requestBodyStart (Lokhttp3/Call;)V + public fun requestFailed (Lokhttp3/Call;Ljava/io/IOException;)V + public fun requestHeadersEnd (Lokhttp3/Call;Lokhttp3/Request;)V + public fun requestHeadersStart (Lokhttp3/Call;)V + public fun responseBodyEnd (Lokhttp3/Call;J)V + public fun responseBodyStart (Lokhttp3/Call;)V + public fun responseFailed (Lokhttp3/Call;Ljava/io/IOException;)V + public fun responseHeadersEnd (Lokhttp3/Call;Lokhttp3/Response;)V + public fun responseHeadersStart (Lokhttp3/Call;)V + public fun secureConnectEnd (Lokhttp3/Call;Lokhttp3/Handshake;)V + public fun secureConnectStart (Lokhttp3/Call;)V +} + +public final class io/sentry/android/okhttp/SentryOkHttpEventListener$Companion { +} + public final class io/sentry/android/okhttp/SentryOkHttpInterceptor : io/sentry/IntegrationName, okhttp3/Interceptor { public fun ()V public fun (Lio/sentry/IHub;)V diff --git a/sentry-android-okhttp/build.gradle.kts b/sentry-android-okhttp/build.gradle.kts index 5d566922f3..ae80ad068e 100644 --- a/sentry-android-okhttp/build.gradle.kts +++ b/sentry-android-okhttp/build.gradle.kts @@ -74,6 +74,7 @@ dependencies { implementation(kotlin(Config.kotlinStdLib, KotlinCompilerVersion.VERSION)) // tests + testImplementation(projects.sentryTestSupport) testImplementation(Config.Libs.okhttp) testImplementation(Config.TestLibs.kotlinTestJunit) testImplementation(Config.TestLibs.androidxJunit) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryNetworkCallEventListener.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryNetworkCallEventListener.kt deleted file mode 100644 index c77cc06350..0000000000 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryNetworkCallEventListener.kt +++ /dev/null @@ -1,345 +0,0 @@ -package io.sentry.android.okhttp - -import android.util.Log -import io.sentry.Breadcrumb -import io.sentry.Hint -import io.sentry.HubAdapter -import io.sentry.IHub -import io.sentry.ISpan -import io.sentry.SpanStatus -import io.sentry.TypeCheckHint -import okhttp3.Call -import okhttp3.Connection -import okhttp3.EventListener -import okhttp3.Handshake -import okhttp3.HttpUrl -import okhttp3.Protocol -import okhttp3.Request -import okhttp3.Response -import java.io.IOException -import java.net.InetAddress -import java.net.InetSocketAddress -import java.net.Proxy -import java.net.URL - -/** - * Logs network performance event metrics to Sentry - * - * Usage - add instance of [SentryNetworkCallEventListener] in [OkHttpClient.eventListenerFactory] - */ -class SentryNetworkCallEventListener( - private val hub: IHub = HubAdapter.getInstance() -) : EventListener() { - - companion object { - private const val CALL_EVENT = "call" - private const val PROXY_SELECT_EVENT = "proxySelect" - private const val DNS_EVENT = "dns" - private const val SECURE_CONNECT_EVENT = "secureConnect" - private const val CONNECT_EVENT = "connect" - private const val CONNECTION_EVENT = "connection" - private const val REQUEST_HEADERS_EVENT = "requestHeaders" - private const val REQUEST_BODY_EVENT = "requestBody" - private const val RESPONSE_HEADERS_EVENT = "responseHeaders" - private const val RESPONSE_BODY_EVENT = "responseBody" - - internal val eventMap: MutableMap = HashMap() - } - - //region Callback functions - - override fun callStart(call: Call) { - eventMap[call] = SentryNetworkCallEvent(hub, call) - } - - override fun proxySelectStart(call: Call, url: HttpUrl) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.startSpan(PROXY_SELECT_EVENT) - } - - override fun proxySelectEnd( - call: Call, - url: HttpUrl, - proxies: List - ) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.finishSpan(PROXY_SELECT_EVENT) { - if (proxies.isNotEmpty()) { - it.setData("proxies", proxies.joinToString { proxy -> proxy.toString() }) - } - } - } - - override fun dnsStart(call: Call, domainName: String) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.startSpan(DNS_EVENT) - } - - override fun dnsEnd( - call: Call, - domainName: String, - inetAddressList: List - ) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.finishSpan(DNS_EVENT) { - it.setData("domainName", domainName) - if (inetAddressList.isNotEmpty()) { - it.setData("dns", inetAddressList.joinToString { address -> address.toString() }) - } - } - } - - override fun connectStart( - call: Call, - inetSocketAddress: InetSocketAddress, - proxy: Proxy - ) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.startSpan(CONNECT_EVENT) - } - - override fun secureConnectStart(call: Call) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.startSpan(SECURE_CONNECT_EVENT) - } - - override fun secureConnectEnd(call: Call, handshake: Handshake?) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.finishSpan(SECURE_CONNECT_EVENT) - } - - override fun connectEnd( - call: Call, - inetSocketAddress: InetSocketAddress, - proxy: Proxy, - protocol: Protocol? - ) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.setProtocol(protocol?.name) - networkEvent.finishSpan(CONNECT_EVENT) - } - - override fun connectFailed( - call: Call, - inetSocketAddress: InetSocketAddress, - proxy: Proxy, - protocol: Protocol?, - ioe: IOException - ) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.setProtocol(protocol?.name) - networkEvent.setError(ioe.message) - networkEvent.finishSpan(CONNECT_EVENT) { - it.throwable = ioe - it.status = SpanStatus.INTERNAL_ERROR - } - } - - override fun connectionAcquired(call: Call, connection: Connection) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.startSpan(CONNECTION_EVENT) - } - - override fun connectionReleased(call: Call, connection: Connection) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.finishSpan(CONNECTION_EVENT) - } - - override fun requestHeadersStart(call: Call) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.startSpan(REQUEST_HEADERS_EVENT) - } - - override fun requestHeadersEnd(call: Call, request: Request) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.finishSpan(REQUEST_HEADERS_EVENT) - } - - override fun requestBodyStart(call: Call) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.startSpan(REQUEST_BODY_EVENT) - } - - override fun requestBodyEnd(call: Call, byteCount: Long) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.finishSpan(REQUEST_BODY_EVENT) { - if (byteCount > 0) { - it.setData("request_body_size", byteCount) - } - } - networkEvent.setRequestBodyLength(byteCount) - } - - override fun requestFailed(call: Call, ioe: IOException) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.setError(ioe.message) - networkEvent.finishSpan(REQUEST_HEADERS_EVENT) { - if (!it.isFinished) { - it.status = SpanStatus.INTERNAL_ERROR - it.throwable = ioe - } - } - networkEvent.finishSpan(REQUEST_BODY_EVENT) { - it.status = SpanStatus.INTERNAL_ERROR - it.throwable = ioe - } - } - - override fun responseHeadersStart(call: Call) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.startSpan(RESPONSE_HEADERS_EVENT) - } - - override fun responseHeadersEnd(call: Call, response: Response) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.setResponse(response) - networkEvent.finishSpan(RESPONSE_HEADERS_EVENT) { - it.setData("status_code", response.code) - } - } - - override fun responseBodyStart(call: Call) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.startSpan(RESPONSE_BODY_EVENT) - } - - override fun responseBodyEnd(call: Call, byteCount: Long) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.setResponseBodyLength(byteCount) - networkEvent.finishSpan(RESPONSE_BODY_EVENT) { - if (byteCount > 0) { - it.setData("response_body_size", byteCount) - } - } - } - - override fun responseFailed(call: Call, ioe: IOException) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.setError(ioe.message) - networkEvent.finishSpan(RESPONSE_HEADERS_EVENT) { - if (!it.isFinished) { - it.status = SpanStatus.INTERNAL_ERROR - it.throwable = ioe - } - } - networkEvent.finishSpan(RESPONSE_BODY_EVENT) { - it.status = SpanStatus.INTERNAL_ERROR - it.throwable = ioe - } - } - - override fun callEnd(call: Call) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.finishSpan(CALL_EVENT) - } - - override fun callFailed(call: Call, ioe: IOException) { - val networkEvent: SentryNetworkCallEvent = eventMap[call] ?: return - networkEvent.setError(ioe.message) - networkEvent.finishSpan(CALL_EVENT) { - it.status = SpanStatus.INTERNAL_ERROR - it.throwable = ioe - } - } - - //endregion - - internal class SentryNetworkCallEvent(private val hub: IHub, private val call: Call) { - private val eventSpans: MutableMap = HashMap() - private val breadcrumb: Breadcrumb - internal val rootSpan: ISpan? - private var response: Response? = null - - init { - val url: String = call.request().url.toString() - val trimmedUrl: String = trimUrl(url) - val host: String = call.request().url.host - val encodedPath: String = call.request().url.encodedPath - val method: String = call.request().method - val requestBodyLength: Long? = call.request().body?.contentLength() - - rootSpan = hub.span?.startChild("http.client", "$method $url") - - breadcrumb = Breadcrumb.http(url, method) - // todo check PII - remove magic strings - check breadcrumb and span keys - breadcrumb.setData("URL", url) - breadcrumb.setData("Filtered URL", trimmedUrl) - breadcrumb.setData("Host", host) - breadcrumb.setData("Path", encodedPath) - breadcrumb.setData("Method", method) - breadcrumb.setData("Success", true) - if (requestBodyLength != null && requestBodyLength > -1) { - breadcrumb.setData("RequestBody Length", requestBodyLength) - } - } - - private fun trimUrl(url: String): String { - val uuidRegex = Regex("[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}") - val trimmedUrl = url.replace(uuidRegex, "*") - if (URL(trimmedUrl).query == null) { - return trimmedUrl - } - return trimmedUrl.replace(URL(trimmedUrl).query, "").replace("?", "") - } - - fun setResponse(response: Response) { - this.response = response - breadcrumb.setData("protocol", response.protocol.name) - breadcrumb.setData("status_code", response.code) - } - - fun setProtocol(protocolName: String?) { - if (protocolName != null) { - breadcrumb.setData("protocol", protocolName) - } - } - - fun setRequestBodyLength(byteCount: Long) { - if (byteCount > -1) { - breadcrumb.setData("request_body_size", byteCount) - } - } - - fun setResponseBodyLength(byteCount: Long) { - if (byteCount > -1) { - breadcrumb.setData("response_body_size", byteCount) - } - } - - fun setError(errorMessage: String?) { - breadcrumb.setData("Success", false) - if (errorMessage != null) { - breadcrumb.setData("Error Message", errorMessage) - } - } - - fun startSpan(event: String) { - val span = rootSpan?.startChild("http.client", event) ?: return - Log.e("BBB", "BBB - start $event") - eventSpans[event] = span - } - - fun finishSpan(event: String, beforeFinish: ((span: ISpan) -> Unit)? = null) { - if (event == CALL_EVENT) { - if (rootSpan == null) { - eventMap.remove(call) - return - } - eventSpans.values.filter { !it.isFinished }.forEach { it.finish(SpanStatus.DEADLINE_EXCEEDED) } - beforeFinish?.invoke(rootSpan) - rootSpan.finish() - - val hint = Hint() - hint.set(TypeCheckHint.OKHTTP_REQUEST, call.request()) - response?.let { hint.set(TypeCheckHint.OKHTTP_RESPONSE, it) } - - hub.addBreadcrumb(breadcrumb, hint) - return - } - val span = eventSpans[event] ?: return - Log.e("BBB", "BBB - finish $event") - beforeFinish?.invoke(span) - span.finish() - } - } -} diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt new file mode 100644 index 0000000000..7a751c7751 --- /dev/null +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt @@ -0,0 +1,138 @@ +package io.sentry.android.okhttp + +import io.sentry.Breadcrumb +import io.sentry.Hint +import io.sentry.IHub +import io.sentry.ISpan +import io.sentry.SpanStatus +import io.sentry.TypeCheckHint +import io.sentry.util.UrlUtils +import okhttp3.Call +import okhttp3.Response +import java.net.URL + +internal class SentryOkHttpEvent(private val hub: IHub, private val call: Call) { + private val eventSpans: MutableMap = HashMap() + private val breadcrumb: Breadcrumb + internal val callRootSpan: ISpan? + private var response: Response? = null + + init { + val urlDetails = UrlUtils.parse(call.request().url.toString()) + val url = urlDetails.urlOrFallback + val trimmedUrl: String = trimUrl(url) + val host: String = call.request().url.host + val encodedPath: String = call.request().url.encodedPath + val method: String = call.request().method + + // We start the call span that will contain all the others + callRootSpan = hub.span?.startChild("http.client", "$method $url") + + // We setup a breadcrumb with all meaningful data + breadcrumb = Breadcrumb.http(url, method) + breadcrumb.setData("url", url) + breadcrumb.setData("filtered_url", trimmedUrl) + breadcrumb.setData("host", host) + breadcrumb.setData("path", encodedPath) + breadcrumb.setData("method", method) + breadcrumb.setData("success", true) + + // We add the same data to the root call span + callRootSpan?.setData("url", url) + callRootSpan?.setData("filtered_url", trimmedUrl) + callRootSpan?.setData("host", host) + callRootSpan?.setData("path", encodedPath) + callRootSpan?.setData("method", method) + callRootSpan?.setData("success", true) + } + + private fun trimUrl(url: String): String { + // Remove any uuid from the url and replace it with a "*" + val uuidRegex = Regex("[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}") + val trimmedUrl = url.replace(uuidRegex, "*") + if (URL(trimmedUrl).query == null) { + return trimmedUrl + } + // Remove any parameter from the url + return trimmedUrl.replace(URL(trimmedUrl).query, "").replace("?", "") + } + + /** + * Sets the [Response] that will be sent in the breadcrumb [Hint]. + * Also, it sets the protocol and status code in the breadcrumb and the call root span. + */ + fun setResponse(response: Response) { + this.response = response + breadcrumb.setData("protocol", response.protocol.name) + breadcrumb.setData("status_code", response.code) + callRootSpan?.setData("protocol", response.protocol.name) + callRootSpan?.setData("status_code", response.code) + callRootSpan?.status = SpanStatus.fromHttpStatusCode(response.code) + } + + fun setProtocol(protocolName: String?) { + if (protocolName != null) { + breadcrumb.setData("protocol", protocolName) + callRootSpan?.setData("protocol", protocolName) + } + } + + fun setRequestBodySize(byteCount: Long) { + if (byteCount > -1) { + breadcrumb.setData("request_body_size", byteCount) + callRootSpan?.setData("request_body_size", byteCount) + } + } + + fun setResponseBodySize(byteCount: Long) { + if (byteCount > -1) { + breadcrumb.setData("response_body_size", byteCount) + callRootSpan?.setData("response_body_size", byteCount) + } + } + + /** + * Sets the success flag in the breadcrumb and the call root span to false. + * Also sets the [errorMessage] if not null. + */ + fun setError(errorMessage: String?) { + breadcrumb.setData("success", false) + callRootSpan?.setData("success", false) + if (errorMessage != null) { + breadcrumb.setData("error_message", errorMessage) + callRootSpan?.setData("error_message", errorMessage) + } + } + + /** Starts a span, if the callRootSpan is not null. */ + fun startSpan(event: String) { + val span = callRootSpan?.startChild("http.client", event) ?: return + eventSpans[event] = span + } + + /** Finishes a previously started span, and runs [beforeFinish] on it and on the call root span. */ + fun finishSpan(event: String, beforeFinish: ((span: ISpan) -> Unit)? = null) { + val span = eventSpans[event] ?: return + beforeFinish?.invoke(span) + callRootSpan?.let { beforeFinish?.invoke(it) } + span.finish() + } + + /** Finishes the call root span, and runs [beforeFinish] on it. Then a breadcrumb is sent. */ + fun finishEvent(beforeFinish: ((span: ISpan) -> Unit)? = null) { + callRootSpan ?: return + + // We forcefully finish all spans, even if they should already have been finished through finishSpan() + eventSpans.values.filter { !it.isFinished }.forEach { it.finish(SpanStatus.DEADLINE_EXCEEDED) } + beforeFinish?.invoke(callRootSpan) + callRootSpan.finish() + + // We put data in the hint and send a breadcrumb + val hint = Hint() + hint.set(TypeCheckHint.OKHTTP_REQUEST, call.request()) + response?.let { hint.set(TypeCheckHint.OKHTTP_RESPONSE, it) } + + hub.addBreadcrumb(breadcrumb, hint) + return + } +} diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt new file mode 100644 index 0000000000..e013d8d7ea --- /dev/null +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt @@ -0,0 +1,248 @@ +package io.sentry.android.okhttp + +import io.sentry.HubAdapter +import io.sentry.IHub +import io.sentry.SpanStatus +import okhttp3.Call +import okhttp3.Connection +import okhttp3.EventListener +import okhttp3.Handshake +import okhttp3.HttpUrl +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import java.io.IOException +import java.net.InetAddress +import java.net.InetSocketAddress +import java.net.Proxy + +/** + * Logs network performance event metrics to Sentry + * + * Usage - add instance of [SentryOkHttpEventListener] in [OkHttpClient.eventListener] + * + * ``` + * val client = OkHttpClient.Builder() + * .eventListener(SentryOkHttpEventListener()) + * .addInterceptor(SentryOkHttpInterceptor()) + * .build() + * ``` + */ +@Suppress("TooManyFunctions") +class SentryOkHttpEventListener( + private val hub: IHub = HubAdapter.getInstance() +) : EventListener() { + + companion object { + private const val PROXY_SELECT_EVENT = "proxySelect" + private const val DNS_EVENT = "dns" + private const val SECURE_CONNECT_EVENT = "secureConnect" + private const val CONNECT_EVENT = "connect" + private const val CONNECTION_EVENT = "connection" + private const val REQUEST_HEADERS_EVENT = "requestHeaders" + private const val REQUEST_BODY_EVENT = "requestBody" + private const val RESPONSE_HEADERS_EVENT = "responseHeaders" + private const val RESPONSE_BODY_EVENT = "responseBody" + + internal val eventMap: MutableMap = HashMap() + } + + override fun callStart(call: Call) { + eventMap[call] = SentryOkHttpEvent(hub, call) + } + + override fun proxySelectStart(call: Call, url: HttpUrl) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.startSpan(PROXY_SELECT_EVENT) + } + + override fun proxySelectEnd( + call: Call, + url: HttpUrl, + proxies: List + ) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.finishSpan(PROXY_SELECT_EVENT) { + if (proxies.isNotEmpty()) { + it.setData("proxies", proxies.joinToString { proxy -> proxy.toString() }) + } + } + } + + override fun dnsStart(call: Call, domainName: String) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.startSpan(DNS_EVENT) + } + + override fun dnsEnd( + call: Call, + domainName: String, + inetAddressList: List + ) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.finishSpan(DNS_EVENT) { + it.setData("domain_name", domainName) + if (inetAddressList.isNotEmpty()) { + it.setData("dns_addresses", inetAddressList.joinToString { address -> address.toString() }) + } + } + } + + override fun connectStart( + call: Call, + inetSocketAddress: InetSocketAddress, + proxy: Proxy + ) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.startSpan(CONNECT_EVENT) + } + + override fun secureConnectStart(call: Call) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.startSpan(SECURE_CONNECT_EVENT) + } + + override fun secureConnectEnd(call: Call, handshake: Handshake?) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.finishSpan(SECURE_CONNECT_EVENT) + } + + override fun connectEnd( + call: Call, + inetSocketAddress: InetSocketAddress, + proxy: Proxy, + protocol: Protocol? + ) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.setProtocol(protocol?.name) + okHttpEvent.finishSpan(CONNECT_EVENT) + } + + override fun connectFailed( + call: Call, + inetSocketAddress: InetSocketAddress, + proxy: Proxy, + protocol: Protocol?, + ioe: IOException + ) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.setProtocol(protocol?.name) + okHttpEvent.setError(ioe.message) + okHttpEvent.finishSpan(CONNECT_EVENT) { + it.throwable = ioe + it.status = SpanStatus.INTERNAL_ERROR + } + } + + override fun connectionAcquired(call: Call, connection: Connection) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.startSpan(CONNECTION_EVENT) + } + + override fun connectionReleased(call: Call, connection: Connection) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.finishSpan(CONNECTION_EVENT) + } + + override fun requestHeadersStart(call: Call) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.startSpan(REQUEST_HEADERS_EVENT) + } + + override fun requestHeadersEnd(call: Call, request: Request) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.finishSpan(REQUEST_HEADERS_EVENT) + } + + override fun requestBodyStart(call: Call) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.startSpan(REQUEST_BODY_EVENT) + } + + override fun requestBodyEnd(call: Call, byteCount: Long) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.finishSpan(REQUEST_BODY_EVENT) { + if (byteCount > 0) { + it.setData("request_body_size", byteCount) + } + } + okHttpEvent.setRequestBodySize(byteCount) + } + + override fun requestFailed(call: Call, ioe: IOException) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.setError(ioe.message) + // requestFailed can happen after requestHeaders or requestBody. + // If requestHeaders already finished, we don't change its status. + okHttpEvent.finishSpan(REQUEST_HEADERS_EVENT) { + if (!it.isFinished) { + it.status = SpanStatus.INTERNAL_ERROR + it.throwable = ioe + } + } + okHttpEvent.finishSpan(REQUEST_BODY_EVENT) { + it.status = SpanStatus.INTERNAL_ERROR + it.throwable = ioe + } + } + + override fun responseHeadersStart(call: Call) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.startSpan(RESPONSE_HEADERS_EVENT) + } + + override fun responseHeadersEnd(call: Call, response: Response) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.setResponse(response) + okHttpEvent.finishSpan(RESPONSE_HEADERS_EVENT) { + it.setData("status_code", response.code) + it.status = SpanStatus.fromHttpStatusCode(response.code) + } + } + + override fun responseBodyStart(call: Call) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.startSpan(RESPONSE_BODY_EVENT) + } + + override fun responseBodyEnd(call: Call, byteCount: Long) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.setResponseBodySize(byteCount) + okHttpEvent.finishSpan(RESPONSE_BODY_EVENT) { + if (byteCount > 0) { + it.setData("response_body_size", byteCount) + } + } + } + + override fun responseFailed(call: Call, ioe: IOException) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.setError(ioe.message) + // responseFailed can happen after responseHeaders or responseBody. + // If responseHeaders already finished, we don't change its status. + okHttpEvent.finishSpan(RESPONSE_HEADERS_EVENT) { + if (!it.isFinished) { + it.status = SpanStatus.INTERNAL_ERROR + it.throwable = ioe + } + } + okHttpEvent.finishSpan(RESPONSE_BODY_EVENT) { + it.status = SpanStatus.INTERNAL_ERROR + it.throwable = ioe + } + } + + override fun callEnd(call: Call) { + val okHttpEvent = eventMap.remove(call) ?: return + okHttpEvent.finishEvent() + } + + override fun callFailed(call: Call, ioe: IOException) { + val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + okHttpEvent.setError(ioe.message) + okHttpEvent.finishEvent { + it.status = SpanStatus.INTERNAL_ERROR + it.throwable = ioe + } + } +} 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 index efa5f880b3..d4b124637a 100644 --- 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 @@ -66,14 +66,15 @@ class SentryOkHttpInterceptor( val url = urlDetails.urlOrFallback val method = request.method - // read transaction from the bound scope val span: ISpan? val isFromEventListener: Boolean - if (SentryNetworkCallEventListener.eventMap.containsKey(chain.call())) { - span = SentryNetworkCallEventListener.eventMap[chain.call()]?.rootSpan + if (SentryOkHttpEventListener.eventMap.containsKey(chain.call())) { + // read the span from the event listener + span = SentryOkHttpEventListener.eventMap[chain.call()]?.callRootSpan isFromEventListener = true } else { + // read the span from the bound scope span = hub.span?.startChild("http.client", "$method $url") isFromEventListener = false } @@ -116,45 +117,51 @@ class SentryOkHttpInterceptor( } finally { finishSpan(span, request, response, isFromEventListener) - // The SentryNetworkCallEventListener will send the breadcrumb itself if used for this call + // The SentryOkHttpEventListener will send the breadcrumb itself if used for this call if (!isFromEventListener) { - val breadcrumb = Breadcrumb.http(request.url.toString(), request.method, code) - request.body?.contentLength().ifHasValidLength { - breadcrumb.setData("request_body_size", it) - } - - val hint = Hint() - .also { it.set(OKHTTP_REQUEST, request) } - response?.let { - it.body?.contentLength().ifHasValidLength { responseBodySize -> - breadcrumb.setData("response_body_size", responseBodySize) - } + sendBreadcrumb(request, code, response) + } + } + } - hint[OKHTTP_RESPONSE] = it - } + private fun sendBreadcrumb(request: Request, code: Int?, response: Response?) { + val breadcrumb = Breadcrumb.http(request.url.toString(), request.method, code) + request.body?.contentLength().ifHasValidLength { + breadcrumb.setData("request_body_size", it) + } - hub.addBreadcrumb(breadcrumb, hint) + val hint = Hint().also { it.set(OKHTTP_REQUEST, request) } + response?.let { + it.body?.contentLength().ifHasValidLength { responseBodySize -> + breadcrumb.setData("response_body_size", responseBodySize) } + + hint[OKHTTP_RESPONSE] = it } + + hub.addBreadcrumb(breadcrumb, hint) } private fun finishSpan(span: ISpan?, request: Request, response: Response?, isFromEventListener: Boolean) { - if (span != null) { - if (beforeSpan != null) { - val result = beforeSpan.execute(span, request, response) - if (result == null) { - // span is dropped - span.spanContext.sampled = false - } else { - if (!isFromEventListener) { - span.finish() - } - } + if (span == null) { + return + } + if (beforeSpan != null) { + val result = beforeSpan.execute(span, request, response) + 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() } } + } else { + // The SentryOkHttpEventListener will finish the span itself if used for this call + if (!isFromEventListener) { + span.finish() + } } } diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt new file mode 100644 index 0000000000..76c4c0c4fb --- /dev/null +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt @@ -0,0 +1,222 @@ +package io.sentry.android.okhttp + +import io.sentry.BaggageHeader +import io.sentry.IHub +import io.sentry.SentryOptions +import io.sentry.SentryTraceHeader +import io.sentry.SentryTracer +import io.sentry.SpanStatus +import io.sentry.TransactionContext +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.RequestBody.Companion.toRequestBody +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import okhttp3.mockwebserver.SocketPolicy +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class SentryOkHttpEventListenerTest { + + class Fixture { + val hub = mock() + val server = MockWebServer() + lateinit var sentryTracer: SentryTracer + lateinit var options: SentryOptions + + @SuppressWarnings("LongParameterList") + fun getSut( + isSpanActive: Boolean = true, + useInterceptor: Boolean = false, + httpStatusCode: Int = 201, + responseBody: String = "success", + socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN, + sendDefaultPii: Boolean = false + ): OkHttpClient { + options = SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + isSendDefaultPii = sendDefaultPii + } + whenever(hub.options).thenReturn(options) + + sentryTracer = SentryTracer(TransactionContext("name", "op"), hub) + + if (isSpanActive) { + whenever(hub.span).thenReturn(sentryTracer) + } + server.enqueue( + MockResponse() + .setBody(responseBody) + .addHeader("myResponseHeader", "myValue") + .setSocketPolicy(socketPolicy) + .setResponseCode(httpStatusCode) + ) + + val builder = OkHttpClient.Builder() + if (useInterceptor) { + builder.addInterceptor(SentryOkHttpInterceptor(hub)) + } + return builder.eventListener(SentryOkHttpEventListener(hub)).build() + } + } + + private val fixture = Fixture() + + private fun getRequest(url: String = "/hello"): Request { + return Request.Builder() + .addHeader("myHeader", "myValue") + .get() + .url(fixture.server.url(url)) + .build() + } + + private fun postRequest(url: String = "/hello", body: String): Request { + return Request.Builder() + .addHeader("myHeader", "myValue") + .post(body.toRequestBody()) + .url(fixture.server.url(url)) + .build() + } + + @Test + fun `when there is an active span and the SentryOkHttpInterceptor, adds sentry trace headers to the request`() { + val sut = fixture.getSut(useInterceptor = true) + sut.newCall(getRequest()).execute() + val recorderRequest = fixture.server.takeRequest() + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Suppress("MaxLineLength") + @Test + fun `when there is an active span but no SentryOkHttpInterceptor, sentry trace headers are not added to the request`() { + val sut = fixture.getSut() + sut.newCall(getRequest()).execute() + val recorderRequest = fixture.server.takeRequest() + assertNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Test + fun `creates a span around the request`() { + val sut = fixture.getSut() + val request = getRequest() + val call = sut.newCall(request) + val response = call.execute() + val okHttpEvent = SentryOkHttpEventListener.eventMap[call] + val callSpan = okHttpEvent?.callRootSpan + response.close() + assertNotNull(callSpan) + assertEquals(callSpan, fixture.sentryTracer.children.first()) + assertEquals("http.client", callSpan.operation) + assertEquals("GET ${request.url}", callSpan.description) + assertEquals(SpanStatus.OK, callSpan.status) + assertTrue(callSpan.isFinished) + } + + @Test + fun `creates a span for each event`() { + val sut = fixture.getSut() + val request = getRequest() + val call = sut.newCall(request) + val response = call.execute() + val okHttpEvent = SentryOkHttpEventListener.eventMap[call] + val callSpan = okHttpEvent?.callRootSpan + response.close() + assertEquals(8, fixture.sentryTracer.children.size) + fixture.sentryTracer.children.forEachIndexed { index, span -> + assertTrue(span.isFinished) + when (index) { + 0 -> { + assertEquals(callSpan, span) + assertEquals("GET ${request.url}", span.description) + assertNotNull(span.data["proxies"]) + assertNotNull(span.data["domain_name"]) + assertNotNull(span.data["dns_addresses"]) + assertEquals(201, span.data["status_code"]) + } + 1 -> { + assertEquals("proxySelect", span.description) + assertNotNull(span.data["proxies"]) + } + 2 -> { + assertEquals("dns", span.description) + assertNotNull(span.data["domain_name"]) + assertNotNull(span.data["dns_addresses"]) + } + 3 -> { + assertEquals("connect", span.description) + } + 4 -> { + assertEquals("connection", span.description) + } + 5 -> { + assertEquals("requestHeaders", span.description) + } + 6 -> { + assertEquals("responseHeaders", span.description) + assertEquals(201, span.data["status_code"]) + } + 7 -> { + assertEquals("responseBody", span.description) + } + } + } + } + + @Test + fun `has requestBody span for requests with body`() { + val sut = fixture.getSut() + val requestBody = "request body sent in the request" + val request = postRequest(body = requestBody) + val call = sut.newCall(request) + val response = call.execute() + val okHttpEvent = SentryOkHttpEventListener.eventMap[call] + val callSpan = okHttpEvent?.callRootSpan + response.close() + assertEquals(9, fixture.sentryTracer.children.size) + val requestBodySpan = fixture.sentryTracer.children.firstOrNull { it.description == "requestBody" } + assertNotNull(requestBodySpan) + assertEquals(requestBody.toByteArray().size.toLong(), requestBodySpan.data["request_body_size"]) + assertEquals(requestBody.toByteArray().size.toLong(), callSpan?.getData("request_body_size")) + } + + @Test + fun `has response_body_size data if body is consumed`() { + val sut = fixture.getSut() + val requestBody = "request body sent in the request" + val request = postRequest(body = requestBody) + val call = sut.newCall(request) + val response = call.execute() + val okHttpEvent = SentryOkHttpEventListener.eventMap[call] + val callSpan = okHttpEvent?.callRootSpan + + // Consume the response + val responseBytes = response.body?.byteStream()?.readBytes() + assertNotNull(responseBytes) + + response.close() + val requestBodySpan = fixture.sentryTracer.children.firstOrNull { it.description == "responseBody" } + assertNotNull(requestBodySpan) + assertEquals(responseBytes.size.toLong(), requestBodySpan.data["response_body_size"]) + assertEquals(responseBytes.size.toLong(), callSpan?.getData("response_body_size")) + } + + @Test + fun `root call span status depends on http status code`() { + val sut = fixture.getSut(httpStatusCode = 404) + val request = getRequest() + val call = sut.newCall(request) + val response = call.execute() + val okHttpEvent = SentryOkHttpEventListener.eventMap[call] + val callSpan = okHttpEvent?.callRootSpan + response.close() + assertNotNull(callSpan) + assertEquals(SpanStatus.fromHttpStatusCode(404), callSpan.status) + } +} diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt new file mode 100644 index 0000000000..f47caf2cf0 --- /dev/null +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt @@ -0,0 +1,390 @@ +package io.sentry.android.okhttp + +import io.sentry.Breadcrumb +import io.sentry.IHub +import io.sentry.ISpan +import io.sentry.SentryOptions +import io.sentry.SentryTracer +import io.sentry.Span +import io.sentry.SpanOptions +import io.sentry.TracesSamplingDecision +import io.sentry.TransactionContext +import io.sentry.TypeCheckHint +import io.sentry.test.getProperty +import okhttp3.OkHttpClient +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.mockwebserver.MockWebServer +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.check +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import java.util.UUID +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class SentryOkHttpEventTest { + private class Fixture { + val hub = mock() + val server = MockWebServer() + val span: ISpan + val request: Request + val response: Response + + init { + whenever(hub.options).thenReturn( + SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + } + ) + + span = Span( + TransactionContext("name", "op", TracesSamplingDecision(true)), + SentryTracer(TransactionContext("name", "op", TracesSamplingDecision(true)), hub), + hub, + null, + SpanOptions() + ) + + request = Request.Builder() + .addHeader("myHeader", "myValue") + .get() + .url(server.url("/hello")) + .build() + + response = Response.Builder() + .code(200) + .message("message") + .request(request) + .protocol(Protocol.HTTP_1_1) + .build() + } + + fun getSut(currentSpan: ISpan? = span, requestUrl: String ? = null): SentryOkHttpEvent { + whenever(hub.span).thenReturn(currentSpan) + val okhttpClient = OkHttpClient.Builder().build() + val call = if (requestUrl == null) { + okhttpClient.newCall(request) + } else { + okhttpClient.newCall( + Request.Builder() + .addHeader("myHeader", "myValue") + .get() + .url(server.url(requestUrl)) + .build() + ) + } + return SentryOkHttpEvent(hub, call) + } + } + + private val fixture = Fixture() + + @Test + fun `when there is no active span, root span is null`() { + val sut = fixture.getSut(currentSpan = null) + assertNull(sut.callRootSpan) + } + + @Test + fun `when there is an active span, a root span is created`() { + val sut = fixture.getSut() + val callSpan = sut.callRootSpan + assertNotNull(callSpan) + assertEquals("http.client", callSpan.operation) + assertEquals("${fixture.request.method} ${fixture.request.url}", callSpan.description) + assertEquals(fixture.request.url.toString(), callSpan.getData("url")) + assertEquals(fixture.request.url.toString(), callSpan.getData("filtered_url")) + assertEquals(fixture.request.url.host, callSpan.getData("host")) + assertEquals(fixture.request.url.encodedPath, callSpan.getData("path")) + assertEquals(fixture.request.method, callSpan.getData("method")) + assertTrue(callSpan.getData("success") as Boolean) + } + + @Test + fun `filtered_url data in root span does not contain uids or parameters`() { + val sut = fixture.getSut(requestUrl = "/hello/${UUID.randomUUID()}/?param1=1¶m2=2") + val callSpan = sut.callRootSpan + val filteredUrl = callSpan?.getData("filtered_url") + assertNotNull(filteredUrl) + assertEquals(fixture.server.url("/hello/*/").toString(), filteredUrl) + } + + @Test + fun `when root span is null, no breadcrumb is created`() { + val sut = fixture.getSut(currentSpan = null) + assertNull(sut.callRootSpan) + sut.finishEvent() + verify(fixture.hub, never()).addBreadcrumb(any(), anyOrNull()) + } + + @Test + fun `when root span is null, no span is created`() { + val sut = fixture.getSut(currentSpan = null) + assertNull(sut.callRootSpan) + sut.startSpan("span") + assertTrue(sut.getEventSpans().isEmpty()) + } + + @Test + fun `when event is finished, root span is finished`() { + val sut = fixture.getSut() + val rootSpan = sut.callRootSpan + assertNotNull(rootSpan) + assertFalse(rootSpan.isFinished) + sut.finishEvent() + assertTrue(rootSpan.isFinished) + } + + @Test + fun `when startSpan, a new span is started`() { + val sut = fixture.getSut() + assertTrue(sut.getEventSpans().isEmpty()) + sut.startSpan("span") + val spans = sut.getEventSpans() + assertEquals(1, spans.size) + assertTrue(spans.containsKey("span")) + assertFalse(spans["span"]!!.isFinished) + } + + @Test + fun `when finishSpan, a span is finished if previously started`() { + val sut = fixture.getSut() + assertTrue(sut.getEventSpans().isEmpty()) + sut.startSpan("span") + val spans = sut.getEventSpans() + assertFalse(spans["span"]!!.isFinished) + sut.finishSpan("span") + assertTrue(spans["span"]!!.isFinished) + } + + @Test + fun `when finishSpan, a callback is called before the span is finished`() { + val sut = fixture.getSut() + var called = false + assertTrue(sut.getEventSpans().isEmpty()) + sut.startSpan("span") + val spans = sut.getEventSpans() + assertFalse(spans["span"]!!.isFinished) + sut.finishSpan("span") { + called = true + assertFalse(it.isFinished) + } + assertTrue(spans["span"]!!.isFinished) + assertTrue(called) + } + + @Test + fun `when finishSpan, a callback is called with the current span and the root call span is finished`() { + val sut = fixture.getSut() + var called = 0 + sut.startSpan("span") + sut.finishSpan("span") { + if (called == 0) { + assertEquals("span", it.description) + } else { + assertEquals(sut.callRootSpan, it) + } + called++ + assertFalse(it.isFinished) + } + assertEquals(2, called) + } + + @Test + fun `finishSpan is ignored if the span was not previously started`() { + val sut = fixture.getSut() + var called = false + assertTrue(sut.getEventSpans().isEmpty()) + sut.finishSpan("span") { called = true } + assertTrue(sut.getEventSpans().isEmpty()) + assertFalse(called) + } + + @Test + fun `when finishEvent, a callback is called with the call root span before it is finished`() { + val sut = fixture.getSut() + var called = false + sut.finishEvent { + called = true + assertEquals(sut.callRootSpan, it) + } + assertTrue(called) + } + + @Test + fun `when finishEvent, all running spans are finished`() { + val sut = fixture.getSut() + sut.startSpan("span") + val spans = sut.getEventSpans() + assertFalse(spans["span"]!!.isFinished) + sut.finishEvent() + assertTrue(spans["span"]!!.isFinished) + } + + @Test + fun `when finishEvent, a breadcrumb is captured with request in the hint`() { + val sut = fixture.getSut() + sut.finishEvent() + verify(fixture.hub).addBreadcrumb( + check { + assertEquals(fixture.request.url.toString(), it.data["url"]) + assertEquals(fixture.request.url.toString(), it.data["filtered_url"]) + assertEquals(fixture.request.url.host, it.data["host"]) + assertEquals(fixture.request.url.encodedPath, it.data["path"]) + assertEquals(fixture.request.method, it.data["method"]) + assertTrue(it.data["success"] as Boolean) + }, + check { + assertEquals(fixture.request, it[TypeCheckHint.OKHTTP_REQUEST]) + } + ) + } + + @Test + fun `setResponse set protocol and code in the breadcrumb and root span, and response in the hint`() { + val sut = fixture.getSut() + sut.setResponse(fixture.response) + + assertEquals(fixture.response.protocol.name, sut.callRootSpan?.getData("protocol")) + assertEquals(fixture.response.code, sut.callRootSpan?.getData("status_code")) + sut.finishEvent() + + verify(fixture.hub).addBreadcrumb( + check { + assertEquals(fixture.response.protocol.name, it.data["protocol"]) + assertEquals(fixture.response.code, it.data["status_code"]) + }, + check { + assertEquals(fixture.response, it[TypeCheckHint.OKHTTP_RESPONSE]) + } + ) + } + + @Test + fun `setProtocol set protocol in the breadcrumb and in the root span`() { + val sut = fixture.getSut() + sut.setProtocol("protocol") + assertEquals("protocol", sut.callRootSpan?.getData("protocol")) + sut.finishEvent() + verify(fixture.hub).addBreadcrumb( + check { + assertEquals("protocol", it.data["protocol"]) + }, + any() + ) + } + + @Test + fun `setProtocol is ignored if protocol is null`() { + val sut = fixture.getSut() + sut.setProtocol(null) + assertNull(sut.callRootSpan?.getData("protocol")) + sut.finishEvent() + verify(fixture.hub).addBreadcrumb( + check { + assertNull(it.data["protocol"]) + }, + any() + ) + } + + @Test + fun `setRequestBodySize set RequestBodySize in the breadcrumb and in the root span`() { + val sut = fixture.getSut() + sut.setRequestBodySize(10) + assertEquals(10L, sut.callRootSpan?.getData("request_body_size")) + sut.finishEvent() + verify(fixture.hub).addBreadcrumb( + check { + assertEquals(10L, it.data["request_body_size"]) + }, + any() + ) + } + + @Test + fun `setRequestBodySize is ignored if RequestBodySize is negative`() { + val sut = fixture.getSut() + sut.setRequestBodySize(-1) + assertNull(sut.callRootSpan?.getData("request_body_size")) + sut.finishEvent() + verify(fixture.hub).addBreadcrumb( + check { + assertNull(it.data["request_body_size"]) + }, + any() + ) + } + + @Test + fun `setResponseBodySize set ResponseBodySize in the breadcrumb and in the root span`() { + val sut = fixture.getSut() + sut.setResponseBodySize(10) + assertEquals(10L, sut.callRootSpan?.getData("response_body_size")) + sut.finishEvent() + verify(fixture.hub).addBreadcrumb( + check { + assertEquals(10L, it.data["response_body_size"]) + }, + any() + ) + } + + @Test + fun `setResponseBodySize is ignored if ResponseBodySize is negative`() { + val sut = fixture.getSut() + sut.setResponseBodySize(-1) + assertNull(sut.callRootSpan?.getData("response_body_size")) + sut.finishEvent() + verify(fixture.hub).addBreadcrumb( + check { + assertNull(it.data["response_body_size"]) + }, + any() + ) + } + + @Test + fun `setError set success to false and errorMessage in the breadcrumb and in the root span`() { + val sut = fixture.getSut() + sut.setError("errorMessage") + assertFalse(sut.callRootSpan?.getData("success") as Boolean) + assertEquals("errorMessage", sut.callRootSpan.getData("error_message")) + sut.finishEvent() + verify(fixture.hub).addBreadcrumb( + check { + assertFalse(it.data["success"] as Boolean) + assertEquals("errorMessage", it.data["error_message"]) + }, + any() + ) + } + + @Test + fun `setError sets success to false in the breadcrumb and in the root span even if errorMessage is null`() { + val sut = fixture.getSut() + sut.setError(null) + assertFalse(sut.callRootSpan?.getData("success") as Boolean) + assertNull(sut.callRootSpan.getData("error_message")) + sut.finishEvent() + verify(fixture.hub).addBreadcrumb( + check { + assertFalse(it.data["success"] as Boolean) + assertNull(it.data["error_message"]) + }, + any() + ) + } + + /** Retrieve all the spans started in the event using reflection. */ + private fun SentryOkHttpEvent.getEventSpans() = getProperty>("eventSpans") +} diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt index eed10ea8c6..b9620615d8 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt @@ -510,4 +510,15 @@ class SentryOkHttpInterceptorTest { } verify(fixture.hub, never()).captureEvent(any(), any()) } + + @Test + fun `when a call is captured by SentryOkHttpEventListener no span nor breadcrumb is created`() { + val sut = fixture.getSut(responseBody = "response body") + val call = sut.newCall(postRequest()) + SentryOkHttpEventListener.eventMap[call] = mock() + call.execute() + val httpClientSpan = fixture.sentryTracer.children.firstOrNull() + assertNull(httpClientSpan) + verify(fixture.hub, never()).addBreadcrumb(any(), anyOrNull()) + } } From 6853cc5016d6dc25777cd90dd3a3bb564de41901 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 17 Apr 2023 12:09:39 +0200 Subject: [PATCH 03/10] updated changelog and sample app --- CHANGELOG.md | 1 + .../java/io/sentry/samples/android/GithubAPI.kt | 16 +++++++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 100cbfaca0..e5fd6c7675 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Features +- Add OkHttp event spans ([#2659](https://github.com/getsentry/sentry-java/pull/2659)) - Attach Trace Context when an ANR is detected (ANRv1) ([#2583](https://github.com/getsentry/sentry-java/pull/2583)) - Make log4j2 integration compatible with log4j 3.0 ([#2634](https://github.com/getsentry/sentry-java/pull/2634)) - Instead of relying on package scanning, we now use an annotation processor to generate `Log4j2Plugins.dat` diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt index aef2b64a0e..09ad9a2d07 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/GithubAPI.kt @@ -1,6 +1,7 @@ package io.sentry.samples.android import io.sentry.HttpStatusCodeRange +import io.sentry.android.okhttp.SentryOkHttpEventListener import io.sentry.android.okhttp.SentryOkHttpInterceptor import okhttp3.OkHttpClient import retrofit2.Retrofit @@ -8,14 +9,15 @@ import retrofit2.converter.gson.GsonConverterFactory object GithubAPI { - private val client = OkHttpClient.Builder().addInterceptor( - SentryOkHttpInterceptor( - captureFailedRequests = true, - failedRequestStatusCodes = listOf( - HttpStatusCodeRange(400, 599) + private val client = OkHttpClient.Builder().eventListener(SentryOkHttpEventListener()) + .addInterceptor( + SentryOkHttpInterceptor( + captureFailedRequests = true, + failedRequestStatusCodes = listOf( + HttpStatusCodeRange(400, 599) + ) ) - ) - ).build() + ).build() private val retrofit = Retrofit.Builder() .baseUrl("https://api.github.com/") From b1d9eab7446dd6cb8a5001e8a3f13d6edf812fc3 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 21 Apr 2023 12:18:36 +0200 Subject: [PATCH 04/10] moved uuid regex as static member --- .../main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt index 7a751c7751..5a20aa3683 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt @@ -11,6 +11,8 @@ import okhttp3.Call import okhttp3.Response import java.net.URL +private val uuidRegex by lazy { Regex("[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}") } + internal class SentryOkHttpEvent(private val hub: IHub, private val call: Call) { private val eventSpans: MutableMap = HashMap() private val breadcrumb: Breadcrumb @@ -48,7 +50,6 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val call: Call) private fun trimUrl(url: String): String { // Remove any uuid from the url and replace it with a "*" - val uuidRegex = Regex("[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}") val trimmedUrl = url.replace(uuidRegex, "*") if (URL(trimmedUrl).query == null) { return trimmedUrl From a8b3171d9593911df71ea6cda5e05f2d2e313de4 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 26 Apr 2023 13:53:05 +0200 Subject: [PATCH 05/10] updated changelog SentryOkHttpEventListener now accepts an instance of EventListener or EventListener.Factory and propagate all the calls to it --- CHANGELOG.md | 9 +- .../api/sentry-android-okhttp.api | 8 +- .../okhttp/SentryOkHttpEventListener.kt | 53 ++++++- .../okhttp/SentryOkHttpEventListenerTest.kt | 148 +++++++++++++++++- 4 files changed, 206 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d60bfce80..922eda3dd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,14 @@ ### Features -- Add OkHttp event spans ([#2659](https://github.com/getsentry/sentry-java/pull/2659)) +- More granular http requests instrumentation with a new SentryOkHttpEventListener ([#2659](https://github.com/getsentry/sentry-java/pull/2659)) + - Create spans for time spent on: + - Proxy selection + - DNS resolution + - HTTPS setup + - Connection + - Requesting headers + - Receiving response - Attach Trace Context when an ANR is detected (ANRv1) ([#2583](https://github.com/getsentry/sentry-java/pull/2583)) - Make log4j2 integration compatible with log4j 3.0 ([#2634](https://github.com/getsentry/sentry-java/pull/2634)) - Instead of relying on package scanning, we now use an annotation processor to generate `Log4j2Plugins.dat` diff --git a/sentry-android-okhttp/api/sentry-android-okhttp.api b/sentry-android-okhttp/api/sentry-android-okhttp.api index 8dc1a8e55f..ec5a385f94 100644 --- a/sentry-android-okhttp/api/sentry-android-okhttp.api +++ b/sentry-android-okhttp/api/sentry-android-okhttp.api @@ -9,8 +9,12 @@ public final class io/sentry/android/okhttp/BuildConfig { public final class io/sentry/android/okhttp/SentryOkHttpEventListener : okhttp3/EventListener { public static final field Companion Lio/sentry/android/okhttp/SentryOkHttpEventListener$Companion; public fun ()V - public fun (Lio/sentry/IHub;)V - public synthetic fun (Lio/sentry/IHub;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lio/sentry/IHub;Lkotlin/jvm/functions/Function1;)V + public synthetic fun (Lio/sentry/IHub;Lkotlin/jvm/functions/Function1;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lio/sentry/IHub;Lokhttp3/EventListener$Factory;)V + public synthetic fun (Lio/sentry/IHub;Lokhttp3/EventListener$Factory;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lio/sentry/IHub;Lokhttp3/EventListener;)V + public synthetic fun (Lio/sentry/IHub;Lokhttp3/EventListener;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun callEnd (Lokhttp3/Call;)V public fun callFailed (Lokhttp3/Call;Ljava/io/IOException;)V public fun callStart (Lokhttp3/Call;)V diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt index e013d8d7ea..8a4b770e6d 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt @@ -27,12 +27,24 @@ import java.net.Proxy * .addInterceptor(SentryOkHttpInterceptor()) * .build() * ``` + * + * If you already use a [OkHttpClient.eventListener], you can pass it in the constructor. + * + * ``` + * val client = OkHttpClient.Builder() + * .eventListener(SentryOkHttpEventListener(myEventListener)) + * .addInterceptor(SentryOkHttpInterceptor()) + * .build() + * ``` */ @Suppress("TooManyFunctions") class SentryOkHttpEventListener( - private val hub: IHub = HubAdapter.getInstance() + private val hub: IHub = HubAdapter.getInstance(), + private val originalEventListenerCreator: ((call: Call) -> EventListener)? = null ) : EventListener() { + private var originalEventListener: EventListener? = null + companion object { private const val PROXY_SELECT_EVENT = "proxySelect" private const val DNS_EVENT = "dns" @@ -47,11 +59,24 @@ class SentryOkHttpEventListener( internal val eventMap: MutableMap = HashMap() } + constructor(hub: IHub = HubAdapter.getInstance(), originalEventListener: EventListener) : this( + hub, + originalEventListenerCreator = { originalEventListener } + ) + + constructor(hub: IHub = HubAdapter.getInstance(), originalEventListenerFactory: Factory) : this( + hub, + originalEventListenerCreator = { originalEventListenerFactory.create(it) } + ) + override fun callStart(call: Call) { + originalEventListener = originalEventListenerCreator?.invoke(call) + originalEventListener?.callStart(call) eventMap[call] = SentryOkHttpEvent(hub, call) } override fun proxySelectStart(call: Call, url: HttpUrl) { + originalEventListener?.proxySelectStart(call, url) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(PROXY_SELECT_EVENT) } @@ -61,6 +86,7 @@ class SentryOkHttpEventListener( url: HttpUrl, proxies: List ) { + originalEventListener?.proxySelectEnd(call, url, proxies) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.finishSpan(PROXY_SELECT_EVENT) { if (proxies.isNotEmpty()) { @@ -70,6 +96,7 @@ class SentryOkHttpEventListener( } override fun dnsStart(call: Call, domainName: String) { + originalEventListener?.dnsStart(call, domainName) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(DNS_EVENT) } @@ -79,6 +106,7 @@ class SentryOkHttpEventListener( domainName: String, inetAddressList: List ) { + originalEventListener?.dnsEnd(call, domainName, inetAddressList) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.finishSpan(DNS_EVENT) { it.setData("domain_name", domainName) @@ -93,16 +121,19 @@ class SentryOkHttpEventListener( inetSocketAddress: InetSocketAddress, proxy: Proxy ) { + originalEventListener?.connectStart(call, inetSocketAddress, proxy) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(CONNECT_EVENT) } override fun secureConnectStart(call: Call) { + originalEventListener?.secureConnectStart(call) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(SECURE_CONNECT_EVENT) } override fun secureConnectEnd(call: Call, handshake: Handshake?) { + originalEventListener?.secureConnectEnd(call, handshake) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.finishSpan(SECURE_CONNECT_EVENT) } @@ -113,6 +144,7 @@ class SentryOkHttpEventListener( proxy: Proxy, protocol: Protocol? ) { + originalEventListener?.connectEnd(call, inetSocketAddress, proxy, protocol) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.setProtocol(protocol?.name) okHttpEvent.finishSpan(CONNECT_EVENT) @@ -125,6 +157,7 @@ class SentryOkHttpEventListener( protocol: Protocol?, ioe: IOException ) { + originalEventListener?.connectFailed(call, inetSocketAddress, proxy, protocol, ioe) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.setProtocol(protocol?.name) okHttpEvent.setError(ioe.message) @@ -135,31 +168,37 @@ class SentryOkHttpEventListener( } override fun connectionAcquired(call: Call, connection: Connection) { + originalEventListener?.connectionAcquired(call, connection) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(CONNECTION_EVENT) } override fun connectionReleased(call: Call, connection: Connection) { + originalEventListener?.connectionReleased(call, connection) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.finishSpan(CONNECTION_EVENT) } override fun requestHeadersStart(call: Call) { + originalEventListener?.requestHeadersStart(call) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(REQUEST_HEADERS_EVENT) } override fun requestHeadersEnd(call: Call, request: Request) { + originalEventListener?.requestHeadersEnd(call, request) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.finishSpan(REQUEST_HEADERS_EVENT) } override fun requestBodyStart(call: Call) { + originalEventListener?.requestBodyStart(call) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(REQUEST_BODY_EVENT) } override fun requestBodyEnd(call: Call, byteCount: Long) { + originalEventListener?.requestBodyEnd(call, byteCount) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.finishSpan(REQUEST_BODY_EVENT) { if (byteCount > 0) { @@ -170,6 +209,7 @@ class SentryOkHttpEventListener( } override fun requestFailed(call: Call, ioe: IOException) { + originalEventListener?.requestFailed(call, ioe) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.setError(ioe.message) // requestFailed can happen after requestHeaders or requestBody. @@ -187,11 +227,13 @@ class SentryOkHttpEventListener( } override fun responseHeadersStart(call: Call) { + originalEventListener?.responseHeadersStart(call) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(RESPONSE_HEADERS_EVENT) } override fun responseHeadersEnd(call: Call, response: Response) { + originalEventListener?.responseHeadersEnd(call, response) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.setResponse(response) okHttpEvent.finishSpan(RESPONSE_HEADERS_EVENT) { @@ -201,11 +243,13 @@ class SentryOkHttpEventListener( } override fun responseBodyStart(call: Call) { + originalEventListener?.responseBodyStart(call) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(RESPONSE_BODY_EVENT) } override fun responseBodyEnd(call: Call, byteCount: Long) { + originalEventListener?.responseBodyEnd(call, byteCount) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.setResponseBodySize(byteCount) okHttpEvent.finishSpan(RESPONSE_BODY_EVENT) { @@ -216,6 +260,7 @@ class SentryOkHttpEventListener( } override fun responseFailed(call: Call, ioe: IOException) { + originalEventListener?.responseFailed(call, ioe) val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.setError(ioe.message) // responseFailed can happen after responseHeaders or responseBody. @@ -233,12 +278,14 @@ class SentryOkHttpEventListener( } override fun callEnd(call: Call) { - val okHttpEvent = eventMap.remove(call) ?: return + originalEventListener?.callEnd(call) + val okHttpEvent: SentryOkHttpEvent = eventMap.remove(call) ?: return okHttpEvent.finishEvent() } override fun callFailed(call: Call, ioe: IOException) { - val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return + originalEventListener?.callFailed(call, ioe) + val okHttpEvent: SentryOkHttpEvent = eventMap.remove(call) ?: return okHttpEvent.setError(ioe.message) okHttpEvent.finishEvent { it.status = SpanStatus.INTERNAL_ERROR diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt index 76c4c0c4fb..c9310b44b2 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt @@ -7,13 +7,19 @@ import io.sentry.SentryTraceHeader import io.sentry.SentryTracer import io.sentry.SpanStatus import io.sentry.TransactionContext +import okhttp3.EventListener import okhttp3.OkHttpClient +import okhttp3.Protocol import okhttp3.Request import okhttp3.RequestBody.Companion.toRequestBody +import okhttp3.Response import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy +import org.mockito.kotlin.any +import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import kotlin.test.Test import kotlin.test.assertEquals @@ -26,17 +32,24 @@ class SentryOkHttpEventListenerTest { class Fixture { val hub = mock() val server = MockWebServer() + val mockEventListener = mock() + val mockEventListenerFactory = mock() lateinit var sentryTracer: SentryTracer lateinit var options: SentryOptions + lateinit var sentryOkHttpEventListener: SentryOkHttpEventListener + + init { + whenever(mockEventListenerFactory.create(any())).thenReturn(mockEventListener) + } @SuppressWarnings("LongParameterList") fun getSut( isSpanActive: Boolean = true, useInterceptor: Boolean = false, httpStatusCode: Int = 201, - responseBody: String = "success", - socketPolicy: SocketPolicy = SocketPolicy.KEEP_OPEN, - sendDefaultPii: Boolean = false + sendDefaultPii: Boolean = false, + eventListener: EventListener? = null, + eventListenerFactory: EventListener.Factory? = null ): OkHttpClient { options = SentryOptions().apply { dsn = "https://key@sentry.io/proj" @@ -51,9 +64,9 @@ class SentryOkHttpEventListenerTest { } server.enqueue( MockResponse() - .setBody(responseBody) + .setBody("responseBody") .addHeader("myResponseHeader", "myValue") - .setSocketPolicy(socketPolicy) + .setSocketPolicy(SocketPolicy.KEEP_OPEN) .setResponseCode(httpStatusCode) ) @@ -61,7 +74,12 @@ class SentryOkHttpEventListenerTest { if (useInterceptor) { builder.addInterceptor(SentryOkHttpInterceptor(hub)) } - return builder.eventListener(SentryOkHttpEventListener(hub)).build() + sentryOkHttpEventListener = when { + eventListenerFactory != null -> SentryOkHttpEventListener(hub, eventListenerFactory) + eventListener != null -> SentryOkHttpEventListener(hub, eventListener) + else -> SentryOkHttpEventListener(hub) + } + return builder.eventListener(sentryOkHttpEventListener).build() } } @@ -219,4 +237,122 @@ class SentryOkHttpEventListenerTest { assertNotNull(callSpan) assertEquals(SpanStatus.fromHttpStatusCode(404), callSpan.status) } + + @Test + fun `propagate all calls to the event listener passed in the ctor`() { + val sut = fixture.getSut(eventListener = fixture.mockEventListener, httpStatusCode = 500) + val listener = fixture.sentryOkHttpEventListener + val request = postRequest(body = "requestBody") + val call = sut.newCall(request) + val response = mock() + whenever(response.protocol).thenReturn(Protocol.HTTP_1_1) + + listener.callStart(call) + verify(fixture.mockEventListener).callStart(eq(call)) + listener.proxySelectStart(call, mock()) + verify(fixture.mockEventListener).proxySelectStart(eq(call), any()) + listener.proxySelectEnd(call, mock(), listOf(mock())) + verify(fixture.mockEventListener).proxySelectEnd(eq(call), any(), any()) + listener.dnsStart(call, "domainName") + verify(fixture.mockEventListener).dnsStart(eq(call), eq("domainName")) + listener.dnsEnd(call, "domainName", listOf(mock())) + verify(fixture.mockEventListener).dnsEnd(eq(call), eq("domainName"), any()) + listener.connectStart(call, mock(), mock()) + verify(fixture.mockEventListener).connectStart(eq(call), any(), any()) + listener.secureConnectStart(call) + verify(fixture.mockEventListener).secureConnectStart(eq(call)) + listener.secureConnectEnd(call, mock()) + verify(fixture.mockEventListener).secureConnectEnd(eq(call), any()) + listener.connectEnd(call, mock(), mock(), mock()) + verify(fixture.mockEventListener).connectEnd(eq(call), any(), any(), any()) + listener.connectFailed(call, mock(), mock(), mock(), mock()) + verify(fixture.mockEventListener).connectFailed(eq(call), any(), any(), any(), any()) + listener.connectionAcquired(call, mock()) + verify(fixture.mockEventListener).connectionAcquired(eq(call), any()) + listener.connectionReleased(call, mock()) + verify(fixture.mockEventListener).connectionReleased(eq(call), any()) + listener.requestHeadersStart(call) + verify(fixture.mockEventListener).requestHeadersStart(eq(call)) + listener.requestHeadersEnd(call, mock()) + verify(fixture.mockEventListener).requestHeadersEnd(eq(call), any()) + listener.requestBodyStart(call) + verify(fixture.mockEventListener).requestBodyStart(eq(call)) + listener.requestBodyEnd(call, 10) + verify(fixture.mockEventListener).requestBodyEnd(eq(call), eq(10)) + listener.requestFailed(call, mock()) + verify(fixture.mockEventListener).requestFailed(eq(call), any()) + listener.responseHeadersStart(call) + verify(fixture.mockEventListener).responseHeadersStart(eq(call)) + listener.responseHeadersEnd(call, response) + verify(fixture.mockEventListener).responseHeadersEnd(eq(call), any()) + listener.responseBodyStart(call) + verify(fixture.mockEventListener).responseBodyStart(eq(call)) + listener.responseBodyEnd(call, 10) + verify(fixture.mockEventListener).responseBodyEnd(eq(call), eq(10)) + listener.responseFailed(call, mock()) + verify(fixture.mockEventListener).responseFailed(eq(call), any()) + listener.callEnd(call) + verify(fixture.mockEventListener).callEnd(eq(call)) + listener.callFailed(call, mock()) + verify(fixture.mockEventListener).callFailed(eq(call), any()) + } + + @Test + fun `propagate all calls to the event listener factory passed in the ctor`() { + val sut = fixture.getSut(eventListenerFactory = fixture.mockEventListenerFactory, httpStatusCode = 500) + val listener = fixture.sentryOkHttpEventListener + val request = postRequest(body = "requestBody") + val call = sut.newCall(request) + val response = mock() + whenever(response.protocol).thenReturn(Protocol.HTTP_1_1) + + listener.callStart(call) + verify(fixture.mockEventListener).callStart(eq(call)) + listener.proxySelectStart(call, mock()) + verify(fixture.mockEventListener).proxySelectStart(eq(call), any()) + listener.proxySelectEnd(call, mock(), listOf(mock())) + verify(fixture.mockEventListener).proxySelectEnd(eq(call), any(), any()) + listener.dnsStart(call, "domainName") + verify(fixture.mockEventListener).dnsStart(eq(call), eq("domainName")) + listener.dnsEnd(call, "domainName", listOf(mock())) + verify(fixture.mockEventListener).dnsEnd(eq(call), eq("domainName"), any()) + listener.connectStart(call, mock(), mock()) + verify(fixture.mockEventListener).connectStart(eq(call), any(), any()) + listener.secureConnectStart(call) + verify(fixture.mockEventListener).secureConnectStart(eq(call)) + listener.secureConnectEnd(call, mock()) + verify(fixture.mockEventListener).secureConnectEnd(eq(call), any()) + listener.connectEnd(call, mock(), mock(), mock()) + verify(fixture.mockEventListener).connectEnd(eq(call), any(), any(), any()) + listener.connectFailed(call, mock(), mock(), mock(), mock()) + verify(fixture.mockEventListener).connectFailed(eq(call), any(), any(), any(), any()) + listener.connectionAcquired(call, mock()) + verify(fixture.mockEventListener).connectionAcquired(eq(call), any()) + listener.connectionReleased(call, mock()) + verify(fixture.mockEventListener).connectionReleased(eq(call), any()) + listener.requestHeadersStart(call) + verify(fixture.mockEventListener).requestHeadersStart(eq(call)) + listener.requestHeadersEnd(call, mock()) + verify(fixture.mockEventListener).requestHeadersEnd(eq(call), any()) + listener.requestBodyStart(call) + verify(fixture.mockEventListener).requestBodyStart(eq(call)) + listener.requestBodyEnd(call, 10) + verify(fixture.mockEventListener).requestBodyEnd(eq(call), eq(10)) + listener.requestFailed(call, mock()) + verify(fixture.mockEventListener).requestFailed(eq(call), any()) + listener.responseHeadersStart(call) + verify(fixture.mockEventListener).responseHeadersStart(eq(call)) + listener.responseHeadersEnd(call, response) + verify(fixture.mockEventListener).responseHeadersEnd(eq(call), any()) + listener.responseBodyStart(call) + verify(fixture.mockEventListener).responseBodyStart(eq(call)) + listener.responseBodyEnd(call, 10) + verify(fixture.mockEventListener).responseBodyEnd(eq(call), eq(10)) + listener.responseFailed(call, mock()) + verify(fixture.mockEventListener).responseFailed(eq(call), any()) + listener.callEnd(call) + verify(fixture.mockEventListener).callEnd(eq(call)) + listener.callFailed(call, mock()) + verify(fixture.mockEventListener).callFailed(eq(call), any()) + } } From d2cd6c5c0c26c024d045fd2c24fc127c83072a50 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 8 May 2023 13:36:38 +0200 Subject: [PATCH 06/10] updated span data to align with https://develop.sentry.dev/sdk/performance/span-data-conventions/ OkHttp event spans are now children of specific spans. E.g. secureConnect is a child of connect updated changelog --- CHANGELOG.md | 2 + .../android/okhttp/SentryOkHttpEvent.kt | 33 +++++-- .../okhttp/SentryOkHttpEventListener.kt | 22 ++--- .../android/okhttp/SentryOkHttpInterceptor.kt | 4 +- .../okhttp/SentryOkHttpEventListenerTest.kt | 8 +- .../android/okhttp/SentryOkHttpEventTest.kt | 95 +++++++++++++++++-- .../okhttp/SentryOkHttpInterceptorTest.kt | 4 +- 7 files changed, 132 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 922eda3dd5..45e3380d2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ - Connection - Requesting headers - Receiving response + - You can attach the event listener to your OkHttpClient through `client.eventListener(new SentryOkHttpEventListener()).addInterceptor(new SentryOkHttpInterceptor()).build();` + - In case you already have an event listener you can use the SentryOkHttpEventListener as well through `client.eventListener(new SentryOkHttpEventListener(myListener)).addInterceptor(new SentryOkHttpInterceptor()).build();` - Attach Trace Context when an ANR is detected (ANRv1) ([#2583](https://github.com/getsentry/sentry-java/pull/2583)) - Make log4j2 integration compatible with log4j 3.0 ([#2634](https://github.com/getsentry/sentry-java/pull/2634)) - Instead of relying on package scanning, we now use an annotation processor to generate `Log4j2Plugins.dat` diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt index 5a20aa3683..230ab38c0e 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt @@ -6,6 +6,13 @@ import io.sentry.IHub import io.sentry.ISpan import io.sentry.SpanStatus import io.sentry.TypeCheckHint +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.CONNECTION_EVENT +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.CONNECT_EVENT +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.REQUEST_BODY_EVENT +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.REQUEST_HEADERS_EVENT +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_BODY_EVENT +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_HEADERS_EVENT +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.SECURE_CONNECT_EVENT import io.sentry.util.UrlUtils import okhttp3.Call import okhttp3.Response @@ -30,13 +37,15 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val call: Call) // We start the call span that will contain all the others callRootSpan = hub.span?.startChild("http.client", "$method $url") + urlDetails.applyToSpan(callRootSpan) + // We setup a breadcrumb with all meaningful data breadcrumb = Breadcrumb.http(url, method) breadcrumb.setData("url", url) breadcrumb.setData("filtered_url", trimmedUrl) breadcrumb.setData("host", host) breadcrumb.setData("path", encodedPath) - breadcrumb.setData("method", method) + breadcrumb.setData("http.method", method) breadcrumb.setData("success", true) // We add the same data to the root call span @@ -44,7 +53,7 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val call: Call) callRootSpan?.setData("filtered_url", trimmedUrl) callRootSpan?.setData("host", host) callRootSpan?.setData("path", encodedPath) - callRootSpan?.setData("method", method) + callRootSpan?.setData("http.method", method) callRootSpan?.setData("success", true) } @@ -80,15 +89,15 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val call: Call) fun setRequestBodySize(byteCount: Long) { if (byteCount > -1) { - breadcrumb.setData("request_body_size", byteCount) - callRootSpan?.setData("request_body_size", byteCount) + breadcrumb.setData("http.request_content_length", byteCount) + callRootSpan?.setData("http.request_content_length", byteCount) } } fun setResponseBodySize(byteCount: Long) { if (byteCount > -1) { - breadcrumb.setData("response_body_size", byteCount) - callRootSpan?.setData("response_body_size", byteCount) + breadcrumb.setData("http.response_content_length", byteCount) + callRootSpan?.setData("http.response_content_length", byteCount) } } @@ -107,7 +116,17 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val call: Call) /** Starts a span, if the callRootSpan is not null. */ fun startSpan(event: String) { - val span = callRootSpan?.startChild("http.client", event) ?: return + // Find the parent of the span being created. E.g. secureConnect is child of connect + val parentSpan = when (event) { + // PROXY_SELECT, DNS, CONNECT and CONNECTION are not children of one another + SECURE_CONNECT_EVENT -> eventSpans[CONNECT_EVENT] + REQUEST_HEADERS_EVENT -> eventSpans[CONNECTION_EVENT] + REQUEST_BODY_EVENT -> eventSpans[CONNECTION_EVENT] + RESPONSE_HEADERS_EVENT -> eventSpans[CONNECTION_EVENT] + RESPONSE_BODY_EVENT -> eventSpans[CONNECTION_EVENT] + else -> callRootSpan + } ?: callRootSpan + val span = parentSpan?.startChild("http.client", event) ?: return eventSpans[event] = span } diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt index 8a4b770e6d..22f8f0915a 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt @@ -46,15 +46,15 @@ class SentryOkHttpEventListener( private var originalEventListener: EventListener? = null companion object { - private const val PROXY_SELECT_EVENT = "proxySelect" - private const val DNS_EVENT = "dns" - private const val SECURE_CONNECT_EVENT = "secureConnect" - private const val CONNECT_EVENT = "connect" - private const val CONNECTION_EVENT = "connection" - private const val REQUEST_HEADERS_EVENT = "requestHeaders" - private const val REQUEST_BODY_EVENT = "requestBody" - private const val RESPONSE_HEADERS_EVENT = "responseHeaders" - private const val RESPONSE_BODY_EVENT = "responseBody" + internal const val PROXY_SELECT_EVENT = "proxySelect" + internal const val DNS_EVENT = "dns" + internal const val SECURE_CONNECT_EVENT = "secureConnect" + internal const val CONNECT_EVENT = "connect" + internal const val CONNECTION_EVENT = "connection" + internal const val REQUEST_HEADERS_EVENT = "requestHeaders" + internal const val REQUEST_BODY_EVENT = "requestBody" + internal const val RESPONSE_HEADERS_EVENT = "responseHeaders" + internal const val RESPONSE_BODY_EVENT = "responseBody" internal val eventMap: MutableMap = HashMap() } @@ -202,7 +202,7 @@ class SentryOkHttpEventListener( val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.finishSpan(REQUEST_BODY_EVENT) { if (byteCount > 0) { - it.setData("request_body_size", byteCount) + it.setData("http.request_content_length", byteCount) } } okHttpEvent.setRequestBodySize(byteCount) @@ -254,7 +254,7 @@ class SentryOkHttpEventListener( okHttpEvent.setResponseBodySize(byteCount) okHttpEvent.finishSpan(RESPONSE_BODY_EVENT) { if (byteCount > 0) { - it.setData("response_body_size", byteCount) + it.setData("http.response_content_length", byteCount) } } } 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 index d4b124637a..f434764c69 100644 --- 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 @@ -127,13 +127,13 @@ class SentryOkHttpInterceptor( private fun sendBreadcrumb(request: Request, code: Int?, response: Response?) { val breadcrumb = Breadcrumb.http(request.url.toString(), request.method, code) request.body?.contentLength().ifHasValidLength { - breadcrumb.setData("request_body_size", it) + breadcrumb.setData("http.request_content_length", it) } val hint = Hint().also { it.set(OKHTTP_REQUEST, request) } response?.let { it.body?.contentLength().ifHasValidLength { responseBodySize -> - breadcrumb.setData("response_body_size", responseBodySize) + breadcrumb.setData("http.response_content_length", responseBodySize) } hint[OKHTTP_RESPONSE] = it diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt index c9310b44b2..0bccd8b410 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt @@ -200,8 +200,8 @@ class SentryOkHttpEventListenerTest { assertEquals(9, fixture.sentryTracer.children.size) val requestBodySpan = fixture.sentryTracer.children.firstOrNull { it.description == "requestBody" } assertNotNull(requestBodySpan) - assertEquals(requestBody.toByteArray().size.toLong(), requestBodySpan.data["request_body_size"]) - assertEquals(requestBody.toByteArray().size.toLong(), callSpan?.getData("request_body_size")) + assertEquals(requestBody.toByteArray().size.toLong(), requestBodySpan.data["http.request_content_length"]) + assertEquals(requestBody.toByteArray().size.toLong(), callSpan?.getData("http.request_content_length")) } @Test @@ -221,8 +221,8 @@ class SentryOkHttpEventListenerTest { response.close() val requestBodySpan = fixture.sentryTracer.children.firstOrNull { it.description == "responseBody" } assertNotNull(requestBodySpan) - assertEquals(responseBytes.size.toLong(), requestBodySpan.data["response_body_size"]) - assertEquals(responseBytes.size.toLong(), callSpan?.getData("response_body_size")) + assertEquals(responseBytes.size.toLong(), requestBodySpan.data["http.response_content_length"]) + assertEquals(responseBytes.size.toLong(), callSpan?.getData("http.response_content_length")) } @Test diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt index f47caf2cf0..2b4f1239b3 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt @@ -10,6 +10,13 @@ import io.sentry.SpanOptions import io.sentry.TracesSamplingDecision import io.sentry.TransactionContext import io.sentry.TypeCheckHint +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.CONNECTION_EVENT +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.CONNECT_EVENT +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.REQUEST_BODY_EVENT +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.REQUEST_HEADERS_EVENT +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_BODY_EVENT +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_HEADERS_EVENT +import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.SECURE_CONNECT_EVENT import io.sentry.test.getProperty import okhttp3.OkHttpClient import okhttp3.Protocol @@ -105,7 +112,7 @@ class SentryOkHttpEventTest { assertEquals(fixture.request.url.toString(), callSpan.getData("filtered_url")) assertEquals(fixture.request.url.host, callSpan.getData("host")) assertEquals(fixture.request.url.encodedPath, callSpan.getData("path")) - assertEquals(fixture.request.method, callSpan.getData("method")) + assertEquals(fixture.request.method, callSpan.getData("http.method")) assertTrue(callSpan.getData("success") as Boolean) } @@ -240,7 +247,7 @@ class SentryOkHttpEventTest { assertEquals(fixture.request.url.toString(), it.data["filtered_url"]) assertEquals(fixture.request.url.host, it.data["host"]) assertEquals(fixture.request.url.encodedPath, it.data["path"]) - assertEquals(fixture.request.method, it.data["method"]) + assertEquals(fixture.request.method, it.data["http.method"]) assertTrue(it.data["success"] as Boolean) }, check { @@ -301,11 +308,11 @@ class SentryOkHttpEventTest { fun `setRequestBodySize set RequestBodySize in the breadcrumb and in the root span`() { val sut = fixture.getSut() sut.setRequestBodySize(10) - assertEquals(10L, sut.callRootSpan?.getData("request_body_size")) + assertEquals(10L, sut.callRootSpan?.getData("http.request_content_length")) sut.finishEvent() verify(fixture.hub).addBreadcrumb( check { - assertEquals(10L, it.data["request_body_size"]) + assertEquals(10L, it.data["http.request_content_length"]) }, any() ) @@ -315,11 +322,11 @@ class SentryOkHttpEventTest { fun `setRequestBodySize is ignored if RequestBodySize is negative`() { val sut = fixture.getSut() sut.setRequestBodySize(-1) - assertNull(sut.callRootSpan?.getData("request_body_size")) + assertNull(sut.callRootSpan?.getData("http.request_content_length")) sut.finishEvent() verify(fixture.hub).addBreadcrumb( check { - assertNull(it.data["request_body_size"]) + assertNull(it.data["http.request_content_length"]) }, any() ) @@ -329,11 +336,11 @@ class SentryOkHttpEventTest { fun `setResponseBodySize set ResponseBodySize in the breadcrumb and in the root span`() { val sut = fixture.getSut() sut.setResponseBodySize(10) - assertEquals(10L, sut.callRootSpan?.getData("response_body_size")) + assertEquals(10L, sut.callRootSpan?.getData("http.response_content_length")) sut.finishEvent() verify(fixture.hub).addBreadcrumb( check { - assertEquals(10L, it.data["response_body_size"]) + assertEquals(10L, it.data["http.response_content_length"]) }, any() ) @@ -343,11 +350,11 @@ class SentryOkHttpEventTest { fun `setResponseBodySize is ignored if ResponseBodySize is negative`() { val sut = fixture.getSut() sut.setResponseBodySize(-1) - assertNull(sut.callRootSpan?.getData("response_body_size")) + assertNull(sut.callRootSpan?.getData("http.response_content_length")) sut.finishEvent() verify(fixture.hub).addBreadcrumb( check { - assertNull(it.data["response_body_size"]) + assertNull(it.data["http.response_content_length"]) }, any() ) @@ -385,6 +392,74 @@ class SentryOkHttpEventTest { ) } + @Test + fun `secureConnect span is child of connect span`() { + val sut = fixture.getSut() + sut.startSpan(CONNECT_EVENT) + sut.startSpan(SECURE_CONNECT_EVENT) + val spans = sut.getEventSpans() + val secureConnectSpan = spans[SECURE_CONNECT_EVENT] as Span? + val connectSpan = spans[CONNECT_EVENT] as Span? + assertNotNull(secureConnectSpan) + assertNotNull(connectSpan) + assertEquals(connectSpan.spanId, secureConnectSpan.parentSpanId) + } + + @Test + fun `secureConnect span is child of root span if connect span is not available`() { + val sut = fixture.getSut() + sut.startSpan(SECURE_CONNECT_EVENT) + val spans = sut.getEventSpans() + val rootSpan = sut.callRootSpan as Span? + val secureConnectSpan = spans[SECURE_CONNECT_EVENT] as Span? + assertNotNull(secureConnectSpan) + assertNotNull(rootSpan) + assertEquals(rootSpan.spanId, secureConnectSpan.parentSpanId) + } + + @Test + fun `request and response spans are children of connection span`() { + val sut = fixture.getSut() + sut.startSpan(CONNECTION_EVENT) + sut.startSpan(REQUEST_HEADERS_EVENT) + sut.startSpan(REQUEST_BODY_EVENT) + sut.startSpan(RESPONSE_HEADERS_EVENT) + sut.startSpan(RESPONSE_BODY_EVENT) + val spans = sut.getEventSpans() + val connectionSpan = spans[CONNECTION_EVENT] as Span? + val requestHeadersSpan = spans[REQUEST_HEADERS_EVENT] as Span? + val requestBodySpan = spans[REQUEST_BODY_EVENT] as Span? + val responseHeadersSpan = spans[RESPONSE_HEADERS_EVENT] as Span? + val responseBodySpan = spans[RESPONSE_BODY_EVENT] as Span? + assertNotNull(connectionSpan) + assertEquals(connectionSpan.spanId, requestHeadersSpan?.parentSpanId) + assertEquals(connectionSpan.spanId, requestBodySpan?.parentSpanId) + assertEquals(connectionSpan.spanId, responseHeadersSpan?.parentSpanId) + assertEquals(connectionSpan.spanId, responseBodySpan?.parentSpanId) + } + + @Test + fun `request and response spans are children of root span if connection span is not available`() { + val sut = fixture.getSut() + sut.startSpan(REQUEST_HEADERS_EVENT) + sut.startSpan(REQUEST_BODY_EVENT) + sut.startSpan(RESPONSE_HEADERS_EVENT) + sut.startSpan(RESPONSE_BODY_EVENT) + val spans = sut.getEventSpans() + val connectionSpan = spans[CONNECTION_EVENT] as Span? + val requestHeadersSpan = spans[REQUEST_HEADERS_EVENT] as Span? + val requestBodySpan = spans[REQUEST_BODY_EVENT] as Span? + val responseHeadersSpan = spans[RESPONSE_HEADERS_EVENT] as Span? + val responseBodySpan = spans[RESPONSE_BODY_EVENT] as Span? + val rootSpan = sut.callRootSpan as Span? + assertNotNull(rootSpan) + assertNull(connectionSpan) + assertEquals(rootSpan.spanId, requestHeadersSpan?.parentSpanId) + assertEquals(rootSpan.spanId, requestBodySpan?.parentSpanId) + assertEquals(rootSpan.spanId, responseHeadersSpan?.parentSpanId) + assertEquals(rootSpan.spanId, responseBodySpan?.parentSpanId) + } + /** Retrieve all the spans started in the event using reflection. */ private fun SentryOkHttpEvent.getEventSpans() = getProperty>("eventSpans") } diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt index b9620615d8..6550af9c5c 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt @@ -253,8 +253,8 @@ class SentryOkHttpInterceptorTest { verify(fixture.hub).addBreadcrumb( check { assertEquals("http", it.type) - assertEquals(13L, it.data["response_body_size"]) - assertEquals(12L, it.data["request_body_size"]) + assertEquals(13L, it.data["http.response_content_length"]) + assertEquals(12L, it.data["http.request_content_length"]) }, anyOrNull() ) From 9cd233c8311c2dfdc1b4c577bbdeaa4b1b95e2f6 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 22 May 2023 13:46:34 +0200 Subject: [PATCH 07/10] SentryOkHttpEvent now takes a Request instead of a Call in the ctor Removed `http.` prefix from the breadcrumb Updated sub-spans with operation from `http.client` to `http.client.details` removed filtered_url from SentryOkHttpEvent --- .../android/okhttp/SentryOkHttpEvent.kt | 38 ++--- .../okhttp/SentryOkHttpEventListener.kt | 77 +++++++++- .../okhttp/SentryOkHttpEventListenerTest.kt | 144 +++++++++--------- .../android/okhttp/SentryOkHttpEventTest.kt | 71 ++++----- 4 files changed, 187 insertions(+), 143 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt index 230ab38c0e..43ba274de9 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt @@ -14,25 +14,21 @@ import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_BOD import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_HEADERS_EVENT import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.SECURE_CONNECT_EVENT import io.sentry.util.UrlUtils -import okhttp3.Call +import okhttp3.Request import okhttp3.Response -import java.net.URL -private val uuidRegex by lazy { Regex("[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}") } - -internal class SentryOkHttpEvent(private val hub: IHub, private val call: Call) { +internal class SentryOkHttpEvent(private val hub: IHub, private val request: Request) { private val eventSpans: MutableMap = HashMap() private val breadcrumb: Breadcrumb internal val callRootSpan: ISpan? private var response: Response? = null init { - val urlDetails = UrlUtils.parse(call.request().url.toString()) + val urlDetails = UrlUtils.parse(request.url.toString()) val url = urlDetails.urlOrFallback - val trimmedUrl: String = trimUrl(url) - val host: String = call.request().url.host - val encodedPath: String = call.request().url.encodedPath - val method: String = call.request().method + val host: String = request.url.host + val encodedPath: String = request.url.encodedPath + val method: String = request.method // We start the call span that will contain all the others callRootSpan = hub.span?.startChild("http.client", "$method $url") @@ -42,31 +38,19 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val call: Call) // We setup a breadcrumb with all meaningful data breadcrumb = Breadcrumb.http(url, method) breadcrumb.setData("url", url) - breadcrumb.setData("filtered_url", trimmedUrl) breadcrumb.setData("host", host) breadcrumb.setData("path", encodedPath) - breadcrumb.setData("http.method", method) + breadcrumb.setData("method", method) breadcrumb.setData("success", true) // We add the same data to the root call span callRootSpan?.setData("url", url) - callRootSpan?.setData("filtered_url", trimmedUrl) callRootSpan?.setData("host", host) callRootSpan?.setData("path", encodedPath) callRootSpan?.setData("http.method", method) callRootSpan?.setData("success", true) } - private fun trimUrl(url: String): String { - // Remove any uuid from the url and replace it with a "*" - val trimmedUrl = url.replace(uuidRegex, "*") - if (URL(trimmedUrl).query == null) { - return trimmedUrl - } - // Remove any parameter from the url - return trimmedUrl.replace(URL(trimmedUrl).query, "").replace("?", "") - } - /** * Sets the [Response] that will be sent in the breadcrumb [Hint]. * Also, it sets the protocol and status code in the breadcrumb and the call root span. @@ -89,14 +73,14 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val call: Call) fun setRequestBodySize(byteCount: Long) { if (byteCount > -1) { - breadcrumb.setData("http.request_content_length", byteCount) + breadcrumb.setData("request_content_length", byteCount) callRootSpan?.setData("http.request_content_length", byteCount) } } fun setResponseBodySize(byteCount: Long) { if (byteCount > -1) { - breadcrumb.setData("http.response_content_length", byteCount) + breadcrumb.setData("response_content_length", byteCount) callRootSpan?.setData("http.response_content_length", byteCount) } } @@ -126,7 +110,7 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val call: Call) RESPONSE_BODY_EVENT -> eventSpans[CONNECTION_EVENT] else -> callRootSpan } ?: callRootSpan - val span = parentSpan?.startChild("http.client", event) ?: return + val span = parentSpan?.startChild("http.client.details", event) ?: return eventSpans[event] = span } @@ -149,7 +133,7 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val call: Call) // We put data in the hint and send a breadcrumb val hint = Hint() - hint.set(TypeCheckHint.OKHTTP_REQUEST, call.request()) + hint.set(TypeCheckHint.OKHTTP_REQUEST, request) response?.let { hint.set(TypeCheckHint.OKHTTP_RESPONSE, it) } hub.addBreadcrumb(breadcrumb, hint) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt index 22f8f0915a..686cd16ae2 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt @@ -72,11 +72,18 @@ class SentryOkHttpEventListener( override fun callStart(call: Call) { originalEventListener = originalEventListenerCreator?.invoke(call) originalEventListener?.callStart(call) - eventMap[call] = SentryOkHttpEvent(hub, call) + // If the wrapped EventListener is ours, we can just delegate the calls, + // without creating other events that would create duplicates + if (canCreateEventSpan()) { + eventMap[call] = SentryOkHttpEvent(hub, call.request()) + } } override fun proxySelectStart(call: Call, url: HttpUrl) { originalEventListener?.proxySelectStart(call, url) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(PROXY_SELECT_EVENT) } @@ -87,6 +94,9 @@ class SentryOkHttpEventListener( proxies: List ) { originalEventListener?.proxySelectEnd(call, url, proxies) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.finishSpan(PROXY_SELECT_EVENT) { if (proxies.isNotEmpty()) { @@ -97,6 +107,9 @@ class SentryOkHttpEventListener( override fun dnsStart(call: Call, domainName: String) { originalEventListener?.dnsStart(call, domainName) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(DNS_EVENT) } @@ -107,6 +120,9 @@ class SentryOkHttpEventListener( inetAddressList: List ) { originalEventListener?.dnsEnd(call, domainName, inetAddressList) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.finishSpan(DNS_EVENT) { it.setData("domain_name", domainName) @@ -122,18 +138,27 @@ class SentryOkHttpEventListener( proxy: Proxy ) { originalEventListener?.connectStart(call, inetSocketAddress, proxy) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(CONNECT_EVENT) } override fun secureConnectStart(call: Call) { originalEventListener?.secureConnectStart(call) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(SECURE_CONNECT_EVENT) } override fun secureConnectEnd(call: Call, handshake: Handshake?) { originalEventListener?.secureConnectEnd(call, handshake) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.finishSpan(SECURE_CONNECT_EVENT) } @@ -145,6 +170,9 @@ class SentryOkHttpEventListener( protocol: Protocol? ) { originalEventListener?.connectEnd(call, inetSocketAddress, proxy, protocol) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.setProtocol(protocol?.name) okHttpEvent.finishSpan(CONNECT_EVENT) @@ -158,6 +186,9 @@ class SentryOkHttpEventListener( ioe: IOException ) { originalEventListener?.connectFailed(call, inetSocketAddress, proxy, protocol, ioe) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.setProtocol(protocol?.name) okHttpEvent.setError(ioe.message) @@ -169,36 +200,54 @@ class SentryOkHttpEventListener( override fun connectionAcquired(call: Call, connection: Connection) { originalEventListener?.connectionAcquired(call, connection) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(CONNECTION_EVENT) } override fun connectionReleased(call: Call, connection: Connection) { originalEventListener?.connectionReleased(call, connection) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.finishSpan(CONNECTION_EVENT) } override fun requestHeadersStart(call: Call) { originalEventListener?.requestHeadersStart(call) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(REQUEST_HEADERS_EVENT) } override fun requestHeadersEnd(call: Call, request: Request) { originalEventListener?.requestHeadersEnd(call, request) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.finishSpan(REQUEST_HEADERS_EVENT) } override fun requestBodyStart(call: Call) { originalEventListener?.requestBodyStart(call) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(REQUEST_BODY_EVENT) } override fun requestBodyEnd(call: Call, byteCount: Long) { originalEventListener?.requestBodyEnd(call, byteCount) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.finishSpan(REQUEST_BODY_EVENT) { if (byteCount > 0) { @@ -210,6 +259,9 @@ class SentryOkHttpEventListener( override fun requestFailed(call: Call, ioe: IOException) { originalEventListener?.requestFailed(call, ioe) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.setError(ioe.message) // requestFailed can happen after requestHeaders or requestBody. @@ -228,12 +280,18 @@ class SentryOkHttpEventListener( override fun responseHeadersStart(call: Call) { originalEventListener?.responseHeadersStart(call) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(RESPONSE_HEADERS_EVENT) } override fun responseHeadersEnd(call: Call, response: Response) { originalEventListener?.responseHeadersEnd(call, response) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.setResponse(response) okHttpEvent.finishSpan(RESPONSE_HEADERS_EVENT) { @@ -244,12 +302,18 @@ class SentryOkHttpEventListener( override fun responseBodyStart(call: Call) { originalEventListener?.responseBodyStart(call) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.startSpan(RESPONSE_BODY_EVENT) } override fun responseBodyEnd(call: Call, byteCount: Long) { originalEventListener?.responseBodyEnd(call, byteCount) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.setResponseBodySize(byteCount) okHttpEvent.finishSpan(RESPONSE_BODY_EVENT) { @@ -261,6 +325,9 @@ class SentryOkHttpEventListener( override fun responseFailed(call: Call, ioe: IOException) { originalEventListener?.responseFailed(call, ioe) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return okHttpEvent.setError(ioe.message) // responseFailed can happen after responseHeaders or responseBody. @@ -285,6 +352,9 @@ class SentryOkHttpEventListener( override fun callFailed(call: Call, ioe: IOException) { originalEventListener?.callFailed(call, ioe) + if (!canCreateEventSpan()) { + return + } val okHttpEvent: SentryOkHttpEvent = eventMap.remove(call) ?: return okHttpEvent.setError(ioe.message) okHttpEvent.finishEvent { @@ -292,4 +362,9 @@ class SentryOkHttpEventListener( it.throwable = ioe } } + + private fun canCreateEventSpan(): Boolean { + // If the wrapped EventListener is ours, we shouldn't create spans, as the originalEventListener already did it + return originalEventListener !is SentryOkHttpEventListener + } } diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt index 0bccd8b410..51afc20c0c 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt @@ -7,6 +7,7 @@ import io.sentry.SentryTraceHeader import io.sentry.SentryTracer import io.sentry.SpanStatus import io.sentry.TransactionContext +import okhttp3.Call import okhttp3.EventListener import okhttp3.OkHttpClient import okhttp3.Protocol @@ -19,6 +20,7 @@ import okhttp3.mockwebserver.SocketPolicy import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.spy import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import kotlin.test.Test @@ -246,55 +248,7 @@ class SentryOkHttpEventListenerTest { val call = sut.newCall(request) val response = mock() whenever(response.protocol).thenReturn(Protocol.HTTP_1_1) - - listener.callStart(call) - verify(fixture.mockEventListener).callStart(eq(call)) - listener.proxySelectStart(call, mock()) - verify(fixture.mockEventListener).proxySelectStart(eq(call), any()) - listener.proxySelectEnd(call, mock(), listOf(mock())) - verify(fixture.mockEventListener).proxySelectEnd(eq(call), any(), any()) - listener.dnsStart(call, "domainName") - verify(fixture.mockEventListener).dnsStart(eq(call), eq("domainName")) - listener.dnsEnd(call, "domainName", listOf(mock())) - verify(fixture.mockEventListener).dnsEnd(eq(call), eq("domainName"), any()) - listener.connectStart(call, mock(), mock()) - verify(fixture.mockEventListener).connectStart(eq(call), any(), any()) - listener.secureConnectStart(call) - verify(fixture.mockEventListener).secureConnectStart(eq(call)) - listener.secureConnectEnd(call, mock()) - verify(fixture.mockEventListener).secureConnectEnd(eq(call), any()) - listener.connectEnd(call, mock(), mock(), mock()) - verify(fixture.mockEventListener).connectEnd(eq(call), any(), any(), any()) - listener.connectFailed(call, mock(), mock(), mock(), mock()) - verify(fixture.mockEventListener).connectFailed(eq(call), any(), any(), any(), any()) - listener.connectionAcquired(call, mock()) - verify(fixture.mockEventListener).connectionAcquired(eq(call), any()) - listener.connectionReleased(call, mock()) - verify(fixture.mockEventListener).connectionReleased(eq(call), any()) - listener.requestHeadersStart(call) - verify(fixture.mockEventListener).requestHeadersStart(eq(call)) - listener.requestHeadersEnd(call, mock()) - verify(fixture.mockEventListener).requestHeadersEnd(eq(call), any()) - listener.requestBodyStart(call) - verify(fixture.mockEventListener).requestBodyStart(eq(call)) - listener.requestBodyEnd(call, 10) - verify(fixture.mockEventListener).requestBodyEnd(eq(call), eq(10)) - listener.requestFailed(call, mock()) - verify(fixture.mockEventListener).requestFailed(eq(call), any()) - listener.responseHeadersStart(call) - verify(fixture.mockEventListener).responseHeadersStart(eq(call)) - listener.responseHeadersEnd(call, response) - verify(fixture.mockEventListener).responseHeadersEnd(eq(call), any()) - listener.responseBodyStart(call) - verify(fixture.mockEventListener).responseBodyStart(eq(call)) - listener.responseBodyEnd(call, 10) - verify(fixture.mockEventListener).responseBodyEnd(eq(call), eq(10)) - listener.responseFailed(call, mock()) - verify(fixture.mockEventListener).responseFailed(eq(call), any()) - listener.callEnd(call) - verify(fixture.mockEventListener).callEnd(eq(call)) - listener.callFailed(call, mock()) - verify(fixture.mockEventListener).callFailed(eq(call), any()) + verifyDelegation(listener, fixture.mockEventListener, call, response) } @Test @@ -305,54 +259,98 @@ class SentryOkHttpEventListenerTest { val call = sut.newCall(request) val response = mock() whenever(response.protocol).thenReturn(Protocol.HTTP_1_1) + verifyDelegation(listener, fixture.mockEventListener, call, response) + } + + @Test + fun `propagate all calls to the SentryOkHttpEventListener passed in the ctor`() { + val originalListener = spy(SentryOkHttpEventListener(fixture.hub, fixture.mockEventListener)) + val sut = fixture.getSut(eventListener = originalListener, httpStatusCode = 500) + val listener = fixture.sentryOkHttpEventListener + val request = postRequest(body = "requestBody") + val call = sut.newCall(request) + val response = mock() + whenever(response.protocol).thenReturn(Protocol.HTTP_1_1) + verifyDelegation(listener, originalListener, call, response) + } + + @Test + fun `propagate all calls to the SentryOkHttpEventListener factory passed in the ctor`() { + val originalListener = spy(SentryOkHttpEventListener(fixture.hub, fixture.mockEventListener)) + val sut = fixture.getSut(eventListenerFactory = { originalListener }, httpStatusCode = 500) + val listener = fixture.sentryOkHttpEventListener + val request = postRequest(body = "requestBody") + val call = sut.newCall(request) + val response = mock() + whenever(response.protocol).thenReturn(Protocol.HTTP_1_1) + verifyDelegation(listener, originalListener, call, response) + } + + @Test + fun `does not duplicated spans if an SentryOkHttpEventListener is passed in the ctor`() { + val originalListener = spy(SentryOkHttpEventListener(fixture.hub, fixture.mockEventListener)) + val sut = fixture.getSut(eventListener = originalListener) + val request = postRequest(body = "requestBody") + val call = sut.newCall(request) + val response = call.execute() + response.close() + // Spans are created by the originalListener, so the listener doesn't create duplicates + assertEquals(9, fixture.sentryTracer.children.size) + } + private fun verifyDelegation( + listener: SentryOkHttpEventListener, + originalListener: EventListener, + call: Call, + response: Response + ) { listener.callStart(call) - verify(fixture.mockEventListener).callStart(eq(call)) + verify(originalListener).callStart(eq(call)) listener.proxySelectStart(call, mock()) - verify(fixture.mockEventListener).proxySelectStart(eq(call), any()) + verify(originalListener).proxySelectStart(eq(call), any()) listener.proxySelectEnd(call, mock(), listOf(mock())) - verify(fixture.mockEventListener).proxySelectEnd(eq(call), any(), any()) + verify(originalListener).proxySelectEnd(eq(call), any(), any()) listener.dnsStart(call, "domainName") - verify(fixture.mockEventListener).dnsStart(eq(call), eq("domainName")) + verify(originalListener).dnsStart(eq(call), eq("domainName")) listener.dnsEnd(call, "domainName", listOf(mock())) - verify(fixture.mockEventListener).dnsEnd(eq(call), eq("domainName"), any()) + verify(originalListener).dnsEnd(eq(call), eq("domainName"), any()) listener.connectStart(call, mock(), mock()) - verify(fixture.mockEventListener).connectStart(eq(call), any(), any()) + verify(originalListener).connectStart(eq(call), any(), any()) listener.secureConnectStart(call) - verify(fixture.mockEventListener).secureConnectStart(eq(call)) + verify(originalListener).secureConnectStart(eq(call)) listener.secureConnectEnd(call, mock()) - verify(fixture.mockEventListener).secureConnectEnd(eq(call), any()) + verify(originalListener).secureConnectEnd(eq(call), any()) listener.connectEnd(call, mock(), mock(), mock()) - verify(fixture.mockEventListener).connectEnd(eq(call), any(), any(), any()) + verify(originalListener).connectEnd(eq(call), any(), any(), any()) listener.connectFailed(call, mock(), mock(), mock(), mock()) - verify(fixture.mockEventListener).connectFailed(eq(call), any(), any(), any(), any()) + verify(originalListener).connectFailed(eq(call), any(), any(), any(), any()) listener.connectionAcquired(call, mock()) - verify(fixture.mockEventListener).connectionAcquired(eq(call), any()) + verify(originalListener).connectionAcquired(eq(call), any()) listener.connectionReleased(call, mock()) - verify(fixture.mockEventListener).connectionReleased(eq(call), any()) + verify(originalListener).connectionReleased(eq(call), any()) listener.requestHeadersStart(call) - verify(fixture.mockEventListener).requestHeadersStart(eq(call)) + verify(originalListener).requestHeadersStart(eq(call)) listener.requestHeadersEnd(call, mock()) - verify(fixture.mockEventListener).requestHeadersEnd(eq(call), any()) + verify(originalListener).requestHeadersEnd(eq(call), any()) listener.requestBodyStart(call) - verify(fixture.mockEventListener).requestBodyStart(eq(call)) + verify(originalListener).requestBodyStart(eq(call)) listener.requestBodyEnd(call, 10) - verify(fixture.mockEventListener).requestBodyEnd(eq(call), eq(10)) + verify(originalListener).requestBodyEnd(eq(call), eq(10)) listener.requestFailed(call, mock()) - verify(fixture.mockEventListener).requestFailed(eq(call), any()) + verify(originalListener).requestFailed(eq(call), any()) listener.responseHeadersStart(call) - verify(fixture.mockEventListener).responseHeadersStart(eq(call)) + verify(originalListener).responseHeadersStart(eq(call)) listener.responseHeadersEnd(call, response) - verify(fixture.mockEventListener).responseHeadersEnd(eq(call), any()) + verify(originalListener).responseHeadersEnd(eq(call), any()) listener.responseBodyStart(call) - verify(fixture.mockEventListener).responseBodyStart(eq(call)) + verify(originalListener).responseBodyStart(eq(call)) listener.responseBodyEnd(call, 10) - verify(fixture.mockEventListener).responseBodyEnd(eq(call), eq(10)) + verify(originalListener).responseBodyEnd(eq(call), eq(10)) listener.responseFailed(call, mock()) - verify(fixture.mockEventListener).responseFailed(eq(call), any()) + verify(originalListener).responseFailed(eq(call), any()) listener.callEnd(call) - verify(fixture.mockEventListener).callEnd(eq(call)) + verify(originalListener).callEnd(eq(call)) listener.callFailed(call, mock()) - verify(fixture.mockEventListener).callFailed(eq(call), any()) + verify(originalListener).callFailed(eq(call), any()) } } diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt index 2b4f1239b3..910d9e229d 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt @@ -18,7 +18,6 @@ import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_BOD import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_HEADERS_EVENT import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.SECURE_CONNECT_EVENT import io.sentry.test.getProperty -import okhttp3.OkHttpClient import okhttp3.Protocol import okhttp3.Request import okhttp3.Response @@ -30,7 +29,6 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.whenever -import java.util.UUID import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -43,7 +41,7 @@ class SentryOkHttpEventTest { val hub = mock() val server = MockWebServer() val span: ISpan - val request: Request + val mockRequest: Request val response: Response init { @@ -61,7 +59,7 @@ class SentryOkHttpEventTest { SpanOptions() ) - request = Request.Builder() + mockRequest = Request.Builder() .addHeader("myHeader", "myValue") .get() .url(server.url("/hello")) @@ -70,26 +68,23 @@ class SentryOkHttpEventTest { response = Response.Builder() .code(200) .message("message") - .request(request) + .request(mockRequest) .protocol(Protocol.HTTP_1_1) .build() } fun getSut(currentSpan: ISpan? = span, requestUrl: String ? = null): SentryOkHttpEvent { whenever(hub.span).thenReturn(currentSpan) - val okhttpClient = OkHttpClient.Builder().build() - val call = if (requestUrl == null) { - okhttpClient.newCall(request) + val request = if (requestUrl == null) { + mockRequest } else { - okhttpClient.newCall( - Request.Builder() - .addHeader("myHeader", "myValue") - .get() - .url(server.url(requestUrl)) - .build() - ) + Request.Builder() + .addHeader("myHeader", "myValue") + .get() + .url(server.url(requestUrl)) + .build() } - return SentryOkHttpEvent(hub, call) + return SentryOkHttpEvent(hub, request) } } @@ -107,24 +102,14 @@ class SentryOkHttpEventTest { val callSpan = sut.callRootSpan assertNotNull(callSpan) assertEquals("http.client", callSpan.operation) - assertEquals("${fixture.request.method} ${fixture.request.url}", callSpan.description) - assertEquals(fixture.request.url.toString(), callSpan.getData("url")) - assertEquals(fixture.request.url.toString(), callSpan.getData("filtered_url")) - assertEquals(fixture.request.url.host, callSpan.getData("host")) - assertEquals(fixture.request.url.encodedPath, callSpan.getData("path")) - assertEquals(fixture.request.method, callSpan.getData("http.method")) + assertEquals("${fixture.mockRequest.method} ${fixture.mockRequest.url}", callSpan.description) + assertEquals(fixture.mockRequest.url.toString(), callSpan.getData("url")) + assertEquals(fixture.mockRequest.url.host, callSpan.getData("host")) + assertEquals(fixture.mockRequest.url.encodedPath, callSpan.getData("path")) + assertEquals(fixture.mockRequest.method, callSpan.getData("http.method")) assertTrue(callSpan.getData("success") as Boolean) } - @Test - fun `filtered_url data in root span does not contain uids or parameters`() { - val sut = fixture.getSut(requestUrl = "/hello/${UUID.randomUUID()}/?param1=1¶m2=2") - val callSpan = sut.callRootSpan - val filteredUrl = callSpan?.getData("filtered_url") - assertNotNull(filteredUrl) - assertEquals(fixture.server.url("/hello/*/").toString(), filteredUrl) - } - @Test fun `when root span is null, no breadcrumb is created`() { val sut = fixture.getSut(currentSpan = null) @@ -158,8 +143,11 @@ class SentryOkHttpEventTest { sut.startSpan("span") val spans = sut.getEventSpans() assertEquals(1, spans.size) + val span = spans["span"] + assertNotNull(span) assertTrue(spans.containsKey("span")) - assertFalse(spans["span"]!!.isFinished) + assertEquals("http.client.details", span.operation) + assertFalse(span.isFinished) } @Test @@ -243,15 +231,14 @@ class SentryOkHttpEventTest { sut.finishEvent() verify(fixture.hub).addBreadcrumb( check { - assertEquals(fixture.request.url.toString(), it.data["url"]) - assertEquals(fixture.request.url.toString(), it.data["filtered_url"]) - assertEquals(fixture.request.url.host, it.data["host"]) - assertEquals(fixture.request.url.encodedPath, it.data["path"]) - assertEquals(fixture.request.method, it.data["http.method"]) + assertEquals(fixture.mockRequest.url.toString(), it.data["url"]) + assertEquals(fixture.mockRequest.url.host, it.data["host"]) + assertEquals(fixture.mockRequest.url.encodedPath, it.data["path"]) + assertEquals(fixture.mockRequest.method, it.data["method"]) assertTrue(it.data["success"] as Boolean) }, check { - assertEquals(fixture.request, it[TypeCheckHint.OKHTTP_REQUEST]) + assertEquals(fixture.mockRequest, it[TypeCheckHint.OKHTTP_REQUEST]) } ) } @@ -312,7 +299,7 @@ class SentryOkHttpEventTest { sut.finishEvent() verify(fixture.hub).addBreadcrumb( check { - assertEquals(10L, it.data["http.request_content_length"]) + assertEquals(10L, it.data["request_content_length"]) }, any() ) @@ -326,7 +313,7 @@ class SentryOkHttpEventTest { sut.finishEvent() verify(fixture.hub).addBreadcrumb( check { - assertNull(it.data["http.request_content_length"]) + assertNull(it.data["request_content_length"]) }, any() ) @@ -340,7 +327,7 @@ class SentryOkHttpEventTest { sut.finishEvent() verify(fixture.hub).addBreadcrumb( check { - assertEquals(10L, it.data["http.response_content_length"]) + assertEquals(10L, it.data["response_content_length"]) }, any() ) @@ -354,7 +341,7 @@ class SentryOkHttpEventTest { sut.finishEvent() verify(fixture.hub).addBreadcrumb( check { - assertNull(it.data["http.response_content_length"]) + assertNull(it.data["response_content_length"]) }, any() ) From 66d6ab8f6675092b633230d81d1bece3aba3118d Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Tue, 23 May 2023 18:55:25 +0200 Subject: [PATCH 08/10] removed duplicated breadcrumb data removed `success` from http span as it's implied by the http.status_code http sub-spans operation now contains the event, and the description is now empty --- .../sentry/android/okhttp/SentryOkHttpEvent.kt | 18 +++++------------- .../okhttp/SentryOkHttpEventListenerTest.kt | 18 +++++++++--------- .../android/okhttp/SentryOkHttpEventTest.kt | 15 +++++---------- 3 files changed, 19 insertions(+), 32 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt index 43ba274de9..a0ec988948 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt @@ -16,9 +16,10 @@ import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.SECURE_CONNE import io.sentry.util.UrlUtils import okhttp3.Request import okhttp3.Response +import java.util.concurrent.ConcurrentHashMap internal class SentryOkHttpEvent(private val hub: IHub, private val request: Request) { - private val eventSpans: MutableMap = HashMap() + private val eventSpans: MutableMap = ConcurrentHashMap() private val breadcrumb: Breadcrumb internal val callRootSpan: ISpan? private var response: Response? = null @@ -37,18 +38,14 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req // We setup a breadcrumb with all meaningful data breadcrumb = Breadcrumb.http(url, method) - breadcrumb.setData("url", url) breadcrumb.setData("host", host) breadcrumb.setData("path", encodedPath) - breadcrumb.setData("method", method) - breadcrumb.setData("success", true) // We add the same data to the root call span callRootSpan?.setData("url", url) callRootSpan?.setData("host", host) callRootSpan?.setData("path", encodedPath) callRootSpan?.setData("http.method", method) - callRootSpan?.setData("success", true) } /** @@ -60,7 +57,7 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req breadcrumb.setData("protocol", response.protocol.name) breadcrumb.setData("status_code", response.code) callRootSpan?.setData("protocol", response.protocol.name) - callRootSpan?.setData("status_code", response.code) + callRootSpan?.setData("http.status_code", response.code) callRootSpan?.status = SpanStatus.fromHttpStatusCode(response.code) } @@ -85,13 +82,8 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req } } - /** - * Sets the success flag in the breadcrumb and the call root span to false. - * Also sets the [errorMessage] if not null. - */ + /** Sets the [errorMessage] if not null. */ fun setError(errorMessage: String?) { - breadcrumb.setData("success", false) - callRootSpan?.setData("success", false) if (errorMessage != null) { breadcrumb.setData("error_message", errorMessage) callRootSpan?.setData("error_message", errorMessage) @@ -110,7 +102,7 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req RESPONSE_BODY_EVENT -> eventSpans[CONNECTION_EVENT] else -> callRootSpan } ?: callRootSpan - val span = parentSpan?.startChild("http.client.details", event) ?: return + val span = parentSpan?.startChild("http.client.$event") ?: return eventSpans[event] = span } diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt index 51afc20c0c..556d3668da 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt @@ -161,29 +161,29 @@ class SentryOkHttpEventListenerTest { assertEquals(201, span.data["status_code"]) } 1 -> { - assertEquals("proxySelect", span.description) + assertEquals("http.client.proxySelect", span.operation) assertNotNull(span.data["proxies"]) } 2 -> { - assertEquals("dns", span.description) + assertEquals("http.client.dns", span.operation) assertNotNull(span.data["domain_name"]) assertNotNull(span.data["dns_addresses"]) } 3 -> { - assertEquals("connect", span.description) + assertEquals("http.client.connect", span.operation) } 4 -> { - assertEquals("connection", span.description) + assertEquals("http.client.connection", span.operation) } 5 -> { - assertEquals("requestHeaders", span.description) + assertEquals("http.client.requestHeaders", span.operation) } 6 -> { - assertEquals("responseHeaders", span.description) + assertEquals("http.client.responseHeaders", span.operation) assertEquals(201, span.data["status_code"]) } 7 -> { - assertEquals("responseBody", span.description) + assertEquals("http.client.responseBody", span.operation) } } } @@ -200,7 +200,7 @@ class SentryOkHttpEventListenerTest { val callSpan = okHttpEvent?.callRootSpan response.close() assertEquals(9, fixture.sentryTracer.children.size) - val requestBodySpan = fixture.sentryTracer.children.firstOrNull { it.description == "requestBody" } + val requestBodySpan = fixture.sentryTracer.children.firstOrNull { it.operation == "http.client.requestBody" } assertNotNull(requestBodySpan) assertEquals(requestBody.toByteArray().size.toLong(), requestBodySpan.data["http.request_content_length"]) assertEquals(requestBody.toByteArray().size.toLong(), callSpan?.getData("http.request_content_length")) @@ -221,7 +221,7 @@ class SentryOkHttpEventListenerTest { assertNotNull(responseBytes) response.close() - val requestBodySpan = fixture.sentryTracer.children.firstOrNull { it.description == "responseBody" } + val requestBodySpan = fixture.sentryTracer.children.firstOrNull { it.operation == "http.client.responseBody" } assertNotNull(requestBodySpan) assertEquals(responseBytes.size.toLong(), requestBodySpan.data["http.response_content_length"]) assertEquals(responseBytes.size.toLong(), callSpan?.getData("http.response_content_length")) diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt index 910d9e229d..e653e057f4 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt @@ -107,7 +107,6 @@ class SentryOkHttpEventTest { assertEquals(fixture.mockRequest.url.host, callSpan.getData("host")) assertEquals(fixture.mockRequest.url.encodedPath, callSpan.getData("path")) assertEquals(fixture.mockRequest.method, callSpan.getData("http.method")) - assertTrue(callSpan.getData("success") as Boolean) } @Test @@ -146,7 +145,7 @@ class SentryOkHttpEventTest { val span = spans["span"] assertNotNull(span) assertTrue(spans.containsKey("span")) - assertEquals("http.client.details", span.operation) + assertEquals("http.client.span", span.operation) assertFalse(span.isFinished) } @@ -184,7 +183,7 @@ class SentryOkHttpEventTest { sut.startSpan("span") sut.finishSpan("span") { if (called == 0) { - assertEquals("span", it.description) + assertEquals("http.client.span", it.operation) } else { assertEquals(sut.callRootSpan, it) } @@ -235,7 +234,6 @@ class SentryOkHttpEventTest { assertEquals(fixture.mockRequest.url.host, it.data["host"]) assertEquals(fixture.mockRequest.url.encodedPath, it.data["path"]) assertEquals(fixture.mockRequest.method, it.data["method"]) - assertTrue(it.data["success"] as Boolean) }, check { assertEquals(fixture.mockRequest, it[TypeCheckHint.OKHTTP_REQUEST]) @@ -249,7 +247,7 @@ class SentryOkHttpEventTest { sut.setResponse(fixture.response) assertEquals(fixture.response.protocol.name, sut.callRootSpan?.getData("protocol")) - assertEquals(fixture.response.code, sut.callRootSpan?.getData("status_code")) + assertEquals(fixture.response.code, sut.callRootSpan?.getData("http.status_code")) sut.finishEvent() verify(fixture.hub).addBreadcrumb( @@ -351,12 +349,10 @@ class SentryOkHttpEventTest { fun `setError set success to false and errorMessage in the breadcrumb and in the root span`() { val sut = fixture.getSut() sut.setError("errorMessage") - assertFalse(sut.callRootSpan?.getData("success") as Boolean) - assertEquals("errorMessage", sut.callRootSpan.getData("error_message")) + assertEquals("errorMessage", sut.callRootSpan?.getData("error_message")) sut.finishEvent() verify(fixture.hub).addBreadcrumb( check { - assertFalse(it.data["success"] as Boolean) assertEquals("errorMessage", it.data["error_message"]) }, any() @@ -367,12 +363,11 @@ class SentryOkHttpEventTest { fun `setError sets success to false in the breadcrumb and in the root span even if errorMessage is null`() { val sut = fixture.getSut() sut.setError(null) - assertFalse(sut.callRootSpan?.getData("success") as Boolean) + assertNotNull(sut.callRootSpan) assertNull(sut.callRootSpan.getData("error_message")) sut.finishEvent() verify(fixture.hub).addBreadcrumb( check { - assertFalse(it.data["success"] as Boolean) assertNull(it.data["error_message"]) }, any() From 06d74ffb6401246cdbe7d0777066478fdbf7c0e1 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 25 May 2023 11:52:00 +0200 Subject: [PATCH 09/10] added SentryOkHttpEventListener empty constructors --- .../api/sentry-android-okhttp.api | 2 ++ .../sentry/android/okhttp/SentryOkHttpEvent.kt | 15 +++++++++------ .../okhttp/SentryOkHttpEventListener.kt | 18 +++++++++++++++++- .../okhttp/SentryOkHttpInterceptorTest.kt | 2 ++ 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/sentry-android-okhttp/api/sentry-android-okhttp.api b/sentry-android-okhttp/api/sentry-android-okhttp.api index ec5a385f94..11c140061e 100644 --- a/sentry-android-okhttp/api/sentry-android-okhttp.api +++ b/sentry-android-okhttp/api/sentry-android-okhttp.api @@ -15,6 +15,8 @@ public final class io/sentry/android/okhttp/SentryOkHttpEventListener : okhttp3/ public synthetic fun (Lio/sentry/IHub;Lokhttp3/EventListener$Factory;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public fun (Lio/sentry/IHub;Lokhttp3/EventListener;)V public synthetic fun (Lio/sentry/IHub;Lokhttp3/EventListener;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lokhttp3/EventListener$Factory;)V + public fun (Lokhttp3/EventListener;)V public fun callEnd (Lokhttp3/Call;)V public fun callFailed (Lokhttp3/Call;Ljava/io/IOException;)V public fun callStart (Lokhttp3/Call;)V diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt index a0ec988948..ed48231638 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt @@ -18,6 +18,9 @@ import okhttp3.Request import okhttp3.Response import java.util.concurrent.ConcurrentHashMap +private const val PROTOCOL_KEY = "protocol" +private const val ERROR_MESSAGE_KEY = "error_message" + internal class SentryOkHttpEvent(private val hub: IHub, private val request: Request) { private val eventSpans: MutableMap = ConcurrentHashMap() private val breadcrumb: Breadcrumb @@ -54,17 +57,17 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req */ fun setResponse(response: Response) { this.response = response - breadcrumb.setData("protocol", response.protocol.name) + breadcrumb.setData(PROTOCOL_KEY, response.protocol.name) breadcrumb.setData("status_code", response.code) - callRootSpan?.setData("protocol", response.protocol.name) + callRootSpan?.setData(PROTOCOL_KEY, response.protocol.name) callRootSpan?.setData("http.status_code", response.code) callRootSpan?.status = SpanStatus.fromHttpStatusCode(response.code) } fun setProtocol(protocolName: String?) { if (protocolName != null) { - breadcrumb.setData("protocol", protocolName) - callRootSpan?.setData("protocol", protocolName) + breadcrumb.setData(PROTOCOL_KEY, protocolName) + callRootSpan?.setData(PROTOCOL_KEY, protocolName) } } @@ -85,8 +88,8 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req /** Sets the [errorMessage] if not null. */ fun setError(errorMessage: String?) { if (errorMessage != null) { - breadcrumb.setData("error_message", errorMessage) - callRootSpan?.setData("error_message", errorMessage) + breadcrumb.setData(ERROR_MESSAGE_KEY, errorMessage) + callRootSpan?.setData(ERROR_MESSAGE_KEY, errorMessage) } } diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt index 686cd16ae2..0553aa2cf2 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt @@ -15,6 +15,7 @@ import java.io.IOException import java.net.InetAddress import java.net.InetSocketAddress import java.net.Proxy +import java.util.concurrent.ConcurrentHashMap /** * Logs network performance event metrics to Sentry @@ -56,9 +57,24 @@ class SentryOkHttpEventListener( internal const val RESPONSE_HEADERS_EVENT = "responseHeaders" internal const val RESPONSE_BODY_EVENT = "responseBody" - internal val eventMap: MutableMap = HashMap() + internal val eventMap: MutableMap = ConcurrentHashMap() } + constructor() : this( + HubAdapter.getInstance(), + originalEventListenerCreator = null + ) + + constructor(originalEventListener: EventListener) : this( + HubAdapter.getInstance(), + originalEventListenerCreator = { originalEventListener } + ) + + constructor(originalEventListenerFactory: Factory) : this( + HubAdapter.getInstance(), + originalEventListenerCreator = { originalEventListenerFactory.create(it) } + ) + constructor(hub: IHub = HubAdapter.getInstance(), originalEventListener: EventListener) : this( hub, originalEventListenerCreator = { originalEventListener } diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt index 6550af9c5c..ba94ca5a03 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt @@ -267,6 +267,7 @@ class SentryOkHttpInterceptorTest { fixture.getSut() val interceptor = SentryOkHttpInterceptor(fixture.hub) val chain = mock() + whenever(chain.call()).thenReturn(mock()) whenever(chain.proceed(any())).thenThrow(IOException()) whenever(chain.request()).thenReturn(getRequest()) @@ -499,6 +500,7 @@ class SentryOkHttpInterceptorTest { captureFailedRequests = true ) val chain = mock() + whenever(chain.call()).thenReturn(mock()) whenever(chain.proceed(any())).thenThrow(IOException()) whenever(chain.request()).thenReturn(getRequest()) From 247ceb354ee1e6aef390ecebde7584ed14c01bcf Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 26 May 2023 10:54:10 +0200 Subject: [PATCH 10/10] updated http operations to be snake_case --- .../android/okhttp/SentryOkHttpEventListener.kt | 12 ++++++------ .../android/okhttp/SentryOkHttpEventListenerTest.kt | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt index 0553aa2cf2..e2f653ed46 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEventListener.kt @@ -47,15 +47,15 @@ class SentryOkHttpEventListener( private var originalEventListener: EventListener? = null companion object { - internal const val PROXY_SELECT_EVENT = "proxySelect" + internal const val PROXY_SELECT_EVENT = "proxy_select" internal const val DNS_EVENT = "dns" - internal const val SECURE_CONNECT_EVENT = "secureConnect" + internal const val SECURE_CONNECT_EVENT = "secure_connect" internal const val CONNECT_EVENT = "connect" internal const val CONNECTION_EVENT = "connection" - internal const val REQUEST_HEADERS_EVENT = "requestHeaders" - internal const val REQUEST_BODY_EVENT = "requestBody" - internal const val RESPONSE_HEADERS_EVENT = "responseHeaders" - internal const val RESPONSE_BODY_EVENT = "responseBody" + internal const val REQUEST_HEADERS_EVENT = "request_headers" + internal const val REQUEST_BODY_EVENT = "request_body" + internal const val RESPONSE_HEADERS_EVENT = "response_headers" + internal const val RESPONSE_BODY_EVENT = "response_body" internal val eventMap: MutableMap = ConcurrentHashMap() } diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt index 556d3668da..36da21c7fe 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt @@ -161,7 +161,7 @@ class SentryOkHttpEventListenerTest { assertEquals(201, span.data["status_code"]) } 1 -> { - assertEquals("http.client.proxySelect", span.operation) + assertEquals("http.client.proxy_select", span.operation) assertNotNull(span.data["proxies"]) } 2 -> { @@ -176,14 +176,14 @@ class SentryOkHttpEventListenerTest { assertEquals("http.client.connection", span.operation) } 5 -> { - assertEquals("http.client.requestHeaders", span.operation) + assertEquals("http.client.request_headers", span.operation) } 6 -> { - assertEquals("http.client.responseHeaders", span.operation) + assertEquals("http.client.response_headers", span.operation) assertEquals(201, span.data["status_code"]) } 7 -> { - assertEquals("http.client.responseBody", span.operation) + assertEquals("http.client.response_body", span.operation) } } } @@ -200,7 +200,7 @@ class SentryOkHttpEventListenerTest { val callSpan = okHttpEvent?.callRootSpan response.close() assertEquals(9, fixture.sentryTracer.children.size) - val requestBodySpan = fixture.sentryTracer.children.firstOrNull { it.operation == "http.client.requestBody" } + val requestBodySpan = fixture.sentryTracer.children.firstOrNull { it.operation == "http.client.request_body" } assertNotNull(requestBodySpan) assertEquals(requestBody.toByteArray().size.toLong(), requestBodySpan.data["http.request_content_length"]) assertEquals(requestBody.toByteArray().size.toLong(), callSpan?.getData("http.request_content_length")) @@ -221,7 +221,7 @@ class SentryOkHttpEventListenerTest { assertNotNull(responseBytes) response.close() - val requestBodySpan = fixture.sentryTracer.children.firstOrNull { it.operation == "http.client.responseBody" } + val requestBodySpan = fixture.sentryTracer.children.firstOrNull { it.operation == "http.client.response_body" } assertNotNull(requestBodySpan) assertEquals(responseBytes.size.toLong(), requestBodySpan.data["http.response_content_length"]) assertEquals(responseBytes.size.toLong(), callSpan?.getData("http.response_content_length"))