From a48e53f10b715c98456cb7d9db2f3aac5b8fe557 Mon Sep 17 00:00:00 2001 From: Nikita Ogorodnikov Date: Thu, 19 May 2022 09:03:32 +0200 Subject: [PATCH 1/3] RUMM-2165: Add APM sampling for network requests tracing --- dd-sdk-android-glide/README.md | 4 +- dd-sdk-android-glide/apiSurface | 2 +- .../android/glide/DatadogGlideModule.kt | 12 +- dd-sdk-android/README.md | 10 +- dd-sdk-android/apiSurface | 10 +- .../propagation/DatadogHttpCodec.java | 3 - .../com/datadog/android/DatadogInterceptor.kt | 28 ++- .../com/datadog/android/rum/RumInterceptor.kt | 13 +- .../android/tracing/TracedRequestListener.kt | 3 +- .../android/tracing/TracingInterceptor.kt | 126 +++++++---- .../datadog/android/DatadogInterceptorTest.kt | 197 +++++++++++++++++- .../DatadogInterceptorWithoutRumTest.kt | 1 + .../DatadogInterceptorWithoutTracesTest.kt | 26 ++- .../datadog/android/rum/RumInterceptorTest.kt | 64 ++++++ ...nterceptorNonDdTracerNotSendingSpanTest.kt | 131 +++++++++++- .../TracingInterceptorNonDdTracerTest.kt | 141 ++++++++++++- .../TracingInterceptorNotSendingSpanTest.kt | 120 +++++++++++ .../android/tracing/TracingInterceptorTest.kt | 137 +++++++++++- docs/trace_collection.md | 22 +- .../android/sample/SampleApplication.kt | 4 +- .../sample/picture/SampleGlideModule.kt | 2 - 21 files changed, 966 insertions(+), 90 deletions(-) create mode 100644 dd-sdk-android/src/test/kotlin/com/datadog/android/rum/RumInterceptorTest.kt diff --git a/dd-sdk-android-glide/README.md b/dd-sdk-android-glide/README.md index 44a348fe8f..ccc29883b2 100644 --- a/dd-sdk-android-glide/README.md +++ b/dd-sdk-android-glide/README.md @@ -48,10 +48,12 @@ Doing so will automatically track Glide's network requests (creating both APM Tr @GlideModule class CustomGlideModule : DatadogGlideModule( - listOf("example.com", "example.eu") + listOf("example.com", "example.eu"), traceSamplingRate = 20f ) ``` +Network requests are sampled with an adjustable sampling rate. A sampling of 20% is applied by default. + ## Contributing diff --git a/dd-sdk-android-glide/apiSurface b/dd-sdk-android-glide/apiSurface index bcd2c92989..80fff8b785 100644 --- a/dd-sdk-android-glide/apiSurface +++ b/dd-sdk-android-glide/apiSurface @@ -1,5 +1,5 @@ open class com.datadog.android.glide.DatadogGlideModule : com.bumptech.glide.module.AppGlideModule - constructor(List = emptyList()) + constructor(List = emptyList(), Float = DEFAULT_SAMPLING_RATE) override fun registerComponents(android.content.Context, com.bumptech.glide.Glide, com.bumptech.glide.Registry) override fun applyOptions(android.content.Context, com.bumptech.glide.GlideBuilder) open fun getClientBuilder(): okhttp3.OkHttpClient.Builder diff --git a/dd-sdk-android-glide/src/main/kotlin/com/datadog/android/glide/DatadogGlideModule.kt b/dd-sdk-android-glide/src/main/kotlin/com/datadog/android/glide/DatadogGlideModule.kt index a8a5cf7bbb..e820f85929 100644 --- a/dd-sdk-android-glide/src/main/kotlin/com/datadog/android/glide/DatadogGlideModule.kt +++ b/dd-sdk-android-glide/src/main/kotlin/com/datadog/android/glide/DatadogGlideModule.kt @@ -33,10 +33,14 @@ import okhttp3.Request * - be wrapped in a Span and have trace id injected to get a full flame-graph in APM. * If no host provided the interceptor won't trace any OkHttp [Request], nor propagate tracing * information to the backend, but RUM Resource events will still be sent for each request. + * @param samplingRate the sampling rate for APM traces created for auto-instrumented + * requests. It must be a value between `0.0` and `100.0`. A value of `0.0` means no trace will + * be kept, `100.0` means all traces will be kept (default value is `20.0`). */ open class DatadogGlideModule @JvmOverloads constructor( - private val firstPartyHosts: List = emptyList() + private val firstPartyHosts: List = emptyList(), + private val samplingRate: Float = DEFAULT_SAMPLING_RATE ) : AppGlideModule() { // region AppGlideModule @@ -76,9 +80,13 @@ open class DatadogGlideModule */ open fun getClientBuilder(): OkHttpClient.Builder { return OkHttpClient.Builder() - .addInterceptor((DatadogInterceptor(firstPartyHosts))) + .addInterceptor(DatadogInterceptor(firstPartyHosts, traceSamplingRate = samplingRate)) .eventListenerFactory(DatadogEventListener.Factory()) } // endregion + + private companion object { + private const val DEFAULT_SAMPLING_RATE: Float = 20f + } } diff --git a/dd-sdk-android/README.md b/dd-sdk-android/README.md index 1167472c40..bfa7310abf 100644 --- a/dd-sdk-android/README.md +++ b/dd-sdk-android/README.md @@ -29,7 +29,7 @@ class SampleApplication : Application() { ### Setup for Europe -If you're targetting our [Europe servers](https://datadoghq.eu), you can +If you're targeting our [Europe servers](https://datadoghq.eu), you can initialize the library like this: ```kotlin @@ -139,12 +139,12 @@ do so by providing a map alongside the message, each entry being added as an att In Java you can do so as follows: ```java - mLogger.d( + Map attributes = new HashMap<>(); + attributes.put("http.url", url); + logger.d( "onPageStarted", null, - new HashMap() {{ - put("http.url", url); - }} + attributes ); ``` diff --git a/dd-sdk-android/apiSurface b/dd-sdk-android/apiSurface index e9ebcdc527..eb767be27c 100644 --- a/dd-sdk-android/apiSurface +++ b/dd-sdk-android/apiSurface @@ -52,8 +52,8 @@ class com.datadog.android.DatadogEventListener : okhttp3.EventListener class Factory : okhttp3.EventListener.Factory override fun create(okhttp3.Call): okhttp3.EventListener open class com.datadog.android.DatadogInterceptor : com.datadog.android.tracing.TracingInterceptor - constructor(List, com.datadog.android.tracing.TracedRequestListener = NoOpTracedRequestListener(), com.datadog.android.rum.RumResourceAttributesProvider = NoOpRumResourceAttributesProvider()) - constructor(com.datadog.android.tracing.TracedRequestListener = NoOpTracedRequestListener(), com.datadog.android.rum.RumResourceAttributesProvider = NoOpRumResourceAttributesProvider()) + constructor(List, com.datadog.android.tracing.TracedRequestListener = NoOpTracedRequestListener(), com.datadog.android.rum.RumResourceAttributesProvider = NoOpRumResourceAttributesProvider(), Float = DEFAULT_TRACE_SAMPLING_RATE) + constructor(com.datadog.android.tracing.TracedRequestListener = NoOpTracedRequestListener(), com.datadog.android.rum.RumResourceAttributesProvider = NoOpRumResourceAttributesProvider(), Float = DEFAULT_TRACE_SAMPLING_RATE) override fun intercept(okhttp3.Interceptor.Chain): okhttp3.Response override fun onRequestIntercepted(okhttp3.Request, io.opentracing.Span?, okhttp3.Response?, Throwable?) override fun canSendSpan(): Boolean @@ -357,7 +357,7 @@ enum com.datadog.android.rum.RumErrorSource - AGENT - WEBVIEW class com.datadog.android.rum.RumInterceptor : com.datadog.android.DatadogInterceptor - constructor(List = emptyList(), RumResourceAttributesProvider = NoOpRumResourceAttributesProvider()) + constructor(List = emptyList(), RumResourceAttributesProvider = NoOpRumResourceAttributesProvider(), Float = DEFAULT_TRACE_SAMPLING_RATE) interface com.datadog.android.rum.RumMonitor fun startView(Any, String, Map = emptyMap()) fun stopView(Any, Map = emptyMap()) @@ -1439,8 +1439,8 @@ class com.datadog.android.tracing.AndroidTracer : com.datadog.opentracing.DDTrac interface com.datadog.android.tracing.TracedRequestListener fun onRequestIntercepted(okhttp3.Request, io.opentracing.Span, okhttp3.Response?, Throwable?) open class com.datadog.android.tracing.TracingInterceptor : okhttp3.Interceptor - constructor(List, TracedRequestListener = NoOpTracedRequestListener()) - constructor(TracedRequestListener = NoOpTracedRequestListener()) + constructor(List, TracedRequestListener = NoOpTracedRequestListener(), Float = DEFAULT_TRACE_SAMPLING_RATE) + constructor(TracedRequestListener = NoOpTracedRequestListener(), Float = DEFAULT_TRACE_SAMPLING_RATE) override fun intercept(okhttp3.Interceptor.Chain): okhttp3.Response protected open fun onRequestIntercepted(okhttp3.Request, io.opentracing.Span?, okhttp3.Response?, Throwable?) companion object diff --git a/dd-sdk-android/src/main/java/com/datadog/opentracing/propagation/DatadogHttpCodec.java b/dd-sdk-android/src/main/java/com/datadog/opentracing/propagation/DatadogHttpCodec.java index b2ead9f23b..48350e99d8 100644 --- a/dd-sdk-android/src/main/java/com/datadog/opentracing/propagation/DatadogHttpCodec.java +++ b/dd-sdk-android/src/main/java/com/datadog/opentracing/propagation/DatadogHttpCodec.java @@ -26,7 +26,6 @@ class DatadogHttpCodec { private static final String TRACE_ID_KEY = "x-datadog-trace-id"; private static final String SPAN_ID_KEY = "x-datadog-parent-id"; private static final String SAMPLING_PRIORITY_KEY = "x-datadog-sampling-priority"; - private static final String SELECTED_FOR_SAMPLE_KEY = "x-datadog-sampled"; private static final String ORIGIN_KEY = "x-datadog-origin"; private DatadogHttpCodec() { @@ -50,8 +49,6 @@ public void inject(final DDSpanContext context, final TextMapInject carrier) { // always use max sampling priority for Android traces carrier.put(SAMPLING_PRIORITY_KEY, "1"); - // makes this request trace selected for sampling - carrier.put(SELECTED_FOR_SAMPLE_KEY, "1"); } } diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/DatadogInterceptor.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/DatadogInterceptor.kt index 2296bafdcd..62fc4b5e3b 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/DatadogInterceptor.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/DatadogInterceptor.kt @@ -6,10 +6,13 @@ package com.datadog.android +import androidx.annotation.FloatRange import com.datadog.android.core.configuration.Configuration import com.datadog.android.core.internal.CoreFeature import com.datadog.android.core.internal.net.FirstPartyHostDetector import com.datadog.android.core.internal.net.identifyRequest +import com.datadog.android.core.internal.sampling.RateBasedSampler +import com.datadog.android.core.internal.sampling.Sampler import com.datadog.android.core.internal.utils.devLogger import com.datadog.android.core.internal.utils.sdkLogger import com.datadog.android.rum.GlobalRum @@ -65,8 +68,9 @@ import okhttp3.Response * ``` * * @param tracedHosts a list of all the hosts that you want to be automatically tracked - * by our APM [TracingInterceptor]. If no host provided the interceptor won't trace - * any OkHttpRequest, nor propagate tracing information to the backend. + * by our APM [TracingInterceptor]. If no host provided (via this argument or global + * configuration [Configuration.Builder.setFirstPartyHosts]) the interceptor won't trace + * any [okhttp3.Request], nor propagate tracing information to the backend. * Please note that the host constraint will only be applied on the [TracingInterceptor] and we will * continue to dispatch RUM Resource events for each request without applying any host filtering. * @param tracedRequestListener which listens on the intercepted [okhttp3.Request] and offers @@ -80,12 +84,14 @@ internal constructor( tracedRequestListener: TracedRequestListener, firstPartyHostDetector: FirstPartyHostDetector, internal val rumResourceAttributesProvider: RumResourceAttributesProvider, + traceSampler: Sampler, localTracerFactory: () -> Tracer ) : TracingInterceptor( tracedHosts, tracedRequestListener, firstPartyHostDetector, ORIGIN_RUM, + traceSampler, localTracerFactory ) { @@ -97,24 +103,30 @@ internal constructor( * Requests made to a URL with any one of these hosts (or any subdomain) will: * - be considered a first party RUM Resource and categorised as such in your RUM dashboard; * - be wrapped in a Span and have trace id injected to get a full flame-graph in APM. - * If no host provided the interceptor won't trace any OkHttp [Request], nor propagate tracing + * If no host provided (via this argument or global configuration [Configuration.Builder.setFirstPartyHosts]) + * the interceptor won't trace any OkHttp [Request], nor propagate tracing * information to the backend, but RUM Resource events will still be sent for each request. * @param tracedRequestListener which listens on the intercepted [okhttp3.Request] and offers * the possibility to modify the created [io.opentracing.Span]. * @param rumResourceAttributesProvider which listens on the intercepted [okhttp3.Request] * and offers the possibility to add custom attributes to the RUM resource events. + * @param traceSamplingRate the sampling rate for APM traces created for auto-instrumented + * requests. It must be a value between `0.0` and `100.0`. A value of `0.0` means no trace will + * be kept, `100.0` means all traces will be kept (default value is `20.0`). */ @JvmOverloads constructor( firstPartyHosts: List, tracedRequestListener: TracedRequestListener = NoOpTracedRequestListener(), rumResourceAttributesProvider: RumResourceAttributesProvider = - NoOpRumResourceAttributesProvider() + NoOpRumResourceAttributesProvider(), + @FloatRange(from = 0.0, to = 100.0) traceSamplingRate: Float = DEFAULT_TRACE_SAMPLING_RATE ) : this( tracedHosts = firstPartyHosts, tracedRequestListener = tracedRequestListener, firstPartyHostDetector = CoreFeature.firstPartyHostDetector, rumResourceAttributesProvider = rumResourceAttributesProvider, + traceSampler = RateBasedSampler(traceSamplingRate / 100), localTracerFactory = { AndroidTracer.Builder().build() } ) @@ -126,17 +138,22 @@ internal constructor( * the possibility to modify the created [io.opentracing.Span]. * @param rumResourceAttributesProvider which listens on the intercepted [okhttp3.Request] * and offers the possibility to add custom attributes to the RUM resource events. + * @param traceSamplingRate the sampling rate for APM traces created for auto-instrumented + * requests. It must be a value between `0.0` and `100.0`. A value of `0.0` means no trace will + * be kept, `100.0` means all traces will be kept (default value is `20.0`). */ @JvmOverloads constructor( tracedRequestListener: TracedRequestListener = NoOpTracedRequestListener(), rumResourceAttributesProvider: RumResourceAttributesProvider = - NoOpRumResourceAttributesProvider() + NoOpRumResourceAttributesProvider(), + @FloatRange(from = 0.0, to = 100.0) traceSamplingRate: Float = DEFAULT_TRACE_SAMPLING_RATE ) : this( tracedHosts = emptyList(), tracedRequestListener = tracedRequestListener, firstPartyHostDetector = CoreFeature.firstPartyHostDetector, rumResourceAttributesProvider = rumResourceAttributesProvider, + traceSampler = RateBasedSampler(traceSamplingRate / 100), localTracerFactory = { AndroidTracer.Builder().build() } ) @@ -169,7 +186,6 @@ internal constructor( throwable: Throwable? ) { super.onRequestIntercepted(request, span, response, throwable) - if (RumFeature.isInitialized()) { if (response != null) { handleResponse(request, response, span) diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/RumInterceptor.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/RumInterceptor.kt index 91efbf97b2..439fde74dd 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/RumInterceptor.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/RumInterceptor.kt @@ -6,6 +6,7 @@ package com.datadog.android.rum +import androidx.annotation.FloatRange import com.datadog.android.DatadogInterceptor import com.datadog.android.core.configuration.Configuration import com.datadog.android.rum.tracking.ViewTrackingStrategy @@ -36,16 +37,22 @@ import okhttp3.Request * Requests made to a URL with any one of these hosts (or any subdomain) will: * - be considered a first party RUM Resource and categorised as such in your RUM dashboard; * - be wrapped in a Span and have trace id injected to get a full flame-graph in APM. - * If no host provided the interceptor won't trace any OkHttp [Request], nor propagate tracing + * If no host provided (via this argument or global configuration [Configuration.Builder.setFirstPartyHosts]) + * the interceptor won't trace any OkHttp [Request], nor propagate tracing * information to the backend, but RUM Resource events will still be sent for each request. * @param rumResourceAttributesProvider which listens on the intercepted [okhttp3.Request] * and offers the possibility to add custom attributes to the RUM resource events. + * @param traceSamplingRate the sampling rate for APM traces created for auto-instrumented + * requests. It must be a value between `0.0` and `100.0`. A value of `0.0` means no trace will + * be kept, `100.0` means all traces will be kept (default value is `20.0`). */ class RumInterceptor( firstPartyHosts: List = emptyList(), rumResourceAttributesProvider: RumResourceAttributesProvider = - NoOpRumResourceAttributesProvider() + NoOpRumResourceAttributesProvider(), + @FloatRange(from = 0.0, to = 100.0) traceSamplingRate: Float = DEFAULT_TRACE_SAMPLING_RATE ) : DatadogInterceptor( firstPartyHosts, - rumResourceAttributesProvider = rumResourceAttributesProvider + rumResourceAttributesProvider = rumResourceAttributesProvider, + traceSamplingRate = traceSamplingRate ) diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/TracedRequestListener.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/TracedRequestListener.kt index 99e1a6252e..032a270aeb 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/TracedRequestListener.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/TracedRequestListener.kt @@ -20,7 +20,8 @@ interface TracedRequestListener { /** * Notifies that a span was automatically created around an OkHttp [Request]. - * You can update the given [Span] (e.g.: add custom tags / baggage items) before it is persisted. + * You can update the given [Span] (e.g.: add custom tags / baggage items) before it + * is persisted. Won't be called if [Request] wasn't sampled. * @param request the intercepted [Request] * @param span the [Span] created around the intercepted [Request] * @param response the [Request] response in case of any diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/TracingInterceptor.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/TracingInterceptor.kt index 9df110d666..624b0f5066 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/TracingInterceptor.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/TracingInterceptor.kt @@ -6,9 +6,13 @@ package com.datadog.android.tracing +import androidx.annotation.FloatRange import com.datadog.android.DatadogInterceptor +import com.datadog.android.core.configuration.Configuration import com.datadog.android.core.internal.CoreFeature import com.datadog.android.core.internal.net.FirstPartyHostDetector +import com.datadog.android.core.internal.sampling.RateBasedSampler +import com.datadog.android.core.internal.sampling.Sampler import com.datadog.android.core.internal.utils.devLogger import com.datadog.android.core.internal.utils.loggableStackTrace import com.datadog.android.core.internal.utils.sdkLogger @@ -56,8 +60,9 @@ import okhttp3.Response * ``` * * @param tracedHosts a list of all the hosts that you want to be automatically tracked - * by this interceptor. If no host is provided the interceptor won't trace any OkHttpRequest, - * nor propagate tracing information to the backend. + * by this interceptor. If no host is provided (via this argument or global + * configuration [Configuration.Builder.setFirstPartyHosts]) the interceptor won't trace + * any [okhttp3.Request], nor propagate tracing information to the backend. * @param tracedRequestListener a listener for automatically created [Span]s * */ @@ -68,6 +73,7 @@ internal constructor( internal val tracedRequestListener: TracedRequestListener, internal val firstPartyHostDetector: FirstPartyHostDetector, internal val traceOrigin: String?, + internal val traceSampler: Sampler, internal val localTracerFactory: () -> Tracer ) : Interceptor { @@ -85,19 +91,25 @@ internal constructor( * Creates a [TracingInterceptor] to automatically create a trace around OkHttp [Request]s. * * @param tracedHosts a list of all the hosts that you want to be automatically tracked - * by this interceptor. If no host is provided the interceptor won't trace any OkHttp [Request], + * by this interceptor. If no host is provided (via this argument or global + * configuration [Configuration.Builder.setFirstPartyHosts]) the interceptor won't trace any OkHttp [Request], * nor propagate tracing information to the backend. * @param tracedRequestListener a listener for automatically created [Span]s + * @param traceSamplingRate the sampling rate for APM traces created for auto-instrumented + * requests. It must be a value between `0.0` and `100.0`. A value of `0.0` means no trace will + * be kept, `100.0` means all traces will be kept (default value is `20.0`). */ @JvmOverloads constructor( tracedHosts: List, - tracedRequestListener: TracedRequestListener = NoOpTracedRequestListener() + tracedRequestListener: TracedRequestListener = NoOpTracedRequestListener(), + @FloatRange(from = 0.0, to = 100.0) traceSamplingRate: Float = DEFAULT_TRACE_SAMPLING_RATE ) : this( tracedHosts, tracedRequestListener, CoreFeature.firstPartyHostDetector, null, + RateBasedSampler(traceSamplingRate / 100), { AndroidTracer.Builder().build() } ) @@ -105,15 +117,20 @@ internal constructor( * Creates a [TracingInterceptor] to automatically create a trace around OkHttp [Request]s. * * @param tracedRequestListener a listener for automatically created [Span]s + * @param traceSamplingRate the sampling rate for APM traces created for auto-instrumented + * requests. It must be a value between `0.0` and `100.0`. A value of `0.0` means no trace will + * be kept, `100.0` means all traces will be kept (default value is `20.0`). */ @JvmOverloads constructor( - tracedRequestListener: TracedRequestListener = NoOpTracedRequestListener() + tracedRequestListener: TracedRequestListener = NoOpTracedRequestListener(), + @FloatRange(from = 0.0, to = 100.0) traceSamplingRate: Float = DEFAULT_TRACE_SAMPLING_RATE ) : this( emptyList(), tracedRequestListener, CoreFeature.firstPartyHostDetector, null, + RateBasedSampler(traceSamplingRate / 100), { AndroidTracer.Builder().build() } ) @@ -178,7 +195,12 @@ internal constructor( request: Request, tracer: Tracer ): Response { - val span = buildSpan(tracer, request) + val span = if (traceSampler.sample()) { + buildSpan(tracer, request) + } else { + null + } + val updatedRequest = try { updateRequest(request, tracer, span).build() } catch (e: IllegalStateException) { @@ -271,20 +293,31 @@ internal constructor( private fun updateRequest( request: Request, tracer: Tracer, - span: Span + span: Span? ): Request.Builder { val tracedRequestBuilder = request.newBuilder() - tracer.inject( - span.context(), - Format.Builtin.TEXT_MAP_INJECT, - TextMapInject { key, value -> - // By default the `addHeader` method adds a value and doesn't replace it - // We need to remove the old trace/span info to use the one for the current span - tracedRequestBuilder.removeHeader(key) - tracedRequestBuilder.addHeader(key, value) + if (span == null) { + listOf( + SAMPLING_PRIORITY_HEADER, + TRACE_ID_HEADER, + SPAN_ID_HEADER + ).forEach { + tracedRequestBuilder.removeHeader(it) } - ) + tracedRequestBuilder.addHeader(SAMPLING_PRIORITY_HEADER, DROP_SAMPLING_DECISION) + } else { + tracer.inject( + span.context(), + Format.Builtin.TEXT_MAP_INJECT, + TextMapInject { key, value -> + // By default the `addHeader` method adds a value and doesn't replace it + // We need to remove the old trace/span info to use the one for the current span + tracedRequestBuilder.removeHeader(key) + tracedRequestBuilder.addHeader(key, value) + } + ) + } return tracedRequestBuilder } @@ -292,38 +325,46 @@ internal constructor( private fun handleResponse( request: Request, response: Response, - span: Span + span: Span? ) { - val statusCode = response.code() - span.setTag(Tags.HTTP_STATUS.key, statusCode) - if (statusCode in 400..499) { - (span as? MutableSpan)?.isError = true - } - if (statusCode == 404) { - (span as? MutableSpan)?.resourceName = RESOURCE_NAME_404 - } - onRequestIntercepted(request, span, response, null) - if (canSendSpan()) { - span.finish() + if (span == null) { + onRequestIntercepted(request, null, response, null) } else { - (span as? MutableSpan)?.drop() + val statusCode = response.code() + span.setTag(Tags.HTTP_STATUS.key, statusCode) + if (statusCode in 400..499) { + (span as? MutableSpan)?.isError = true + } + if (statusCode == 404) { + (span as? MutableSpan)?.resourceName = RESOURCE_NAME_404 + } + onRequestIntercepted(request, span, response, null) + if (canSendSpan()) { + span.finish() + } else { + (span as? MutableSpan)?.drop() + } } } private fun handleThrowable( request: Request, throwable: Throwable, - span: Span + span: Span? ) { - (span as? MutableSpan)?.isError = true - span.setTag(DDTags.ERROR_MSG, throwable.message) - span.setTag(DDTags.ERROR_TYPE, throwable.javaClass.name) - span.setTag(DDTags.ERROR_STACK, throwable.loggableStackTrace()) - onRequestIntercepted(request, span, null, throwable) - if (canSendSpan()) { - span.finish() + if (span == null) { + onRequestIntercepted(request, null, null, throwable) } else { - (span as? MutableSpan)?.drop() + (span as? MutableSpan)?.isError = true + span.setTag(DDTags.ERROR_MSG, throwable.message) + span.setTag(DDTags.ERROR_TYPE, throwable.javaClass.name) + span.setTag(DDTags.ERROR_STACK, throwable.loggableStackTrace()) + onRequestIntercepted(request, span, null, throwable) + if (canSendSpan()) { + span.finish() + } else { + (span as? MutableSpan)?.drop() + } } } @@ -351,5 +392,14 @@ internal constructor( "You added a TracingInterceptor to your OkHttpClient, " + "but you didn't register any Tracer. " + "We automatically created a local tracer for you." + + internal const val DEFAULT_TRACE_SAMPLING_RATE: Float = 20f + + // taken from DatadogHttpCodec + internal const val TRACE_ID_HEADER = "x-datadog-trace-id" + internal const val SPAN_ID_HEADER = "x-datadog-parent-id" + internal const val SAMPLING_PRIORITY_HEADER = "x-datadog-sampling-priority" + + internal const val DROP_SAMPLING_DECISION = "0" } } diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/DatadogInterceptorTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/DatadogInterceptorTest.kt index db8ffdd3b8..fc68aaf6ac 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/DatadogInterceptorTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/DatadogInterceptorTest.kt @@ -8,6 +8,7 @@ package com.datadog.android import com.datadog.android.core.configuration.Configuration import com.datadog.android.core.internal.net.identifyRequest +import com.datadog.android.core.internal.sampling.RateBasedSampler import com.datadog.android.rum.NoOpRumResourceAttributesProvider import com.datadog.android.rum.RumAttributes import com.datadog.android.rum.RumErrorSource @@ -85,6 +86,7 @@ internal class DatadogInterceptorTest : TracingInterceptorNotSendingSpanTest() { tracedRequestListener = mockRequestListener, firstPartyHostDetector = mockDetector, rumResourceAttributesProvider = mockRumAttributesProvider, + traceSampler = mockTraceSampler, localTracerFactory = factory ) } @@ -113,7 +115,7 @@ internal class DatadogInterceptorTest : TracingInterceptorNotSendingSpanTest() { } @Test - fun `M instantiate with default callbacks W init()`() { + fun `M instantiate with default values W init() { no tracing hosts specified }`() { // When val interceptor = DatadogInterceptor() @@ -123,10 +125,16 @@ internal class DatadogInterceptorTest : TracingInterceptorNotSendingSpanTest() { .isInstanceOf(NoOpRumResourceAttributesProvider::class.java) assertThat(interceptor.tracedRequestListener) .isInstanceOf(NoOpTracedRequestListener::class.java) + assertThat(interceptor.traceSampler) + .isInstanceOf(RateBasedSampler::class.java) + val traceSampler = interceptor.traceSampler as RateBasedSampler + assertThat(traceSampler.sampleRate).isEqualTo( + TracingInterceptor.DEFAULT_TRACE_SAMPLING_RATE / 100 + ) } @Test - fun `M instantiate with default callbacks W init()`( + fun `M instantiate with default values W init() { traced hosts specified }`( @StringForgery(regex = "[a-z]+\\.[a-z]{3}") hosts: List ) { // When @@ -138,6 +146,12 @@ internal class DatadogInterceptorTest : TracingInterceptorNotSendingSpanTest() { .isInstanceOf(NoOpRumResourceAttributesProvider::class.java) assertThat(interceptor.tracedRequestListener) .isInstanceOf(NoOpTracedRequestListener::class.java) + assertThat(interceptor.traceSampler) + .isInstanceOf(RateBasedSampler::class.java) + val traceSampler = interceptor.traceSampler as RateBasedSampler + assertThat(traceSampler.sampleRate).isEqualTo( + TracingInterceptor.DEFAULT_TRACE_SAMPLING_RATE / 100 + ) } @Test @@ -179,6 +193,44 @@ internal class DatadogInterceptorTest : TracingInterceptorNotSendingSpanTest() { } } + @Test + fun `𝕄 start and stop RUM Resource 𝕎 intercept() {successful request + not sampled}`( + @IntForgery(min = 200, max = 300) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + stubChain(mockChain, statusCode) + val expectedStartAttrs = emptyMap() + // no span -> shouldn't have trace/spans IDs + val expectedStopAttrs = fakeAttributes + val requestId = identifyRequest(fakeRequest) + val mimeType = fakeMediaType?.type() + val kind = when { + mimeType != null -> RumResourceKind.fromMimeType(mimeType) + else -> RumResourceKind.NATIVE + } + + // When + testedInterceptor.intercept(mockChain) + + // Then + inOrder(rumMonitor.mockInstance) { + verify(rumMonitor.mockInstance).startResource( + requestId, + fakeMethod, + fakeUrl, + expectedStartAttrs + ) + verify(rumMonitor.mockInstance).stopResource( + requestId, + statusCode, + fakeResponseBody.toByteArray().size.toLong(), + kind, + expectedStopAttrs + ) + } + } + @Test fun `𝕄 start and stop RUM Resource 𝕎 intercept() {successful request empty response}`( @IntForgery(min = 200, max = 300) statusCode: Int @@ -226,6 +278,52 @@ internal class DatadogInterceptorTest : TracingInterceptorNotSendingSpanTest() { } } + @Test + fun `𝕄 start and stop RUM Resource 𝕎 intercept() {successful request empty response + !smp}`( + @IntForgery(min = 200, max = 300) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + stubChain(mockChain) { + Response.Builder() + .request(fakeRequest) + .protocol(Protocol.HTTP_2) + .code(statusCode) + .message("HTTP $statusCode") + .body(ResponseBody.create(fakeMediaType, "")) + .header(TracingInterceptor.HEADER_CT, fakeMediaType?.type().orEmpty()) + .build() + } + // no span -> shouldn't have trace/spans IDs + val expectedStopAttrs = fakeAttributes + val requestId = identifyRequest(fakeRequest) + val mimeType = fakeMediaType?.type() + val kind = when { + mimeType != null -> RumResourceKind.fromMimeType(mimeType) + else -> RumResourceKind.NATIVE + } + + // When + testedInterceptor.intercept(mockChain) + + // Then + inOrder(rumMonitor.mockInstance) { + verify(rumMonitor.mockInstance).startResource( + requestId, + fakeMethod, + fakeUrl, + emptyMap() + ) + verify(rumMonitor.mockInstance).stopResource( + requestId, + statusCode, + null, + kind, + expectedStopAttrs + ) + } + } + @Test fun `𝕄 start and stop RUM Resource 𝕎 intercept() {successful request throwing response}`( @IntForgery(min = 200, max = 300) statusCode: Int @@ -284,6 +382,63 @@ internal class DatadogInterceptorTest : TracingInterceptorNotSendingSpanTest() { } } + @Test + fun `𝕄 start and stop RUM Resource 𝕎 intercept() {success request throwing response + !smp}`( + @IntForgery(min = 200, max = 300) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + stubChain(mockChain) { + Response.Builder() + .request(fakeRequest) + .protocol(Protocol.HTTP_2) + .code(statusCode) + .message("HTTP $statusCode") + .header(TracingInterceptor.HEADER_CT, fakeMediaType?.type().orEmpty()) + .body(object : ResponseBody() { + override fun contentType(): MediaType? = fakeMediaType + + override fun contentLength(): Long = fakeResponseBody.length.toLong() + + override fun source(): BufferedSource { + return mock().apply { + whenever(this.request(any())) doThrow IOException() + } + } + }) + .build() + } + val expectedStartAttrs = emptyMap() + // no span -> shouldn't have trace/spans IDs + val expectedStopAttrs = fakeAttributes + val requestId = identifyRequest(fakeRequest) + val mimeType = fakeMediaType?.type() + val kind = when { + mimeType != null -> RumResourceKind.fromMimeType(mimeType) + else -> RumResourceKind.NATIVE + } + + // When + testedInterceptor.intercept(mockChain) + + // Then + inOrder(rumMonitor.mockInstance) { + verify(rumMonitor.mockInstance).startResource( + requestId, + fakeMethod, + fakeUrl, + expectedStartAttrs + ) + verify(rumMonitor.mockInstance).stopResource( + requestId, + statusCode, + null, + kind, + expectedStopAttrs + ) + } + } + @Test fun `𝕄 start and stop RUM Resource 𝕎 intercept() {failing request}`( @IntForgery(min = 400, max = 500) statusCode: Int @@ -323,6 +478,44 @@ internal class DatadogInterceptorTest : TracingInterceptorNotSendingSpanTest() { } } + @Test + fun `𝕄 start and stop RUM Resource 𝕎 intercept() {failing request + not sampled}`( + @IntForgery(min = 400, max = 500) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + stubChain(mockChain, statusCode) + val expectedStartAttrs = emptyMap() + // no span -> shouldn't have trace/spans IDs + val expectedStopAttrs = fakeAttributes + val requestId = identifyRequest(fakeRequest) + val mimeType = fakeMediaType?.type() + val kind = when { + mimeType != null -> RumResourceKind.fromMimeType(mimeType) + else -> RumResourceKind.NATIVE + } + + // When + testedInterceptor.intercept(mockChain) + + // Then + inOrder(rumMonitor.mockInstance) { + verify(rumMonitor.mockInstance).startResource( + requestId, + fakeMethod, + fakeUrl, + expectedStartAttrs + ) + verify(rumMonitor.mockInstance).stopResource( + requestId, + statusCode, + fakeResponseBody.toByteArray().size.toLong(), + kind, + expectedStopAttrs + ) + } + } + @Test fun `𝕄 start and stop RUM Resource 𝕎 intercept() {throwing request}`( @Forgery throwable: Throwable diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/DatadogInterceptorWithoutRumTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/DatadogInterceptorWithoutRumTest.kt index e5278ea7a4..61df1b35b5 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/DatadogInterceptorWithoutRumTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/DatadogInterceptorWithoutRumTest.kt @@ -56,6 +56,7 @@ internal class DatadogInterceptorWithoutRumTest : TracingInterceptorTest() { tracedRequestListener = mockRequestListener, firstPartyHostDetector = mockDetector, rumResourceAttributesProvider = mockRumAttributesProvider, + traceSampler = mockTraceSampler, localTracerFactory = factory ) } diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/DatadogInterceptorWithoutTracesTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/DatadogInterceptorWithoutTracesTest.kt index 80fb451e0a..3f46570adf 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/DatadogInterceptorWithoutTracesTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/DatadogInterceptorWithoutTracesTest.kt @@ -11,6 +11,7 @@ import android.util.Log import com.datadog.android.core.configuration.Configuration import com.datadog.android.core.internal.net.FirstPartyHostDetector import com.datadog.android.core.internal.net.identifyRequest +import com.datadog.android.core.internal.sampling.Sampler import com.datadog.android.rum.RumErrorSource import com.datadog.android.rum.RumResourceAttributesProvider import com.datadog.android.rum.RumResourceKind @@ -38,6 +39,7 @@ import com.nhaarman.mockitokotlin2.doReturn import com.nhaarman.mockitokotlin2.doThrow import com.nhaarman.mockitokotlin2.inOrder import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.verifyZeroInteractions import com.nhaarman.mockitokotlin2.whenever import fr.xgouchet.elmyr.Forge import fr.xgouchet.elmyr.annotation.Forgery @@ -105,6 +107,9 @@ internal class DatadogInterceptorWithoutTracesTest { @Mock lateinit var mockSpan: DDSpan + @Mock + lateinit var mockTraceSampler: Sampler + // endregion // region Fakes @@ -148,6 +153,7 @@ internal class DatadogInterceptorWithoutTracesTest { whenever(mockSpan.context()) doReturn mockSpanContext whenever(mockSpanContext.toSpanId()) doReturn fakeSpanId whenever(mockSpanContext.toTraceId()) doReturn fakeTraceId + whenever(mockTraceSampler.sample()) doReturn true val mediaType = forge.anElementFrom("application", "image", "text", "model") + "/" + forge.anAlphabeticalString() @@ -157,7 +163,8 @@ internal class DatadogInterceptorWithoutTracesTest { tracedHosts = emptyList(), tracedRequestListener = mockRequestListener, firstPartyHostDetector = mockDetector, - rumResourceAttributesProvider = mockRumAttributesProvider + rumResourceAttributesProvider = mockRumAttributesProvider, + traceSampler = mockTraceSampler ) { mockLocalTracer } TracingFeature.initialize( appContext.mockInstance, @@ -322,6 +329,23 @@ internal class DatadogInterceptorWithoutTracesTest { Assertions.assertThat(response).isSameAs(fakeResponse) } + @Test + fun `𝕄 not create a Span 𝕎 intercept() for completed request {not sampled}`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + verifyZeroInteractions(mockSpan, mockSpanBuilder, mockLocalTracer) + Assertions.assertThat(response).isSameAs(fakeResponse) + } + // region Internal private fun stubChain(chain: Interceptor.Chain, statusCode: Int) { diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/rum/RumInterceptorTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/rum/RumInterceptorTest.kt new file mode 100644 index 0000000000..49c99d2f26 --- /dev/null +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/rum/RumInterceptorTest.kt @@ -0,0 +1,64 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +package com.datadog.android.rum + +import com.datadog.android.core.internal.sampling.RateBasedSampler +import com.datadog.android.tracing.NoOpTracedRequestListener +import com.datadog.android.tracing.TracingInterceptor +import com.datadog.android.tracing.TracingInterceptorNotSendingSpanTest +import com.datadog.android.utils.config.GlobalRumMonitorTestConfiguration +import com.datadog.android.utils.forge.Configurator +import com.datadog.tools.unit.annotations.TestConfigurationsProvider +import com.datadog.tools.unit.extensions.TestConfigurationExtension +import com.datadog.tools.unit.extensions.config.TestConfiguration +import fr.xgouchet.elmyr.junit5.ForgeConfiguration +import fr.xgouchet.elmyr.junit5.ForgeExtension +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith +import org.junit.jupiter.api.extension.Extensions +import org.mockito.junit.jupiter.MockitoExtension +import org.mockito.junit.jupiter.MockitoSettings +import org.mockito.quality.Strictness + +@Extensions( + ExtendWith(MockitoExtension::class), + ExtendWith(ForgeExtension::class), + ExtendWith(TestConfigurationExtension::class) +) +@MockitoSettings(strictness = Strictness.LENIENT) +@ForgeConfiguration(Configurator::class) +internal class RumInterceptorTest : TracingInterceptorNotSendingSpanTest() { + + @Test + fun `M instantiate with default values W init()`() { + // When + val interceptor = RumInterceptor() + + // Then + assertThat(interceptor.tracedHosts).isEmpty() + assertThat(interceptor.rumResourceAttributesProvider) + .isInstanceOf(NoOpRumResourceAttributesProvider::class.java) + assertThat(interceptor.tracedRequestListener) + .isInstanceOf(NoOpTracedRequestListener::class.java) + assertThat(interceptor.traceSampler) + .isInstanceOf(RateBasedSampler::class.java) + val traceSampler = interceptor.traceSampler as RateBasedSampler + assertThat(traceSampler.sampleRate) + .isEqualTo(TracingInterceptor.DEFAULT_TRACE_SAMPLING_RATE / 100) + } + + companion object { + val rumMonitor = GlobalRumMonitorTestConfiguration() + + @TestConfigurationsProvider + @JvmStatic + fun getTestConfigurations(): List { + return listOf(logger, appContext, coreFeature, rumMonitor) + } + } +} diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerNotSendingSpanTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerNotSendingSpanTest.kt index 01e6309032..74e0c4db23 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerNotSendingSpanTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerNotSendingSpanTest.kt @@ -11,16 +11,19 @@ import android.util.Log import com.datadog.android.Datadog import com.datadog.android.core.configuration.Configuration import com.datadog.android.core.internal.net.FirstPartyHostDetector +import com.datadog.android.core.internal.sampling.Sampler import com.datadog.android.core.internal.utils.loggableStackTrace import com.datadog.android.tracing.internal.TracingFeature import com.datadog.android.utils.config.ApplicationContextTestConfiguration import com.datadog.android.utils.config.CoreFeatureTestConfiguration import com.datadog.android.utils.config.LoggerTestConfiguration import com.datadog.android.utils.forge.Configurator +import com.datadog.opentracing.DDTracer import com.datadog.tools.unit.annotations.TestConfigurationsProvider import com.datadog.tools.unit.extensions.TestConfigurationExtension import com.datadog.tools.unit.extensions.config.TestConfiguration import com.datadog.tools.unit.setStaticValue +import com.datadog.trace.api.interceptor.MutableSpan import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.argumentCaptor @@ -109,6 +112,9 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest { @Mock lateinit var mockDetector: FirstPartyHostDetector + @Mock + lateinit var mockTraceSampler: Sampler + // endregion // region Fakes @@ -158,6 +164,7 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest { whenever(mockSpan.context()) doReturn mockSpanContext whenever(mockSpanContext.toSpanId()) doReturn fakeSpanId whenever(mockSpanContext.toTraceId()) doReturn fakeTraceId + whenever(mockTraceSampler.sample()) doReturn true fakeMediaType = if (forge.aBool()) { val mediaType = forge.anElementFrom("application", "image", "text", "model") + @@ -195,6 +202,7 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest { mockRequestListener, mockDetector, fakeOrigin, + mockTraceSampler, factory ) { override fun canSendSpan(): Boolean { @@ -203,10 +211,6 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest { } } - open fun getExpectedOrigin(): String { - return fakeOrigin - } - @Test fun `𝕄 inject tracing header 𝕎 intercept() {global known host}`( @StringForgery key: String, @@ -229,6 +233,30 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest { } } + @Test + fun `𝕄 inject non-tracing header 𝕎 intercept() {global known host + not sampled}`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + verifyZeroInteractions(mockTracer, mockLocalTracer) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(TracingInterceptor.SAMPLING_PRIORITY_HEADER)) + .isEqualTo("0") + assertThat(lastValue.header(TracingInterceptor.TRACE_ID_HEADER)).isNull() + assertThat(lastValue.header(TracingInterceptor.SPAN_ID_HEADER)).isNull() + } + } + @Test fun `𝕄 inject tracing header 𝕎 intercept() {local known host}`( @StringForgery key: String, @@ -254,6 +282,33 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest { } } + @Test + fun `𝕄 inject non-tracing header 𝕎 intercept() {local known host + not sampled}`( + @IntForgery(min = 200, max = 600) statusCode: Int, + forge: Forge + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + fakeUrl = forgeUrl(forge, forge.anElementFrom(fakeLocalHosts)) + fakeRequest = forgeRequest(forge) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(false) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + verifyZeroInteractions(mockTracer, mockLocalTracer) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(TracingInterceptor.SAMPLING_PRIORITY_HEADER)) + .isEqualTo("0") + assertThat(lastValue.header(TracingInterceptor.TRACE_ID_HEADER)).isNull() + assertThat(lastValue.header(TracingInterceptor.SPAN_ID_HEADER)).isNull() + } + } + @Test fun `𝕄 inject tracing header 𝕎 intercept() for request with parent span`( @StringForgery key: String, @@ -313,10 +368,28 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest { verify(mockSpanBuilder).asChildOf(parentSpanContext) } + @Test + fun `𝕄 not create a span 𝕎 intercept() { not sampled }`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + verifyZeroInteractions(mockTracer, mockSpan, mockSpanBuilder, mockLocalTracer) + } + @Test fun `𝕄 create a span with info 𝕎 intercept() for successful request`( @IntForgery(min = 200, max = 300) statusCode: Int ) { + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) stubChain(mockChain, statusCode) val response = testedInterceptor.intercept(mockChain) @@ -535,6 +608,23 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest { assertThat(response).isSameAs(fakeResponse) } + @Test + fun `𝕄 not call the listener 𝕎 intercept() for completed request { not sampled }`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + verifyZeroInteractions(mockRequestListener) + assertThat(response).isSameAs(fakeResponse) + } + @Test fun `𝕄 call the listener 𝕎 intercept() for throwing request`( @Forgery throwable: Throwable, @@ -565,6 +655,23 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest { verify(mockSpan, never()).finish() } + @Test + fun `𝕄 not call the listener 𝕎 intercept() for throwing request { not sampled }`( + @Forgery throwable: Throwable + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + whenever(mockChain.request()) doReturn fakeRequest + whenever(mockChain.proceed(any())) doThrow throwable + + assertThrows(throwable.message.orEmpty()) { + testedInterceptor.intercept(mockChain) + } + + verifyZeroInteractions(mockRequestListener) + } + @Test fun `M do not update the hostDetector W host list provided`(forge: Forge) { // GIVEN @@ -610,6 +717,7 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest { fun `𝕄 create only one local tracer 𝕎 intercept() called from multiple threads`( @IntForgery(min = 200, max = 300) statusCode: Int ) { + // Given var called = 0 val tracedHosts = listOf(fakeHostIp, fakeHostName) whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) @@ -620,6 +728,17 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest { GlobalTracer::class.java.setStaticValue("isRegistered", false) stubChain(mockChain, statusCode) + // need this setup, otherwise #intercept actually throws NPE, which pollutes the log + val localSpanBuilder: DDTracer.DDSpanBuilder = mock() + val localSpan: Span = mock(extraInterfaces = arrayOf(MutableSpan::class)) + whenever(localSpanBuilder.asChildOf(null as SpanContext?)) doReturn localSpanBuilder + whenever(localSpanBuilder.start()) doReturn localSpan + whenever(localSpan.context()) doReturn mockSpanContext + whenever(mockSpanContext.toSpanId()) doReturn fakeSpanId + whenever(mockSpanContext.toTraceId()) doReturn fakeTraceId + whenever(mockLocalTracer.buildSpan(TracingInterceptor.SPAN_NAME)) doReturn localSpanBuilder + + // When val countDownLatch = CountDownLatch(2) Thread { testedInterceptor.intercept(mockChain) @@ -638,11 +757,11 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest { // region Internal - internal fun stubChain(chain: Interceptor.Chain, statusCode: Int) { + private fun stubChain(chain: Interceptor.Chain, statusCode: Int) { return stubChain(chain) { forgeResponse(statusCode) } } - internal fun stubChain( + private fun stubChain( chain: Interceptor.Chain, responseBuilder: () -> Response ) { diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerTest.kt index 9ea979a659..03282b2995 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerTest.kt @@ -11,16 +11,20 @@ import android.util.Log import com.datadog.android.Datadog import com.datadog.android.core.configuration.Configuration import com.datadog.android.core.internal.net.FirstPartyHostDetector +import com.datadog.android.core.internal.sampling.RateBasedSampler +import com.datadog.android.core.internal.sampling.Sampler import com.datadog.android.core.internal.utils.loggableStackTrace import com.datadog.android.tracing.internal.TracingFeature import com.datadog.android.utils.config.ApplicationContextTestConfiguration import com.datadog.android.utils.config.CoreFeatureTestConfiguration import com.datadog.android.utils.config.LoggerTestConfiguration import com.datadog.android.utils.forge.Configurator +import com.datadog.opentracing.DDTracer import com.datadog.tools.unit.annotations.TestConfigurationsProvider import com.datadog.tools.unit.extensions.TestConfigurationExtension import com.datadog.tools.unit.extensions.config.TestConfiguration import com.datadog.tools.unit.setStaticValue +import com.datadog.trace.api.interceptor.MutableSpan import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.argumentCaptor @@ -109,6 +113,9 @@ internal open class TracingInterceptorNonDdTracerTest { @Mock lateinit var mockDetector: FirstPartyHostDetector + @Mock + lateinit var mockTraceSampler: Sampler + // endregion // region Fakes @@ -155,6 +162,7 @@ internal open class TracingInterceptorNonDdTracerTest { whenever(mockSpan.context()) doReturn mockSpanContext whenever(mockSpanContext.toSpanId()) doReturn fakeSpanId whenever(mockSpanContext.toTraceId()) doReturn fakeTraceId + whenever(mockTraceSampler.sample()) doReturn true val mediaType = forge.anElementFrom("application", "image", "text", "model") + "/" + forge.anAlphabeticalString() @@ -181,6 +189,7 @@ internal open class TracingInterceptorNonDdTracerTest { mockRequestListener, mockDetector, fakeOrigin, + mockTraceSampler, factory ) } @@ -192,7 +201,7 @@ internal open class TracingInterceptorNonDdTracerTest { } @Test - fun `M instantiate with default callbacks W init()`() { + fun `M instantiate with default values W init()`() { // When val interceptor = TracingInterceptor() @@ -200,10 +209,16 @@ internal open class TracingInterceptorNonDdTracerTest { assertThat(interceptor.tracedHosts).isEmpty() assertThat(interceptor.tracedRequestListener) .isInstanceOf(NoOpTracedRequestListener::class.java) + assertThat(interceptor.traceSampler) + .isInstanceOf(RateBasedSampler::class.java) + val traceSampler = interceptor.traceSampler as RateBasedSampler + assertThat(traceSampler.sampleRate).isEqualTo( + TracingInterceptor.DEFAULT_TRACE_SAMPLING_RATE / 100 + ) } @Test - fun `M instantiate with default callbacks W init()`( + fun `M instantiate with default values W init()`( @StringForgery(regex = "[a-z]+\\.[a-z]{3}") hosts: List ) { // When @@ -213,6 +228,12 @@ internal open class TracingInterceptorNonDdTracerTest { assertThat(interceptor.tracedHosts).containsAll(hosts) assertThat(interceptor.tracedRequestListener) .isInstanceOf(NoOpTracedRequestListener::class.java) + assertThat(interceptor.traceSampler) + .isInstanceOf(RateBasedSampler::class.java) + val traceSampler = interceptor.traceSampler as RateBasedSampler + assertThat(traceSampler.sampleRate).isEqualTo( + TracingInterceptor.DEFAULT_TRACE_SAMPLING_RATE / 100 + ) } @Test @@ -237,6 +258,30 @@ internal open class TracingInterceptorNonDdTracerTest { } } + @Test + fun `𝕄 inject non-tracing header 𝕎 intercept() {global known host + not sampled}`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + verifyZeroInteractions(mockTracer, mockLocalTracer) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(TracingInterceptor.SAMPLING_PRIORITY_HEADER)) + .isEqualTo("0") + assertThat(lastValue.header(TracingInterceptor.TRACE_ID_HEADER)).isNull() + assertThat(lastValue.header(TracingInterceptor.SPAN_ID_HEADER)).isNull() + } + } + @Test fun `𝕄 inject tracing header 𝕎 intercept() {local known host}`( @StringForgery key: String, @@ -262,6 +307,33 @@ internal open class TracingInterceptorNonDdTracerTest { } } + @Test + fun `𝕄 inject non-tracing header 𝕎 intercept() {local known host + not sampled}`( + @IntForgery(min = 200, max = 600) statusCode: Int, + forge: Forge + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + fakeUrl = forgeUrlWithQueryParams(forge, forge.anElementFrom(fakeLocalHosts)) + fakeRequest = forgeRequest(forge) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(false) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + verifyZeroInteractions(mockTracer, mockLocalTracer) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(TracingInterceptor.SAMPLING_PRIORITY_HEADER)) + .isEqualTo("0") + assertThat(lastValue.header(TracingInterceptor.TRACE_ID_HEADER)).isNull() + assertThat(lastValue.header(TracingInterceptor.SPAN_ID_HEADER)).isNull() + } + } + @Test fun `𝕄 inject tracing header 𝕎 intercept() for request with parent span`( @StringForgery key: String, @@ -389,6 +461,23 @@ internal open class TracingInterceptorNonDdTracerTest { verify(mockSpanBuilder).asChildOf(parentSpanContext) } + @Test + fun `𝕄 not create a span 𝕎 intercept() { not sampled }`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + verifyZeroInteractions(mockTracer, mockSpan, mockSpanBuilder, mockLocalTracer) + } + @Test fun `𝕄 create a span with info 𝕎 intercept() for successful request`( @IntForgery(min = 200, max = 300) statusCode: Int @@ -633,6 +722,23 @@ internal open class TracingInterceptorNonDdTracerTest { assertThat(response).isSameAs(fakeResponse) } + @Test + fun `𝕄 not call the listener 𝕎 intercept() for completed request { not sampled }`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + verifyZeroInteractions(mockRequestListener) + assertThat(response).isSameAs(fakeResponse) + } + @Test fun `𝕄 call the listener 𝕎 intercept() for throwing request`( @Forgery throwable: Throwable, @@ -663,6 +769,23 @@ internal open class TracingInterceptorNonDdTracerTest { verify(mockSpan).finish() } + @Test + fun `𝕄 not call the listener 𝕎 intercept() for throwing request { not sampled }`( + @Forgery throwable: Throwable + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + whenever(mockChain.request()) doReturn fakeRequest + whenever(mockChain.proceed(any())) doThrow throwable + + assertThrows(throwable.message.orEmpty()) { + testedInterceptor.intercept(mockChain) + } + + verifyZeroInteractions(mockRequestListener) + } + @Test fun `M do not update the hostDetector W host list provided`(forge: Forge) { // GIVEN @@ -708,6 +831,7 @@ internal open class TracingInterceptorNonDdTracerTest { fun `𝕄 create only one local tracer 𝕎 intercept() called from multiple threads`( @IntForgery(min = 200, max = 300) statusCode: Int ) { + // Given var called = 0 testedInterceptor = instantiateTestedInterceptor { called++ @@ -717,6 +841,17 @@ internal open class TracingInterceptorNonDdTracerTest { whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) stubChain(mockChain, statusCode) + // need this setup, otherwise #intercept actually throws NPE, which pollutes the log + val localSpanBuilder: DDTracer.DDSpanBuilder = mock() + val localSpan: Span = mock(extraInterfaces = arrayOf(MutableSpan::class)) + whenever(localSpanBuilder.asChildOf(null as SpanContext?)) doReturn localSpanBuilder + whenever(localSpanBuilder.start()) doReturn localSpan + whenever(localSpan.context()) doReturn mockSpanContext + whenever(mockSpanContext.toSpanId()) doReturn fakeSpanId + whenever(mockSpanContext.toTraceId()) doReturn fakeTraceId + whenever(mockLocalTracer.buildSpan(TracingInterceptor.SPAN_NAME)) doReturn localSpanBuilder + + // When val countDownLatch = CountDownLatch(2) Thread { testedInterceptor.intercept(mockChain) @@ -735,7 +870,7 @@ internal open class TracingInterceptorNonDdTracerTest { // region Internal - internal fun stubChain(chain: Interceptor.Chain, statusCode: Int) { + private fun stubChain(chain: Interceptor.Chain, statusCode: Int) { fakeResponse = forgeResponse(statusCode) whenever(chain.request()) doReturn fakeRequest diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNotSendingSpanTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNotSendingSpanTest.kt index d6a6cd5e12..68807b8da0 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNotSendingSpanTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNotSendingSpanTest.kt @@ -11,6 +11,7 @@ import android.util.Log import com.datadog.android.Datadog import com.datadog.android.core.configuration.Configuration import com.datadog.android.core.internal.net.FirstPartyHostDetector +import com.datadog.android.core.internal.sampling.Sampler import com.datadog.android.core.internal.utils.loggableStackTrace import com.datadog.android.tracing.internal.TracingFeature import com.datadog.android.utils.config.ApplicationContextTestConfiguration @@ -108,6 +109,9 @@ internal open class TracingInterceptorNotSendingSpanTest { @Mock lateinit var mockDetector: FirstPartyHostDetector + @Mock + lateinit var mockTraceSampler: Sampler + // endregion // region Fakes @@ -157,6 +161,7 @@ internal open class TracingInterceptorNotSendingSpanTest { whenever(mockSpan.context()) doReturn mockSpanContext whenever(mockSpanContext.toSpanId()) doReturn fakeSpanId whenever(mockSpanContext.toTraceId()) doReturn fakeTraceId + whenever(mockTraceSampler.sample()) doReturn true fakeMediaType = if (forge.aBool()) { val mediaType = forge.anElementFrom("application", "image", "text", "model") + @@ -194,6 +199,7 @@ internal open class TracingInterceptorNotSendingSpanTest { mockRequestListener, mockDetector, fakeOrigin, + mockTraceSampler, factory ) { override fun canSendSpan(): Boolean { @@ -228,6 +234,30 @@ internal open class TracingInterceptorNotSendingSpanTest { } } + @Test + fun `𝕄 inject non-tracing header 𝕎 intercept() {global known host + not sampled}`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + verifyZeroInteractions(mockTracer, mockLocalTracer) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(TracingInterceptor.SAMPLING_PRIORITY_HEADER)) + .isEqualTo("0") + assertThat(lastValue.header(TracingInterceptor.TRACE_ID_HEADER)).isNull() + assertThat(lastValue.header(TracingInterceptor.SPAN_ID_HEADER)).isNull() + } + } + @Test fun `𝕄 inject tracing header 𝕎 intercept() {local known host}`( @StringForgery key: String, @@ -253,6 +283,33 @@ internal open class TracingInterceptorNotSendingSpanTest { } } + @Test + fun `𝕄 inject non-tracing header 𝕎 intercept() {local known host + not sampled}`( + @IntForgery(min = 200, max = 600) statusCode: Int, + forge: Forge + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + fakeUrl = forgeUrl(forge, forge.anElementFrom(fakeLocalHosts)) + fakeRequest = forgeRequest(forge) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(false) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + verifyZeroInteractions(mockTracer, mockLocalTracer) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(TracingInterceptor.SAMPLING_PRIORITY_HEADER)) + .isEqualTo("0") + assertThat(lastValue.header(TracingInterceptor.TRACE_ID_HEADER)).isNull() + assertThat(lastValue.header(TracingInterceptor.SPAN_ID_HEADER)).isNull() + } + } + @Test fun `𝕄 inject tracing header 𝕎 intercept() for request with parent span`( @StringForgery key: String, @@ -314,6 +371,23 @@ internal open class TracingInterceptorNotSendingSpanTest { verify(mockSpanBuilder).withOrigin(getExpectedOrigin()) } + @Test + fun `𝕄 not create a span 𝕎 intercept() { not sampled }`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + verifyZeroInteractions(mockTracer, mockSpan, mockSpanBuilder, mockLocalTracer) + } + @Test fun `𝕄 create a span with info 𝕎 intercept() for successful request`( @IntForgery(min = 200, max = 300) statusCode: Int @@ -562,6 +636,23 @@ internal open class TracingInterceptorNotSendingSpanTest { assertThat(response).isSameAs(fakeResponse) } + @Test + fun `𝕄 not call the listener 𝕎 intercept() for completed request { not sampled }`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + verifyZeroInteractions(mockRequestListener) + assertThat(response).isSameAs(fakeResponse) + } + @Test fun `𝕄 call the listener 𝕎 intercept() for throwing request`( @Forgery throwable: Throwable, @@ -594,6 +685,23 @@ internal open class TracingInterceptorNotSendingSpanTest { verify(mockSpan as MutableSpan).drop() } + @Test + fun `𝕄 not call the listener 𝕎 intercept() for throwing request { not sampled }`( + @Forgery throwable: Throwable + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + whenever(mockChain.request()) doReturn fakeRequest + whenever(mockChain.proceed(any())) doThrow throwable + + assertThrows(throwable.message.orEmpty()) { + testedInterceptor.intercept(mockChain) + } + + verifyZeroInteractions(mockRequestListener) + } + @Test fun `M do not update the hostDetector W host list provided`(forge: Forge) { // GIVEN @@ -639,6 +747,7 @@ internal open class TracingInterceptorNotSendingSpanTest { fun `𝕄 create only one local tracer 𝕎 intercept() called from multiple threads`( @IntForgery(min = 200, max = 300) statusCode: Int ) { + // Given var called = 0 val tracedHosts = listOf(fakeHostIp, fakeHostName) whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) @@ -649,6 +758,17 @@ internal open class TracingInterceptorNotSendingSpanTest { GlobalTracer::class.java.setStaticValue("isRegistered", false) stubChain(mockChain, statusCode) + // need this setup, otherwise #intercept actually throws NPE, which pollutes the log + val localSpanBuilder: DDTracer.DDSpanBuilder = mock() + val localSpan: Span = mock(extraInterfaces = arrayOf(MutableSpan::class)) + whenever(localSpanBuilder.asChildOf(null as SpanContext?)) doReturn localSpanBuilder + whenever(localSpanBuilder.start()) doReturn localSpan + whenever(localSpan.context()) doReturn mockSpanContext + whenever(mockSpanContext.toSpanId()) doReturn fakeSpanId + whenever(mockSpanContext.toTraceId()) doReturn fakeTraceId + whenever(mockLocalTracer.buildSpan(TracingInterceptor.SPAN_NAME)) doReturn localSpanBuilder + + // When val countDownLatch = CountDownLatch(2) Thread { testedInterceptor.intercept(mockChain) diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorTest.kt index b3ab072dc4..e0b521e73c 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorTest.kt @@ -11,6 +11,8 @@ import android.util.Log import com.datadog.android.Datadog import com.datadog.android.core.configuration.Configuration import com.datadog.android.core.internal.net.FirstPartyHostDetector +import com.datadog.android.core.internal.sampling.RateBasedSampler +import com.datadog.android.core.internal.sampling.Sampler import com.datadog.android.core.internal.utils.loggableStackTrace import com.datadog.android.tracing.internal.TracingFeature import com.datadog.android.utils.config.ApplicationContextTestConfiguration @@ -108,6 +110,9 @@ internal open class TracingInterceptorTest { @Mock lateinit var mockDetector: FirstPartyHostDetector + @Mock + lateinit var mockTraceSampler: Sampler + // endregion // region Fakes @@ -155,6 +160,7 @@ internal open class TracingInterceptorTest { whenever(mockSpan.context()) doReturn mockSpanContext whenever(mockSpanContext.toSpanId()) doReturn fakeSpanId whenever(mockSpanContext.toTraceId()) doReturn fakeTraceId + whenever(mockTraceSampler.sample()) doReturn true val mediaType = forge.anElementFrom("application", "image", "text", "model") + "/" + forge.anAlphabeticalString() @@ -181,6 +187,7 @@ internal open class TracingInterceptorTest { mockRequestListener, mockDetector, fakeOrigin, + mockTraceSampler, factory ) } @@ -196,7 +203,7 @@ internal open class TracingInterceptorTest { } @Test - fun `M instantiate with default callbacks W init()`() { + fun `M instantiate with default values W init()`() { // When val interceptor = TracingInterceptor() @@ -204,10 +211,16 @@ internal open class TracingInterceptorTest { assertThat(interceptor.tracedHosts).isEmpty() assertThat(interceptor.tracedRequestListener) .isInstanceOf(NoOpTracedRequestListener::class.java) + assertThat(interceptor.traceSampler) + .isInstanceOf(RateBasedSampler::class.java) + val traceSampler = interceptor.traceSampler as RateBasedSampler + assertThat(traceSampler.sampleRate).isEqualTo( + TracingInterceptor.DEFAULT_TRACE_SAMPLING_RATE / 100 + ) } @Test - fun `M instantiate with default callbacks W init()`( + fun `M instantiate with default values W init()`( @StringForgery(regex = "[a-z]+\\.[a-z]{3}") hosts: List ) { // When @@ -217,6 +230,12 @@ internal open class TracingInterceptorTest { assertThat(interceptor.tracedHosts).containsAll(hosts) assertThat(interceptor.tracedRequestListener) .isInstanceOf(NoOpTracedRequestListener::class.java) + assertThat(interceptor.traceSampler) + .isInstanceOf(RateBasedSampler::class.java) + val traceSampler = interceptor.traceSampler as RateBasedSampler + assertThat(traceSampler.sampleRate).isEqualTo( + TracingInterceptor.DEFAULT_TRACE_SAMPLING_RATE / 100 + ) } @Test @@ -241,6 +260,30 @@ internal open class TracingInterceptorTest { } } + @Test + fun `𝕄 inject non-tracing header 𝕎 intercept() {global known host + not sampled}`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + verifyZeroInteractions(mockTracer, mockLocalTracer) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(TracingInterceptor.SAMPLING_PRIORITY_HEADER)) + .isEqualTo("0") + assertThat(lastValue.header(TracingInterceptor.TRACE_ID_HEADER)).isNull() + assertThat(lastValue.header(TracingInterceptor.SPAN_ID_HEADER)).isNull() + } + } + @Test fun `𝕄 inject tracing header 𝕎 intercept() {local known host}`( @StringForgery key: String, @@ -266,6 +309,33 @@ internal open class TracingInterceptorTest { } } + @Test + fun `𝕄 inject non-tracing header 𝕎 intercept() {local known host + not sampled}`( + @IntForgery(min = 200, max = 600) statusCode: Int, + forge: Forge + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + fakeUrl = forgeUrlWithQueryParams(forge, forge.anElementFrom(fakeLocalHosts)) + fakeRequest = forgeRequest(forge) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(false) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + verifyZeroInteractions(mockTracer, mockLocalTracer) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(TracingInterceptor.SAMPLING_PRIORITY_HEADER)) + .isEqualTo("0") + assertThat(lastValue.header(TracingInterceptor.TRACE_ID_HEADER)).isNull() + assertThat(lastValue.header(TracingInterceptor.SPAN_ID_HEADER)).isNull() + } + } + @Test fun `𝕄 inject tracing header 𝕎 intercept() for request with parent span`( @StringForgery key: String, @@ -395,6 +465,23 @@ internal open class TracingInterceptorTest { verify(mockSpanBuilder).withOrigin(getExpectedOrigin()) } + @Test + fun `𝕄 not create a span 𝕎 intercept() { not sampled }`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + verifyZeroInteractions(mockTracer, mockSpan, mockSpanBuilder, mockLocalTracer) + } + @Test fun `𝕄 create a span with info 𝕎 intercept() for successful request`( @IntForgery(min = 200, max = 300) statusCode: Int @@ -665,6 +752,23 @@ internal open class TracingInterceptorTest { assertThat(response).isSameAs(fakeResponse) } + @Test + fun `𝕄 not call the listener 𝕎 intercept() for completed request { not sampled }`( + @IntForgery(min = 200, max = 600) statusCode: Int + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + stubChain(mockChain, statusCode) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + verifyZeroInteractions(mockRequestListener) + assertThat(response).isSameAs(fakeResponse) + } + @Test fun `𝕄 call the listener 𝕎 intercept() for throwing request`( @Forgery throwable: Throwable, @@ -696,6 +800,23 @@ internal open class TracingInterceptorTest { verify(mockSpan).finish() } + @Test + fun `𝕄 not call the listener 𝕎 intercept() for throwing request { not sampled }`( + @Forgery throwable: Throwable + ) { + // Given + whenever(mockTraceSampler.sample()).thenReturn(false) + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + whenever(mockChain.request()) doReturn fakeRequest + whenever(mockChain.proceed(any())) doThrow throwable + + assertThrows(throwable.message.orEmpty()) { + testedInterceptor.intercept(mockChain) + } + + verifyZeroInteractions(mockRequestListener) + } + @Test fun `M do not update the hostDetector W host list provided`(forge: Forge) { // GIVEN @@ -741,6 +862,7 @@ internal open class TracingInterceptorTest { fun `𝕄 create only one local tracer 𝕎 intercept() called from multiple threads`( @IntForgery(min = 200, max = 300) statusCode: Int ) { + // Given var called = 0 testedInterceptor = instantiateTestedInterceptor { called++ @@ -750,6 +872,17 @@ internal open class TracingInterceptorTest { whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) stubChain(mockChain, statusCode) + // need this setup, otherwise #intercept actually throws NPE, which pollutes the log + val localSpanBuilder: DDTracer.DDSpanBuilder = mock() + val localSpan: Span = mock(extraInterfaces = arrayOf(MutableSpan::class)) + whenever(localSpanBuilder.asChildOf(null as SpanContext?)) doReturn localSpanBuilder + whenever(localSpanBuilder.start()) doReturn localSpan + whenever(localSpan.context()) doReturn mockSpanContext + whenever(mockSpanContext.toSpanId()) doReturn fakeSpanId + whenever(mockSpanContext.toTraceId()) doReturn fakeTraceId + whenever(mockLocalTracer.buildSpan(TracingInterceptor.SPAN_NAME)) doReturn localSpanBuilder + + // When val countDownLatch = CountDownLatch(2) Thread { testedInterceptor.intercept(mockChain) diff --git a/docs/trace_collection.md b/docs/trace_collection.md index df5e3c0a76..0dba1c6038 100644 --- a/docs/trace_collection.md +++ b/docs/trace_collection.md @@ -435,15 +435,20 @@ If you want to trace your OkHttp requests, you can add the provided [Interceptor {{< tabs >}} {{% tab "Kotlin" %}} ```kotlin -val okHttpClient = OkHttpClient.Builder() - .addInterceptor(DatadogInterceptor(listOf("example.com", "example.eu"))) +val okHttpClient = OkHttpClient.Builder() + .addInterceptor( + DatadogInterceptor(listOf("example.com", "example.eu"), traceSamplingRate = 20f) + ) .build() ``` {{% /tab %}} {{% tab "Java" %}} ```java +float traceSamplingRate = 20f; final OkHttpClient okHttpClient = new OkHttpClient.Builder() - .addInterceptor(new DatadogInterceptor(Arrays.asList("example.com", "example.eu"))) + .addInterceptor( + new DatadogInterceptor(Arrays.asList("example.com", "example.eu"), traceSamplingRate) + ) .build(); ``` {{% /tab %}} @@ -451,6 +456,8 @@ final OkHttpClient okHttpClient = new OkHttpClient.Builder() This creates a span around each request processed by the OkHttpClient (matching the provided hosts), with all the relevant information automatically filled (URL, method, status code, error), and propagates the tracing information to your backend to get a unified trace within Datadog. +Network traces are sampled with an adjustable sampling rate. A sampling of 20% is applied by default. + The interceptor tracks requests at the application level. You can also add a `TracingInterceptor` at the network level to get more details, for example when following redirections. {{< tabs >}} @@ -458,17 +465,18 @@ The interceptor tracks requests at the application level. You can also add a `Tr ```kotlin val tracedHosts = listOf("example.com", "example.eu") val okHttpClient = OkHttpClient.Builder() - .addInterceptor(DatadogInterceptor(tracedHosts)) - .addNetworkInterceptor(TracingInterceptor(tracedHosts)) + .addInterceptor(DatadogInterceptor(tracedHosts, traceSamplingRate = 20f)) + .addNetworkInterceptor(TracingInterceptor(tracedHosts, traceSamplingRate = 20f)) .build() ``` {{% /tab %}} {{% tab "Java" %}} ```java +float traceSamplingRate = 20f; final List tracedHosts = Arrays.asList("example.com", "example.eu"); final OkHttpClient okHttpClient = new OkHttpClient.Builder() - .addInterceptor(new DatadogInterceptor(tracedHosts)) - .addNetworkInterceptor(new TracingInterceptor(tracedHosts)) + .addInterceptor(new DatadogInterceptor(tracedHosts, traceSamplingRate)) + .addNetworkInterceptor(new TracingInterceptor(tracedHosts, traceSamplingRate)) .build(); ``` {{% /tab %}} diff --git a/sample/kotlin/src/main/kotlin/com/datadog/android/sample/SampleApplication.kt b/sample/kotlin/src/main/kotlin/com/datadog/android/sample/SampleApplication.kt index 489e5258a9..716e7b1aec 100644 --- a/sample/kotlin/src/main/kotlin/com/datadog/android/sample/SampleApplication.kt +++ b/sample/kotlin/src/main/kotlin/com/datadog/android/sample/SampleApplication.kt @@ -56,8 +56,8 @@ class SampleApplication : Application() { ) private val okHttpClient = OkHttpClient.Builder() - .addInterceptor(RumInterceptor()) - .addNetworkInterceptor(TracingInterceptor()) + .addInterceptor(RumInterceptor(traceSamplingRate = 100f)) + .addNetworkInterceptor(TracingInterceptor(traceSamplingRate = 100f)) .eventListenerFactory(DatadogEventListener.Factory()) .build() diff --git a/sample/kotlin/src/main/kotlin/com/datadog/android/sample/picture/SampleGlideModule.kt b/sample/kotlin/src/main/kotlin/com/datadog/android/sample/picture/SampleGlideModule.kt index 07dced0ec6..509015af7b 100644 --- a/sample/kotlin/src/main/kotlin/com/datadog/android/sample/picture/SampleGlideModule.kt +++ b/sample/kotlin/src/main/kotlin/com/datadog/android/sample/picture/SampleGlideModule.kt @@ -19,8 +19,6 @@ class SampleGlideModule : DatadogGlideModule(listOf("shopist.io")) { override fun applyOptions(context: Context, builder: GlideBuilder) { super.applyOptions(context, builder) - val size10mb = 10485760L - val factory = InternalCacheDiskCacheFactory(context, "glide", size10mb) builder.setMemoryCache(LruResourceCache(1)) builder.setLogLevel(Log.VERBOSE) } From efafff391d9eea9b60f362a35416cfb577f08168 Mon Sep 17 00:00:00 2001 From: Nikita Ogorodnikov Date: Thu, 19 May 2022 09:27:21 +0200 Subject: [PATCH 2/3] RUMM-2165: Add E2E test for trace sampling of network requests --- .../rum/RumResourceTrackingE2ETests.kt | 48 ++++++++--- .../src/main/AndroidManifest.xml | 1 + .../activities/ResourceTrackingActivity.kt | 2 +- ...esourceTrackingCustomAttributesActivity.kt | 3 +- ...rceTrackingCustomSpanAttributesActivity.kt | 5 +- ...ourceTrackingNetworkInterceptorActivity.kt | 5 +- .../ResourceTrackingTraceSamplingActivity.kt | 80 +++++++++++++++++++ .../android/nightly/server/LocalServer.kt | 15 +++- 8 files changed, 139 insertions(+), 20 deletions(-) create mode 100644 instrumented/nightly-tests/src/main/kotlin/com/datadog/android/nightly/activities/ResourceTrackingTraceSamplingActivity.kt diff --git a/instrumented/nightly-tests/src/androidTest/kotlin/com/datadog/android/nightly/rum/RumResourceTrackingE2ETests.kt b/instrumented/nightly-tests/src/androidTest/kotlin/com/datadog/android/nightly/rum/RumResourceTrackingE2ETests.kt index d244a7f166..ba26e2871a 100644 --- a/instrumented/nightly-tests/src/androidTest/kotlin/com/datadog/android/nightly/rum/RumResourceTrackingE2ETests.kt +++ b/instrumented/nightly-tests/src/androidTest/kotlin/com/datadog/android/nightly/rum/RumResourceTrackingE2ETests.kt @@ -15,6 +15,7 @@ import com.datadog.android.nightly.activities.ResourceTrackingCustomAttributesAc import com.datadog.android.nightly.activities.ResourceTrackingCustomSpanAttributesActivity import com.datadog.android.nightly.activities.ResourceTrackingFirstPartyHostsActivity import com.datadog.android.nightly.activities.ResourceTrackingNetworkInterceptorActivity +import com.datadog.android.nightly.activities.ResourceTrackingTraceSamplingActivity import com.datadog.android.nightly.rules.NightlyTestRule import com.datadog.android.nightly.utils.defaultConfigurationBuilder import com.datadog.android.nightly.utils.initializeSdk @@ -34,7 +35,7 @@ internal class RumResourceTrackingE2ETests { /** * apiMethodSignature: com.datadog.android.core.configuration.Configuration$Builder#fun useViewTrackingStrategy(com.datadog.android.rum.tracking.ViewTrackingStrategy): Builder * apiMethodSignature: com.datadog.android.rum.tracking.ActivityViewTrackingStrategy#constructor(Boolean, ComponentPredicate = AcceptAllActivities()) - * apiMethodSignature: com.datadog.android.rum.RumInterceptor#constructor(List = emptyList(), RumResourceAttributesProvider = NoOpRumResourceAttributesProvider()) + * apiMethodSignature: com.datadog.android.rum.RumInterceptor#constructor(List = emptyList(), RumResourceAttributesProvider = NoOpRumResourceAttributesProvider(), Float = DEFAULT_TRACE_SAMPLING_RATE) */ @Test fun rum_resource_tracking() { @@ -58,8 +59,8 @@ internal class RumResourceTrackingE2ETests { /** * apiMethodSignature: com.datadog.android.core.configuration.Configuration$Builder#fun useViewTrackingStrategy(com.datadog.android.rum.tracking.ViewTrackingStrategy): Builder * apiMethodSignature: com.datadog.android.rum.tracking.ActivityViewTrackingStrategy#constructor(Boolean, ComponentPredicate = AcceptAllActivities()) - * apiMethodSignature: com.datadog.android.rum.RumInterceptor#constructor(List = emptyList(), RumResourceAttributesProvider = NoOpRumResourceAttributesProvider()) - * apiMethodSignature: com.datadog.android.DatadogInterceptor#constructor(List, com.datadog.android.tracing.TracedRequestListener = NoOpTracedRequestListener(), com.datadog.android.rum.RumResourceAttributesProvider = NoOpRumResourceAttributesProvider()) + * apiMethodSignature: com.datadog.android.rum.RumInterceptor#constructor(List = emptyList(), RumResourceAttributesProvider = NoOpRumResourceAttributesProvider(), Float = DEFAULT_TRACE_SAMPLING_RATE) + * apiMethodSignature: com.datadog.android.DatadogInterceptor#constructor(List, com.datadog.android.tracing.TracedRequestListener = NoOpTracedRequestListener(), com.datadog.android.rum.RumResourceAttributesProvider = NoOpRumResourceAttributesProvider(), Float = DEFAULT_TRACE_SAMPLING_RATE) */ @Test fun rum_resource_tracking_with_custom_attributes() { @@ -83,9 +84,9 @@ internal class RumResourceTrackingE2ETests { /** * apiMethodSignature: com.datadog.android.core.configuration.Configuration$Builder#fun useViewTrackingStrategy(com.datadog.android.rum.tracking.ViewTrackingStrategy): Builder * apiMethodSignature: com.datadog.android.rum.tracking.ActivityViewTrackingStrategy#constructor(Boolean, ComponentPredicate = AcceptAllActivities()) - * apiMethodSignature: com.datadog.android.tracing.TracingInterceptor#constructor(TracedRequestListener = NoOpTracedRequestListener()) - * apiMethodSignature: com.datadog.android.DatadogInterceptor#constructor(com.datadog.android.tracing.TracedRequestListener = NoOpTracedRequestListener(), com.datadog.android.rum.RumResourceAttributesProvider = NoOpRumResourceAttributesProvider()) - * apiMethodSignature: com.datadog.android.rum.RumInterceptor#constructor(List = emptyList(), RumResourceAttributesProvider = NoOpRumResourceAttributesProvider()) + * apiMethodSignature: com.datadog.android.tracing.TracingInterceptor#constructor(TracedRequestListener = NoOpTracedRequestListener(), Float = DEFAULT_TRACE_SAMPLING_RATE) + * apiMethodSignature: com.datadog.android.DatadogInterceptor#constructor(com.datadog.android.tracing.TracedRequestListener = NoOpTracedRequestListener(), com.datadog.android.rum.RumResourceAttributesProvider = NoOpRumResourceAttributesProvider(), Float = DEFAULT_TRACE_SAMPLING_RATE) + * apiMethodSignature: com.datadog.android.rum.RumInterceptor#constructor(List = emptyList(), RumResourceAttributesProvider = NoOpRumResourceAttributesProvider(), Float = DEFAULT_TRACE_SAMPLING_RATE) * apiMethodSignature: com.datadog.android.core.configuration.Configuration$Builder#fun setFirstPartyHosts(List): Builder */ @Test @@ -115,8 +116,8 @@ internal class RumResourceTrackingE2ETests { /** * apiMethodSignature: com.datadog.android.core.configuration.Configuration$Builder#fun useViewTrackingStrategy(com.datadog.android.rum.tracking.ViewTrackingStrategy): Builder * apiMethodSignature: com.datadog.android.rum.tracking.ActivityViewTrackingStrategy#constructor(Boolean, ComponentPredicate = AcceptAllActivities()) - * apiMethodSignature: com.datadog.android.rum.RumInterceptor#constructor(List = emptyList(), RumResourceAttributesProvider = NoOpRumResourceAttributesProvider()) - * apiMethodSignature: com.datadog.android.tracing.TracingInterceptor#constructor(List, TracedRequestListener = NoOpTracedRequestListener()) + * apiMethodSignature: com.datadog.android.rum.RumInterceptor#constructor(List = emptyList(), RumResourceAttributesProvider = NoOpRumResourceAttributesProvider(), Float = DEFAULT_TRACE_SAMPLING_RATE) + * apiMethodSignature: com.datadog.android.tracing.TracingInterceptor#constructor(List, TracedRequestListener = NoOpTracedRequestListener(), Float = DEFAULT_TRACE_SAMPLING_RATE) */ @Test fun rum_resource_tracking_with_network_interceptor() { @@ -143,8 +144,8 @@ internal class RumResourceTrackingE2ETests { /** * apiMethodSignature: com.datadog.android.core.configuration.Configuration$Builder#fun useViewTrackingStrategy(com.datadog.android.rum.tracking.ViewTrackingStrategy): Builder * apiMethodSignature: com.datadog.android.rum.tracking.ActivityViewTrackingStrategy#constructor(Boolean, ComponentPredicate = AcceptAllActivities()) - * apiMethodSignature: com.datadog.android.rum.RumInterceptor#constructor(List = emptyList(), RumResourceAttributesProvider = NoOpRumResourceAttributesProvider()) - * apiMethodSignature: com.datadog.android.DatadogInterceptor#constructor(List, com.datadog.android.tracing.TracedRequestListener = NoOpTracedRequestListener(), com.datadog.android.rum.RumResourceAttributesProvider = NoOpRumResourceAttributesProvider()) + * apiMethodSignature: com.datadog.android.rum.RumInterceptor#constructor(List = emptyList(), RumResourceAttributesProvider = NoOpRumResourceAttributesProvider(), Float = DEFAULT_TRACE_SAMPLING_RATE) + * apiMethodSignature: com.datadog.android.DatadogInterceptor#constructor(List, com.datadog.android.tracing.TracedRequestListener = NoOpTracedRequestListener(), com.datadog.android.rum.RumResourceAttributesProvider = NoOpRumResourceAttributesProvider(), Float = DEFAULT_TRACE_SAMPLING_RATE) * apiMethodSignature: com.datadog.android.core.configuration.Configuration$Builder#fun setFirstPartyHosts(List): Builder */ @Test @@ -168,4 +169,31 @@ internal class RumResourceTrackingE2ETests { } launch(ResourceTrackingCustomSpanAttributesActivity::class.java) } + + /** + * apiMethodSignature: com.datadog.android.core.configuration.Configuration$Builder#fun useViewTrackingStrategy(com.datadog.android.rum.tracking.ViewTrackingStrategy): Builder + * apiMethodSignature: com.datadog.android.rum.tracking.ActivityViewTrackingStrategy#constructor(Boolean, ComponentPredicate = AcceptAllActivities()) + * apiMethodSignature: com.datadog.android.rum.RumInterceptor#constructor(List = emptyList(), RumResourceAttributesProvider = NoOpRumResourceAttributesProvider(), Float = DEFAULT_TRACE_SAMPLING_RATE) + */ + @Test + fun rum_resource_tracking_trace_sampling_75_percent() { + // this test is backed by 2 monitors: + // 1. RUM monitor - it should check that number of RUM resources is not affected by sampling + // 2. APM monitor - number of traces should be affected by sampling + measureSdkInitialize { + val config = defaultConfigurationBuilder( + logsEnabled = true, + tracesEnabled = true, + crashReportsEnabled = true, + rumEnabled = true + ) + .useViewTrackingStrategy(ActivityViewTrackingStrategy(true)) + .build() + initializeSdk( + InstrumentationRegistry.getInstrumentation().targetContext, + config = config + ) + } + launch(ResourceTrackingTraceSamplingActivity::class.java) + } } diff --git a/instrumented/nightly-tests/src/main/AndroidManifest.xml b/instrumented/nightly-tests/src/main/AndroidManifest.xml index 22001e3372..61d6d9cf34 100644 --- a/instrumented/nightly-tests/src/main/AndroidManifest.xml +++ b/instrumented/nightly-tests/src/main/AndroidManifest.xml @@ -30,6 +30,7 @@ + Unit) { engine = embeddedServer(Netty, PORT) { install(ContentNegotiation) { gson {} } routing { get(PATH) { - call.respondRedirect(redirection, true) + callAction.invoke(call) } } } - engine?.start(wait = false) + engine.start(wait = false) } fun stop() { Thread { - engine?.stop(SHUTDOWN_MS, STOP_TIMEOUT_MS) + engine.stop(SHUTDOWN_MS, STOP_TIMEOUT_MS) }.start() } From 2a08eaebd70e2b0b7159db0dd4c00ab4a4286c15 Mon Sep 17 00:00:00 2001 From: Nikita Ogorodnikov Date: Thu, 19 May 2022 11:36:49 +0200 Subject: [PATCH 3/3] RUMM-2165: Respect sampling decision already made in the okhttp chain --- .../android/tracing/TracingInterceptor.kt | 12 +++- ...nterceptorNonDdTracerNotSendingSpanTest.kt | 71 +++++++++++++++++++ .../TracingInterceptorNonDdTracerTest.kt | 71 +++++++++++++++++++ .../TracingInterceptorNotSendingSpanTest.kt | 71 +++++++++++++++++++ .../android/tracing/TracingInterceptorTest.kt | 71 +++++++++++++++++++ detekt.yml | 1 + docs/trace_collection.md | 2 + 7 files changed, 298 insertions(+), 1 deletion(-) diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/TracingInterceptor.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/TracingInterceptor.kt index 624b0f5066..23c418b4df 100644 --- a/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/TracingInterceptor.kt +++ b/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/TracingInterceptor.kt @@ -21,6 +21,7 @@ import com.datadog.android.tracing.internal.TracingFeature import com.datadog.opentracing.DDTracer import com.datadog.trace.api.DDTags import com.datadog.trace.api.interceptor.MutableSpan +import com.datadog.trace.api.sampling.PrioritySampling import io.opentracing.Span import io.opentracing.SpanContext import io.opentracing.Tracer @@ -195,7 +196,8 @@ internal constructor( request: Request, tracer: Tracer ): Response { - val span = if (traceSampler.sample()) { + val isSampled = extractSamplingDecision(request) ?: traceSampler.sample() + val span = if (isSampled) { buildSpan(tracer, request) } else { null @@ -275,6 +277,14 @@ internal constructor( return span } + private fun extractSamplingDecision(request: Request): Boolean? { + val samplingPriority = request.header(SAMPLING_PRIORITY_HEADER)?.toIntOrNull() + ?: return null + if (samplingPriority == PrioritySampling.UNSET) return null + return samplingPriority == PrioritySampling.USER_KEEP || + samplingPriority == PrioritySampling.SAMPLER_KEEP + } + private fun extractParentContext(tracer: Tracer, request: Request): SpanContext? { val tagContext = request.tag(Span::class.java)?.context() diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerNotSendingSpanTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerNotSendingSpanTest.kt index 74e0c4db23..33332c920d 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerNotSendingSpanTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerNotSendingSpanTest.kt @@ -24,6 +24,7 @@ import com.datadog.tools.unit.extensions.TestConfigurationExtension import com.datadog.tools.unit.extensions.config.TestConfiguration import com.datadog.tools.unit.setStaticValue import com.datadog.trace.api.interceptor.MutableSpan +import com.datadog.trace.api.sampling.PrioritySampling import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.argumentCaptor @@ -368,6 +369,76 @@ internal open class TracingInterceptorNonDdTracerNotSendingSpanTest { verify(mockSpanBuilder).asChildOf(parentSpanContext) } + @Test + fun `𝕄 respect sampling decision 𝕎 intercept() {sampled in upstream interceptor}`( + @IntForgery(min = 200, max = 600) statusCode: Int, + @StringForgery key: String, + @StringForgery(type = StringForgeryType.ALPHA_NUMERICAL) value: String, + forge: Forge + ) { + // Given + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + fakeRequest = forgeRequest(forge) { + it.addHeader( + TracingInterceptor.SAMPLING_PRIORITY_HEADER, + forge.anElementFrom( + PrioritySampling.SAMPLER_KEEP.toString(), + PrioritySampling.USER_KEEP.toString() + ) + ) + } + stubChain(mockChain, statusCode) + whenever(mockTraceSampler.sample()).thenReturn(false) + doAnswer { invocation -> + val carrier = invocation.arguments[2] as TextMapInject + carrier.put(key, value) + }.whenever(mockTracer).inject(any(), any(), any()) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(key)).isEqualTo(value) + } + } + + @Test + fun `𝕄 respect sampling decision 𝕎 intercept() {sampled out in upstream interceptor}`( + @IntForgery(min = 200, max = 600) statusCode: Int, + forge: Forge + ) { + // Given + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + fakeRequest = forgeRequest(forge) { + it.addHeader( + TracingInterceptor.SAMPLING_PRIORITY_HEADER, + forge.anElementFrom( + PrioritySampling.SAMPLER_DROP.toString(), + PrioritySampling.USER_DROP.toString() + ) + ) + } + stubChain(mockChain, statusCode) + whenever(mockTraceSampler.sample()).thenReturn(true) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(TracingInterceptor.SAMPLING_PRIORITY_HEADER)) + .isEqualTo("0") + assertThat(lastValue.header(TracingInterceptor.TRACE_ID_HEADER)).isNull() + assertThat(lastValue.header(TracingInterceptor.SPAN_ID_HEADER)).isNull() + } + verifyZeroInteractions(mockTracer, mockLocalTracer, mockSpanBuilder, mockSpan) + } + @Test fun `𝕄 not create a span 𝕎 intercept() { not sampled }`( @IntForgery(min = 200, max = 600) statusCode: Int diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerTest.kt index 03282b2995..59259b5f27 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNonDdTracerTest.kt @@ -25,6 +25,7 @@ import com.datadog.tools.unit.extensions.TestConfigurationExtension import com.datadog.tools.unit.extensions.config.TestConfiguration import com.datadog.tools.unit.setStaticValue import com.datadog.trace.api.interceptor.MutableSpan +import com.datadog.trace.api.sampling.PrioritySampling import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.argumentCaptor @@ -461,6 +462,76 @@ internal open class TracingInterceptorNonDdTracerTest { verify(mockSpanBuilder).asChildOf(parentSpanContext) } + @Test + fun `𝕄 respect sampling decision 𝕎 intercept() {sampled in upstream interceptor}`( + @IntForgery(min = 200, max = 600) statusCode: Int, + @StringForgery key: String, + @StringForgery(type = StringForgeryType.ALPHA_NUMERICAL) value: String, + forge: Forge + ) { + // Given + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + fakeRequest = forgeRequest(forge) { + it.addHeader( + TracingInterceptor.SAMPLING_PRIORITY_HEADER, + forge.anElementFrom( + PrioritySampling.SAMPLER_KEEP.toString(), + PrioritySampling.USER_KEEP.toString() + ) + ) + } + stubChain(mockChain, statusCode) + whenever(mockTraceSampler.sample()).thenReturn(false) + doAnswer { invocation -> + val carrier = invocation.arguments[2] as TextMapInject + carrier.put(key, value) + }.whenever(mockTracer).inject(any(), any(), any()) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(key)).isEqualTo(value) + } + } + + @Test + fun `𝕄 respect sampling decision 𝕎 intercept() {sampled out in upstream interceptor}`( + @IntForgery(min = 200, max = 600) statusCode: Int, + forge: Forge + ) { + // Given + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + fakeRequest = forgeRequest(forge) { + it.addHeader( + TracingInterceptor.SAMPLING_PRIORITY_HEADER, + forge.anElementFrom( + PrioritySampling.SAMPLER_DROP.toString(), + PrioritySampling.USER_DROP.toString() + ) + ) + } + stubChain(mockChain, statusCode) + whenever(mockTraceSampler.sample()).thenReturn(true) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(TracingInterceptor.SAMPLING_PRIORITY_HEADER)) + .isEqualTo("0") + assertThat(lastValue.header(TracingInterceptor.TRACE_ID_HEADER)).isNull() + assertThat(lastValue.header(TracingInterceptor.SPAN_ID_HEADER)).isNull() + } + verifyZeroInteractions(mockTracer, mockLocalTracer, mockSpanBuilder, mockSpan) + } + @Test fun `𝕄 not create a span 𝕎 intercept() { not sampled }`( @IntForgery(min = 200, max = 600) statusCode: Int diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNotSendingSpanTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNotSendingSpanTest.kt index 68807b8da0..2f7b1265b9 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNotSendingSpanTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorNotSendingSpanTest.kt @@ -25,6 +25,7 @@ import com.datadog.tools.unit.extensions.TestConfigurationExtension import com.datadog.tools.unit.extensions.config.TestConfiguration import com.datadog.tools.unit.setStaticValue import com.datadog.trace.api.interceptor.MutableSpan +import com.datadog.trace.api.sampling.PrioritySampling import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.argumentCaptor @@ -371,6 +372,76 @@ internal open class TracingInterceptorNotSendingSpanTest { verify(mockSpanBuilder).withOrigin(getExpectedOrigin()) } + @Test + fun `𝕄 respect sampling decision 𝕎 intercept() {sampled in upstream interceptor}`( + @IntForgery(min = 200, max = 600) statusCode: Int, + @StringForgery key: String, + @StringForgery(type = StringForgeryType.ALPHA_NUMERICAL) value: String, + forge: Forge + ) { + // Given + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + fakeRequest = forgeRequest(forge) { + it.addHeader( + TracingInterceptor.SAMPLING_PRIORITY_HEADER, + forge.anElementFrom( + PrioritySampling.SAMPLER_KEEP.toString(), + PrioritySampling.USER_KEEP.toString() + ) + ) + } + stubChain(mockChain, statusCode) + whenever(mockTraceSampler.sample()).thenReturn(false) + doAnswer { invocation -> + val carrier = invocation.arguments[2] as TextMapInject + carrier.put(key, value) + }.whenever(mockTracer).inject(any(), any(), any()) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(key)).isEqualTo(value) + } + } + + @Test + fun `𝕄 respect sampling decision 𝕎 intercept() {sampled out in upstream interceptor}`( + @IntForgery(min = 200, max = 600) statusCode: Int, + forge: Forge + ) { + // Given + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + fakeRequest = forgeRequest(forge) { + it.addHeader( + TracingInterceptor.SAMPLING_PRIORITY_HEADER, + forge.anElementFrom( + PrioritySampling.SAMPLER_DROP.toString(), + PrioritySampling.USER_DROP.toString() + ) + ) + } + stubChain(mockChain, statusCode) + whenever(mockTraceSampler.sample()).thenReturn(true) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(TracingInterceptor.SAMPLING_PRIORITY_HEADER)) + .isEqualTo("0") + assertThat(lastValue.header(TracingInterceptor.TRACE_ID_HEADER)).isNull() + assertThat(lastValue.header(TracingInterceptor.SPAN_ID_HEADER)).isNull() + } + verifyZeroInteractions(mockTracer, mockLocalTracer, mockSpanBuilder, mockSpan) + } + @Test fun `𝕄 not create a span 𝕎 intercept() { not sampled }`( @IntForgery(min = 200, max = 600) statusCode: Int diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorTest.kt index e0b521e73c..905a370d18 100644 --- a/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorTest.kt +++ b/dd-sdk-android/src/test/kotlin/com/datadog/android/tracing/TracingInterceptorTest.kt @@ -26,6 +26,7 @@ import com.datadog.tools.unit.extensions.TestConfigurationExtension import com.datadog.tools.unit.extensions.config.TestConfiguration import com.datadog.tools.unit.setStaticValue import com.datadog.trace.api.interceptor.MutableSpan +import com.datadog.trace.api.sampling.PrioritySampling import com.nhaarman.mockitokotlin2.any import com.nhaarman.mockitokotlin2.anyOrNull import com.nhaarman.mockitokotlin2.argumentCaptor @@ -465,6 +466,76 @@ internal open class TracingInterceptorTest { verify(mockSpanBuilder).withOrigin(getExpectedOrigin()) } + @Test + fun `𝕄 respect sampling decision 𝕎 intercept() {sampled in upstream interceptor}`( + @IntForgery(min = 200, max = 600) statusCode: Int, + @StringForgery key: String, + @StringForgery(type = StringForgeryType.ALPHA_NUMERICAL) value: String, + forge: Forge + ) { + // Given + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + fakeRequest = forgeRequest(forge) { + it.addHeader( + TracingInterceptor.SAMPLING_PRIORITY_HEADER, + forge.anElementFrom( + PrioritySampling.SAMPLER_KEEP.toString(), + PrioritySampling.USER_KEEP.toString() + ) + ) + } + stubChain(mockChain, statusCode) + whenever(mockTraceSampler.sample()).thenReturn(false) + doAnswer { invocation -> + val carrier = invocation.arguments[2] as TextMapInject + carrier.put(key, value) + }.whenever(mockTracer).inject(any(), any(), any()) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(key)).isEqualTo(value) + } + } + + @Test + fun `𝕄 respect sampling decision 𝕎 intercept() {sampled out in upstream interceptor}`( + @IntForgery(min = 200, max = 600) statusCode: Int, + forge: Forge + ) { + // Given + whenever(mockDetector.isFirstPartyUrl(HttpUrl.get(fakeUrl))).thenReturn(true) + fakeRequest = forgeRequest(forge) { + it.addHeader( + TracingInterceptor.SAMPLING_PRIORITY_HEADER, + forge.anElementFrom( + PrioritySampling.SAMPLER_DROP.toString(), + PrioritySampling.USER_DROP.toString() + ) + ) + } + stubChain(mockChain, statusCode) + whenever(mockTraceSampler.sample()).thenReturn(true) + + // When + val response = testedInterceptor.intercept(mockChain) + + // Then + assertThat(response).isSameAs(fakeResponse) + argumentCaptor { + verify(mockChain).proceed(capture()) + assertThat(lastValue.header(TracingInterceptor.SAMPLING_PRIORITY_HEADER)) + .isEqualTo("0") + assertThat(lastValue.header(TracingInterceptor.TRACE_ID_HEADER)).isNull() + assertThat(lastValue.header(TracingInterceptor.SPAN_ID_HEADER)).isNull() + } + verifyZeroInteractions(mockTracer, mockLocalTracer, mockSpanBuilder, mockSpan) + } + @Test fun `𝕄 not create a span 𝕎 intercept() { not sampled }`( @IntForgery(min = 200, max = 600) statusCode: Int diff --git a/detekt.yml b/detekt.yml index a362204294..a7ca7a79a0 100644 --- a/detekt.yml +++ b/detekt.yml @@ -1074,6 +1074,7 @@ datadog: - "kotlin.String.substringBefore(kotlin.Char, kotlin.String)" - "kotlin.String.toByteArray(java.nio.charset.Charset) " - "kotlin.String.toByteArray(java.nio.charset.Charset)" + - "kotlin.String.toIntOrNull()" - "kotlin.String.toDoubleOrNull()" - "kotlin.String.toLongOrNull()" - "kotlin.String.toMethod()" diff --git a/docs/trace_collection.md b/docs/trace_collection.md index 0dba1c6038..b4d67bfc47 100644 --- a/docs/trace_collection.md +++ b/docs/trace_collection.md @@ -482,6 +482,8 @@ final OkHttpClient okHttpClient = new OkHttpClient.Builder() {{% /tab %}} {{< /tabs >}} +In this case trace sampling decision made by the upstream interceptor for a particular request will be respected by the downstream interceptor. + Because the way the OkHttp Request is executed (using a Thread pool), the request span won't be automatically linked with the span that triggered the request. You can manually provide a parent span in the `OkHttp Request.Builder` as follows: {{< tabs >}}