From fdf5953d79d0732b8346b7331f7aeafe103da28e Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 14 Jun 2023 22:12:58 +0200 Subject: [PATCH 1/7] TwP --- .../core/ActivityLifecycleIntegration.java | 1 + .../gestures/SentryGestureListener.java | 1 + .../navigation/SentryNavigationListener.kt | 1 + .../android/okhttp/SentryOkHttpInterceptor.kt | 17 ++- .../apollo3/SentryApollo3HttpInterceptor.kt | 13 +- .../sentry/apollo/SentryApolloInterceptor.kt | 46 ++++--- .../sentry/openfeign/SentryFeignClient.java | 51 +++++--- .../opentelemetry/SentrySpanProcessor.java | 8 +- .../boot/jakarta/SentryAutoConfiguration.java | 1 - .../jakarta/SentryAutoConfigurationTest.kt | 12 +- .../spring/boot/SentryAutoConfiguration.java | 1 - .../boot/SentryAutoConfigurationTest.kt | 12 +- .../api/sentry-spring-jakarta.api | 2 +- ...entrySpanClientHttpRequestInterceptor.java | 38 ++++-- .../SentrySpanClientWebRequestFilter.java | 59 +++++---- .../jakarta/tracing/SentryTracingFilter.java | 118 ++++++++--------- .../webflux/AbstractSentryWebFilter.java | 56 ++++---- .../tracing/SentryTracingFilterTest.kt | 9 +- ...entrySpanClientHttpRequestInterceptor.java | 38 ++++-- .../SentrySpanClientWebRequestFilter.java | 55 ++++---- .../spring/tracing/SentryTracingFilter.java | 43 ++---- .../spring/webflux/SentryWebFilter.java | 46 +++---- .../spring/tracing/SentryTracingFilterTest.kt | 4 + sentry/api/sentry.api | 49 ++++++- sentry/src/main/java/io/sentry/Baggage.java | 15 +++ sentry/src/main/java/io/sentry/Hub.java | 51 +++++++- .../src/main/java/io/sentry/HubAdapter.java | 12 ++ sentry/src/main/java/io/sentry/IHub.java | 16 ++- sentry/src/main/java/io/sentry/NoOpHub.java | 12 ++ .../java/io/sentry/PropagationContext.java | 123 ++++++++++++++++++ sentry/src/main/java/io/sentry/Scope.java | 15 +++ sentry/src/main/java/io/sentry/Sentry.java | 19 ++- .../src/main/java/io/sentry/SentryClient.java | 15 ++- .../java/io/sentry/TransactionContext.java | 54 ++------ .../java/io/sentry/util/TracingUtils.java | 106 +++++++++++++++ .../java/io/sentry/TransactionContextTest.kt | 49 ++++--- 36 files changed, 789 insertions(+), 379 deletions(-) create mode 100644 sentry/src/main/java/io/sentry/PropagationContext.java create mode 100644 sentry/src/main/java/io/sentry/util/TracingUtils.java diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index f2717b4324b..a8e705de5c0 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -174,6 +174,7 @@ private void stopPreviousTransactions() { } } + // TODO should we start a new trace here if performance is disabled? private void startTracing(final @NotNull Activity activity) { WeakReference weakActivity = new WeakReference<>(activity); if (performanceEnabled && !isRunningTransaction(activity) && hub != null) { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java index 2bc8a52694f..88539454a07 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java @@ -184,6 +184,7 @@ private void addBreadcrumb( hint); } + // TODO should we start a new trace here if performance is disabled? private void startTracing(final @NotNull UiElement target, final @NotNull String eventType) { if (!(options.isTracingEnabled() && options.isEnableUserInteractionTracing())) { return; diff --git a/sentry-android-navigation/src/main/java/io/sentry/android/navigation/SentryNavigationListener.kt b/sentry-android-navigation/src/main/java/io/sentry/android/navigation/SentryNavigationListener.kt index 9661ee1925b..18e21137eb6 100644 --- a/sentry-android-navigation/src/main/java/io/sentry/android/navigation/SentryNavigationListener.kt +++ b/sentry-android-navigation/src/main/java/io/sentry/android/navigation/SentryNavigationListener.kt @@ -90,6 +90,7 @@ class SentryNavigationListener @JvmOverloads constructor( hub.addBreadcrumb(breadcrumb, hint) } + // TODO should we start a new trace here if performance is disabled? private fun startTracing( controller: NavController, destination: NavDestination, diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index f434764c695..5120e2e2eb6 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -18,6 +18,7 @@ import io.sentry.exception.SentryHttpClientException import io.sentry.protocol.Mechanism import io.sentry.util.HttpUtils import io.sentry.util.PropagationTargetsUtils +import io.sentry.util.TracingUtils import io.sentry.util.UrlUtils import okhttp3.Headers import okhttp3.Interceptor @@ -85,18 +86,20 @@ class SentryOkHttpInterceptor( var code: Int? = null try { val requestBuilder = request.newBuilder() - if (span != null && !span.isNoOp && - PropagationTargetsUtils.contain(hub.options.tracePropagationTargets, request.url.toString()) - ) { - span.toSentryTrace().let { - requestBuilder.addHeader(it.name, it.value) - } - span.toBaggageHeader(request.headers(BaggageHeader.BAGGAGE_HEADER))?.let { + TracingUtils.traceIfAllowed( + hub, + request.url.toString(), + request.headers(BaggageHeader.BAGGAGE_HEADER), + span + ) { tracingHeaders -> + requestBuilder.addHeader(tracingHeaders.sentryTraceHeader.name, tracingHeaders.sentryTraceHeader.value) + tracingHeaders.baggageHeader?.let { requestBuilder.removeHeader(BaggageHeader.BAGGAGE_HEADER) requestBuilder.addHeader(it.name, it.value) } } + request = requestBuilder.build() response = chain.proceed(request) code = response.code diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt index 736fd84256b..df1a9f9d922 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt +++ b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt @@ -18,7 +18,7 @@ import io.sentry.SentryIntegrationPackageStorage import io.sentry.SentryLevel import io.sentry.SpanStatus import io.sentry.TypeCheckHint -import io.sentry.util.PropagationTargetsUtils +import io.sentry.util.TracingUtils import io.sentry.util.UrlUtils import io.sentry.vendor.Base64 @@ -42,14 +42,11 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH var cleanedHeaders = removeSentryInternalHeaders(request.headers).toMutableList() - if (!span.isNoOp && PropagationTargetsUtils.contain(hub.options.tracePropagationTargets, request.url)) { - val sentryTraceHeader = span.toSentryTrace() - val baggageHeader = span.toBaggageHeader(request.headers.filter { it.name == BaggageHeader.BAGGAGE_HEADER }.map { it.value }) - cleanedHeaders.add(HttpHeader(sentryTraceHeader.name, sentryTraceHeader.value)) - - baggageHeader?.let { newHeader -> + TracingUtils.traceIfAllowed(hub, request.url, request.headers.filter { it.name == BaggageHeader.BAGGAGE_HEADER }.map { it.value }, span) { + cleanedHeaders.add(HttpHeader(it.sentryTraceHeader.name, it.sentryTraceHeader.value)) + it.baggageHeader?.let { baggageHeader -> cleanedHeaders = cleanedHeaders.filterNot { it.name == BaggageHeader.BAGGAGE_HEADER }.toMutableList().apply { - add(HttpHeader(newHeader.name, newHeader.value)) + add(HttpHeader(baggageHeader.name, baggageHeader.value)) } } } diff --git a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt index a52871b8fde..297c0222f42 100644 --- a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt +++ b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt @@ -11,6 +11,7 @@ import com.apollographql.apollo.interceptor.ApolloInterceptor.FetchSourceType import com.apollographql.apollo.interceptor.ApolloInterceptor.InterceptorRequest import com.apollographql.apollo.interceptor.ApolloInterceptor.InterceptorResponse import com.apollographql.apollo.interceptor.ApolloInterceptorChain +import com.apollographql.apollo.request.RequestHeaders import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.Hint @@ -23,6 +24,7 @@ import io.sentry.SentryLevel import io.sentry.SpanStatus import io.sentry.TypeCheckHint.APOLLO_REQUEST import io.sentry.TypeCheckHint.APOLLO_RESPONSE +import io.sentry.util.TracingUtils import java.util.concurrent.Executor class SentryApolloInterceptor( @@ -41,25 +43,14 @@ class SentryApolloInterceptor( override fun interceptAsync(request: InterceptorRequest, chain: ApolloInterceptorChain, dispatcher: Executor, callBack: CallBack) { val activeSpan = hub.span if (activeSpan == null) { - chain.proceedAsync(request, dispatcher, callBack) + val headers = addTracingHeaders(request, null) + val modifiedRequest = request.toBuilder().requestHeaders(headers).build() + chain.proceedAsync(modifiedRequest, dispatcher, callBack) } else { val span = startChild(request, activeSpan) - val requestWithHeader = if (span.isNoOp) { - request - } else { - val sentryTraceHeader = span.toSentryTrace() - - // we have no access to URI, no way to verify tracing origins - val requestHeaderBuilder = request.requestHeaders.toBuilder() - requestHeaderBuilder.addHeader(sentryTraceHeader.name, sentryTraceHeader.value) - span.toBaggageHeader(listOf(request.requestHeaders.headerValue(BaggageHeader.BAGGAGE_HEADER))) - ?.let { - requestHeaderBuilder.addHeader(it.name, it.value) - } - val headers = requestHeaderBuilder.build() - request.toBuilder().requestHeaders(headers).build() - } + val headers = addTracingHeaders(request, span) + val requestWithHeader = request.toBuilder().requestHeaders(headers).build() span.setData("operationId", requestWithHeader.operation.operationId()) span.setData("variables", requestWithHeader.operation.variables().valueMap().toString()) @@ -100,6 +91,29 @@ class SentryApolloInterceptor( override fun dispose() {} + private fun addTracingHeaders(request: InterceptorRequest, span: ISpan?): RequestHeaders { + val requestHeaderBuilder = request.requestHeaders.toBuilder() + + if (hub.options.isTraceSampling) { + // we have no access to URI, no way to verify tracing origins + TracingUtils.trace( + hub, + listOf(request.requestHeaders.headerValue(BaggageHeader.BAGGAGE_HEADER)), + span + ) { tracingHeaders -> + requestHeaderBuilder.addHeader( + tracingHeaders.sentryTraceHeader.name, + tracingHeaders.sentryTraceHeader.value + ) + tracingHeaders.baggageHeader?.let { + requestHeaderBuilder.addHeader(it.name, it.value) + } + } + } + + return requestHeaderBuilder.build() + } + private fun startChild(request: InterceptorRequest, activeSpan: ISpan): ISpan { val operation = request.operation.name().name() val operationType = when (request.operation) { diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java index ead779c3e86..8beb6d3c558 100644 --- a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java +++ b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java @@ -11,10 +11,9 @@ import io.sentry.Hint; import io.sentry.IHub; import io.sentry.ISpan; -import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.util.Objects; -import io.sentry.util.PropagationTargetsUtils; +import io.sentry.util.TracingUtils; import io.sentry.util.UrlUtils; import java.io.IOException; import java.util.ArrayList; @@ -46,8 +45,10 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O Response response = null; try { final ISpan activeSpan = hub.getSpan(); + if (activeSpan == null) { - return delegate.execute(request, options); + final @NotNull Request modifiedRequest = maybeAddTracingHeaders(request, null); + return delegate.execute(modifiedRequest, options); } ISpan span = activeSpan.startChild("http.client"); @@ -55,26 +56,10 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O span.setDescription(request.httpMethod().name() + " " + urlDetails.getUrlOrFallback()); urlDetails.applyToSpan(span); - final RequestWrapper requestWrapper = new RequestWrapper(request); - - if (!span.isNoOp() - && PropagationTargetsUtils.contain( - hub.getOptions().getTracePropagationTargets(), request.url())) { - final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); - final @Nullable Collection requestBaggageHeader = - request.headers().get(BaggageHeader.BAGGAGE_HEADER); - final @Nullable BaggageHeader baggageHeader = - span.toBaggageHeader( - requestBaggageHeader != null ? new ArrayList<>(requestBaggageHeader) : null); - requestWrapper.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); - if (baggageHeader != null) { - requestWrapper.removeHeader(BaggageHeader.BAGGAGE_HEADER); - requestWrapper.header(baggageHeader.getName(), baggageHeader.getValue()); - } - } + final @NotNull Request modifiedRequest = maybeAddTracingHeaders(request, span); try { - response = delegate.execute(requestWrapper.build(), options); + response = delegate.execute(modifiedRequest, options); // handles both success and error responses span.setStatus(SpanStatus.fromHttpStatusCode(response.status())); return response; @@ -102,6 +87,30 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O } } + private @NotNull Request maybeAddTracingHeaders( + final @NotNull Request request, final @Nullable ISpan span) { + final RequestWrapper requestWrapper = new RequestWrapper(request); + + TracingUtils.traceIfAllowed( + hub, + request.url(), + new ArrayList<>(request.headers().get(BaggageHeader.BAGGAGE_HEADER)), + span, + tracingHeaders -> { + requestWrapper.header( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + requestWrapper.removeHeader(BaggageHeader.BAGGAGE_HEADER); + requestWrapper.header(baggageHeader.getName(), baggageHeader.getValue()); + } + }); + + return requestWrapper.build(); + } + private void addBreadcrumb(final @NotNull Request request, final @Nullable Response response) { final Breadcrumb breadcrumb = Breadcrumb.http( diff --git a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanProcessor.java b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanProcessor.java index 16f8e99976c..07365170b48 100644 --- a/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanProcessor.java +++ b/sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SentrySpanProcessor.java @@ -18,6 +18,7 @@ import io.sentry.ISpan; import io.sentry.ITransaction; import io.sentry.Instrumenter; +import io.sentry.PropagationContext; import io.sentry.SentryDate; import io.sentry.SentryLevel; import io.sentry.SentryLongDate; @@ -113,13 +114,12 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri null, null, null) - : TransactionContext.fromSentryTrace( + : TransactionContext.fromPropagationContext( transactionName, transactionNameSource, op, - traceData.getSentryTraceHeader(), - traceData.getBaggage(), - spanId); + PropagationContext.fromHeaders( + traceData.getSentryTraceHeader(), traceData.getBaggage(), spanId)); ; transactionContext.setInstrumenter(Instrumenter.OTEL); diff --git a/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java b/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java index 2a0def37229..5c8c96be3b2 100644 --- a/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java +++ b/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java @@ -240,7 +240,6 @@ static class SentrySecurityConfiguration { } @Bean - @Conditional(SentryTracingCondition.class) @ConditionalOnMissingBean(name = "sentryTracingFilter") public FilterRegistrationBean sentryTracingFilter( final @NotNull IHub hub, final @NotNull TransactionNameProvider transactionNameProvider) { diff --git a/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt index 0d7e3637328..b686681b59e 100644 --- a/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt @@ -458,18 +458,10 @@ class SentryAutoConfigurationTest { } @Test - fun `when tracing is set, does not create tracing filter`() { + fun `creates tracing filter`() { contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") .run { - assertThat(it).doesNotHaveBean("sentryTracingFilter") - } - } - - @Test - fun `when tracing is disabled, does not create tracing filter`() { - contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") - .run { - assertThat(it).doesNotHaveBean("sentryTracingFilter") + assertThat(it).hasBean("sentryTracingFilter") } } diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java index c4183c002ad..f7bc5108759 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java @@ -240,7 +240,6 @@ static class SentrySecurityConfiguration { } @Bean - @Conditional(SentryTracingCondition.class) @ConditionalOnMissingBean(name = "sentryTracingFilter") public FilterRegistrationBean sentryTracingFilter( final @NotNull IHub hub, final @NotNull TransactionNameProvider transactionNameProvider) { diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt index b9e858b8e29..c87bcf674f7 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentryAutoConfigurationTest.kt @@ -458,18 +458,10 @@ class SentryAutoConfigurationTest { } @Test - fun `when tracing is set, does not create tracing filter`() { + fun `creates tracing filter`() { contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") .run { - assertThat(it).doesNotHaveBean("sentryTracingFilter") - } - } - - @Test - fun `when tracing is disabled, does not create tracing filter`() { - contextRunner.withPropertyValues("sentry.dsn=http://key@localhost/proj") - .run { - assertThat(it).doesNotHaveBean("sentryTracingFilter") + assertThat(it).hasBean("sentryTracingFilter") } } diff --git a/sentry-spring-jakarta/api/sentry-spring-jakarta.api b/sentry-spring-jakarta/api/sentry-spring-jakarta.api index f4387ce93a4..9550186a690 100644 --- a/sentry-spring-jakarta/api/sentry-spring-jakarta.api +++ b/sentry-spring-jakarta/api/sentry-spring-jakarta.api @@ -168,7 +168,7 @@ public abstract class io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter : protected fun doOnError (Lio/sentry/ITransaction;Ljava/lang/Throwable;)V protected fun maybeStartTransaction (Lio/sentry/IHub;Lorg/springframework/http/server/reactive/ServerHttpRequest;)Lio/sentry/ITransaction; protected fun shouldTraceRequest (Lio/sentry/IHub;Lorg/springframework/http/server/reactive/ServerHttpRequest;)Z - protected fun startTransaction (Lio/sentry/IHub;Lorg/springframework/http/server/reactive/ServerHttpRequest;)Lio/sentry/ITransaction; + protected fun startTransaction (Lio/sentry/IHub;Lorg/springframework/http/server/reactive/ServerHttpRequest;Lio/sentry/PropagationContext;)Lio/sentry/ITransaction; } public final class io/sentry/spring/jakarta/webflux/ReactorUtils { diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java index e53560b101c..086cff16000 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -10,10 +10,9 @@ import io.sentry.Hint; import io.sentry.IHub; import io.sentry.ISpan; -import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.util.Objects; -import io.sentry.util.PropagationTargetsUtils; +import io.sentry.util.TracingUtils; import io.sentry.util.UrlUtils; import java.io.IOException; import org.jetbrains.annotations.NotNull; @@ -42,6 +41,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { try { final ISpan activeSpan = hub.getSpan(); if (activeSpan == null) { + maybeAddTracingHeaders(request, null); return execution.execute(request, body); } @@ -52,18 +52,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { span.setDescription(methodName + " " + urlDetails.getUrlOrFallback()); urlDetails.applyToSpan(span); - if (!span.isNoOp() - && PropagationTargetsUtils.contain( - hub.getOptions().getTracePropagationTargets(), request.getURI())) { - final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); - request.getHeaders().add(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); - @Nullable - BaggageHeader baggage = - span.toBaggageHeader(request.getHeaders().get(BaggageHeader.BAGGAGE_HEADER)); - if (baggage != null) { - request.getHeaders().set(baggage.getName(), baggage.getValue()); - } - } + maybeAddTracingHeaders(request, span); try { response = execution.execute(request, body); @@ -84,6 +73,27 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { } } + private void maybeAddTracingHeaders( + final @NotNull HttpRequest request, final @Nullable ISpan span) { + TracingUtils.traceIfAllowed( + hub, + request.getURI().toString(), + request.getHeaders().get(BaggageHeader.BAGGAGE_HEADER), + span, + tracingHeaders -> { + request + .getHeaders() + .add( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + request.getHeaders().set(baggageHeader.getName(), baggageHeader.getValue()); + } + }); + } + private void addBreadcrumb( final @NotNull HttpRequest request, final @NotNull byte[] body, diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientWebRequestFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientWebRequestFilter.java index 0633eef58cc..03035fcde09 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientWebRequestFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientWebRequestFilter.java @@ -9,10 +9,9 @@ import io.sentry.Hint; import io.sentry.IHub; import io.sentry.ISpan; -import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.util.Objects; -import io.sentry.util.PropagationTargetsUtils; +import io.sentry.util.TracingUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.springframework.web.reactive.function.client.ClientRequest; @@ -34,36 +33,17 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { final @NotNull ClientRequest request, final @NotNull ExchangeFunction next) { final ISpan activeSpan = hub.getSpan(); if (activeSpan == null) { - addBreadcrumb(request, null); - return next.exchange(request); + final @NotNull ClientRequest modifiedRequest = maybeAddTracingHeaders(request, null); + addBreadcrumb(modifiedRequest, null); + return next.exchange(modifiedRequest); } final ISpan span = activeSpan.startChild("http.client"); span.setDescription(request.method().name() + " " + request.url()); - final ClientRequest.Builder requestBuilder = ClientRequest.from(request); - - if (!span.isNoOp() - && PropagationTargetsUtils.contain( - hub.getOptions().getTracePropagationTargets(), request.url())) { - final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); - requestBuilder.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); - - final @Nullable BaggageHeader baggageHeader = - span.toBaggageHeader(request.headers().get(BaggageHeader.BAGGAGE_HEADER)); - - if (baggageHeader != null) { - requestBuilder.headers( - httpHeaders -> { - httpHeaders.remove(BaggageHeader.BAGGAGE_HEADER); - httpHeaders.add(baggageHeader.getName(), baggageHeader.getValue()); - }); - } - } - - final ClientRequest clientRequestWithSentryTraceHeader = requestBuilder.build(); + final @NotNull ClientRequest modifiedRequest = maybeAddTracingHeaders(request, span); - return next.exchange(clientRequestWithSentryTraceHeader) + return next.exchange(modifiedRequest) .flatMap( response -> { span.setStatus(SpanStatus.fromHttpStatusCode(response.statusCode().value())); @@ -81,6 +61,33 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { }); } + private @NotNull ClientRequest maybeAddTracingHeaders( + final @NotNull ClientRequest request, final @Nullable ISpan span) { + final ClientRequest.Builder requestBuilder = ClientRequest.from(request); + + TracingUtils.traceIfAllowed( + hub, + request.url().toString(), + request.headers().get(BaggageHeader.BAGGAGE_HEADER), + span, + tracingHeaders -> { + requestBuilder.header( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + requestBuilder.headers( + httpHeaders -> { + httpHeaders.remove(BaggageHeader.BAGGAGE_HEADER); + httpHeaders.add(baggageHeader.getName(), baggageHeader.getValue()); + }); + } + }); + + return requestBuilder.build(); + } + private void addBreadcrumb( final @NotNull ClientRequest request, final @Nullable ClientResponse response) { final Breadcrumb breadcrumb = diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentryTracingFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentryTracingFilter.java index 8104a5383b4..925a09672a3 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentryTracingFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentryTracingFilter.java @@ -1,18 +1,16 @@ package io.sentry.spring.jakarta.tracing; import com.jakewharton.nopen.annotation.Open; -import io.sentry.Baggage; import io.sentry.BaggageHeader; import io.sentry.CustomSamplingContext; import io.sentry.HubAdapter; import io.sentry.IHub; import io.sentry.ITransaction; -import io.sentry.SentryLevel; +import io.sentry.PropagationContext; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.TransactionContext; import io.sentry.TransactionOptions; -import io.sentry.exception.InvalidSentryTraceHeaderException; import io.sentry.protocol.TransactionNameSource; import io.sentry.util.Objects; import jakarta.servlet.FilterChain; @@ -29,7 +27,10 @@ import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping; -/** Creates {@link ITransaction} around HTTP request executions. */ +/** + * Creates {@link ITransaction} around HTTP request executions if performance is enabled. Otherwise + * just reads tracing information from incoming request. + */ @Open public class SentryTracingFilter extends OncePerRequestFilter { /** Operation used by {@link SentryTransaction} created in {@link SentryTracingFilter}. */ @@ -74,44 +75,57 @@ protected void doFilterInternal( final @NotNull HttpServletResponse httpResponse, final @NotNull FilterChain filterChain) throws ServletException, IOException { - - if (hub.isEnabled() && shouldTraceRequest(httpRequest)) { - final String sentryTraceHeader = httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); - final List baggageHeader = + if (hub.isEnabled()) { + final @Nullable String sentryTraceHeader = + httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); + final @Nullable List baggageHeader = Collections.list(httpRequest.getHeaders(BaggageHeader.BAGGAGE_HEADER)); - - // at this stage we are not able to get real transaction name - final ITransaction transaction = - startTransaction(httpRequest, sentryTraceHeader, baggageHeader); - try { + final @Nullable PropagationContext propagationContext = + hub.continueTrace(sentryTraceHeader, baggageHeader); + if (hub.getOptions().isTracingEnabled() && shouldTraceRequest(httpRequest)) { + doFilterWithTransaction(httpRequest, httpResponse, filterChain, propagationContext); + } else { filterChain.doFilter(httpRequest, httpResponse); - } catch (Throwable e) { - // exceptions that are not handled by Spring - transaction.setStatus(SpanStatus.INTERNAL_ERROR); - throw e; - } finally { - // after all filters run, templated path pattern is available in request attribute - final String transactionName = transactionNameProvider.provideTransactionName(httpRequest); - final TransactionNameSource transactionNameSource = - transactionNameProvider.provideTransactionSource(); - // if transaction name is not resolved, the request has not been processed by a controller - // and we should not report it to Sentry - if (transactionName != null) { - transaction.setName(transactionName, transactionNameSource); - transaction.setOperation(TRANSACTION_OP); - // if exception has been thrown, transaction status is already set to INTERNAL_ERROR, and - // httpResponse.getStatus() returns 200. - if (transaction.getStatus() == null) { - transaction.setStatus(SpanStatus.fromHttpStatusCode(httpResponse.getStatus())); - } - transaction.finish(); - } } } else { filterChain.doFilter(httpRequest, httpResponse); } } + private void doFilterWithTransaction( + HttpServletRequest httpRequest, + HttpServletResponse httpResponse, + FilterChain filterChain, + final @Nullable PropagationContext propagationContext) + throws IOException, ServletException { + // at this stage we are not able to get real transaction name + final ITransaction transaction = startTransaction(httpRequest, propagationContext); + try { + filterChain.doFilter(httpRequest, httpResponse); + } catch (Throwable e) { + // exceptions that are not handled by Spring + transaction.setStatus(SpanStatus.INTERNAL_ERROR); + throw e; + } finally { + // after all filters run, templated path pattern is available in request attribute + final String transactionName = transactionNameProvider.provideTransactionName(httpRequest); + final TransactionNameSource transactionNameSource = + transactionNameProvider.provideTransactionSource(); + // if transaction name is not resolved, the request has not been processed by a controller + // and we should not report it to Sentry + if (transactionName != null) { + transaction.setName(transactionName, transactionNameSource); + transaction.setOperation(TRANSACTION_OP); + // if exception has been thrown, transaction status is already set to INTERNAL_ERROR, and + // httpResponse.getStatus() returns 200. + if (transaction.getStatus() == null) { + transaction.setStatus(SpanStatus.fromHttpStatusCode(httpResponse.getStatus())); + } + transaction.finish(); + } + } + } + private boolean shouldTraceRequest(final @NotNull HttpServletRequest request) { return hub.getOptions().isTraceOptionsRequests() || !HttpMethod.OPTIONS.name().equals(request.getMethod()); @@ -119,37 +133,23 @@ private boolean shouldTraceRequest(final @NotNull HttpServletRequest request) { private ITransaction startTransaction( final @NotNull HttpServletRequest request, - final @Nullable String sentryTraceHeader, - final @Nullable List baggageHeader) { + final @Nullable PropagationContext propagationContext) { final String name = request.getMethod() + " " + request.getRequestURI(); final CustomSamplingContext customSamplingContext = new CustomSamplingContext(); customSamplingContext.set("request", request); - final Baggage baggage = Baggage.fromHeader(baggageHeader, hub.getOptions().getLogger()); - - if (sentryTraceHeader != null) { - try { - final TransactionContext contexts = - TransactionContext.fromSentryTrace( - name, - TransactionNameSource.URL, - "http.server", - new SentryTraceHeader(sentryTraceHeader), - baggage, - null); - - final TransactionOptions transactionOptions = new TransactionOptions(); - transactionOptions.setCustomSamplingContext(customSamplingContext); - transactionOptions.setBindToScope(true); - - return hub.startTransaction(contexts, transactionOptions); - } catch (InvalidSentryTraceHeaderException e) { - hub.getOptions() - .getLogger() - .log(SentryLevel.DEBUG, e, "Failed to parse Sentry trace header: %s", e.getMessage()); - } + if (propagationContext != null) { + final TransactionContext contexts = + TransactionContext.fromPropagationContext( + name, TransactionNameSource.URL, "http.server", propagationContext); + + final TransactionOptions transactionOptions = new TransactionOptions(); + transactionOptions.setCustomSamplingContext(customSamplingContext); + transactionOptions.setBindToScope(true); + + return hub.startTransaction(contexts, transactionOptions); } final TransactionOptions transactionOptions = new TransactionOptions(); diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java index cebfb74f962..80a1b69ee6b 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/AbstractSentryWebFilter.java @@ -3,7 +3,6 @@ import static io.sentry.TypeCheckHint.WEBFLUX_FILTER_REQUEST; import static io.sentry.TypeCheckHint.WEBFLUX_FILTER_RESPONSE; -import io.sentry.Baggage; import io.sentry.BaggageHeader; import io.sentry.Breadcrumb; import io.sentry.CustomSamplingContext; @@ -11,13 +10,12 @@ import io.sentry.IHub; import io.sentry.ITransaction; import io.sentry.NoOpHub; +import io.sentry.PropagationContext; import io.sentry.Sentry; -import io.sentry.SentryLevel; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.TransactionContext; import io.sentry.TransactionOptions; -import io.sentry.exception.InvalidSentryTraceHeaderException; import io.sentry.protocol.TransactionNameSource; import io.sentry.util.Objects; import java.util.List; @@ -46,13 +44,20 @@ public AbstractSentryWebFilter(final @NotNull IHub hub) { protected @Nullable ITransaction maybeStartTransaction( final @NotNull IHub requestHub, final @NotNull ServerHttpRequest request) { - if (requestHub.isEnabled() - && requestHub.getOptions().isTracingEnabled() - && shouldTraceRequest(requestHub, request)) { - return startTransaction(requestHub, request); - } else { - return null; + if (requestHub.isEnabled()) { + final @NotNull HttpHeaders headers = request.getHeaders(); + final @Nullable String sentryTraceHeader = + headers.getFirst(SentryTraceHeader.SENTRY_TRACE_HEADER); + final @Nullable List baggageHeaders = headers.get(BaggageHeader.BAGGAGE_HEADER); + final @Nullable PropagationContext propagationContext = + requestHub.continueTrace(sentryTraceHeader, baggageHeaders); + + if (requestHub.getOptions().isTracingEnabled() && shouldTraceRequest(requestHub, request)) { + return startTransaction(requestHub, request, propagationContext); + } } + + return null; } protected void doFinally( @@ -119,11 +124,9 @@ private void finishTransaction(ServerWebExchange exchange, ITransaction transact } protected @NotNull ITransaction startTransaction( - final @NotNull IHub hub, final @NotNull ServerHttpRequest request) { - final @NotNull HttpHeaders headers = request.getHeaders(); - final @Nullable List sentryTraceHeaders = - headers.get(SentryTraceHeader.SENTRY_TRACE_HEADER); - final @Nullable List baggageHeaders = headers.get(BaggageHeader.BAGGAGE_HEADER); + final @NotNull IHub hub, + final @NotNull ServerHttpRequest request, + final @Nullable PropagationContext propagationContext) { final @NotNull String name = request.getMethod() + " " + request.getURI().getPath(); final @NotNull CustomSamplingContext customSamplingContext = new CustomSamplingContext(); customSamplingContext.set("request", request); @@ -132,25 +135,12 @@ private void finishTransaction(ServerWebExchange exchange, ITransaction transact transactionOptions.setCustomSamplingContext(customSamplingContext); transactionOptions.setBindToScope(true); - if (sentryTraceHeaders != null && sentryTraceHeaders.size() > 0) { - final @NotNull Baggage baggage = - Baggage.fromHeader(baggageHeaders, hub.getOptions().getLogger()); - try { - final @NotNull TransactionContext contexts = - TransactionContext.fromSentryTrace( - name, - TransactionNameSource.URL, - TRANSACTION_OP, - new SentryTraceHeader(sentryTraceHeaders.get(0)), - baggage, - null); - - return hub.startTransaction(contexts, transactionOptions); - } catch (InvalidSentryTraceHeaderException e) { - hub.getOptions() - .getLogger() - .log(SentryLevel.DEBUG, e, "Failed to parse Sentry trace header: %s", e.getMessage()); - } + if (propagationContext != null) { + final @NotNull TransactionContext contexts = + TransactionContext.fromPropagationContext( + name, TransactionNameSource.URL, TRANSACTION_OP, propagationContext); + + return hub.startTransaction(contexts, transactionOptions); } return hub.startTransaction( diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt index 849cbbb12ce..a046e5f30b3 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt @@ -1,6 +1,8 @@ package io.sentry.spring.jakarta.tracing import io.sentry.IHub +import io.sentry.ILogger +import io.sentry.PropagationContext import io.sentry.SentryOptions import io.sentry.SentryTracer import io.sentry.SpanId @@ -18,6 +20,7 @@ import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check import org.mockito.kotlin.mock import org.mockito.kotlin.never +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever @@ -40,7 +43,9 @@ class SentryTracingFilterTest { val transactionNameProvider = mock() val options = SentryOptions().apply { dsn = "https://key@sentry.io/proj" + enableTracing = true } + val logger = mock() init { whenever(hub.options).thenReturn(options) @@ -59,6 +64,7 @@ class SentryTracingFilterTest { response.status = status whenever(hub.startTransaction(any(), check { assertTrue(it.isBindToScope) })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } whenever(hub.isEnabled).thenReturn(isEnabled) + whenever(hub.continueTrace(any(), any())).thenAnswer { PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?) } return SentryTracingFilter(hub, transactionNameProvider) } } @@ -205,7 +211,8 @@ class SentryTracingFilterTest { verify(fixture.chain).doFilter(fixture.request, fixture.response) verify(fixture.hub).isEnabled - verify(fixture.hub).options + verify(fixture.hub).continueTrace(anyOrNull(), anyOrNull()) + verify(fixture.hub, times(2)).options verifyNoMoreInteractions(fixture.hub) verify(fixture.transactionNameProvider, never()).provideTransactionName(any()) } diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java index 09a4bcc8a05..d983cc7d2a5 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -10,10 +10,9 @@ import io.sentry.Hint; import io.sentry.IHub; import io.sentry.ISpan; -import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.util.Objects; -import io.sentry.util.PropagationTargetsUtils; +import io.sentry.util.TracingUtils; import io.sentry.util.UrlUtils; import java.io.IOException; import org.jetbrains.annotations.NotNull; @@ -42,6 +41,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { try { final ISpan activeSpan = hub.getSpan(); if (activeSpan == null) { + maybeAddTracingHeaders(request, null); return execution.execute(request, body); } @@ -52,18 +52,7 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { urlDetails.applyToSpan(span); span.setDescription(methodName + " " + urlDetails.getUrlOrFallback()); - if (!span.isNoOp() - && PropagationTargetsUtils.contain( - hub.getOptions().getTracePropagationTargets(), request.getURI())) { - final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); - request.getHeaders().add(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); - @Nullable - BaggageHeader baggage = - span.toBaggageHeader(request.getHeaders().get(BaggageHeader.BAGGAGE_HEADER)); - if (baggage != null) { - request.getHeaders().set(baggage.getName(), baggage.getValue()); - } - } + maybeAddTracingHeaders(request, span); try { response = execution.execute(request, body); @@ -84,6 +73,27 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { } } + private void maybeAddTracingHeaders( + final @NotNull HttpRequest request, final @Nullable ISpan span) { + TracingUtils.traceIfAllowed( + hub, + request.getURI().toString(), + request.getHeaders().get(BaggageHeader.BAGGAGE_HEADER), + span, + tracingHeaders -> { + request + .getHeaders() + .add( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + request.getHeaders().set(baggageHeader.getName(), baggageHeader.getValue()); + } + }); + } + private void addBreadcrumb( final @NotNull HttpRequest request, final @NotNull byte[] body, diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java index 1c82865ed7a..83fc8cc39c6 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java @@ -9,10 +9,9 @@ import io.sentry.Hint; import io.sentry.IHub; import io.sentry.ISpan; -import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.util.Objects; -import io.sentry.util.PropagationTargetsUtils; +import io.sentry.util.TracingUtils; import io.sentry.util.UrlUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -36,7 +35,7 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { final ISpan activeSpan = hub.getSpan(); if (activeSpan == null) { addBreadcrumb(request, null); - return next.exchange(request); + return next.exchange(maybeAddHeaders(request, null)); } final ISpan span = activeSpan.startChild("http.client"); @@ -44,28 +43,7 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { span.setDescription(request.method().name() + " " + urlDetails.getUrlOrFallback()); urlDetails.applyToSpan(span); - final ClientRequest.Builder requestBuilder = ClientRequest.from(request); - - if (!span.isNoOp() - && PropagationTargetsUtils.contain( - hub.getOptions().getTracePropagationTargets(), request.url())) { - - final SentryTraceHeader sentryTraceHeader = span.toSentryTrace(); - requestBuilder.header(sentryTraceHeader.getName(), sentryTraceHeader.getValue()); - - final @Nullable BaggageHeader baggageHeader = - span.toBaggageHeader(request.headers().get(BaggageHeader.BAGGAGE_HEADER)); - - if (baggageHeader != null) { - requestBuilder.headers( - httpHeaders -> { - httpHeaders.remove(BaggageHeader.BAGGAGE_HEADER); - httpHeaders.add(baggageHeader.getName(), baggageHeader.getValue()); - }); - } - } - - final ClientRequest clientRequestWithSentryTraceHeader = requestBuilder.build(); + final ClientRequest clientRequestWithSentryTraceHeader = maybeAddHeaders(request, span); return next.exchange(clientRequestWithSentryTraceHeader) .flatMap( @@ -85,6 +63,33 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { }); } + private ClientRequest maybeAddHeaders( + final @NotNull ClientRequest request, final @Nullable ISpan span) { + final ClientRequest.Builder requestBuilder = ClientRequest.from(request); + + TracingUtils.traceIfAllowed( + hub, + request.url().toString(), + request.headers().get(BaggageHeader.BAGGAGE_HEADER), + span, + tracingHeaders -> { + requestBuilder.header( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + requestBuilder.headers( + httpHeaders -> { + httpHeaders.remove(BaggageHeader.BAGGAGE_HEADER); + httpHeaders.add(baggageHeader.getName(), baggageHeader.getValue()); + }); + } + }); + + return requestBuilder.build(); + } + private void addBreadcrumb( final @NotNull ClientRequest request, final @Nullable ClientResponse response) { final Breadcrumb breadcrumb = diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java index 1086950b942..69950c1b563 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java @@ -1,18 +1,16 @@ package io.sentry.spring.tracing; import com.jakewharton.nopen.annotation.Open; -import io.sentry.Baggage; import io.sentry.BaggageHeader; import io.sentry.CustomSamplingContext; import io.sentry.HubAdapter; import io.sentry.IHub; import io.sentry.ITransaction; -import io.sentry.SentryLevel; +import io.sentry.PropagationContext; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.TransactionContext; import io.sentry.TransactionOptions; -import io.sentry.exception.InvalidSentryTraceHeaderException; import io.sentry.protocol.TransactionNameSource; import io.sentry.util.Objects; import java.io.IOException; @@ -79,10 +77,11 @@ protected void doFilterInternal( final String sentryTraceHeader = httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); final List baggageHeader = Collections.list(httpRequest.getHeaders(BaggageHeader.BAGGAGE_HEADER)); + final @Nullable PropagationContext propagationContext = + hub.continueTrace(sentryTraceHeader, baggageHeader); // at this stage we are not able to get real transaction name - final ITransaction transaction = - startTransaction(httpRequest, sentryTraceHeader, baggageHeader); + final ITransaction transaction = startTransaction(httpRequest, propagationContext); try { filterChain.doFilter(httpRequest, httpResponse); } catch (Throwable e) { @@ -119,37 +118,23 @@ private boolean shouldTraceRequest(final @NotNull HttpServletRequest request) { private ITransaction startTransaction( final @NotNull HttpServletRequest request, - final @Nullable String sentryTraceHeader, - final @Nullable List baggageHeader) { + final @Nullable PropagationContext propagationContext) { final String name = request.getMethod() + " " + request.getRequestURI(); final CustomSamplingContext customSamplingContext = new CustomSamplingContext(); customSamplingContext.set("request", request); - final Baggage baggage = Baggage.fromHeader(baggageHeader, hub.getOptions().getLogger()); + if (propagationContext != null) { + final TransactionContext contexts = + TransactionContext.fromPropagationContext( + name, TransactionNameSource.URL, "http.server", propagationContext); - if (sentryTraceHeader != null) { - try { - final TransactionContext contexts = - TransactionContext.fromSentryTrace( - name, - TransactionNameSource.URL, - "http.server", - new SentryTraceHeader(sentryTraceHeader), - baggage, - null); - - final TransactionOptions transactionOptions = new TransactionOptions(); - transactionOptions.setCustomSamplingContext(customSamplingContext); - transactionOptions.setBindToScope(true); - - return hub.startTransaction(contexts, transactionOptions); - } catch (InvalidSentryTraceHeaderException e) { - hub.getOptions() - .getLogger() - .log(SentryLevel.DEBUG, e, "Failed to parse Sentry trace header: %s", e.getMessage()); - } + final TransactionOptions transactionOptions = new TransactionOptions(); + transactionOptions.setCustomSamplingContext(customSamplingContext); + transactionOptions.setBindToScope(true); + + return hub.startTransaction(contexts, transactionOptions); } final TransactionOptions transactionOptions = new TransactionOptions(); diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java index 6e297392820..8c554e5dcb1 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java @@ -3,7 +3,6 @@ import static io.sentry.TypeCheckHint.WEBFLUX_FILTER_REQUEST; import static io.sentry.TypeCheckHint.WEBFLUX_FILTER_RESPONSE; -import io.sentry.Baggage; import io.sentry.BaggageHeader; import io.sentry.Breadcrumb; import io.sentry.CustomSamplingContext; @@ -11,13 +10,12 @@ import io.sentry.IHub; import io.sentry.ITransaction; import io.sentry.NoOpHub; +import io.sentry.PropagationContext; import io.sentry.Sentry; -import io.sentry.SentryLevel; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.TransactionContext; import io.sentry.TransactionOptions; -import io.sentry.exception.InvalidSentryTraceHeaderException; import io.sentry.protocol.TransactionNameSource; import io.sentry.util.Objects; import java.util.List; @@ -57,9 +55,16 @@ public Mono filter( final boolean isTracingEnabled = requestHub.getOptions().isTracingEnabled(); final @NotNull ServerHttpRequest request = serverWebExchange.getRequest(); + final @NotNull HttpHeaders headers = request.getHeaders(); + final @Nullable String sentryTraceHeader = + headers.getFirst(SentryTraceHeader.SENTRY_TRACE_HEADER); + final @Nullable List baggageHeaders = headers.get(BaggageHeader.BAGGAGE_HEADER); + final @Nullable PropagationContext propagationContext = + requestHub.continueTrace(sentryTraceHeader, baggageHeaders); + final @Nullable ITransaction transaction = isTracingEnabled && shouldTraceRequest(requestHub, request) - ? startTransaction(requestHub, request) + ? startTransaction(requestHub, request, propagationContext) : null; return webFilterChain @@ -105,11 +110,9 @@ private boolean shouldTraceRequest( } private @NotNull ITransaction startTransaction( - final @NotNull IHub hub, final @NotNull ServerHttpRequest request) { - final @NotNull HttpHeaders headers = request.getHeaders(); - final @Nullable List sentryTraceHeaders = - headers.get(SentryTraceHeader.SENTRY_TRACE_HEADER); - final @Nullable List baggageHeaders = headers.get(BaggageHeader.BAGGAGE_HEADER); + final @NotNull IHub hub, + final @NotNull ServerHttpRequest request, + final @Nullable PropagationContext propagationContext) { final @NotNull String name = request.getMethod() + " " + request.getURI().getPath(); final @NotNull CustomSamplingContext customSamplingContext = new CustomSamplingContext(); customSamplingContext.set("request", request); @@ -118,25 +121,12 @@ private boolean shouldTraceRequest( transactionOptions.setCustomSamplingContext(customSamplingContext); transactionOptions.setBindToScope(true); - if (sentryTraceHeaders != null && sentryTraceHeaders.size() > 0) { - final @NotNull Baggage baggage = - Baggage.fromHeader(baggageHeaders, hub.getOptions().getLogger()); - try { - final @NotNull TransactionContext contexts = - TransactionContext.fromSentryTrace( - name, - TransactionNameSource.URL, - TRANSACTION_OP, - new SentryTraceHeader(sentryTraceHeaders.get(0)), - baggage, - null); - - return hub.startTransaction(contexts, transactionOptions); - } catch (InvalidSentryTraceHeaderException e) { - hub.getOptions() - .getLogger() - .log(SentryLevel.DEBUG, e, "Failed to parse Sentry trace header: %s", e.getMessage()); - } + if (propagationContext != null) { + final @NotNull TransactionContext contexts = + TransactionContext.fromPropagationContext( + name, TransactionNameSource.URL, TRANSACTION_OP, propagationContext); + + return hub.startTransaction(contexts, transactionOptions); } return hub.startTransaction( diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt index cd72743d917..19b27bd7d25 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt @@ -1,6 +1,8 @@ package io.sentry.spring.tracing import io.sentry.IHub +import io.sentry.ILogger +import io.sentry.PropagationContext import io.sentry.SentryOptions import io.sentry.SentryTracer import io.sentry.SpanId @@ -41,6 +43,7 @@ class SentryTracingFilterTest { val options = SentryOptions().apply { dsn = "https://key@sentry.io/proj" } + val logger = mock() init { whenever(hub.options).thenReturn(options) @@ -59,6 +62,7 @@ class SentryTracingFilterTest { response.status = status whenever(hub.startTransaction(any(), check { assertTrue(it.isBindToScope) })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } whenever(hub.isEnabled).thenReturn(isEnabled) + whenever(hub.continueTrace(any(), any())).thenAnswer { PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?) } return SentryTracingFilter(hub, transactionNameProvider) } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 72fa5937703..f5891409e1e 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -60,6 +60,7 @@ public final class io/sentry/Baggage { public fun setTransaction (Ljava/lang/String;)V public fun setUserId (Ljava/lang/String;)V public fun setUserSegment (Ljava/lang/String;)V + public fun setValuesFromScope (Lio/sentry/Scope;Lio/sentry/SentryOptions;)V public fun setValuesFromTransaction (Lio/sentry/ITransaction;Lio/sentry/protocol/User;Lio/sentry/SentryOptions;Lio/sentry/TracesSamplingDecision;)V public fun toHeaderString (Ljava/lang/String;)Ljava/lang/String; public fun toTraceContext ()Lio/sentry/TraceContext; @@ -329,6 +330,7 @@ public final class io/sentry/HttpStatusCodeRange { public final class io/sentry/Hub : io/sentry/IHub { public fun (Lio/sentry/SentryOptions;)V public fun addBreadcrumb (Lio/sentry/Breadcrumb;Lio/sentry/Hint;)V + public fun baggageHeader (Ljava/util/List;)Lio/sentry/BaggageHeader; public fun bindClient (Lio/sentry/ISentryClient;)V public fun captureEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; @@ -344,6 +346,7 @@ public final class io/sentry/Hub : io/sentry/IHub { public synthetic fun clone ()Ljava/lang/Object; public fun close ()V public fun configureScope (Lio/sentry/ScopeCallback;)V + public fun continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/PropagationContext; public fun endSession ()V public fun flush (J)V public fun getLastEventId ()Lio/sentry/protocol/SentryId; @@ -372,6 +375,7 @@ public final class io/sentry/Hub : io/sentry/IHub { public final class io/sentry/HubAdapter : io/sentry/IHub { public fun addBreadcrumb (Lio/sentry/Breadcrumb;Lio/sentry/Hint;)V + public fun baggageHeader (Ljava/util/List;)Lio/sentry/BaggageHeader; public fun bindClient (Lio/sentry/ISentryClient;)V public fun captureEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; @@ -387,6 +391,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub { public synthetic fun clone ()Ljava/lang/Object; public fun close ()V public fun configureScope (Lio/sentry/ScopeCallback;)V + public fun continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/PropagationContext; public fun endSession ()V public fun flush (J)V public static fun getInstance ()Lio/sentry/HubAdapter; @@ -433,6 +438,7 @@ public abstract interface class io/sentry/IHub { public abstract fun addBreadcrumb (Lio/sentry/Breadcrumb;Lio/sentry/Hint;)V public fun addBreadcrumb (Ljava/lang/String;)V public fun addBreadcrumb (Ljava/lang/String;Ljava/lang/String;)V + public abstract fun baggageHeader (Ljava/util/List;)Lio/sentry/BaggageHeader; public abstract fun bindClient (Lio/sentry/ISentryClient;)V public fun captureEnvelope (Lio/sentry/SentryEnvelope;)Lio/sentry/protocol/SentryId; public abstract fun captureEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; @@ -457,6 +463,7 @@ public abstract interface class io/sentry/IHub { public abstract fun clone ()Lio/sentry/IHub; public abstract fun close ()V public abstract fun configureScope (Lio/sentry/ScopeCallback;)V + public abstract fun continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/PropagationContext; public abstract fun endSession ()V public abstract fun flush (J)V public abstract fun getLastEventId ()Lio/sentry/protocol/SentryId; @@ -800,6 +807,7 @@ public final class io/sentry/NoOpEnvelopeReader : io/sentry/IEnvelopeReader { public final class io/sentry/NoOpHub : io/sentry/IHub { public fun addBreadcrumb (Lio/sentry/Breadcrumb;Lio/sentry/Hint;)V + public fun baggageHeader (Ljava/util/List;)Lio/sentry/BaggageHeader; public fun bindClient (Lio/sentry/ISentryClient;)V public fun captureEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; @@ -815,6 +823,7 @@ public final class io/sentry/NoOpHub : io/sentry/IHub { public synthetic fun clone ()Ljava/lang/Object; public fun close ()V public fun configureScope (Lio/sentry/ScopeCallback;)V + public fun continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/PropagationContext; public fun endSession ()V public fun flush (J)V public static fun getInstance ()Lio/sentry/NoOpHub; @@ -1108,6 +1117,25 @@ public final class io/sentry/ProfilingTransactionData$JsonKeys { public fun ()V } +public final class io/sentry/PropagationContext { + public fun ()V + public fun (Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Lio/sentry/SpanId;Lio/sentry/Baggage;Ljava/lang/Boolean;)V + public static fun fromHeaders (Lio/sentry/ILogger;Ljava/lang/String;Ljava/lang/String;)Lio/sentry/PropagationContext; + public static fun fromHeaders (Lio/sentry/ILogger;Ljava/lang/String;Ljava/util/List;)Lio/sentry/PropagationContext; + public static fun fromHeaders (Lio/sentry/SentryTraceHeader;Lio/sentry/Baggage;Lio/sentry/SpanId;)Lio/sentry/PropagationContext; + public fun getBaggage ()Lio/sentry/Baggage; + public fun getParentSpanId ()Lio/sentry/SpanId; + public fun getSpanId ()Lio/sentry/SpanId; + public fun getTraceId ()Lio/sentry/protocol/SentryId; + public fun isSampled ()Ljava/lang/Boolean; + public fun setBaggage (Lio/sentry/Baggage;)V + public fun setParentSpanId (Lio/sentry/SpanId;)V + public fun setSampled (Ljava/lang/Boolean;)V + public fun setSpanId (Lio/sentry/SpanId;)V + public fun setTraceId (Lio/sentry/protocol/SentryId;)V + public fun traceContext ()Lio/sentry/TraceContext; +} + public final class io/sentry/RequestDetails { public fun (Ljava/lang/String;Ljava/util/Map;)V public fun getHeaders ()Ljava/util/Map; @@ -1132,6 +1160,7 @@ public final class io/sentry/Scope { public fun clearTransaction ()V public fun getContexts ()Lio/sentry/protocol/Contexts; public fun getLevel ()Lio/sentry/SentryLevel; + public fun getPropagationContext ()Lio/sentry/PropagationContext; public fun getRequest ()Lio/sentry/protocol/Request; public fun getSession ()Lio/sentry/Session; public fun getSpan ()Lio/sentry/ISpan; @@ -1152,6 +1181,7 @@ public final class io/sentry/Scope { public fun setExtra (Ljava/lang/String;Ljava/lang/String;)V public fun setFingerprint (Ljava/util/List;)V public fun setLevel (Lio/sentry/SentryLevel;)V + public fun setPropagationContext (Lio/sentry/PropagationContext;)V public fun setRequest (Lio/sentry/protocol/Request;)V public fun setTag (Ljava/lang/String;Ljava/lang/String;)V public fun setTransaction (Lio/sentry/ITransaction;)V @@ -1202,6 +1232,7 @@ public final class io/sentry/Sentry { public static fun addBreadcrumb (Lio/sentry/Breadcrumb;Lio/sentry/Hint;)V public static fun addBreadcrumb (Ljava/lang/String;)V public static fun addBreadcrumb (Ljava/lang/String;Ljava/lang/String;)V + public static fun baggageHeader (Ljava/util/List;)Lio/sentry/BaggageHeader; public static fun bindClient (Lio/sentry/ISentryClient;)V public static fun captureEvent (Lio/sentry/SentryEvent;)Lio/sentry/protocol/SentryId; public static fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; @@ -1220,6 +1251,7 @@ public final class io/sentry/Sentry { public static fun cloneMainHub ()Lio/sentry/IHub; public static fun close ()V public static fun configureScope (Lio/sentry/ScopeCallback;)V + public static fun continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/PropagationContext; public static fun endSession ()V public static fun flush (J)V public static fun getCurrentHub ()Lio/sentry/IHub; @@ -2222,8 +2254,7 @@ public final class io/sentry/TransactionContext : io/sentry/SpanContext { public fun (Ljava/lang/String;Ljava/lang/String;)V public fun (Ljava/lang/String;Ljava/lang/String;Lio/sentry/TracesSamplingDecision;)V public fun (Ljava/lang/String;Ljava/lang/String;Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Lio/sentry/protocol/TransactionNameSource;Lio/sentry/SpanId;Lio/sentry/TracesSamplingDecision;Lio/sentry/Baggage;)V - public static fun fromSentryTrace (Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;Ljava/lang/String;Lio/sentry/SentryTraceHeader;)Lio/sentry/TransactionContext; - public static fun fromSentryTrace (Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;Ljava/lang/String;Lio/sentry/SentryTraceHeader;Lio/sentry/Baggage;Lio/sentry/SpanId;)Lio/sentry/TransactionContext; + public static fun fromPropagationContext (Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;Ljava/lang/String;Lio/sentry/PropagationContext;)Lio/sentry/TransactionContext; public static fun fromSentryTrace (Ljava/lang/String;Ljava/lang/String;Lio/sentry/SentryTraceHeader;)Lio/sentry/TransactionContext; public fun getBaggage ()Lio/sentry/Baggage; public fun getInstrumenter ()Lio/sentry/Instrumenter; @@ -4140,6 +4171,20 @@ public final class io/sentry/util/StringUtils { public static fun removeSurrounding (Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String; } +public final class io/sentry/util/TracingUtils { + public fun ()V + public static fun maybeUpdateBaggage (Lio/sentry/Scope;Lio/sentry/SentryOptions;)V + public static fun startNewTrace (Lio/sentry/IHub;)V + public static fun trace (Lio/sentry/IHub;Ljava/util/List;Lio/sentry/ISpan;Lio/sentry/util/HintUtils$SentryConsumer;)V + public static fun traceIfAllowed (Lio/sentry/IHub;Ljava/lang/String;Ljava/util/List;Lio/sentry/ISpan;Lio/sentry/util/HintUtils$SentryConsumer;)V +} + +public final class io/sentry/util/TracingUtils$TracingHeaders { + public fun (Lio/sentry/SentryTraceHeader;Lio/sentry/BaggageHeader;)V + public fun getBaggageHeader ()Lio/sentry/BaggageHeader; + public fun getSentryTraceHeader ()Lio/sentry/SentryTraceHeader; +} + public final class io/sentry/util/UrlUtils { public static final field SENSITIVE_DATA_SUBSTITUTE Ljava/lang/String; public fun ()V diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index 1500d1c36f2..6c5ad41c6f7 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -352,6 +352,21 @@ public void setValuesFromTransaction( setSampleRate(sampleRateToString(sampleRate(samplingDecision))); } + @ApiStatus.Internal + public void setValuesFromScope(final @NotNull Scope scope, final @NotNull SentryOptions options) { + final @NotNull PropagationContext propagationContext = scope.getPropagationContext(); + final @Nullable User user = scope.getUser(); + setTraceId(propagationContext.getTraceId().toString()); + setPublicKey(new Dsn(options.getDsn()).getPublicKey()); + setRelease(options.getRelease()); + setEnvironment(options.getEnvironment()); + setUserSegment(user != null ? getSegment(user) : null); + + // TODO vvv should we set null explicitly? + setTransaction(null); + setSampleRate(null); + } + private static @Nullable String getSegment(final @NotNull User user) { if (user.getSegment() != null) { return user.getSegment(); diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 38a2250b9dd..e78e9b847a8 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -11,12 +11,14 @@ import io.sentry.util.HintUtils; import io.sentry.util.Objects; import io.sentry.util.Pair; +import io.sentry.util.TracingUtils; import java.io.Closeable; import java.lang.ref.WeakReference; import java.util.Collections; import java.util.List; import java.util.Map; import java.util.WeakHashMap; +import java.util.concurrent.atomic.AtomicReference; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -743,19 +745,22 @@ public void flush(long timeoutMillis) { @Override public @Nullable SentryTraceHeader traceHeaders() { - SentryTraceHeader traceHeader = null; + AtomicReference traceHeader = new AtomicReference<>(); if (!isEnabled()) { options .getLogger() .log( SentryLevel.WARNING, "Instance is disabled and this 'traceHeaders' call is a no-op."); } else { - final ISpan span = stack.peek().getScope().getSpan(); - if (span != null && !span.isNoOp()) { - traceHeader = span.toSentryTrace(); - } + TracingUtils.trace( + this, + null, + getSpan(), + tracingHeaders -> { + traceHeader.set(tracingHeaders.getSentryTraceHeader()); + }); } - return traceHeader; + return traceHeader.get(); } @Override @@ -818,4 +823,38 @@ private Scope buildLocalScope( } return scope; } + + @Override + public @Nullable PropagationContext continueTrace( + final @Nullable String sentryTraceHeader, final @Nullable List baggageHeaders) { + @NotNull + PropagationContext propagationContext = + PropagationContext.fromHeaders(getOptions().getLogger(), sentryTraceHeader, baggageHeaders); + configureScope( + (scope) -> { + scope.setPropagationContext(propagationContext); + }); + return propagationContext; + } + + @Override + public @Nullable BaggageHeader baggageHeader(@Nullable List thirdPartyBaggageHeaders) { + final @NotNull AtomicReference baggageHeader = new AtomicReference<>(); + if (!isEnabled()) { + options + .getLogger() + .log( + SentryLevel.WARNING, + "Instance is disabled and this 'baggageHeader' call is a no-op."); + } else { + TracingUtils.trace( + this, + thirdPartyBaggageHeaders, + getSpan(), + tracingHeaders -> { + baggageHeader.set(tracingHeaders.getBaggageHeader()); + }); + } + return baggageHeader.get(); + } } diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index b777076d0fe..7b9942210f3 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -234,4 +234,16 @@ public void setSpanContext( public void reportFullyDisplayed() { Sentry.reportFullyDisplayed(); } + + @Override + public @Nullable PropagationContext continueTrace( + final @Nullable String sentryTraceHeader, final @Nullable List baggageHeaders) { + return Sentry.continueTrace(sentryTraceHeader, baggageHeaders); + } + + @Override + public @Nullable BaggageHeader baggageHeader( + final @Nullable List thirdPartyBaggageHeaders) { + return Sentry.baggageHeader(thirdPartyBaggageHeaders); + } } diff --git a/sentry/src/main/java/io/sentry/IHub.java b/sentry/src/main/java/io/sentry/IHub.java index b99e99ab82a..fcc12f9f383 100644 --- a/sentry/src/main/java/io/sentry/IHub.java +++ b/sentry/src/main/java/io/sentry/IHub.java @@ -524,7 +524,7 @@ ITransaction startTransaction( } /** - * Returns trace header of active transaction or {@code null} if no transaction is active. + * Returns trace header of active transaction, or scope if no transaction is active. * * @return trace header or null */ @@ -589,4 +589,18 @@ void setSpanContext( default void reportFullDisplayed() { reportFullyDisplayed(); } + + /** + * Continue a trace based on HTTP header values. If no "sentry-trace" header is provided a random + * trace ID and span ID is created. + * + * @param sentryTraceHeader "sentry-trace" header + * @param baggageHeaders "baggage" headers + */ + @Nullable + PropagationContext continueTrace( + final @Nullable String sentryTraceHeader, final @Nullable List baggageHeaders); + + @Nullable + BaggageHeader baggageHeader(@Nullable List thirdPartyBaggageHeaders); } diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java index 6735fe832ff..cd485ef6301 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -189,4 +189,16 @@ public void setSpanContext( @Override public void reportFullyDisplayed() {} + + @Override + public @Nullable PropagationContext continueTrace( + final @Nullable String sentryTraceHeader, final @Nullable List baggageHeaders) { + return null; + } + + @Override + public @Nullable BaggageHeader baggageHeader( + final @Nullable List thirdPartyBaggageHeaders) { + return null; + } } diff --git a/sentry/src/main/java/io/sentry/PropagationContext.java b/sentry/src/main/java/io/sentry/PropagationContext.java new file mode 100644 index 00000000000..e1b8ddf0516 --- /dev/null +++ b/sentry/src/main/java/io/sentry/PropagationContext.java @@ -0,0 +1,123 @@ +package io.sentry; + +import io.sentry.exception.InvalidSentryTraceHeaderException; +import io.sentry.protocol.SentryId; +import java.util.Arrays; +import java.util.List; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public final class PropagationContext { + + public static PropagationContext fromHeaders( + final @NotNull ILogger logger, + final @Nullable String sentryTraceHeader, + final @Nullable String baggageHeader) { + return fromHeaders(logger, sentryTraceHeader, Arrays.asList(baggageHeader)); + } + + public static @NotNull PropagationContext fromHeaders( + final @NotNull ILogger logger, + final @Nullable String sentryTraceHeaderString, + final @Nullable List baggageHeaderStrings) { + if (sentryTraceHeaderString == null) { + return new PropagationContext(); + } + + try { + final @NotNull SentryTraceHeader traceHeader = new SentryTraceHeader(sentryTraceHeaderString); + final @NotNull Baggage baggage = Baggage.fromHeader(baggageHeaderStrings); + return fromHeaders(traceHeader, baggage, null); + } catch (InvalidSentryTraceHeaderException e) { + logger.log(SentryLevel.DEBUG, e, "Failed to parse Sentry trace header: %s", e.getMessage()); + return new PropagationContext(); + } + } + + public static @NotNull PropagationContext fromHeaders( + final @NotNull SentryTraceHeader sentryTraceHeader, + final @Nullable Baggage baggage, + final @Nullable SpanId spanId) { + final @NotNull SpanId spanIdToUse = spanId == null ? new SpanId() : spanId; + + return new PropagationContext( + sentryTraceHeader.getTraceId(), + spanIdToUse, + sentryTraceHeader.getSpanId(), + baggage, + sentryTraceHeader.isSampled()); + } + + private @NotNull SentryId traceId; + private @NotNull SpanId spanId; + private @Nullable SpanId parentSpanId; + + private @Nullable Boolean sampled; + + private @Nullable Baggage baggage; + + public PropagationContext() { + this(new SentryId(), new SpanId(), null, null, null); + } + + public PropagationContext( + final @NotNull SentryId traceId, + final @NotNull SpanId spanId, + final @Nullable SpanId parentSpanId, + final @Nullable Baggage baggage, + final @Nullable Boolean sampled) { + this.traceId = traceId; + this.spanId = spanId; + this.parentSpanId = parentSpanId; + this.baggage = baggage; + this.sampled = sampled; + } + + public @NotNull SentryId getTraceId() { + return traceId; + } + + public void setTraceId(final @NotNull SentryId traceId) { + this.traceId = traceId; + } + + public @NotNull SpanId getSpanId() { + return spanId; + } + + public void setSpanId(final @NotNull SpanId spanId) { + this.spanId = spanId; + } + + public @Nullable SpanId getParentSpanId() { + return parentSpanId; + } + + public void setParentSpanId(final @Nullable SpanId parentSpanId) { + this.parentSpanId = parentSpanId; + } + + public @Nullable Baggage getBaggage() { + return baggage; + } + + public void setBaggage(final @Nullable Baggage baggage) { + this.baggage = baggage; + } + + public @Nullable Boolean isSampled() { + return sampled; + } + + public void setSampled(final @Nullable Boolean sampled) { + this.sampled = sampled; + } + + public @Nullable TraceContext traceContext() { + if (baggage != null) { + return baggage.toTraceContext(); + } + + return null; + } +} diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index 997f93c05d7..d73ba91bb29 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -71,6 +71,8 @@ public final class Scope { /** Scope's attachments */ private @NotNull List attachments = new CopyOnWriteArrayList<>(); + private @NotNull PropagationContext propagationContext; + /** * Scope's ctor * @@ -79,6 +81,7 @@ public final class Scope { public Scope(final @NotNull SentryOptions options) { this.options = Objects.requireNonNull(options, "SentryOptions is required."); this.breadcrumbs = createBreadcrumbsList(this.options.getMaxBreadcrumbs()); + this.propagationContext = new PropagationContext(); } Scope(final @NotNull Scope scope) { @@ -134,6 +137,8 @@ public Scope(final @NotNull SentryOptions options) { this.contexts = new Contexts(scope.contexts); this.attachments = new CopyOnWriteArrayList<>(scope.attachments); + + this.propagationContext = scope.propagationContext; } /** @@ -799,6 +804,16 @@ public void withTransaction(final @NotNull IWithTransaction callback) { return session; } + @ApiStatus.Internal + public void setPropagationContext(final @NotNull PropagationContext propagationContext) { + this.propagationContext = propagationContext; + } + + @ApiStatus.Internal + public @NotNull PropagationContext getPropagationContext() { + return propagationContext; + } + /** the IWithTransaction callback */ @ApiStatus.Internal public interface IWithTransaction { diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 0cd640c8c48..99ddd7baedc 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -903,7 +903,7 @@ public static void endSession() { } /** - * Returns trace header of active transaction or {@code null} if no transaction is active. + * TODO Returns trace header of active transaction or {@code null} if no transaction is active. * * @return trace header or null */ @@ -969,4 +969,21 @@ public interface OptionsConfiguration { */ void configure(@NotNull T options); } + + /** + * Continue a trace based on HTTP header values. If no "sentry-trace" header is provided a random + * trace ID and span ID is created. + * + * @param sentryTraceHeader "sentry-trace" header + * @param baggageHeaders "baggage" headers + */ + public static @Nullable PropagationContext continueTrace( + final @Nullable String sentryTraceHeader, final @Nullable List baggageHeaders) { + return getCurrentHub().continueTrace(sentryTraceHeader, baggageHeaders); + } + + public static @Nullable BaggageHeader baggageHeader( + @Nullable List thirdPartyBaggageHeaders) { + return getCurrentHub().baggageHeader(thirdPartyBaggageHeaders); + } } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 7570611b58e..89629a7bb5d 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -11,6 +11,7 @@ import io.sentry.transport.ITransport; import io.sentry.util.HintUtils; import io.sentry.util.Objects; +import io.sentry.util.TracingUtils; import java.io.Closeable; import java.io.IOException; import java.security.SecureRandom; @@ -171,10 +172,16 @@ private boolean shouldApplyScopeData( } try { - final TraceContext traceContext = - scope != null && scope.getTransaction() != null - ? scope.getTransaction().traceContext() - : null; + @Nullable TraceContext traceContext = null; + if (scope != null) { + if (scope.getTransaction() != null) { + traceContext = scope.getTransaction().traceContext(); + } else { + TracingUtils.maybeUpdateBaggage(scope, options); + traceContext = scope.getPropagationContext().traceContext(); + } + } + final boolean shouldSendAttachments = event != null; List attachments = shouldSendAttachments ? getAttachments(hint) : null; final SentryEnvelope envelope = diff --git a/sentry/src/main/java/io/sentry/TransactionContext.java b/sentry/src/main/java/io/sentry/TransactionContext.java index 4463f9ae4f0..3f24cfd4805 100644 --- a/sentry/src/main/java/io/sentry/TransactionContext.java +++ b/sentry/src/main/java/io/sentry/TransactionContext.java @@ -26,58 +26,32 @@ public final class TransactionContext extends SpanContext { final @NotNull String name, final @NotNull String operation, final @NotNull SentryTraceHeader sentryTrace) { - return fromSentryTrace(name, TransactionNameSource.CUSTOM, operation, sentryTrace, null, null); - } - - /** - * Creates {@link TransactionContext} from sentry-trace header. - * - * @param name - the transaction name - * @param transactionNameSource - source of the transaction name - * @param operation - the operation - * @param sentryTrace - the sentry-trace header - * @return the transaction contexts - */ - @ApiStatus.Internal - public static @NotNull TransactionContext fromSentryTrace( - final @NotNull String name, - final @NotNull TransactionNameSource transactionNameSource, - final @NotNull String operation, - final @NotNull SentryTraceHeader sentryTrace) { @Nullable Boolean parentSampled = sentryTrace.isSampled(); + TracesSamplingDecision samplingDecision = + parentSampled == null ? null : new TracesSamplingDecision(parentSampled); + return new TransactionContext( name, operation, sentryTrace.getTraceId(), new SpanId(), - transactionNameSource, + TransactionNameSource.CUSTOM, sentryTrace.getSpanId(), - parentSampled == null ? null : new TracesSamplingDecision(parentSampled), + samplingDecision, null); } - /** - * Creates {@link TransactionContext} from sentry-trace header. - * - * @param name - the transaction name - * @param transactionNameSource - source of the transaction name - * @param operation - the operation - * @param sentryTrace - the sentry-trace header - * @param baggage - the baggage header - * @return the transaction contexts - */ - @ApiStatus.Internal - public static @NotNull TransactionContext fromSentryTrace( + public static TransactionContext fromPropagationContext( final @NotNull String name, final @NotNull TransactionNameSource transactionNameSource, final @NotNull String operation, - final @NotNull SentryTraceHeader sentryTrace, - final @Nullable Baggage baggage, - final @Nullable SpanId spanId) { - @Nullable Boolean parentSampled = sentryTrace.isSampled(); + final @NotNull PropagationContext propagationContext) { + @Nullable Boolean parentSampled = propagationContext.isSampled(); TracesSamplingDecision samplingDecision = parentSampled == null ? null : new TracesSamplingDecision(parentSampled); + @Nullable Baggage baggage = propagationContext.getBaggage(); + if (baggage != null) { baggage.freeze(); @@ -90,15 +64,13 @@ public final class TransactionContext extends SpanContext { } } - final @NotNull SpanId spanIdToUse = spanId == null ? new SpanId() : spanId; - return new TransactionContext( name, operation, - sentryTrace.getTraceId(), - spanIdToUse, + propagationContext.getTraceId(), + propagationContext.getSpanId(), transactionNameSource, - sentryTrace.getSpanId(), + propagationContext.getParentSpanId(), samplingDecision, baggage); } diff --git a/sentry/src/main/java/io/sentry/util/TracingUtils.java b/sentry/src/main/java/io/sentry/util/TracingUtils.java new file mode 100644 index 00000000000..8dad62bd903 --- /dev/null +++ b/sentry/src/main/java/io/sentry/util/TracingUtils.java @@ -0,0 +1,106 @@ +package io.sentry.util; + +import io.sentry.Baggage; +import io.sentry.BaggageHeader; +import io.sentry.IHub; +import io.sentry.ISpan; +import io.sentry.PropagationContext; +import io.sentry.Scope; +import io.sentry.SentryOptions; +import io.sentry.SentryTraceHeader; +import java.util.List; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public final class TracingUtils { + + public static void startNewTrace(final @NotNull IHub hub) { + hub.configureScope( + scope -> { + scope.setPropagationContext(new PropagationContext()); + }); + } + + public static void traceIfAllowed( + final @NotNull IHub hub, + final @NotNull String requestUrl, + @Nullable List thirdPartyBaggageHeaders, + final @Nullable ISpan span, + final @NotNull HintUtils.SentryConsumer callback) { + final @NotNull SentryOptions sentryOptions = hub.getOptions(); + if (sentryOptions.isTraceSampling() && isAllowedToSendTo(requestUrl, sentryOptions)) { + trace(hub, thirdPartyBaggageHeaders, span, callback); + } + } + + public static void trace( + final @NotNull IHub hub, + @Nullable List thirdPartyBaggageHeaders, + final @Nullable ISpan span, + final @NotNull HintUtils.SentryConsumer callback) { + final @NotNull SentryOptions sentryOptions = hub.getOptions(); + + if (span != null && !span.isNoOp()) { + callback.accept( + new TracingHeaders(span.toSentryTrace(), span.toBaggageHeader(thirdPartyBaggageHeaders))); + } else { + hub.configureScope( + (scope) -> { + maybeUpdateBaggage(scope, sentryOptions); + final @NotNull PropagationContext propagationContext = scope.getPropagationContext(); + + final @Nullable Baggage baggage = propagationContext.getBaggage(); + @Nullable BaggageHeader baggageHeader = null; + if (baggage != null) { + baggageHeader = + BaggageHeader.fromBaggageAndOutgoingHeader(baggage, thirdPartyBaggageHeaders); + } + + callback.accept( + new TracingHeaders( + new SentryTraceHeader( + propagationContext.getTraceId(), propagationContext.getSpanId(), null), + baggageHeader)); + }); + } + } + + public static void maybeUpdateBaggage( + final @NotNull Scope scope, final @NotNull SentryOptions sentryOptions) { + final @NotNull PropagationContext propagationContext = scope.getPropagationContext(); + @Nullable Baggage baggage = propagationContext.getBaggage(); + if (baggage == null) { + baggage = new Baggage(sentryOptions.getLogger()); + propagationContext.setBaggage(baggage); + } + if (baggage.isMutable()) { + baggage.setValuesFromScope(scope, sentryOptions); + baggage.freeze(); + } + } + + private static boolean isAllowedToSendTo( + final @NotNull String requestUrl, final @NotNull SentryOptions sentryOptions) { + return PropagationTargetsUtils.contain(sentryOptions.getTracePropagationTargets(), requestUrl); + } + + public static final class TracingHeaders { + private final @NotNull SentryTraceHeader sentryTraceHeader; + private final @Nullable BaggageHeader baggageHeader; + + public TracingHeaders( + final @NotNull SentryTraceHeader sentryTraceHeader, + final @Nullable BaggageHeader baggageHeader) { + this.sentryTraceHeader = sentryTraceHeader; + this.baggageHeader = baggageHeader; + } + + public @NotNull SentryTraceHeader getSentryTraceHeader() { + return sentryTraceHeader; + } + + public @Nullable BaggageHeader getBaggageHeader() { + return baggageHeader; + } + } +} diff --git a/sentry/src/test/java/io/sentry/TransactionContextTest.kt b/sentry/src/test/java/io/sentry/TransactionContextTest.kt index 4c40a65272c..dc55c6d501f 100644 --- a/sentry/src/test/java/io/sentry/TransactionContextTest.kt +++ b/sentry/src/test/java/io/sentry/TransactionContextTest.kt @@ -2,6 +2,7 @@ package io.sentry import io.sentry.protocol.SentryId import io.sentry.protocol.TransactionNameSource +import org.mockito.kotlin.mock import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -30,10 +31,14 @@ class TransactionContextTest { } @Test - fun `when context is created from trace header and baggage header, parent sampling decision of false is set from trace header`() { - val traceHeader = SentryTraceHeader(SentryId(), SpanId(), false) - val baggageHeader = Baggage.fromHeader("sentry-trace_id=a,sentry-transaction=sentryTransaction,sentry-sample_rate=0.3") - val context = TransactionContext.fromSentryTrace("name", TransactionNameSource.CUSTOM, "op", traceHeader, baggageHeader, null) + fun `when context is created from propagation context, parent sampling decision of false is set from trace header`() { + val logger = mock() + val propagationContext = PropagationContext.fromHeaders( + logger, + SentryTraceHeader(SentryId(), SpanId(), false).value, + "sentry-trace_id=a,sentry-transaction=sentryTransaction,sentry-sample_rate=0.3" + ) + val context = TransactionContext.fromPropagationContext("name", TransactionNameSource.CUSTOM, "op", propagationContext) assertNull(context.sampled) assertNull(context.profileSampled) assertFalse(context.parentSampled!!) @@ -41,10 +46,14 @@ class TransactionContextTest { } @Test - fun `when context is created from trace header and baggage header, parent sampling decision of false is set from trace header if no sample rate is available`() { - val traceHeader = SentryTraceHeader(SentryId(), SpanId(), false) - val baggageHeader = Baggage.fromHeader("sentry-trace_id=a,sentry-transaction=sentryTransaction") - val context = TransactionContext.fromSentryTrace("name", TransactionNameSource.CUSTOM, "op", traceHeader, baggageHeader, null) + fun `when context is created from propagation context, parent sampling decision of false is set from trace header if no sample rate is available`() { + val logger = mock() + val propagationContext = PropagationContext.fromHeaders( + logger, + SentryTraceHeader(SentryId(), SpanId(), false).value, + "sentry-trace_id=a,sentry-transaction=sentryTransaction" + ) + val context = TransactionContext.fromPropagationContext("name", TransactionNameSource.CUSTOM, "op", propagationContext) assertNull(context.sampled) assertNull(context.profileSampled) assertFalse(context.parentSampled!!) @@ -52,10 +61,14 @@ class TransactionContextTest { } @Test - fun `when context is created from trace header and baggage header, parent sampling decision of true is set from trace header`() { - val traceHeader = SentryTraceHeader(SentryId(), SpanId(), true) - val baggageHeader = Baggage.fromHeader("sentry-trace_id=a,sentry-transaction=sentryTransaction,sentry-sample_rate=0.3") - val context = TransactionContext.fromSentryTrace("name", TransactionNameSource.CUSTOM, "op", traceHeader, baggageHeader, null) + fun `when context is created from propagation context, parent sampling decision of true is set from trace header`() { + val logger = mock() + val propagationContext = PropagationContext.fromHeaders( + logger, + SentryTraceHeader(SentryId(), SpanId(), true).value, + "sentry-trace_id=a,sentry-transaction=sentryTransaction,sentry-sample_rate=0.3" + ) + val context = TransactionContext.fromPropagationContext("name", TransactionNameSource.CUSTOM, "op", propagationContext) assertNull(context.sampled) assertNull(context.profileSampled) assertTrue(context.parentSampled!!) @@ -63,10 +76,14 @@ class TransactionContextTest { } @Test - fun `when context is created from trace header and baggage header, parent sampling decision of true is set from trace header if no sample rate is available`() { - val traceHeader = SentryTraceHeader(SentryId(), SpanId(), true) - val baggageHeader = Baggage.fromHeader("sentry-trace_id=a,sentry-transaction=sentryTransaction") - val context = TransactionContext.fromSentryTrace("name", TransactionNameSource.CUSTOM, "op", traceHeader, baggageHeader, null) + fun `when context is created from propagation context, parent sampling decision of true is set from trace header if no sample rate is available`() { + val logger = mock() + val propagationContext = PropagationContext.fromHeaders( + logger, + SentryTraceHeader(SentryId(), SpanId(), true).value, + "sentry-trace_id=a,sentry-transaction=sentryTransaction" + ) + val context = TransactionContext.fromPropagationContext("name", TransactionNameSource.CUSTOM, "op", propagationContext) assertNull(context.sampled) assertNull(context.profileSampled) assertTrue(context.parentSampled!!) From 6eba5bca8a5834874516da9d03faa5c2db484719 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 15 Jun 2023 14:58:36 +0200 Subject: [PATCH 2/7] fix tests --- .../main/java/io/sentry/openfeign/SentryFeignClient.java | 6 ++++-- .../jakarta/webflux/SentryWebFluxTracingFilterTest.kt | 5 +++++ .../spring/webflux/SentryWebFluxTracingFilterTest.kt | 5 +++++ sentry/src/main/java/io/sentry/PropagationContext.java | 2 +- sentry/src/test/java/io/sentry/HubTest.kt | 9 +++++++-- sentry/src/test/java/io/sentry/SentryClientTest.kt | 8 +++++--- 6 files changed, 27 insertions(+), 8 deletions(-) diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java index 8beb6d3c558..1802d5d3d6f 100644 --- a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java +++ b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java @@ -89,12 +89,14 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O private @NotNull Request maybeAddTracingHeaders( final @NotNull Request request, final @Nullable ISpan span) { - final RequestWrapper requestWrapper = new RequestWrapper(request); + final @NotNull RequestWrapper requestWrapper = new RequestWrapper(request); + final @Nullable Collection requestBaggageHeaders = + request.headers().get(BaggageHeader.BAGGAGE_HEADER); TracingUtils.traceIfAllowed( hub, request.url(), - new ArrayList<>(request.headers().get(BaggageHeader.BAGGAGE_HEADER)), + (requestBaggageHeaders != null ? new ArrayList<>(requestBaggageHeaders) : null), span, tracingHeaders -> { requestWrapper.header( diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt index 8cb30dcb505..b32831e8265 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt @@ -3,6 +3,8 @@ package io.sentry.spring.jakarta.webflux import io.sentry.Breadcrumb import io.sentry.Hint import io.sentry.IHub +import io.sentry.ILogger +import io.sentry.PropagationContext import io.sentry.ScopeCallback import io.sentry.Sentry import io.sentry.SentryOptions @@ -50,6 +52,7 @@ class SentryWebFluxTracingFilterTest { dsn = "https://key@sentry.io/proj" enableTracing = true } + val logger = mock() init { whenever(hub.options).thenReturn(options) @@ -69,6 +72,7 @@ class SentryWebFluxTracingFilterTest { whenever(hub.startTransaction(any(), check { assertTrue(it.isBindToScope) })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } whenever(hub.isEnabled).thenReturn(isEnabled) whenever(chain.filter(any())).thenReturn(Mono.create { s -> s.success() }) + whenever(hub.continueTrace(anyOrNull(), anyOrNull())).thenAnswer { PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?) } return SentryWebFilter(hub) } } @@ -235,6 +239,7 @@ class SentryWebFluxTracingFilterTest { verify(fixture.hub, times(3)).isEnabled verify(fixture.hub, times(2)).options + verify(fixture.hub).continueTrace(anyOrNull(), anyOrNull()) verify(fixture.hub).pushScope() verify(fixture.hub).addBreadcrumb(any(), any()) verify(fixture.hub).configureScope(any()) diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt index 388dbd94463..c8b1af22849 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt @@ -3,6 +3,8 @@ package io.sentry.spring.webflux import io.sentry.Breadcrumb import io.sentry.Hint import io.sentry.IHub +import io.sentry.ILogger +import io.sentry.PropagationContext import io.sentry.ScopeCallback import io.sentry.Sentry import io.sentry.SentryOptions @@ -50,6 +52,7 @@ class SentryWebFluxTracingFilterTest { dsn = "https://key@sentry.io/proj" enableTracing = true } + val logger = mock() init { whenever(hub.options).thenReturn(options) @@ -69,6 +72,7 @@ class SentryWebFluxTracingFilterTest { whenever(hub.startTransaction(any(), check { assertTrue(it.isBindToScope) })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } whenever(hub.isEnabled).thenReturn(isEnabled) whenever(chain.filter(any())).thenReturn(Mono.create { s -> s.success() }) + whenever(hub.continueTrace(anyOrNull(), anyOrNull())).thenAnswer { PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?) } return SentryWebFilter(hub) } } @@ -236,6 +240,7 @@ class SentryWebFluxTracingFilterTest { verify(fixture.hub).isEnabled verify(fixture.hub, times(2)).options + verify(fixture.hub).continueTrace(anyOrNull(), anyOrNull()) verify(fixture.hub).pushScope() verify(fixture.hub).addBreadcrumb(any(), any()) verify(fixture.hub).configureScope(any()) diff --git a/sentry/src/main/java/io/sentry/PropagationContext.java b/sentry/src/main/java/io/sentry/PropagationContext.java index e1b8ddf0516..cc538194e56 100644 --- a/sentry/src/main/java/io/sentry/PropagationContext.java +++ b/sentry/src/main/java/io/sentry/PropagationContext.java @@ -26,7 +26,7 @@ public static PropagationContext fromHeaders( try { final @NotNull SentryTraceHeader traceHeader = new SentryTraceHeader(sentryTraceHeaderString); - final @NotNull Baggage baggage = Baggage.fromHeader(baggageHeaderStrings); + final @NotNull Baggage baggage = Baggage.fromHeader(baggageHeaderStrings, logger); return fromHeaders(traceHeader, baggage, null); } catch (InvalidSentryTraceHeaderException e) { logger.log(SentryLevel.DEBUG, e, "Failed to parse Sentry trace header: %s", e.getMessage()); diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt index baab6e4e573..fcb0132d0d1 100644 --- a/sentry/src/test/java/io/sentry/HubTest.kt +++ b/sentry/src/test/java/io/sentry/HubTest.kt @@ -1580,10 +1580,15 @@ class HubTest { //region startTransaction tests @Test - fun `when traceHeaders and no transaction is active, traceHeaders are null`() { + fun `when traceHeaders and no transaction is active, traceHeaders are generated from scope`() { val hub = generateHub() - assertNull(hub.traceHeaders()) + var spanId: SpanId? = null + hub.configureScope { spanId = it.propagationContext.spanId } + + val traceHeader = hub.traceHeaders() + assertNotNull(traceHeader) + assertEquals(spanId, traceHeader.spanId) } @Test diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 19dff0a5040..441d206a6d2 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -1340,12 +1340,14 @@ class SentryClientTest { } @Test - fun `when scope does not have an active transaction, trace state is not set on the envelope`() { + fun `when scope does not have an active transaction, trace state is set on the envelope from scope`() { val sut = fixture.getSut() - sut.captureEvent(SentryEvent(), createScope()) + val scope = createScope() + sut.captureEvent(SentryEvent(), scope) verify(fixture.transport).send( check { - assertNull(it.header.traceContext) + assertNotNull(it.header.traceContext) + assertEquals(scope.propagationContext.traceId, it.header.traceContext?.traceId) }, anyOrNull() ) From 537b3fb5105a731a97ff8fe3a3f1c188a3cfa524 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 19 Jun 2023 09:20:03 +0200 Subject: [PATCH 3/7] also add headers for okhttp if no span is active; add tests; fix existing tests --- .../okhttp/SentryOkHttpInterceptorTest.kt | 18 ++++++++- .../apollo3/SentryApollo3HttpInterceptor.kt | 39 +++++++++++-------- .../apollo3/SentryApollo3InterceptorTest.kt | 33 +++++++++++----- .../apollo/SentryApolloInterceptorTest.kt | 27 ++++++++----- .../sentry/openfeign/SentryFeignClientTest.kt | 19 ++++++++- .../SentrySpanRestTemplateCustomizerTest.kt | 28 +++++++++++++ .../SentrySpanWebClientCustomizerTest.kt | 35 +++++++++++++++++ .../SentrySpanRestTemplateCustomizerTest.kt | 30 ++++++++++++++ .../boot/SentrySpanWebClientCustomizerTest.kt | 37 ++++++++++++++++++ 9 files changed, 228 insertions(+), 38 deletions(-) diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt index ba94ca5a038..d14fa0e3f8a 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpInterceptorTest.kt @@ -7,6 +7,8 @@ import io.sentry.Breadcrumb import io.sentry.Hint import io.sentry.HttpStatusCodeRange import io.sentry.IHub +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -26,6 +28,7 @@ import okhttp3.mockwebserver.SocketPolicy import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -46,6 +49,7 @@ class SentryOkHttpInterceptorTest { val server = MockWebServer() lateinit var sentryTracer: SentryTracer lateinit var options: SentryOptions + lateinit var scope: Scope @SuppressWarnings("LongParameterList") fun getSut( @@ -75,7 +79,9 @@ class SentryOkHttpInterceptorTest { } isSendDefaultPii = sendDefaultPii } + scope = Scope(options) whenever(hub.options).thenReturn(options) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) sentryTracer = SentryTracer(TransactionContext("name", "op"), hub) @@ -179,10 +185,20 @@ class SentryOkHttpInterceptorTest { } @Test - fun `when there is no active span, does not add sentry trace header to the request`() { + fun `when there is no active span, adds sentry trace header to the request from scope`() { val sut = fixture.getSut(isSpanActive = false) sut.newCall(getRequest()).execute() val recorderRequest = fixture.server.takeRequest() + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Test + fun `when there is no active span and host if not allowed, does not add sentry trace header to the request`() { + val sut = fixture.getSut(isSpanActive = false) + fixture.options.setTracePropagationTargets(listOf("some-host-that-does-not-exist")) + sut.newCall(getRequest()).execute() + val recorderRequest = fixture.server.takeRequest() assertNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) assertNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt index df1a9f9d922..8871d3ac5fa 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt +++ b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt @@ -30,32 +30,37 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH SentryIntegrationPackageStorage.getInstance().addPackage("maven:io.sentry:sentry-apollo-3", BuildConfig.VERSION_NAME) } + private fun maybeAddTracingHeaders(hub: IHub, request: HttpRequest, span: ISpan?): HttpRequest { + var cleanedHeaders = removeSentryInternalHeaders(request.headers).toMutableList() + + TracingUtils.traceIfAllowed(hub, request.url, request.headers.filter { it.name == BaggageHeader.BAGGAGE_HEADER }.map { it.value }, span) { + cleanedHeaders.add(HttpHeader(it.sentryTraceHeader.name, it.sentryTraceHeader.value)) + it.baggageHeader?.let { baggageHeader -> + cleanedHeaders = cleanedHeaders.filterNot { it.name == BaggageHeader.BAGGAGE_HEADER }.toMutableList().apply { + add(HttpHeader(baggageHeader.name, baggageHeader.value)) + } + } + } + + val requestBuilder = request.newBuilder().apply { + headers(cleanedHeaders) + } + + return requestBuilder.build() + } + override suspend fun intercept( request: HttpRequest, chain: HttpInterceptorChain ): HttpResponse { val activeSpan = hub.span return if (activeSpan == null) { - chain.proceed(request) + val modifiedRequest = maybeAddTracingHeaders(hub, request, null) + chain.proceed(modifiedRequest) } else { val span = startChild(request, activeSpan) + val modifiedRequest = maybeAddTracingHeaders(hub, request, span) - var cleanedHeaders = removeSentryInternalHeaders(request.headers).toMutableList() - - TracingUtils.traceIfAllowed(hub, request.url, request.headers.filter { it.name == BaggageHeader.BAGGAGE_HEADER }.map { it.value }, span) { - cleanedHeaders.add(HttpHeader(it.sentryTraceHeader.name, it.sentryTraceHeader.value)) - it.baggageHeader?.let { baggageHeader -> - cleanedHeaders = cleanedHeaders.filterNot { it.name == BaggageHeader.BAGGAGE_HEADER }.toMutableList().apply { - add(HttpHeader(baggageHeader.name, baggageHeader.value)) - } - } - } - - val requestBuilder = request.newBuilder().apply { - headers(cleanedHeaders) - } - - val modifiedRequest = requestBuilder.build() var httpResponse: HttpResponse? = null var statusCode: Int? = null diff --git a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt index a45f58d7145..973adad123d 100644 --- a/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt +++ b/sentry-apollo-3/src/test/java/io/sentry/apollo3/SentryApollo3InterceptorTest.kt @@ -6,6 +6,8 @@ import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub import io.sentry.ITransaction +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -21,8 +23,10 @@ import kotlinx.coroutines.runBlocking import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy +import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -36,14 +40,15 @@ class SentryApollo3InterceptorTest { class Fixture { val server = MockWebServer() - val hub = mock().apply { - whenever(options).thenReturn( - SentryOptions().apply { - dsn = "https://key@sentry.io/proj" - isTraceSampling = true - sdkVersion = SdkVersion("test", "1.2.3") - } - ) + val options = + SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + sdkVersion = SdkVersion("test", "1.2.3") + } + val scope = Scope(options) + val hub = mock().also { + whenever(it.options).thenReturn(options) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(it).configureScope(any()) } private var httpInterceptor = SentryApollo3HttpInterceptor(hub) @@ -140,7 +145,8 @@ class SentryApollo3InterceptorTest { } @Test - fun `when there is no active span, does not add sentry trace header to the request`() { + fun `does not add sentry trace header to the request if host is disallowed`() { + fixture.options.setTracePropagationTargets(listOf("some-host-that-does-not-exist")) executeQuery(isSpanActive = false) val recorderRequest = fixture.server.takeRequest() @@ -148,6 +154,15 @@ class SentryApollo3InterceptorTest { assertNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } + @Test + fun `when there is no active span, does not add sentry trace header to the request`() { + executeQuery(isSpanActive = false) + + val recorderRequest = fixture.server.takeRequest() + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + @Test fun `when there is an active span, adds sentry trace headers to the request`() { executeQuery() diff --git a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt index 97bb307b343..0412f678755 100644 --- a/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt +++ b/sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt @@ -3,9 +3,12 @@ package io.sentry.apollo import com.apollographql.apollo.ApolloClient import com.apollographql.apollo.coroutines.await import com.apollographql.apollo.exception.ApolloException +import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub import io.sentry.ITransaction +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -20,26 +23,30 @@ import kotlinx.coroutines.runBlocking import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy +import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull -import kotlin.test.assertNull class SentryApolloInterceptorTest { class Fixture { val server = MockWebServer() - val hub = mock().apply { - whenever(options).thenReturn( - SentryOptions().apply { - dsn = "https://key@sentry.io/proj" - sdkVersion = SdkVersion("test", "1.2.3") - } + val options = SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + sdkVersion = SdkVersion("test", "1.2.3") + } + val scope = Scope(options) + val hub = mock().also { + whenever(it.options).thenReturn(options) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(it).configureScope( + any() ) } private var interceptor = SentryApolloInterceptor(hub) @@ -129,11 +136,12 @@ class SentryApolloInterceptorTest { } @Test - fun `when there is no active span, does not add sentry trace header to the request`() { + fun `when there is no active span, adds sentry trace header to the request from scope`() { executeQuery(isSpanActive = false) val recorderRequest = fixture.server.takeRequest() - assertNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } @Test @@ -141,6 +149,7 @@ class SentryApolloInterceptorTest { executeQuery() val recorderRequest = fixture.server.takeRequest() assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } @Test diff --git a/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt b/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt index d6068bc2371..e54e062765b 100644 --- a/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt +++ b/sentry-openfeign/src/test/kotlin/io/sentry/openfeign/SentryFeignClientTest.kt @@ -8,6 +8,8 @@ import feign.RequestLine import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -18,6 +20,7 @@ import okhttp3.mockwebserver.MockWebServer import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -39,9 +42,11 @@ class SentryFeignClientTest { val sentryOptions = SentryOptions().apply { dsn = "http://key@localhost/proj" } + val scope = Scope(sentryOptions) init { whenever(hub.options).thenReturn(sentryOptions) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) sentryTracer = SentryTracer(TransactionContext("name", "op"), hub) } @@ -113,8 +118,18 @@ class SentryFeignClientTest { } @Test - fun `when there is no active span, does not add sentry trace header to the request`() { - fixture.sentryOptions.isTraceSampling = true + fun `when there is no active span, adds sentry trace header to the request from scope`() { + fixture.sentryOptions.dsn = "https://key@sentry.io/proj" + val sut = fixture.getSut(isSpanActive = false) + sut.getOk() + val recorderRequest = fixture.server.takeRequest() + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Test + fun `when there is no active span, does not add sentry trace header to the request if host is disallowed`() { + fixture.sentryOptions.setTracePropagationTargets(listOf("some-host-that-does-not-exist")) fixture.sentryOptions.dsn = "https://key@sentry.io/proj" val sut = fixture.getSut(isSpanActive = false) sut.getOk() diff --git a/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestTemplateCustomizerTest.kt b/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestTemplateCustomizerTest.kt index 4afaa9983b3..2c69175fa50 100644 --- a/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestTemplateCustomizerTest.kt +++ b/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanRestTemplateCustomizerTest.kt @@ -3,6 +3,8 @@ package io.sentry.spring.boot.jakarta import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -13,8 +15,10 @@ import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy import org.assertj.core.api.Assertions.assertThat +import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -38,9 +42,11 @@ class SentrySpanRestTemplateCustomizerTest { val transaction: SentryTracer internal val customizer = SentrySpanRestTemplateCustomizer(hub) val url = mockServer.url("/test/123").toString() + val scope = Scope(sentryOptions) init { whenever(hub.options).thenReturn(sentryOptions) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) transaction = SentryTracer(TransactionContext("aTransaction", "op", TracesSamplingDecision(true)), hub) } @@ -164,6 +170,28 @@ class SentrySpanRestTemplateCustomizerTest { assertThat(fixture.transaction.spans).isEmpty() } + @Test + fun `when transaction is not active, adds tracing headers from scope`() { + val sut = fixture.getSut(isTransactionActive = false) + val headers = HttpHeaders() + headers.add("baggage", "thirdPartyBaggage=someValue") + headers.add("baggage", "secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue") + + val requestEntity = HttpEntity(headers) + + sut.exchange(fixture.url, HttpMethod.GET, requestEntity, String::class.java) + + val recorderRequest = fixture.mockServer.takeRequest() + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + + val baggageHeaderValues = recorderRequest.headers.values(BaggageHeader.BAGGAGE_HEADER) + assertEquals(baggageHeaderValues.size, 1) + assertTrue(baggageHeaderValues[0].startsWith("thirdPartyBaggage=someValue,secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue")) + assertTrue(baggageHeaderValues[0].contains("sentry-public_key=key")) + assertTrue(baggageHeaderValues[0].contains("sentry-trace_id")) + } + @Test fun `avoids duplicate registration`() { val restTemplate = fixture.getSut(isTransactionActive = true) diff --git a/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanWebClientCustomizerTest.kt b/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanWebClientCustomizerTest.kt index 2f298992d0e..2aaea06efcb 100644 --- a/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanWebClientCustomizerTest.kt +++ b/sentry-spring-boot-starter-jakarta/src/test/kotlin/io/sentry/spring/boot/jakarta/SentrySpanWebClientCustomizerTest.kt @@ -1,8 +1,10 @@ package io.sentry.spring.boot.jakarta +import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -16,8 +18,10 @@ import okhttp3.mockwebserver.RecordedRequest import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach +import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -33,6 +37,7 @@ import kotlin.test.assertNull class SentrySpanWebClientCustomizerTest { class Fixture { lateinit var sentryOptions: SentryOptions + lateinit var scope: Scope val hub = mock() var mockServer = MockWebServer() lateinit var transaction: SentryTracer @@ -47,7 +52,9 @@ class SentrySpanWebClientCustomizerTest { } dsn = "http://key@localhost/proj" } + scope = Scope(sentryOptions) whenever(hub.options).thenReturn(sentryOptions) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) transaction = SentryTracer(TransactionContext("aTransaction", "op", TracesSamplingDecision(true)), hub) val webClientBuilder = WebClient.builder() customizer.customize(webClientBuilder) @@ -124,6 +131,33 @@ class SentrySpanWebClientCustomizerTest { .block() val recordedRequest = fixture.mockServer.takeRequest() assertNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Test + fun `when no transaction is active and server is not listed in tracing origins, does not add sentry trace header to the request`() { + fixture.getSut(isTransactionActive = false, includeMockServerInTracingOrigins = false) + .get() + .uri(fixture.mockServer.url("/test/123").toUri()) + .retrieve() + .bodyToMono(String::class.java) + .block() + val recordedRequest = fixture.mockServer.takeRequest() + assertNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Test + fun `when no transaction is active, adds sentry trace header to the request from scope`() { + fixture.getSut(isTransactionActive = false, includeMockServerInTracingOrigins = true) + .get() + .uri(fixture.mockServer.url("/test/123").toUri()) + .retrieve() + .bodyToMono(String::class.java) + .block() + val recordedRequest = fixture.mockServer.takeRequest() + assertNotNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } @Test @@ -136,6 +170,7 @@ class SentrySpanWebClientCustomizerTest { .block() val recordedRequest = fixture.mockServer.takeRequest() assertNotNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } @Test diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt index cf60680dee2..21c989fb7df 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanRestTemplateCustomizerTest.kt @@ -3,6 +3,8 @@ package io.sentry.spring.boot import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -13,8 +15,10 @@ import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy import org.assertj.core.api.Assertions.assertThat +import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -38,9 +42,13 @@ class SentrySpanRestTemplateCustomizerTest { val transaction: SentryTracer internal val customizer = SentrySpanRestTemplateCustomizer(hub) val url = mockServer.url("/test/123").toString() + val scope = Scope(sentryOptions) init { whenever(hub.options).thenReturn(sentryOptions) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope( + any() + ) transaction = SentryTracer(TransactionContext("aTransaction", "op", TracesSamplingDecision(true)), hub) } @@ -164,6 +172,28 @@ class SentrySpanRestTemplateCustomizerTest { assertThat(fixture.transaction.spans).isEmpty() } + @Test + fun `when transaction is not active, adds tracing headers from scope`() { + val sut = fixture.getSut(isTransactionActive = false) + val headers = HttpHeaders() + headers.add("baggage", "thirdPartyBaggage=someValue") + headers.add("baggage", "secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue") + + val requestEntity = HttpEntity(headers) + + sut.exchange(fixture.url, HttpMethod.GET, requestEntity, String::class.java) + + val recorderRequest = fixture.mockServer.takeRequest() + assertNotNull(recorderRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recorderRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + + val baggageHeaderValues = recorderRequest.headers.values(BaggageHeader.BAGGAGE_HEADER) + assertEquals(baggageHeaderValues.size, 1) + assertTrue(baggageHeaderValues[0].startsWith("thirdPartyBaggage=someValue,secondThirdPartyBaggage=secondValue; property;propertyKey=propertyValue,anotherThirdPartyBaggage=anotherValue")) + assertTrue(baggageHeaderValues[0].contains("sentry-public_key=key")) + assertTrue(baggageHeaderValues[0].contains("sentry-trace_id")) + } + @Test fun `avoids duplicate registration`() { val restTemplate = fixture.getSut(isTransactionActive = true) diff --git a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanWebClientCustomizerTest.kt b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanWebClientCustomizerTest.kt index e664d69a8a2..ba26fef3e91 100644 --- a/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanWebClientCustomizerTest.kt +++ b/sentry-spring-boot-starter/src/test/kotlin/io/sentry/spring/boot/SentrySpanWebClientCustomizerTest.kt @@ -1,8 +1,10 @@ package io.sentry.spring.boot +import io.sentry.BaggageHeader import io.sentry.Breadcrumb import io.sentry.IHub import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer @@ -16,8 +18,10 @@ import okhttp3.mockwebserver.RecordedRequest import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach +import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -33,6 +37,7 @@ import kotlin.test.assertNull class SentrySpanWebClientCustomizerTest { class Fixture { lateinit var sentryOptions: SentryOptions + lateinit var scope: Scope val hub = mock() var mockServer = MockWebServer() lateinit var transaction: SentryTracer @@ -47,7 +52,11 @@ class SentrySpanWebClientCustomizerTest { } dsn = "http://key@localhost/proj" } + scope = Scope(sentryOptions) whenever(hub.options).thenReturn(sentryOptions) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope( + any() + ) transaction = SentryTracer(TransactionContext("aTransaction", "op", TracesSamplingDecision(true)), hub) val webClientBuilder = WebClient.builder() customizer.customize(webClientBuilder) @@ -124,6 +133,33 @@ class SentrySpanWebClientCustomizerTest { .block() val recordedRequest = fixture.mockServer.takeRequest() assertNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Test + fun `when no transaction is active and server is not listed in tracing origins, does not add sentry trace header to the request`() { + fixture.getSut(isTransactionActive = false, includeMockServerInTracingOrigins = false) + .get() + .uri(fixture.mockServer.url("/test/123").toUri()) + .retrieve() + .bodyToMono(String::class.java) + .block() + val recordedRequest = fixture.mockServer.takeRequest() + assertNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) + } + + @Test + fun `when no transaction is active, adds sentry trace header to the request from scope`() { + fixture.getSut(isTransactionActive = false, includeMockServerInTracingOrigins = true) + .get() + .uri(fixture.mockServer.url("/test/123").toUri()) + .retrieve() + .bodyToMono(String::class.java) + .block() + val recordedRequest = fixture.mockServer.takeRequest() + assertNotNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } @Test @@ -136,6 +172,7 @@ class SentrySpanWebClientCustomizerTest { .block() val recordedRequest = fixture.mockServer.takeRequest() assertNotNull(recordedRequest.headers[SentryTraceHeader.SENTRY_TRACE_HEADER]) + assertNotNull(recordedRequest.headers[BaggageHeader.BAGGAGE_HEADER]) } @Test From 686eb4ebcbcea65e0271ec9e1dd69b9ff35a7b01 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 19 Jun 2023 10:10:53 +0200 Subject: [PATCH 4/7] Change trace and traceIfAllowed to return instead of taking a callback --- .../android/okhttp/SentryOkHttpInterceptor.kt | 2 +- .../apollo3/SentryApollo3HttpInterceptor.kt | 2 +- .../sentry/apollo/SentryApolloInterceptor.kt | 2 +- .../sentry/openfeign/SentryFeignClient.java | 34 ++++++++-------- ...entrySpanClientHttpRequestInterceptor.java | 34 ++++++++-------- .../SentrySpanClientWebRequestFilter.java | 40 ++++++++++--------- ...entrySpanClientHttpRequestInterceptor.java | 34 ++++++++-------- .../SentrySpanClientWebRequestFilter.java | 40 ++++++++++--------- sentry/api/sentry.api | 4 +- sentry/src/main/java/io/sentry/Hub.java | 35 +++++++--------- .../java/io/sentry/util/TracingUtils.java | 23 ++++++----- 11 files changed, 129 insertions(+), 121 deletions(-) diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt index 5120e2e2eb6..10625b32cd9 100644 --- a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt +++ b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt @@ -92,7 +92,7 @@ class SentryOkHttpInterceptor( request.url.toString(), request.headers(BaggageHeader.BAGGAGE_HEADER), span - ) { tracingHeaders -> + )?.let { tracingHeaders -> requestBuilder.addHeader(tracingHeaders.sentryTraceHeader.name, tracingHeaders.sentryTraceHeader.value) tracingHeaders.baggageHeader?.let { requestBuilder.removeHeader(BaggageHeader.BAGGAGE_HEADER) diff --git a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt index 8871d3ac5fa..cd34092ca4b 100644 --- a/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt +++ b/sentry-apollo-3/src/main/java/io/sentry/apollo3/SentryApollo3HttpInterceptor.kt @@ -33,7 +33,7 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH private fun maybeAddTracingHeaders(hub: IHub, request: HttpRequest, span: ISpan?): HttpRequest { var cleanedHeaders = removeSentryInternalHeaders(request.headers).toMutableList() - TracingUtils.traceIfAllowed(hub, request.url, request.headers.filter { it.name == BaggageHeader.BAGGAGE_HEADER }.map { it.value }, span) { + TracingUtils.traceIfAllowed(hub, request.url, request.headers.filter { it.name == BaggageHeader.BAGGAGE_HEADER }.map { it.value }, span)?.let { cleanedHeaders.add(HttpHeader(it.sentryTraceHeader.name, it.sentryTraceHeader.value)) it.baggageHeader?.let { baggageHeader -> cleanedHeaders = cleanedHeaders.filterNot { it.name == BaggageHeader.BAGGAGE_HEADER }.toMutableList().apply { diff --git a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt index 297c0222f42..45abbd5862c 100644 --- a/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt +++ b/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt @@ -100,7 +100,7 @@ class SentryApolloInterceptor( hub, listOf(request.requestHeaders.headerValue(BaggageHeader.BAGGAGE_HEADER)), span - ) { tracingHeaders -> + )?.let { tracingHeaders -> requestHeaderBuilder.addHeader( tracingHeaders.sentryTraceHeader.name, tracingHeaders.sentryTraceHeader.value diff --git a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java index 1802d5d3d6f..51cc41a921d 100644 --- a/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java +++ b/sentry-openfeign/src/main/java/io/sentry/openfeign/SentryFeignClient.java @@ -93,22 +93,24 @@ public Response execute(final @NotNull Request request, final @NotNull Request.O final @Nullable Collection requestBaggageHeaders = request.headers().get(BaggageHeader.BAGGAGE_HEADER); - TracingUtils.traceIfAllowed( - hub, - request.url(), - (requestBaggageHeaders != null ? new ArrayList<>(requestBaggageHeaders) : null), - span, - tracingHeaders -> { - requestWrapper.header( - tracingHeaders.getSentryTraceHeader().getName(), - tracingHeaders.getSentryTraceHeader().getValue()); - - final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); - if (baggageHeader != null) { - requestWrapper.removeHeader(BaggageHeader.BAGGAGE_HEADER); - requestWrapper.header(baggageHeader.getName(), baggageHeader.getValue()); - } - }); + final @Nullable TracingUtils.TracingHeaders tracingHeaders = + TracingUtils.traceIfAllowed( + hub, + request.url(), + (requestBaggageHeaders != null ? new ArrayList<>(requestBaggageHeaders) : null), + span); + + if (tracingHeaders != null) { + requestWrapper.header( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + requestWrapper.removeHeader(BaggageHeader.BAGGAGE_HEADER); + requestWrapper.header(baggageHeader.getName(), baggageHeader.getValue()); + } + } return requestWrapper.build(); } diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java index 086cff16000..2817a55cd4b 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -75,23 +75,25 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { private void maybeAddTracingHeaders( final @NotNull HttpRequest request, final @Nullable ISpan span) { - TracingUtils.traceIfAllowed( - hub, - request.getURI().toString(), - request.getHeaders().get(BaggageHeader.BAGGAGE_HEADER), - span, - tracingHeaders -> { - request - .getHeaders() - .add( - tracingHeaders.getSentryTraceHeader().getName(), - tracingHeaders.getSentryTraceHeader().getValue()); + final @Nullable TracingUtils.TracingHeaders tracingHeaders = + TracingUtils.traceIfAllowed( + hub, + request.getURI().toString(), + request.getHeaders().get(BaggageHeader.BAGGAGE_HEADER), + span); - final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); - if (baggageHeader != null) { - request.getHeaders().set(baggageHeader.getName(), baggageHeader.getValue()); - } - }); + if (tracingHeaders != null) { + request + .getHeaders() + .add( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + request.getHeaders().set(baggageHeader.getName(), baggageHeader.getValue()); + } + } } private void addBreadcrumb( diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientWebRequestFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientWebRequestFilter.java index 03035fcde09..6c4d8227795 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientWebRequestFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentrySpanClientWebRequestFilter.java @@ -65,25 +65,27 @@ public SentrySpanClientWebRequestFilter(final @NotNull IHub hub) { final @NotNull ClientRequest request, final @Nullable ISpan span) { final ClientRequest.Builder requestBuilder = ClientRequest.from(request); - TracingUtils.traceIfAllowed( - hub, - request.url().toString(), - request.headers().get(BaggageHeader.BAGGAGE_HEADER), - span, - tracingHeaders -> { - requestBuilder.header( - tracingHeaders.getSentryTraceHeader().getName(), - tracingHeaders.getSentryTraceHeader().getValue()); - - final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); - if (baggageHeader != null) { - requestBuilder.headers( - httpHeaders -> { - httpHeaders.remove(BaggageHeader.BAGGAGE_HEADER); - httpHeaders.add(baggageHeader.getName(), baggageHeader.getValue()); - }); - } - }); + final @Nullable TracingUtils.TracingHeaders tracingHeaders = + TracingUtils.traceIfAllowed( + hub, + request.url().toString(), + request.headers().get(BaggageHeader.BAGGAGE_HEADER), + span); + + if (tracingHeaders != null) { + requestBuilder.header( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + requestBuilder.headers( + httpHeaders -> { + httpHeaders.remove(BaggageHeader.BAGGAGE_HEADER); + httpHeaders.add(baggageHeader.getName(), baggageHeader.getValue()); + }); + } + } return requestBuilder.build(); } diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java index d983cc7d2a5..427ac41a507 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientHttpRequestInterceptor.java @@ -75,23 +75,25 @@ public SentrySpanClientHttpRequestInterceptor(final @NotNull IHub hub) { private void maybeAddTracingHeaders( final @NotNull HttpRequest request, final @Nullable ISpan span) { - TracingUtils.traceIfAllowed( - hub, - request.getURI().toString(), - request.getHeaders().get(BaggageHeader.BAGGAGE_HEADER), - span, - tracingHeaders -> { - request - .getHeaders() - .add( - tracingHeaders.getSentryTraceHeader().getName(), - tracingHeaders.getSentryTraceHeader().getValue()); + final @Nullable TracingUtils.TracingHeaders tracingHeaders = + TracingUtils.traceIfAllowed( + hub, + request.getURI().toString(), + request.getHeaders().get(BaggageHeader.BAGGAGE_HEADER), + span); - final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); - if (baggageHeader != null) { - request.getHeaders().set(baggageHeader.getName(), baggageHeader.getValue()); - } - }); + if (tracingHeaders != null) { + request + .getHeaders() + .add( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + request.getHeaders().set(baggageHeader.getName(), baggageHeader.getValue()); + } + } } private void addBreadcrumb( diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java index 83fc8cc39c6..f76cfd21a24 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentrySpanClientWebRequestFilter.java @@ -67,25 +67,27 @@ private ClientRequest maybeAddHeaders( final @NotNull ClientRequest request, final @Nullable ISpan span) { final ClientRequest.Builder requestBuilder = ClientRequest.from(request); - TracingUtils.traceIfAllowed( - hub, - request.url().toString(), - request.headers().get(BaggageHeader.BAGGAGE_HEADER), - span, - tracingHeaders -> { - requestBuilder.header( - tracingHeaders.getSentryTraceHeader().getName(), - tracingHeaders.getSentryTraceHeader().getValue()); - - final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); - if (baggageHeader != null) { - requestBuilder.headers( - httpHeaders -> { - httpHeaders.remove(BaggageHeader.BAGGAGE_HEADER); - httpHeaders.add(baggageHeader.getName(), baggageHeader.getValue()); - }); - } - }); + final @Nullable TracingUtils.TracingHeaders tracingHeaders = + TracingUtils.traceIfAllowed( + hub, + request.url().toString(), + request.headers().get(BaggageHeader.BAGGAGE_HEADER), + span); + + if (tracingHeaders != null) { + requestBuilder.header( + tracingHeaders.getSentryTraceHeader().getName(), + tracingHeaders.getSentryTraceHeader().getValue()); + + final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); + if (baggageHeader != null) { + requestBuilder.headers( + httpHeaders -> { + httpHeaders.remove(BaggageHeader.BAGGAGE_HEADER); + httpHeaders.add(baggageHeader.getName(), baggageHeader.getValue()); + }); + } + } return requestBuilder.build(); } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index f5891409e1e..8d4b79ed13a 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -4175,8 +4175,8 @@ public final class io/sentry/util/TracingUtils { public fun ()V public static fun maybeUpdateBaggage (Lio/sentry/Scope;Lio/sentry/SentryOptions;)V public static fun startNewTrace (Lio/sentry/IHub;)V - public static fun trace (Lio/sentry/IHub;Ljava/util/List;Lio/sentry/ISpan;Lio/sentry/util/HintUtils$SentryConsumer;)V - public static fun traceIfAllowed (Lio/sentry/IHub;Ljava/lang/String;Ljava/util/List;Lio/sentry/ISpan;Lio/sentry/util/HintUtils$SentryConsumer;)V + public static fun trace (Lio/sentry/IHub;Ljava/util/List;Lio/sentry/ISpan;)Lio/sentry/util/TracingUtils$TracingHeaders; + public static fun traceIfAllowed (Lio/sentry/IHub;Ljava/lang/String;Ljava/util/List;Lio/sentry/ISpan;)Lio/sentry/util/TracingUtils$TracingHeaders; } public final class io/sentry/util/TracingUtils$TracingHeaders { diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index e78e9b847a8..27886ce9994 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -18,7 +18,6 @@ import java.util.List; import java.util.Map; import java.util.WeakHashMap; -import java.util.concurrent.atomic.AtomicReference; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -745,22 +744,20 @@ public void flush(long timeoutMillis) { @Override public @Nullable SentryTraceHeader traceHeaders() { - AtomicReference traceHeader = new AtomicReference<>(); if (!isEnabled()) { options .getLogger() .log( SentryLevel.WARNING, "Instance is disabled and this 'traceHeaders' call is a no-op."); } else { - TracingUtils.trace( - this, - null, - getSpan(), - tracingHeaders -> { - traceHeader.set(tracingHeaders.getSentryTraceHeader()); - }); + final @Nullable TracingUtils.TracingHeaders headers = + TracingUtils.trace(this, null, getSpan()); + if (headers != null) { + return headers.getSentryTraceHeader(); + } } - return traceHeader.get(); + + return null; } @Override @@ -839,7 +836,6 @@ private Scope buildLocalScope( @Override public @Nullable BaggageHeader baggageHeader(@Nullable List thirdPartyBaggageHeaders) { - final @NotNull AtomicReference baggageHeader = new AtomicReference<>(); if (!isEnabled()) { options .getLogger() @@ -847,14 +843,13 @@ private Scope buildLocalScope( SentryLevel.WARNING, "Instance is disabled and this 'baggageHeader' call is a no-op."); } else { - TracingUtils.trace( - this, - thirdPartyBaggageHeaders, - getSpan(), - tracingHeaders -> { - baggageHeader.set(tracingHeaders.getBaggageHeader()); - }); - } - return baggageHeader.get(); + final @Nullable TracingUtils.TracingHeaders headers = + TracingUtils.trace(this, thirdPartyBaggageHeaders, getSpan()); + if (headers != null) { + return headers.getBaggageHeader(); + } + } + + return null; } } diff --git a/sentry/src/main/java/io/sentry/util/TracingUtils.java b/sentry/src/main/java/io/sentry/util/TracingUtils.java index 8dad62bd903..80a1c31c2fa 100644 --- a/sentry/src/main/java/io/sentry/util/TracingUtils.java +++ b/sentry/src/main/java/io/sentry/util/TracingUtils.java @@ -9,6 +9,7 @@ import io.sentry.SentryOptions; import io.sentry.SentryTraceHeader; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -21,29 +22,30 @@ public static void startNewTrace(final @NotNull IHub hub) { }); } - public static void traceIfAllowed( + public static @Nullable TracingHeaders traceIfAllowed( final @NotNull IHub hub, final @NotNull String requestUrl, @Nullable List thirdPartyBaggageHeaders, - final @Nullable ISpan span, - final @NotNull HintUtils.SentryConsumer callback) { + final @Nullable ISpan span) { final @NotNull SentryOptions sentryOptions = hub.getOptions(); if (sentryOptions.isTraceSampling() && isAllowedToSendTo(requestUrl, sentryOptions)) { - trace(hub, thirdPartyBaggageHeaders, span, callback); + return trace(hub, thirdPartyBaggageHeaders, span); } + + return null; } - public static void trace( + public static @Nullable TracingHeaders trace( final @NotNull IHub hub, @Nullable List thirdPartyBaggageHeaders, - final @Nullable ISpan span, - final @NotNull HintUtils.SentryConsumer callback) { + final @Nullable ISpan span) { final @NotNull SentryOptions sentryOptions = hub.getOptions(); if (span != null && !span.isNoOp()) { - callback.accept( - new TracingHeaders(span.toSentryTrace(), span.toBaggageHeader(thirdPartyBaggageHeaders))); + return new TracingHeaders( + span.toSentryTrace(), span.toBaggageHeader(thirdPartyBaggageHeaders)); } else { + final @NotNull AtomicReference returnValue = new AtomicReference<>(); hub.configureScope( (scope) -> { maybeUpdateBaggage(scope, sentryOptions); @@ -56,12 +58,13 @@ public static void trace( BaggageHeader.fromBaggageAndOutgoingHeader(baggage, thirdPartyBaggageHeaders); } - callback.accept( + returnValue.set( new TracingHeaders( new SentryTraceHeader( propagationContext.getTraceId(), propagationContext.getSpanId(), null), baggageHeader)); }); + return returnValue.get(); } } From 4e850afa3b7ce3a0b493f5128f5d489a0c926ae3 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 19 Jun 2023 12:16:40 +0200 Subject: [PATCH 5/7] Start new trace in ActivityLifecycleIntegratin --- .../core/ActivityLifecycleIntegration.java | 131 +++++++++--------- .../core/ActivityLifecycleIntegrationTest.kt | 12 +- 2 files changed, 75 insertions(+), 68 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index a8e705de5c0..aa757ddc845 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -31,6 +31,7 @@ import io.sentry.protocol.MeasurementValue; import io.sentry.protocol.TransactionNameSource; import io.sentry.util.Objects; +import io.sentry.util.TracingUtils; import java.io.Closeable; import java.io.IOException; import java.lang.ref.WeakReference; @@ -122,11 +123,9 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio fullyDisplayedReporter = this.options.getFullyDisplayedReporter(); timeToFullDisplaySpanEnabled = this.options.isEnableTimeToFullDisplayTracing(); - if (this.options.isEnableActivityLifecycleBreadcrumbs() || performanceEnabled) { - application.registerActivityLifecycleCallbacks(this); - this.options.getLogger().log(SentryLevel.DEBUG, "ActivityLifecycleIntegration installed."); - addIntegrationToSdkVersion(); - } + application.registerActivityLifecycleCallbacks(this); + this.options.getLogger().log(SentryLevel.DEBUG, "ActivityLifecycleIntegration installed."); + addIntegrationToSdkVersion(); } private boolean isPerformanceEnabled(final @NotNull SentryAndroidOptions options) { @@ -174,10 +173,11 @@ private void stopPreviousTransactions() { } } - // TODO should we start a new trace here if performance is disabled? private void startTracing(final @NotNull Activity activity) { WeakReference weakActivity = new WeakReference<>(activity); - if (performanceEnabled && !isRunningTransaction(activity) && hub != null) { + if (!performanceEnabled && hub != null) { + TracingUtils.startNewTrace(hub); + } else if (performanceEnabled && !isRunningTransaction(activity) && hub != null) { // as we allow a single transaction running on the bound Scope, we finish the previous ones stopPreviousTransactions(); @@ -368,12 +368,15 @@ public synchronized void onActivityCreated( @Override public synchronized void onActivityStarted(final @NotNull Activity activity) { - // The docs on the screen rendering performance tracing - // (https://firebase.google.com/docs/perf-mon/screen-traces?platform=android#definition), - // state that the tracing starts for every Activity class when the app calls .onActivityStarted. - // Adding an Activity in onActivityCreated leads to Window.FEATURE_NO_TITLE not - // working. Moving this to onActivityStarted fixes the problem. - activityFramesTracker.addActivity(activity); + if (performanceEnabled || options.isEnableActivityLifecycleBreadcrumbs()) { + // The docs on the screen rendering performance tracing + // (https://firebase.google.com/docs/perf-mon/screen-traces?platform=android#definition), + // state that the tracing starts for every Activity class when the app calls + // .onActivityStarted. + // Adding an Activity in onActivityCreated leads to Window.FEATURE_NO_TITLE not + // working. Moving this to onActivityStarted fixes the problem. + activityFramesTracker.addActivity(activity); + } addBreadcrumb(activity, "started"); } @@ -381,31 +384,32 @@ public synchronized void onActivityStarted(final @NotNull Activity activity) { @SuppressLint("NewApi") @Override public synchronized void onActivityResumed(final @NotNull Activity activity) { - - // app start span - @Nullable final SentryDate appStartStartTime = AppStartState.getInstance().getAppStartTime(); - @Nullable final SentryDate appStartEndTime = AppStartState.getInstance().getAppStartEndTime(); - // in case the SentryPerformanceProvider is disabled it does not set the app start times, - // and we need to set the end time manually here, - // the start time gets set manually in SentryAndroid.init() - if (appStartStartTime != null && appStartEndTime == null) { - AppStartState.getInstance().setAppStartEnd(); - } - finishAppStartSpan(); - - final @Nullable ISpan ttidSpan = ttidSpanMap.get(activity); - final @Nullable ISpan ttfdSpan = ttfdSpanMap.get(activity); - final View rootView = activity.findViewById(android.R.id.content); - if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN - && rootView != null) { - FirstDrawDoneListener.registerForNextDraw( - rootView, () -> onFirstFrameDrawn(ttfdSpan, ttidSpan), buildInfoProvider); - } else { - // Posting a task to the main thread's handler will make it executed after it finished - // its current job. That is, right after the activity draws the layout. - mainHandler.post(() -> onFirstFrameDrawn(ttfdSpan, ttidSpan)); + if (performanceEnabled || options.isEnableActivityLifecycleBreadcrumbs()) { + // app start span + @Nullable final SentryDate appStartStartTime = AppStartState.getInstance().getAppStartTime(); + @Nullable final SentryDate appStartEndTime = AppStartState.getInstance().getAppStartEndTime(); + // in case the SentryPerformanceProvider is disabled it does not set the app start times, + // and we need to set the end time manually here, + // the start time gets set manually in SentryAndroid.init() + if (appStartStartTime != null && appStartEndTime == null) { + AppStartState.getInstance().setAppStartEnd(); + } + finishAppStartSpan(); + + final @Nullable ISpan ttidSpan = ttidSpanMap.get(activity); + final @Nullable ISpan ttfdSpan = ttfdSpanMap.get(activity); + final View rootView = activity.findViewById(android.R.id.content); + if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN + && rootView != null) { + FirstDrawDoneListener.registerForNextDraw( + rootView, () -> onFirstFrameDrawn(ttfdSpan, ttidSpan), buildInfoProvider); + } else { + // Posting a task to the main thread's handler will make it executed after it finished + // its current job. That is, right after the activity draws the layout. + mainHandler.post(() -> onFirstFrameDrawn(ttfdSpan, ttidSpan)); + } + addBreadcrumb(activity, "resumed"); } - addBreadcrumb(activity, "resumed"); } @Override @@ -451,35 +455,38 @@ public synchronized void onActivitySaveInstanceState( @Override public synchronized void onActivityDestroyed(final @NotNull Activity activity) { - addBreadcrumb(activity, "destroyed"); - - // in case the appStartSpan isn't completed yet, we finish it as cancelled to avoid - // memory leak - finishSpan(appStartSpan, SpanStatus.CANCELLED); + if (performanceEnabled || options.isEnableActivityLifecycleBreadcrumbs()) { + addBreadcrumb(activity, "destroyed"); - // we finish the ttidSpan as cancelled in case it isn't completed yet - final ISpan ttidSpan = ttidSpanMap.get(activity); - final ISpan ttfdSpan = ttfdSpanMap.get(activity); - finishSpan(ttidSpan, SpanStatus.DEADLINE_EXCEEDED); - - // we finish the ttfdSpan as deadline_exceeded in case it isn't completed yet - finishExceededTtfdSpan(ttfdSpan, ttidSpan); - cancelTtfdAutoClose(); + // in case the appStartSpan isn't completed yet, we finish it as cancelled to avoid + // memory leak + finishSpan(appStartSpan, SpanStatus.CANCELLED); - // in case people opt-out enableActivityLifecycleTracingAutoFinish and forgot to finish it, - // we make sure to finish it when the activity gets destroyed. - stopTracing(activity, true); + // we finish the ttidSpan as cancelled in case it isn't completed yet + final ISpan ttidSpan = ttidSpanMap.get(activity); + final ISpan ttfdSpan = ttfdSpanMap.get(activity); + finishSpan(ttidSpan, SpanStatus.DEADLINE_EXCEEDED); - // set it to null in case its been just finished as cancelled - appStartSpan = null; - ttidSpanMap.remove(activity); - ttfdSpanMap.remove(activity); + // we finish the ttfdSpan as deadline_exceeded in case it isn't completed yet + finishExceededTtfdSpan(ttfdSpan, ttidSpan); + cancelTtfdAutoClose(); - // clear it up, so we don't start again for the same activity if the activity is in the activity - // stack still. - // if the activity is opened again and not in memory, transactions will be created normally. - if (performanceEnabled) { - activitiesWithOngoingTransactions.remove(activity); + // in case people opt-out enableActivityLifecycleTracingAutoFinish and forgot to finish it, + // we make sure to finish it when the activity gets destroyed. + stopTracing(activity, true); + + // set it to null in case its been just finished as cancelled + appStartSpan = null; + ttidSpanMap.remove(activity); + ttfdSpanMap.remove(activity); + + // clear it up, so we don't start again for the same activity if the activity is in the + // activity + // stack still. + // if the activity is opened again and not in memory, transactions will be created normally. + if (performanceEnabled) { + activitiesWithOngoingTransactions.remove(activity); + } } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index ce66f1dcc7c..71fba2b57f1 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -141,13 +141,13 @@ class ActivityLifecycleIntegrationTest { } @Test - fun `When activity lifecycle breadcrumb and tracing are disabled, it doesn't register callback`() { + fun `When activity lifecycle breadcrumb and tracing are disabled, it still registers callback`() { val sut = fixture.getSut() fixture.options.isEnableActivityLifecycleBreadcrumbs = false sut.register(fixture.hub, fixture.options) - verify(fixture.application, never()).registerActivityLifecycleCallbacks(any()) + verify(fixture.application).registerActivityLifecycleCallbacks(any()) } @Test @@ -162,7 +162,7 @@ class ActivityLifecycleIntegrationTest { } @Test - fun `When activity lifecycle breadcrumb is disabled and tracesSampleRate is set but tracing is disabled, it does not register callback`() { + fun `When activity lifecycle breadcrumb is disabled and tracesSampleRate is set but tracing is disabled, it still registers callback`() { val sut = fixture.getSut() fixture.options.isEnableActivityLifecycleBreadcrumbs = false fixture.options.tracesSampleRate = 1.0 @@ -170,7 +170,7 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) - verify(fixture.application, never()).registerActivityLifecycleCallbacks(any()) + verify(fixture.application).registerActivityLifecycleCallbacks(any()) } @Test @@ -196,7 +196,7 @@ class ActivityLifecycleIntegrationTest { } @Test - fun `When activity lifecycle breadcrumb and tracing activity flag are disabled, it doesn't register callback`() { + fun `When activity lifecycle breadcrumb and tracing activity flag are disabled, its still registers callback`() { val sut = fixture.getSut() fixture.options.isEnableActivityLifecycleBreadcrumbs = false fixture.options.tracesSampleRate = 1.0 @@ -205,7 +205,7 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) - verify(fixture.application, never()).registerActivityLifecycleCallbacks(any()) + verify(fixture.application).registerActivityLifecycleCallbacks(any()) } @Test From 15ad792e209a444cf3fc0adb04d1c398680e9788 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 19 Jun 2023 14:32:49 +0200 Subject: [PATCH 6/7] Also start a new trace in SentryNavigationListener --- .../core/ActivityLifecycleIntegrationTest.kt | 23 ++++++++++++ .../navigation/SentryNavigationListener.kt | 3 +- .../SentryNavigationListenerTest.kt | 35 ++++++++++++++----- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index 71fba2b57f1..da3e315d274 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -16,6 +16,7 @@ import io.sentry.FullyDisplayedReporter import io.sentry.Hub import io.sentry.ISentryExecutorService import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.Sentry import io.sentry.SentryDate import io.sentry.SentryLevel @@ -32,6 +33,7 @@ import io.sentry.protocol.MeasurementValue import io.sentry.protocol.TransactionNameSource import io.sentry.test.getProperty import org.junit.runner.RunWith +import org.mockito.ArgumentCaptor import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argumentCaptor @@ -54,6 +56,7 @@ import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotEquals import kotlin.test.assertNotNull +import kotlin.test.assertNotSame import kotlin.test.assertNull import kotlin.test.assertSame import kotlin.test.assertTrue @@ -1384,6 +1387,26 @@ class ActivityLifecycleIntegrationTest { assertFalse(ttfdSpan2.isFinished) } + @Test + fun `starts new trace if performance is disabled`() { + val sut = fixture.getSut() + val activity = mock() + fixture.options.enableTracing = false + + val argumentCaptor: ArgumentCaptor = ArgumentCaptor.forClass(ScopeCallback::class.java) + val scope = Scope(fixture.options) + val propagationContextAtStart = scope.propagationContext + whenever(fixture.hub.configureScope(argumentCaptor.capture())).thenAnswer { + argumentCaptor.value.run(scope) + } + + sut.register(fixture.hub, fixture.options) + sut.onActivityCreated(activity, fixture.bundle) + + verify(fixture.hub).configureScope(any()) + assertNotSame(propagationContextAtStart, scope.propagationContext) + } + private fun runFirstDraw(view: View) { // Removes OnDrawListener in the next OnGlobalLayout after onDraw view.viewTreeObserver.dispatchOnDraw() diff --git a/sentry-android-navigation/src/main/java/io/sentry/android/navigation/SentryNavigationListener.kt b/sentry-android-navigation/src/main/java/io/sentry/android/navigation/SentryNavigationListener.kt index 18e21137eb6..625b612f0bf 100644 --- a/sentry-android-navigation/src/main/java/io/sentry/android/navigation/SentryNavigationListener.kt +++ b/sentry-android-navigation/src/main/java/io/sentry/android/navigation/SentryNavigationListener.kt @@ -19,6 +19,7 @@ import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.TypeCheckHint import io.sentry.protocol.TransactionNameSource +import io.sentry.util.TracingUtils import java.lang.ref.WeakReference /** @@ -90,13 +91,13 @@ class SentryNavigationListener @JvmOverloads constructor( hub.addBreadcrumb(breadcrumb, hint) } - // TODO should we start a new trace here if performance is disabled? private fun startTracing( controller: NavController, destination: NavDestination, arguments: Map ) { if (!isPerformanceEnabled) { + TracingUtils.startNewTrace(hub) return } diff --git a/sentry-android-navigation/src/test/java/io/sentry/android/navigation/SentryNavigationListenerTest.kt b/sentry-android-navigation/src/test/java/io/sentry/android/navigation/SentryNavigationListenerTest.kt index e1e2be48308..d7ddfed02f1 100644 --- a/sentry-android-navigation/src/test/java/io/sentry/android/navigation/SentryNavigationListenerTest.kt +++ b/sentry-android-navigation/src/test/java/io/sentry/android/navigation/SentryNavigationListenerTest.kt @@ -18,6 +18,7 @@ import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.protocol.TransactionNameSource import org.junit.runner.RunWith +import org.mockito.ArgumentCaptor import org.mockito.kotlin.any import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.check @@ -29,6 +30,7 @@ import org.mockito.kotlin.whenever import org.robolectric.annotation.Config import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNotSame import kotlin.test.assertNull @RunWith(AndroidJUnit4::class) @@ -43,6 +45,7 @@ class SentryNavigationListenerTest { val context = mock() val resources = mock() val scope = mock() + lateinit var options: SentryOptions lateinit var transaction: SentryTracer @@ -56,14 +59,13 @@ class SentryNavigationListenerTest { hasViewIdInRes: Boolean = true, transaction: SentryTracer? = null ): SentryNavigationListener { - whenever(hub.options).thenReturn( - SentryOptions().apply { - dsn = "http://key@localhost/proj" - setTracesSampleRate( - tracesSampleRate - ) - } - ) + options = SentryOptions().apply { + dsn = "http://key@localhost/proj" + setTracesSampleRate( + tracesSampleRate + ) + } + whenever(hub.options).thenReturn(options) this.transaction = transaction ?: SentryTracer( TransactionContext( @@ -347,4 +349,21 @@ class SentryNavigationListenerTest { captor.thirdValue.accept(fixture.transaction) verify(fixture.scope).clearTransaction() } + + @Test + fun `starts new trace if performance is disabled`() { + val sut = fixture.getSut(enableTracing = false) + + val argumentCaptor: ArgumentCaptor = ArgumentCaptor.forClass(ScopeCallback::class.java) + val scope = Scope(fixture.options) + val propagationContextAtStart = scope.propagationContext + whenever(fixture.hub.configureScope(argumentCaptor.capture())).thenAnswer { + argumentCaptor.value.run(scope) + } + + sut.onDestinationChanged(fixture.navController, fixture.destination, null) + + verify(fixture.hub).configureScope(any()) + assertNotSame(propagationContextAtStart, scope.propagationContext) + } } From 3cb799cfbc6b6d2d24204e8270a270d003db368b Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 20 Jun 2023 07:14:24 +0200 Subject: [PATCH 7/7] Add tests for continueTrace in spring filters --- .../tracing/SentryTracingFilterTest.kt | 29 +++++++- .../webflux/SentryWebFluxTracingFilterTest.kt | 32 ++++++++- .../spring/tracing/SentryTracingFilter.java | 68 +++++++++++-------- .../spring/tracing/SentryTracingFilterTest.kt | 34 +++++++++- .../webflux/SentryWebFluxTracingFilterTest.kt | 32 ++++++++- 5 files changed, 163 insertions(+), 32 deletions(-) diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt index a046e5f30b3..a62df4ee72a 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt @@ -11,6 +11,7 @@ import io.sentry.TraceContext import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.protocol.SentryId +import io.sentry.protocol.SentryTransaction import io.sentry.protocol.TransactionNameSource import jakarta.servlet.FilterChain import jakarta.servlet.http.HttpServletRequest @@ -18,6 +19,7 @@ import org.assertj.core.api.Assertions.assertThat import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.times @@ -51,7 +53,7 @@ class SentryTracingFilterTest { whenever(hub.options).thenReturn(options) } - fun getSut(isEnabled: Boolean = true, status: Int = 200, sentryTraceHeader: String? = null): SentryTracingFilter { + fun getSut(isEnabled: Boolean = true, status: Int = 200, sentryTraceHeader: String? = null, baggageHeaders: List? = null): SentryTracingFilter { request.requestURI = "/product/12" request.method = "POST" request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/product/{id}") @@ -61,6 +63,9 @@ class SentryTracingFilterTest { request.addHeader("sentry-trace", sentryTraceHeader) whenever(hub.startTransaction(any(), check { it.isBindToScope })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } } + if (baggageHeaders != null) { + request.addHeader("baggage", baggageHeaders) + } response.status = status whenever(hub.startTransaction(any(), check { assertTrue(it.isBindToScope) })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } whenever(hub.isEnabled).thenReturn(isEnabled) @@ -256,4 +261,26 @@ class SentryTracingFilterTest { anyOrNull() ) } + + @Test + fun `continues incoming trace even is performance is disabled`() { + val parentSpanId = SpanId() + val sentryTraceHeaderString = "2722d9f6ec019ade60c776169d9a8904-$parentSpanId-1" + val baggageHeaderStrings = listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET") + fixture.options.enableTracing = false + val filter = fixture.getSut(sentryTraceHeader = sentryTraceHeaderString, baggageHeaders = baggageHeaderStrings) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.chain).doFilter(fixture.request, fixture.response) + + verify(fixture.hub).continueTrace(eq(sentryTraceHeaderString), eq(baggageHeaderStrings)) + + verify(fixture.hub, never()).captureTransaction( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } } diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt index b32831e8265..e61c7e344b5 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt @@ -15,6 +15,7 @@ import io.sentry.TraceContext import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.protocol.SentryId +import io.sentry.protocol.SentryTransaction import io.sentry.protocol.TransactionNameSource import io.sentry.spring.jakarta.webflux.AbstractSentryWebFilter.SENTRY_HUB_KEY import org.assertj.core.api.Assertions.assertThat @@ -22,7 +23,9 @@ import org.mockito.Mockito import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions @@ -58,12 +61,15 @@ class SentryWebFluxTracingFilterTest { whenever(hub.options).thenReturn(options) } - fun getSut(isEnabled: Boolean = true, status: HttpStatus = HttpStatus.OK, sentryTraceHeader: String? = null, method: HttpMethod = HttpMethod.POST): SentryWebFilter { + fun getSut(isEnabled: Boolean = true, status: HttpStatus = HttpStatus.OK, sentryTraceHeader: String? = null, baggageHeaders: List? = null, method: HttpMethod = HttpMethod.POST): SentryWebFilter { var requestBuilder = MockServerHttpRequest.method(method, "/product/{id}", 12) if (sentryTraceHeader != null) { requestBuilder = requestBuilder.header("sentry-trace", sentryTraceHeader) whenever(hub.startTransaction(any(), check { it.isBindToScope })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } } + if (baggageHeaders != null) { + requestBuilder = requestBuilder.header("baggage", *baggageHeaders.toTypedArray()) + } request = requestBuilder.build() exchange = MockServerWebExchange.builder(request).build() exchange.attributes.put(SENTRY_HUB_KEY, hub) @@ -291,4 +297,28 @@ class SentryWebFluxTracingFilterTest { ) } } + + @Test + fun `continues incoming trace even is performance is disabled`() { + val parentSpanId = SpanId() + val sentryTraceHeaderString = "2722d9f6ec019ade60c776169d9a8904-$parentSpanId-1" + val baggageHeaderStrings = listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET") + fixture.options.enableTracing = false + val filter = fixture.getSut(sentryTraceHeader = sentryTraceHeaderString, baggageHeaders = baggageHeaderStrings) + + withMockHub { + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.chain).filter(fixture.exchange) + + verify(fixture.hub, never()).captureTransaction( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + + verify(fixture.hub).continueTrace(eq(sentryTraceHeaderString), eq(baggageHeaderStrings)) + } + } } diff --git a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java index 69950c1b563..4d0d796a679 100644 --- a/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java +++ b/sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java @@ -73,44 +73,58 @@ protected void doFilterInternal( final @NotNull FilterChain filterChain) throws ServletException, IOException { - if (hub.isEnabled() && shouldTraceRequest(httpRequest)) { - final String sentryTraceHeader = httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); - final List baggageHeader = + if (hub.isEnabled()) { + final @Nullable String sentryTraceHeader = + httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); + final @Nullable List baggageHeader = Collections.list(httpRequest.getHeaders(BaggageHeader.BAGGAGE_HEADER)); final @Nullable PropagationContext propagationContext = hub.continueTrace(sentryTraceHeader, baggageHeader); - // at this stage we are not able to get real transaction name - final ITransaction transaction = startTransaction(httpRequest, propagationContext); - try { + if (hub.getOptions().isTracingEnabled() && shouldTraceRequest(httpRequest)) { + doFilterWithTransaction(httpRequest, httpResponse, filterChain, propagationContext); + } else { filterChain.doFilter(httpRequest, httpResponse); - } catch (Throwable e) { - // exceptions that are not handled by Spring - transaction.setStatus(SpanStatus.INTERNAL_ERROR); - throw e; - } finally { - // after all filters run, templated path pattern is available in request attribute - final String transactionName = transactionNameProvider.provideTransactionName(httpRequest); - final TransactionNameSource transactionNameSource = - transactionNameProvider.provideTransactionSource(); - // if transaction name is not resolved, the request has not been processed by a controller - // and we should not report it to Sentry - if (transactionName != null) { - transaction.setName(transactionName, transactionNameSource); - transaction.setOperation(TRANSACTION_OP); - // if exception has been thrown, transaction status is already set to INTERNAL_ERROR, and - // httpResponse.getStatus() returns 200. - if (transaction.getStatus() == null) { - transaction.setStatus(SpanStatus.fromHttpStatusCode(httpResponse.getStatus())); - } - transaction.finish(); - } } } else { filterChain.doFilter(httpRequest, httpResponse); } } + private void doFilterWithTransaction( + HttpServletRequest httpRequest, + HttpServletResponse httpResponse, + FilterChain filterChain, + final @Nullable PropagationContext propagationContext) + throws IOException, ServletException { + // at this stage we are not able to get real transaction name + final ITransaction transaction = startTransaction(httpRequest, propagationContext); + try { + filterChain.doFilter(httpRequest, httpResponse); + } catch (Throwable e) { + // exceptions that are not handled by Spring + transaction.setStatus(SpanStatus.INTERNAL_ERROR); + throw e; + } finally { + // after all filters run, templated path pattern is available in request attribute + final String transactionName = transactionNameProvider.provideTransactionName(httpRequest); + final TransactionNameSource transactionNameSource = + transactionNameProvider.provideTransactionSource(); + // if transaction name is not resolved, the request has not been processed by a controller + // and we should not report it to Sentry + if (transactionName != null) { + transaction.setName(transactionName, transactionNameSource); + transaction.setOperation(TRANSACTION_OP); + // if exception has been thrown, transaction status is already set to INTERNAL_ERROR, and + // httpResponse.getStatus() returns 200. + if (transaction.getStatus() == null) { + transaction.setStatus(SpanStatus.fromHttpStatusCode(httpResponse.getStatus())); + } + transaction.finish(); + } + } + } + private boolean shouldTraceRequest(final @NotNull HttpServletRequest request) { return hub.getOptions().isTraceOptionsRequests() || !HttpMethod.OPTIONS.name().equals(request.getMethod()); diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt index 19b27bd7d25..784d7306908 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt @@ -11,13 +11,16 @@ import io.sentry.TraceContext import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.protocol.SentryId +import io.sentry.protocol.SentryTransaction import io.sentry.protocol.TransactionNameSource import org.assertj.core.api.Assertions.assertThat import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever @@ -42,6 +45,7 @@ class SentryTracingFilterTest { val transactionNameProvider = mock() val options = SentryOptions().apply { dsn = "https://key@sentry.io/proj" + enableTracing = true } val logger = mock() @@ -49,7 +53,7 @@ class SentryTracingFilterTest { whenever(hub.options).thenReturn(options) } - fun getSut(isEnabled: Boolean = true, status: Int = 200, sentryTraceHeader: String? = null): SentryTracingFilter { + fun getSut(isEnabled: Boolean = true, status: Int = 200, sentryTraceHeader: String? = null, baggageHeaders: List? = null): SentryTracingFilter { request.requestURI = "/product/12" request.method = "POST" request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/product/{id}") @@ -59,6 +63,9 @@ class SentryTracingFilterTest { request.addHeader("sentry-trace", sentryTraceHeader) whenever(hub.startTransaction(any(), check { it.isBindToScope })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } } + if (baggageHeaders != null) { + request.addHeader("baggage", baggageHeaders) + } response.status = status whenever(hub.startTransaction(any(), check { assertTrue(it.isBindToScope) })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } whenever(hub.isEnabled).thenReturn(isEnabled) @@ -209,7 +216,8 @@ class SentryTracingFilterTest { verify(fixture.chain).doFilter(fixture.request, fixture.response) verify(fixture.hub).isEnabled - verify(fixture.hub).options + verify(fixture.hub, times(2)).options + verify(fixture.hub).continueTrace(anyOrNull(), anyOrNull()) verifyNoMoreInteractions(fixture.hub) verify(fixture.transactionNameProvider, never()).provideTransactionName(any()) } @@ -253,4 +261,26 @@ class SentryTracingFilterTest { anyOrNull() ) } + + @Test + fun `continues incoming trace even is performance is disabled`() { + val parentSpanId = SpanId() + val sentryTraceHeaderString = "2722d9f6ec019ade60c776169d9a8904-$parentSpanId-1" + val baggageHeaderStrings = listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET") + fixture.options.enableTracing = false + val filter = fixture.getSut(sentryTraceHeader = sentryTraceHeaderString, baggageHeaders = baggageHeaderStrings) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.chain).doFilter(fixture.request, fixture.response) + + verify(fixture.hub).continueTrace(eq(sentryTraceHeaderString), eq(baggageHeaderStrings)) + + verify(fixture.hub, never()).captureTransaction( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } } diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt index c8b1af22849..2132b611c94 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt @@ -15,6 +15,7 @@ import io.sentry.TraceContext import io.sentry.TransactionContext import io.sentry.TransactionOptions import io.sentry.protocol.SentryId +import io.sentry.protocol.SentryTransaction import io.sentry.protocol.TransactionNameSource import io.sentry.spring.webflux.SentryWebFilter.SENTRY_HUB_KEY import org.assertj.core.api.Assertions.assertThat @@ -22,7 +23,9 @@ import org.mockito.Mockito import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.eq import org.mockito.kotlin.mock +import org.mockito.kotlin.never import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions @@ -58,12 +61,15 @@ class SentryWebFluxTracingFilterTest { whenever(hub.options).thenReturn(options) } - fun getSut(isEnabled: Boolean = true, status: HttpStatus = HttpStatus.OK, sentryTraceHeader: String? = null, method: HttpMethod = HttpMethod.POST): SentryWebFilter { + fun getSut(isEnabled: Boolean = true, status: HttpStatus = HttpStatus.OK, sentryTraceHeader: String? = null, baggageHeaders: List? = null, method: HttpMethod = HttpMethod.POST): SentryWebFilter { var requestBuilder = MockServerHttpRequest.method(method, "/product/{id}", 12) if (sentryTraceHeader != null) { requestBuilder = requestBuilder.header("sentry-trace", sentryTraceHeader) whenever(hub.startTransaction(any(), check { it.isBindToScope })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, hub) } } + if (baggageHeaders != null) { + requestBuilder = requestBuilder.header("baggage", *baggageHeaders.toTypedArray()) + } request = requestBuilder.build() exchange = MockServerWebExchange.builder(request).build() exchange.attributes.put(SENTRY_HUB_KEY, hub) @@ -292,4 +298,28 @@ class SentryWebFluxTracingFilterTest { ) } } + + @Test + fun `continues incoming trace even is performance is disabled`() { + val parentSpanId = SpanId() + val sentryTraceHeaderString = "2722d9f6ec019ade60c776169d9a8904-$parentSpanId-1" + val baggageHeaderStrings = listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET") + fixture.options.enableTracing = false + val filter = fixture.getSut(sentryTraceHeader = sentryTraceHeaderString, baggageHeaders = baggageHeaderStrings) + + withMockHub { + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.chain).filter(fixture.exchange) + + verify(fixture.hub, never()).captureTransaction( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + + verify(fixture.hub).continueTrace(eq(sentryTraceHeaderString), eq(baggageHeaderStrings)) + } + } }