From fdf5953d79d0732b8346b7331f7aeafe103da28e Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 14 Jun 2023 22:12:58 +0200 Subject: [PATCH 01/22] 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 f2717b4324..a8e705de5c 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 2bc8a52694..88539454a0 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 9661ee1925..18e21137eb 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 f434764c69..5120e2e2eb 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 736fd84256..df1a9f9d92 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 a52871b8fd..297c0222f4 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 ead779c3e8..8beb6d3c55 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 16f8e99976..07365170b4 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 2a0def3722..5c8c96be3b 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 0d7e363732..b686681b59 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 c4183c002a..f7bc510875 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 b9e858b8e2..c87bcf674f 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 f4387ce93a..9550186a69 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 e53560b101..086cff1600 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 0633eef58c..03035fcde0 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 8104a5383b..925a09672a 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 cebfb74f96..80a1b69ee6 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 849cbbb12c..a046e5f30b 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 09a4bcc8a0..d983cc7d2a 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 1c82865ed7..83fc8cc39c 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 1086950b94..69950c1b56 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 6e29739282..8c554e5dcb 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 cd72743d91..19b27bd7d2 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 72fa593770..f5891409e1 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 1500d1c36f..6c5ad41c6f 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 38a2250b9d..e78e9b847a 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 b777076d0f..7b9942210f 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 b99e99ab82..fcc12f9f38 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 6735fe832f..cd485ef630 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 0000000000..e1b8ddf051 --- /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 997f93c05d..d73ba91bb2 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 0cd640c8c4..99ddd7baed 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 7570611b58..89629a7bb5 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 4463f9ae4f..3f24cfd480 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 0000000000..8dad62bd90 --- /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 4c40a65272..dc55c6d501 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 02/22] 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 8beb6d3c55..1802d5d3d6 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 8cb30dcb50..b32831e826 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 388dbd9446..c8b1af2284 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 e1b8ddf051..cc538194e5 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 baab6e4e57..fcb0132d0d 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 19dff0a504..441d206a6d 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 03/22] 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 ba94ca5a03..d14fa0e3f8 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 df1a9f9d92..8871d3ac5f 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 a45f58d714..973adad123 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 97bb307b34..0412f67875 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 d6068bc237..e54e062765 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 4afaa9983b..2c69175fa5 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 2f298992d0..2aaea06efc 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 cf60680dee..21c989fb7d 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 e664d69a8a..ba26fef3e9 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 04/22] 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 5120e2e2eb..10625b32cd 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 8871d3ac5f..cd34092ca4 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 297c0222f4..45abbd5862 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 1802d5d3d6..51cc41a921 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 086cff1600..2817a55cd4 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 03035fcde0..6c4d822779 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 d983cc7d2a..427ac41a50 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 83fc8cc39c..f76cfd21a2 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 f5891409e1..8d4b79ed13 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 e78e9b847a..27886ce999 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 8dad62bd90..80a1c31c2f 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 05/22] 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 a8e705de5c..aa757ddc84 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 ce66f1dcc7..71fba2b57f 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 06/22] 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 71fba2b57f..da3e315d27 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 18e21137eb..625b612f0b 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 e1e2be4830..d7ddfed02f 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 07/22] 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 a046e5f30b..a62df4ee72 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 b32831e826..e61c7e344b 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 69950c1b56..4d0d796a67 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 19b27bd7d2..784d730690 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 c8b1af2284..2132b611c9 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)) + } + } } From 5feaac92ee65dc50171d3ec279cb35e2622f927d Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 20 Jun 2023 08:15:03 +0200 Subject: [PATCH 08/22] Add changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1bfca335cf..8574128a15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Features + +- Tracing headers (`sentry-trace` and `baggage`) are now attached and passed through even if performance is disabled ([#2788](https://github.com/getsentry/sentry-java/pull/2788)) + ## 6.23.0 ### Features From 1b1f062395a8cf058a1b1f06d10666193af087e4 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 20 Jun 2023 10:06:10 +0200 Subject: [PATCH 09/22] Add test for TracingUtils --- .../java/io/sentry/util/TracingUtilsTest.kt | 161 ++++++++++++++++++ 1 file changed, 161 insertions(+) create mode 100644 sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt diff --git a/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt b/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt new file mode 100644 index 0000000000..b353f2a838 --- /dev/null +++ b/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt @@ -0,0 +1,161 @@ +package io.sentry.util + +import io.sentry.Baggage +import io.sentry.IHub +import io.sentry.NoOpSpan +import io.sentry.Scope +import io.sentry.ScopeCallback +import io.sentry.SentryOptions +import io.sentry.SentryTracer +import io.sentry.Span +import io.sentry.SpanOptions +import io.sentry.TracesSamplingDecision +import io.sentry.TransactionContext +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.doAnswer +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class TracingUtilsTest { + + class Fixture { + val options = SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + } + val hub = mock() + val scope = Scope(options) + lateinit var span: Span + val preExistingBaggage = listOf("some-baggage-key=some-baggage-value") + + fun setup() { + whenever(hub.options).thenReturn(options) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) + span = Span( + TransactionContext("name", "op", TracesSamplingDecision(true)), + SentryTracer(TransactionContext("name", "op", TracesSamplingDecision(true)), hub), + hub, + null, + SpanOptions() + ) + } + } + + val fixture = Fixture() + + @Test + fun `returns headers if allowed from scope without span`() { + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, null) + + assertNotNull(headers) + assertNotNull(headers.baggageHeader) + assertEquals(fixture.scope.propagationContext.spanId, headers.sentryTraceHeader.spanId) + assertEquals(fixture.scope.propagationContext.traceId, headers.sentryTraceHeader.traceId) + assertTrue(headers.baggageHeader!!.value.contains("some-baggage-key=some-baggage-value")) + assertTrue(headers.baggageHeader!!.value.contains("sentry-trace_id=${fixture.scope.propagationContext.traceId}")) + assertFalse(fixture.scope.propagationContext.baggage!!.isMutable) + } + + @Test + fun `returns headers if allowed from scope if span is noop`() { + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, NoOpSpan.getInstance()) + + assertNotNull(headers) + assertNotNull(headers.baggageHeader) + assertEquals(fixture.scope.propagationContext.spanId, headers.sentryTraceHeader.spanId) + assertEquals(fixture.scope.propagationContext.traceId, headers.sentryTraceHeader.traceId) + assertTrue(headers.baggageHeader!!.value.contains("some-baggage-key=some-baggage-value")) + assertFalse(fixture.scope.propagationContext.baggage!!.isMutable) + } + + @Test + fun `returns headers if allowed from scope without span leaving frozen baggage alone`() { + fixture.scope.propagationContext.baggage = Baggage.fromHeader("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET").also { it.freeze() } + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, null) + + assertNotNull(headers) + assertNotNull(headers.baggageHeader) + assertEquals(fixture.scope.propagationContext.spanId, headers.sentryTraceHeader.spanId) + assertEquals(fixture.scope.propagationContext.traceId, headers.sentryTraceHeader.traceId) + assertTrue(headers.baggageHeader!!.value.contains("some-baggage-key=some-baggage-value")) + assertTrue(headers.baggageHeader!!.value.contains("sentry-trace_id=2722d9f6ec019ade60c776169d9a8904")) + assertFalse(headers.baggageHeader!!.value.contains("sentry-trace_id=${fixture.scope.propagationContext.traceId}")) + } + + @Test + fun `returns headers if allowed from span`() { + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, fixture.span) + + assertNotNull(headers) + assertNotNull(headers.baggageHeader) + assertEquals(fixture.span.spanId, headers.sentryTraceHeader.spanId) + assertEquals(fixture.span.traceId, headers.sentryTraceHeader.traceId) + assertTrue(headers.baggageHeader!!.value.contains("some-baggage-key=some-baggage-value")) + } + + @Test + fun `does not return headers if not trace sampling without span`() { + fixture.options.isTraceSampling = false + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, null) + + assertNull(headers) + } + + @Test + fun `does not return headers if not trace sampling from span`() { + fixture.options.isTraceSampling = false + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, fixture.span) + + assertNull(headers) + } + + @Test + fun `does not return headers if host is disallowed without span`() { + fixture.options.setTracePropagationTargets(listOf("some-host-that-does-not-exist")) + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, null) + + assertNull(headers) + } + + @Test + fun `does not return headers if host is disallowed from span`() { + fixture.options.setTracePropagationTargets(listOf("some-host-that-does-not-exist")) + fixture.setup() + + val headers = TracingUtils.traceIfAllowed(fixture.hub, "https://sentry.io/hello", fixture.preExistingBaggage, fixture.span) + + assertNull(headers) + } + + @Test + fun `start new trace sets propagation context on scope`() { + fixture.setup() + + val propagationContextBefore = fixture.scope.propagationContext + + TracingUtils.startNewTrace(fixture.hub) + + assertNotEquals(propagationContextBefore.traceId, fixture.scope.propagationContext.traceId) + assertNotEquals(propagationContextBefore.spanId, fixture.scope.propagationContext.spanId) + } +} From 66e4bdbcf7c58eb77ec6ccec5fcca7705145a06b Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 20 Jun 2023 10:19:13 +0200 Subject: [PATCH 10/22] More tests --- .../java/io/sentry/util/TracingUtilsTest.kt | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt b/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt index b353f2a838..e38410641e 100644 --- a/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/TracingUtilsTest.kt @@ -158,4 +158,40 @@ class TracingUtilsTest { assertNotEquals(propagationContextBefore.traceId, fixture.scope.propagationContext.traceId) assertNotEquals(propagationContextBefore.spanId, fixture.scope.propagationContext.spanId) } + + @Test + fun `creates new baggage if none present`() { + fixture.setup() + assertNull(fixture.scope.propagationContext.baggage) + + TracingUtils.maybeUpdateBaggage(fixture.scope, fixture.options) + + assertNotNull(fixture.scope.propagationContext.baggage) + assertEquals(fixture.scope.propagationContext.traceId.toString(), fixture.scope.propagationContext.baggage!!.traceId) + assertFalse(fixture.scope.propagationContext.baggage!!.isMutable) + } + + @Test + fun `updates mutable baggage`() { + fixture.setup() + // not frozen because it doesn't contain sentry-* keys + fixture.scope.propagationContext.baggage = Baggage.fromHeader(fixture.preExistingBaggage) + + TracingUtils.maybeUpdateBaggage(fixture.scope, fixture.options) + + assertEquals(fixture.scope.propagationContext.traceId.toString(), fixture.scope.propagationContext.baggage!!.traceId) + assertFalse(fixture.scope.propagationContext.baggage!!.isMutable) + } + + @Test + fun `does not change frozen baggage`() { + fixture.setup() + // frozen automatically because it contains sentry-* keys + fixture.scope.propagationContext.baggage = Baggage.fromHeader("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=2722d9f6ec019ade60c776169d9a8904,sentry-transaction=HTTP%20GET") + + TracingUtils.maybeUpdateBaggage(fixture.scope, fixture.options) + + assertEquals("2722d9f6ec019ade60c776169d9a8904", fixture.scope.propagationContext.baggage!!.traceId) + assertFalse(fixture.scope.propagationContext.baggage!!.isMutable) + } } From b6bfe7e447c6e29adec7cf75d3c9eb4e85d013c1 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 22 Jun 2023 09:53:44 +0200 Subject: [PATCH 11/22] Switch from AtomicReference to holder class --- .../src/main/java/io/sentry/util/TracingUtils.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/sentry/src/main/java/io/sentry/util/TracingUtils.java b/sentry/src/main/java/io/sentry/util/TracingUtils.java index 80a1c31c2f..7a91b0cf46 100644 --- a/sentry/src/main/java/io/sentry/util/TracingUtils.java +++ b/sentry/src/main/java/io/sentry/util/TracingUtils.java @@ -45,7 +45,7 @@ public static void startNewTrace(final @NotNull IHub hub) { return new TracingHeaders( span.toSentryTrace(), span.toBaggageHeader(thirdPartyBaggageHeaders)); } else { - final @NotNull AtomicReference returnValue = new AtomicReference<>(); + final @NotNull TracingHeadersHolder returnValue = new TracingHeadersHolder(); hub.configureScope( (scope) -> { maybeUpdateBaggage(scope, sentryOptions); @@ -58,13 +58,13 @@ public static void startNewTrace(final @NotNull IHub hub) { BaggageHeader.fromBaggageAndOutgoingHeader(baggage, thirdPartyBaggageHeaders); } - returnValue.set( + returnValue.headers = new TracingHeaders( new SentryTraceHeader( propagationContext.getTraceId(), propagationContext.getSpanId(), null), - baggageHeader)); + baggageHeader); }); - return returnValue.get(); + return returnValue.headers; } } @@ -82,11 +82,15 @@ public static void maybeUpdateBaggage( } } - private static boolean isAllowedToSendTo( + private static boolean shouldAttachTracingHeaders( final @NotNull String requestUrl, final @NotNull SentryOptions sentryOptions) { return PropagationTargetsUtils.contain(sentryOptions.getTracePropagationTargets(), requestUrl); } + private static final class TracingHeadersHolder { + private @Nullable TracingHeaders headers = null; + } + public static final class TracingHeaders { private final @NotNull SentryTraceHeader sentryTraceHeader; private final @Nullable BaggageHeader baggageHeader; From 104ac37b3c345dc3c0383c4365b4badae5a0e763 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Thu, 22 Jun 2023 07:56:17 +0000 Subject: [PATCH 12/22] Format code --- sentry/src/main/java/io/sentry/util/TracingUtils.java | 1 - 1 file changed, 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/util/TracingUtils.java b/sentry/src/main/java/io/sentry/util/TracingUtils.java index 7a91b0cf46..848cf04f65 100644 --- a/sentry/src/main/java/io/sentry/util/TracingUtils.java +++ b/sentry/src/main/java/io/sentry/util/TracingUtils.java @@ -9,7 +9,6 @@ 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; From 2266d58fb187bb48f4278f866be2def92a2a4134 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 22 Jun 2023 14:56:07 +0200 Subject: [PATCH 13/22] Changes according to reviews - continueTrace now returns a TransactionContext making it easier to create a transaction from incoming headers - TransactionContext now has defaults for name, source and op and setters for them - deprecate traceHeaders() and replace with getTraceparent() - rename baggageHeader() to getBaggage() - getBaggage() no longer takes pre existing headers - add trace context to envelope payload from scope if none present and no span on scope - improve JavaDoc - create new PropagationContext when cloning scope --- .../opentelemetry/SentrySpanProcessor.java | 15 +- .../api/sentry-spring-jakarta.api | 2 +- .../jakarta/tracing/SentryTracingFilter.java | 21 ++- .../webflux/AbstractSentryWebFilter.java | 17 +- .../tracing/SentryTracingFilterTest.kt | 2 +- .../webflux/SentryWebFluxTracingFilterTest.kt | 2 +- .../spring/tracing/SentryTracingFilter.java | 21 ++- .../spring/webflux/SentryWebFilter.java | 17 +- .../spring/tracing/SentryTracingFilterTest.kt | 2 +- .../webflux/SentryWebFluxTracingFilterTest.kt | 2 +- sentry/api/sentry.api | 32 ++-- sentry/src/main/java/io/sentry/Baggage.java | 2 - sentry/src/main/java/io/sentry/Hub.java | 52 +++--- .../src/main/java/io/sentry/HubAdapter.java | 17 +- sentry/src/main/java/io/sentry/IHub.java | 31 +++- sentry/src/main/java/io/sentry/NoOpHub.java | 14 +- .../java/io/sentry/PropagationContext.java | 9 + sentry/src/main/java/io/sentry/Scope.java | 2 +- sentry/src/main/java/io/sentry/Sentry.java | 39 ++++- .../src/main/java/io/sentry/SentryClient.java | 10 +- .../java/io/sentry/TransactionContext.java | 58 ++++--- .../java/io/sentry/util/TracingUtils.java | 2 +- .../test/java/io/sentry/SentryClientTest.kt | 163 ++++++++++++++++++ .../java/io/sentry/TransactionContextTest.kt | 9 +- 24 files changed, 393 insertions(+), 148 deletions(-) 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 07365170b4..2b10b5c36e 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 @@ -106,21 +106,14 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri final @NotNull TransactionContext transactionContext = traceData.getSentryTraceHeader() == null ? new TransactionContext( - transactionName, - op, - new SentryId(traceData.getTraceId()), - spanId, - transactionNameSource, - null, - null, - null) + new SentryId(traceData.getTraceId()), spanId, null, null, null) : TransactionContext.fromPropagationContext( - transactionName, - transactionNameSource, - op, PropagationContext.fromHeaders( traceData.getSentryTraceHeader(), traceData.getBaggage(), spanId)); ; + transactionContext.setName(transactionName); + transactionContext.setTransactionNameSource(transactionNameSource); + transactionContext.setOperation(op); transactionContext.setInstrumenter(Instrumenter.OTEL); TransactionOptions transactionOptions = new TransactionOptions(); diff --git a/sentry-spring-jakarta/api/sentry-spring-jakarta.api b/sentry-spring-jakarta/api/sentry-spring-jakarta.api index 9550186a69..32f76fbf7c 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/PropagationContext;)Lio/sentry/ITransaction; + protected fun startTransaction (Lio/sentry/IHub;Lorg/springframework/http/server/reactive/ServerHttpRequest;Lio/sentry/TransactionContext;)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/SentryTracingFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/tracing/SentryTracingFilter.java index 925a09672a..3bbcca3468 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 @@ -6,7 +6,6 @@ import io.sentry.HubAdapter; import io.sentry.IHub; import io.sentry.ITransaction; -import io.sentry.PropagationContext; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.TransactionContext; @@ -80,10 +79,10 @@ protected void doFilterInternal( httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); final @Nullable List baggageHeader = Collections.list(httpRequest.getHeaders(BaggageHeader.BAGGAGE_HEADER)); - final @Nullable PropagationContext propagationContext = + final @Nullable TransactionContext transactionContext = hub.continueTrace(sentryTraceHeader, baggageHeader); if (hub.getOptions().isTracingEnabled() && shouldTraceRequest(httpRequest)) { - doFilterWithTransaction(httpRequest, httpResponse, filterChain, propagationContext); + doFilterWithTransaction(httpRequest, httpResponse, filterChain, transactionContext); } else { filterChain.doFilter(httpRequest, httpResponse); } @@ -96,10 +95,10 @@ private void doFilterWithTransaction( HttpServletRequest httpRequest, HttpServletResponse httpResponse, FilterChain filterChain, - final @Nullable PropagationContext propagationContext) + final @Nullable TransactionContext transactionContext) throws IOException, ServletException { // at this stage we are not able to get real transaction name - final ITransaction transaction = startTransaction(httpRequest, propagationContext); + final ITransaction transaction = startTransaction(httpRequest, transactionContext); try { filterChain.doFilter(httpRequest, httpResponse); } catch (Throwable e) { @@ -133,23 +132,23 @@ private boolean shouldTraceRequest(final @NotNull HttpServletRequest request) { private ITransaction startTransaction( final @NotNull HttpServletRequest request, - final @Nullable PropagationContext propagationContext) { + final @Nullable TransactionContext transactionContext) { final String name = request.getMethod() + " " + request.getRequestURI(); final CustomSamplingContext customSamplingContext = new CustomSamplingContext(); customSamplingContext.set("request", request); - if (propagationContext != null) { - final TransactionContext contexts = - TransactionContext.fromPropagationContext( - name, TransactionNameSource.URL, "http.server", propagationContext); + if (transactionContext != null) { + transactionContext.setName(name); + transactionContext.setTransactionNameSource(TransactionNameSource.URL); + transactionContext.setOperation("http.server"); final TransactionOptions transactionOptions = new TransactionOptions(); transactionOptions.setCustomSamplingContext(customSamplingContext); transactionOptions.setBindToScope(true); - return hub.startTransaction(contexts, transactionOptions); + return hub.startTransaction(transactionContext, 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 80a1b69ee6..8b93ade6cf 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 @@ -10,7 +10,6 @@ import io.sentry.IHub; import io.sentry.ITransaction; import io.sentry.NoOpHub; -import io.sentry.PropagationContext; import io.sentry.Sentry; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; @@ -49,11 +48,11 @@ public AbstractSentryWebFilter(final @NotNull IHub hub) { final @Nullable String sentryTraceHeader = headers.getFirst(SentryTraceHeader.SENTRY_TRACE_HEADER); final @Nullable List baggageHeaders = headers.get(BaggageHeader.BAGGAGE_HEADER); - final @Nullable PropagationContext propagationContext = + final @Nullable TransactionContext transactionContext = requestHub.continueTrace(sentryTraceHeader, baggageHeaders); if (requestHub.getOptions().isTracingEnabled() && shouldTraceRequest(requestHub, request)) { - return startTransaction(requestHub, request, propagationContext); + return startTransaction(requestHub, request, transactionContext); } } @@ -126,7 +125,7 @@ private void finishTransaction(ServerWebExchange exchange, ITransaction transact protected @NotNull ITransaction startTransaction( final @NotNull IHub hub, final @NotNull ServerHttpRequest request, - final @Nullable PropagationContext propagationContext) { + final @Nullable TransactionContext transactionContext) { final @NotNull String name = request.getMethod() + " " + request.getURI().getPath(); final @NotNull CustomSamplingContext customSamplingContext = new CustomSamplingContext(); customSamplingContext.set("request", request); @@ -135,12 +134,12 @@ private void finishTransaction(ServerWebExchange exchange, ITransaction transact transactionOptions.setCustomSamplingContext(customSamplingContext); transactionOptions.setBindToScope(true); - if (propagationContext != null) { - final @NotNull TransactionContext contexts = - TransactionContext.fromPropagationContext( - name, TransactionNameSource.URL, TRANSACTION_OP, propagationContext); + if (transactionContext != null) { + transactionContext.setName(name); + transactionContext.setTransactionNameSource(TransactionNameSource.URL); + transactionContext.setOperation(TRANSACTION_OP); - return hub.startTransaction(contexts, transactionOptions); + return hub.startTransaction(transactionContext, 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 a62df4ee72..bdd93bb60e 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 @@ -69,7 +69,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?) } + whenever(hub.continueTrace(any(), any())).thenAnswer { TransactionContext.fromPropagationContext(PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?)) } return SentryTracingFilter(hub, transactionNameProvider) } } 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 e61c7e344b..87f9130c5d 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 @@ -78,7 +78,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?) } + whenever(hub.continueTrace(anyOrNull(), anyOrNull())).thenAnswer { TransactionContext.fromPropagationContext(PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?)) } return SentryWebFilter(hub) } } 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 4d0d796a67..a2afbf3578 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 @@ -6,7 +6,6 @@ import io.sentry.HubAdapter; import io.sentry.IHub; import io.sentry.ITransaction; -import io.sentry.PropagationContext; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; import io.sentry.TransactionContext; @@ -78,11 +77,11 @@ protected void doFilterInternal( httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); final @Nullable List baggageHeader = Collections.list(httpRequest.getHeaders(BaggageHeader.BAGGAGE_HEADER)); - final @Nullable PropagationContext propagationContext = + final @Nullable TransactionContext transactionContext = hub.continueTrace(sentryTraceHeader, baggageHeader); if (hub.getOptions().isTracingEnabled() && shouldTraceRequest(httpRequest)) { - doFilterWithTransaction(httpRequest, httpResponse, filterChain, propagationContext); + doFilterWithTransaction(httpRequest, httpResponse, filterChain, transactionContext); } else { filterChain.doFilter(httpRequest, httpResponse); } @@ -95,10 +94,10 @@ private void doFilterWithTransaction( HttpServletRequest httpRequest, HttpServletResponse httpResponse, FilterChain filterChain, - final @Nullable PropagationContext propagationContext) + final @Nullable TransactionContext transactionContext) throws IOException, ServletException { // at this stage we are not able to get real transaction name - final ITransaction transaction = startTransaction(httpRequest, propagationContext); + final ITransaction transaction = startTransaction(httpRequest, transactionContext); try { filterChain.doFilter(httpRequest, httpResponse); } catch (Throwable e) { @@ -132,23 +131,23 @@ private boolean shouldTraceRequest(final @NotNull HttpServletRequest request) { private ITransaction startTransaction( final @NotNull HttpServletRequest request, - final @Nullable PropagationContext propagationContext) { + final @Nullable TransactionContext transactionContext) { final String name = request.getMethod() + " " + request.getRequestURI(); final CustomSamplingContext customSamplingContext = new CustomSamplingContext(); customSamplingContext.set("request", request); - if (propagationContext != null) { - final TransactionContext contexts = - TransactionContext.fromPropagationContext( - name, TransactionNameSource.URL, "http.server", propagationContext); + if (transactionContext != null) { + transactionContext.setName(name); + transactionContext.setTransactionNameSource(TransactionNameSource.URL); + transactionContext.setOperation("http.server"); final TransactionOptions transactionOptions = new TransactionOptions(); transactionOptions.setCustomSamplingContext(customSamplingContext); transactionOptions.setBindToScope(true); - return hub.startTransaction(contexts, transactionOptions); + return hub.startTransaction(transactionContext, 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 8c554e5dcb..f5aa5482b6 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 @@ -10,7 +10,6 @@ import io.sentry.IHub; import io.sentry.ITransaction; import io.sentry.NoOpHub; -import io.sentry.PropagationContext; import io.sentry.Sentry; import io.sentry.SentryTraceHeader; import io.sentry.SpanStatus; @@ -59,12 +58,12 @@ public Mono filter( final @Nullable String sentryTraceHeader = headers.getFirst(SentryTraceHeader.SENTRY_TRACE_HEADER); final @Nullable List baggageHeaders = headers.get(BaggageHeader.BAGGAGE_HEADER); - final @Nullable PropagationContext propagationContext = + final @Nullable TransactionContext transactionContext = requestHub.continueTrace(sentryTraceHeader, baggageHeaders); final @Nullable ITransaction transaction = isTracingEnabled && shouldTraceRequest(requestHub, request) - ? startTransaction(requestHub, request, propagationContext) + ? startTransaction(requestHub, request, transactionContext) : null; return webFilterChain @@ -112,7 +111,7 @@ private boolean shouldTraceRequest( private @NotNull ITransaction startTransaction( final @NotNull IHub hub, final @NotNull ServerHttpRequest request, - final @Nullable PropagationContext propagationContext) { + final @Nullable TransactionContext transactionContext) { final @NotNull String name = request.getMethod() + " " + request.getURI().getPath(); final @NotNull CustomSamplingContext customSamplingContext = new CustomSamplingContext(); customSamplingContext.set("request", request); @@ -121,12 +120,12 @@ private boolean shouldTraceRequest( transactionOptions.setCustomSamplingContext(customSamplingContext); transactionOptions.setBindToScope(true); - if (propagationContext != null) { - final @NotNull TransactionContext contexts = - TransactionContext.fromPropagationContext( - name, TransactionNameSource.URL, TRANSACTION_OP, propagationContext); + if (transactionContext != null) { + transactionContext.setName(name); + transactionContext.setTransactionNameSource(TransactionNameSource.URL); + transactionContext.setOperation(TRANSACTION_OP); - return hub.startTransaction(contexts, transactionOptions); + return hub.startTransaction(transactionContext, 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 784d730690..01ebfc53bd 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 @@ -69,7 +69,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?) } + whenever(hub.continueTrace(any(), any())).thenAnswer { TransactionContext.fromPropagationContext(PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?)) } return SentryTracingFilter(hub, transactionNameProvider) } } 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 2132b611c9..8bedbea1f4 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 @@ -78,7 +78,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?) } + whenever(hub.continueTrace(anyOrNull(), anyOrNull())).thenAnswer { TransactionContext.fromPropagationContext(PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?)) } return SentryWebFilter(hub) } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 7b572fe9e7..5cd5574e8e 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -331,7 +331,6 @@ 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; @@ -347,12 +346,14 @@ 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 continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/TransactionContext; public fun endSession ()V public fun flush (J)V + public fun getBaggage ()Lio/sentry/BaggageHeader; public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; public fun getSpan ()Lio/sentry/ISpan; + public fun getTraceparent ()Lio/sentry/SentryTraceHeader; public fun isCrashedLastRun ()Ljava/lang/Boolean; public fun isEnabled ()Z public fun popScope ()V @@ -376,7 +377,6 @@ 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; @@ -392,13 +392,15 @@ 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 continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/TransactionContext; public fun endSession ()V public fun flush (J)V + public fun getBaggage ()Lio/sentry/BaggageHeader; public static fun getInstance ()Lio/sentry/HubAdapter; public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; public fun getSpan ()Lio/sentry/ISpan; + public fun getTraceparent ()Lio/sentry/SentryTraceHeader; public fun isCrashedLastRun ()Ljava/lang/Boolean; public fun isEnabled ()Z public fun popScope ()V @@ -439,7 +441,6 @@ 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; @@ -464,12 +465,14 @@ 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 continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/TransactionContext; public abstract fun endSession ()V public abstract fun flush (J)V + public abstract fun getBaggage ()Lio/sentry/BaggageHeader; public abstract fun getLastEventId ()Lio/sentry/protocol/SentryId; public abstract fun getOptions ()Lio/sentry/SentryOptions; public abstract fun getSpan ()Lio/sentry/ISpan; + public abstract fun getTraceparent ()Lio/sentry/SentryTraceHeader; public abstract fun isCrashedLastRun ()Ljava/lang/Boolean; public abstract fun isEnabled ()Z public abstract fun popScope ()V @@ -808,7 +811,6 @@ 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; @@ -824,13 +826,15 @@ 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 continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/TransactionContext; public fun endSession ()V public fun flush (J)V + public fun getBaggage ()Lio/sentry/BaggageHeader; public static fun getInstance ()Lio/sentry/NoOpHub; public fun getLastEventId ()Lio/sentry/protocol/SentryId; public fun getOptions ()Lio/sentry/SentryOptions; public fun getSpan ()Lio/sentry/ISpan; + public fun getTraceparent ()Lio/sentry/SentryTraceHeader; public fun isCrashedLastRun ()Ljava/lang/Boolean; public fun isEnabled ()Z public fun popScope ()V @@ -1120,6 +1124,7 @@ public final class io/sentry/ProfilingTransactionData$JsonKeys { public final class io/sentry/PropagationContext { public fun ()V + public fun (Lio/sentry/PropagationContext;)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; @@ -1233,7 +1238,6 @@ 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; @@ -1252,12 +1256,14 @@ 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 continueTrace (Ljava/lang/String;Ljava/util/List;)Lio/sentry/TransactionContext; public static fun endSession ()V public static fun flush (J)V + public static fun getBaggage ()Lio/sentry/BaggageHeader; public static fun getCurrentHub ()Lio/sentry/IHub; public static fun getLastEventId ()Lio/sentry/protocol/SentryId; public static fun getSpan ()Lio/sentry/ISpan; + public static fun getTraceparent ()Lio/sentry/SentryTraceHeader; public static fun init ()V public static fun init (Lio/sentry/OptionsContainer;Lio/sentry/Sentry$OptionsConfiguration;)V public static fun init (Lio/sentry/OptionsContainer;Lio/sentry/Sentry$OptionsConfiguration;Z)V @@ -2261,12 +2267,12 @@ public final class io/sentry/TracesSamplingDecision { } public final class io/sentry/TransactionContext : io/sentry/SpanContext { + public fun (Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Lio/sentry/SpanId;Lio/sentry/TracesSamplingDecision;Lio/sentry/Baggage;)V public fun (Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;Ljava/lang/String;)V public fun (Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;Ljava/lang/String;Lio/sentry/TracesSamplingDecision;)V 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 fromPropagationContext (Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;Ljava/lang/String;Lio/sentry/PropagationContext;)Lio/sentry/TransactionContext; + public static fun fromPropagationContext (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; @@ -2275,8 +2281,10 @@ public final class io/sentry/TransactionContext : io/sentry/SpanContext { public fun getParentSamplingDecision ()Lio/sentry/TracesSamplingDecision; public fun getTransactionNameSource ()Lio/sentry/protocol/TransactionNameSource; public fun setInstrumenter (Lio/sentry/Instrumenter;)V + public fun setName (Ljava/lang/String;)V public fun setParentSampled (Ljava/lang/Boolean;)V public fun setParentSampled (Ljava/lang/Boolean;Ljava/lang/Boolean;)V + public fun setTransactionNameSource (Lio/sentry/protocol/TransactionNameSource;)V } public abstract interface class io/sentry/TransactionFinishedCallback { diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index 6c5ad41c6f..e2ef0dc5fd 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -361,8 +361,6 @@ public void setValuesFromScope(final @NotNull Scope scope, final @NotNull Sentry setRelease(options.getRelease()); setEnvironment(options.getEnvironment()); setUserSegment(user != null ? getSegment(user) : null); - - // TODO vvv should we set null explicitly? setTransaction(null); setSampleRate(null); } diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 27886ce999..6b22488a02 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -742,22 +742,11 @@ public void flush(long timeoutMillis) { return transaction; } + @Deprecated + @SuppressWarnings("InlineMeSuggester") @Override public @Nullable SentryTraceHeader traceHeaders() { - if (!isEnabled()) { - options - .getLogger() - .log( - SentryLevel.WARNING, "Instance is disabled and this 'traceHeaders' call is a no-op."); - } else { - final @Nullable TracingUtils.TracingHeaders headers = - TracingUtils.trace(this, null, getSpan()); - if (headers != null) { - return headers.getSentryTraceHeader(); - } - } - - return null; + return getTraceparent(); } @Override @@ -822,29 +811,50 @@ private Scope buildLocalScope( } @Override - public @Nullable PropagationContext continueTrace( - final @Nullable String sentryTraceHeader, final @Nullable List baggageHeaders) { + public @Nullable TransactionContext continueTrace( + final @Nullable String sentryTrace, final @Nullable List baggageHeaders) { @NotNull PropagationContext propagationContext = - PropagationContext.fromHeaders(getOptions().getLogger(), sentryTraceHeader, baggageHeaders); + PropagationContext.fromHeaders(getOptions().getLogger(), sentryTrace, baggageHeaders); configureScope( (scope) -> { scope.setPropagationContext(propagationContext); }); - return propagationContext; + if (options.isTracingEnabled()) { + return TransactionContext.fromPropagationContext(propagationContext); + } else { + return null; + } } @Override - public @Nullable BaggageHeader baggageHeader(@Nullable List thirdPartyBaggageHeaders) { + public @Nullable SentryTraceHeader getTraceparent() { if (!isEnabled()) { options .getLogger() .log( SentryLevel.WARNING, - "Instance is disabled and this 'baggageHeader' call is a no-op."); + "Instance is disabled and this 'getTraceparent' call is a no-op."); } else { final @Nullable TracingUtils.TracingHeaders headers = - TracingUtils.trace(this, thirdPartyBaggageHeaders, getSpan()); + TracingUtils.trace(this, null, getSpan()); + if (headers != null) { + return headers.getSentryTraceHeader(); + } + } + + return null; + } + + @Override + public @Nullable BaggageHeader getBaggage() { + if (!isEnabled()) { + options + .getLogger() + .log(SentryLevel.WARNING, "Instance is disabled and this 'getBaggage' call is a no-op."); + } else { + final @Nullable TracingUtils.TracingHeaders headers = + TracingUtils.trace(this, null, getSpan()); if (headers != null) { return headers.getBaggageHeader(); } diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java index 7b9942210f..e6ec220874 100644 --- a/sentry/src/main/java/io/sentry/HubAdapter.java +++ b/sentry/src/main/java/io/sentry/HubAdapter.java @@ -202,6 +202,7 @@ public void flush(long timeoutMillis) { return Sentry.startTransaction(transactionContext, transactionOptions); } + @Deprecated @Override public @Nullable SentryTraceHeader traceHeaders() { return Sentry.traceHeaders(); @@ -236,14 +237,18 @@ public void reportFullyDisplayed() { } @Override - public @Nullable PropagationContext continueTrace( - final @Nullable String sentryTraceHeader, final @Nullable List baggageHeaders) { - return Sentry.continueTrace(sentryTraceHeader, baggageHeaders); + public @Nullable TransactionContext continueTrace( + final @Nullable String sentryTrace, final @Nullable List baggageHeaders) { + return Sentry.continueTrace(sentryTrace, baggageHeaders); } @Override - public @Nullable BaggageHeader baggageHeader( - final @Nullable List thirdPartyBaggageHeaders) { - return Sentry.baggageHeader(thirdPartyBaggageHeaders); + public @Nullable SentryTraceHeader getTraceparent() { + return Sentry.getTraceparent(); + } + + @Override + public @Nullable BaggageHeader getBaggage() { + return Sentry.getBaggage(); } } diff --git a/sentry/src/main/java/io/sentry/IHub.java b/sentry/src/main/java/io/sentry/IHub.java index fcc12f9f38..97583f66e8 100644 --- a/sentry/src/main/java/io/sentry/IHub.java +++ b/sentry/src/main/java/io/sentry/IHub.java @@ -524,10 +524,13 @@ ITransaction startTransaction( } /** - * Returns trace header of active transaction, or scope if no transaction is active. + * Returns the "sentry-trace" header that allows tracing across services. Can also be used in + * <meta> HTML tags. Also see {@link IHub#getBaggage()}. * - * @return trace header or null + * @deprecated please use {@link IHub#getTraceparent()} instead. + * @return sentry trace header or null */ + @Deprecated @Nullable SentryTraceHeader traceHeaders(); @@ -594,13 +597,29 @@ default void reportFullDisplayed() { * 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 sentryTrace "sentry-trace" header * @param baggageHeaders "baggage" headers + * @return a transaction context for starting a transaction or null if performance is disabled */ @Nullable - PropagationContext continueTrace( - final @Nullable String sentryTraceHeader, final @Nullable List baggageHeaders); + TransactionContext continueTrace( + final @Nullable String sentryTrace, final @Nullable List baggageHeaders); + /** + * Returns the "sentry-trace" header that allows tracing across services. Can also be used in + * <meta> HTML tags. Also see {@link IHub#getBaggage()}. + * + * @return sentry trace header or null + */ + @Nullable + SentryTraceHeader getTraceparent(); + + /** + * Returns the "baggage" header that allows tracing across services. Can also be used in + * <meta> HTML tags. Also see {@link IHub#getTraceparent()}. + * + * @return baggage header or null + */ @Nullable - BaggageHeader baggageHeader(@Nullable List thirdPartyBaggageHeaders); + BaggageHeader getBaggage(); } diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java index cd485ef630..aa5d846975 100644 --- a/sentry/src/main/java/io/sentry/NoOpHub.java +++ b/sentry/src/main/java/io/sentry/NoOpHub.java @@ -162,6 +162,8 @@ public void flush(long timeoutMillis) {} } @Override + @Deprecated + @SuppressWarnings("InlineMeSuggester") public @NotNull SentryTraceHeader traceHeaders() { return new SentryTraceHeader(SentryId.EMPTY_ID, SpanId.EMPTY_ID, true); } @@ -191,14 +193,18 @@ public void setSpanContext( public void reportFullyDisplayed() {} @Override - public @Nullable PropagationContext continueTrace( - final @Nullable String sentryTraceHeader, final @Nullable List baggageHeaders) { + public @Nullable TransactionContext continueTrace( + final @Nullable String sentryTrace, final @Nullable List baggageHeaders) { return null; } @Override - public @Nullable BaggageHeader baggageHeader( - final @Nullable List thirdPartyBaggageHeaders) { + public @Nullable SentryTraceHeader getTraceparent() { + return null; + } + + @Override + public @Nullable BaggageHeader getBaggage() { return null; } } diff --git a/sentry/src/main/java/io/sentry/PropagationContext.java b/sentry/src/main/java/io/sentry/PropagationContext.java index cc538194e5..7f55460852 100644 --- a/sentry/src/main/java/io/sentry/PropagationContext.java +++ b/sentry/src/main/java/io/sentry/PropagationContext.java @@ -60,6 +60,15 @@ public PropagationContext() { this(new SentryId(), new SpanId(), null, null, null); } + public PropagationContext(final @NotNull PropagationContext propagationContext) { + this( + propagationContext.getTraceId(), + propagationContext.getSpanId(), + propagationContext.getParentSpanId(), + propagationContext.getBaggage(), + propagationContext.isSampled()); + } + public PropagationContext( final @NotNull SentryId traceId, final @NotNull SpanId spanId, diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index d73ba91bb2..e5d57578d4 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -138,7 +138,7 @@ public Scope(final @NotNull SentryOptions options) { this.attachments = new CopyOnWriteArrayList<>(scope.attachments); - this.propagationContext = scope.propagationContext; + this.propagationContext = new PropagationContext(scope.propagationContext); } /** diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 99ddd7baed..254c1fffd5 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -903,10 +903,14 @@ public static void endSession() { } /** - * TODO Returns trace header of active transaction or {@code null} if no transaction is active. + * Returns the "sentry-trace" header that allows tracing across services. Can also be used in + * <meta> HTML tags. Also see {@link Sentry#getBaggage()}. * - * @return trace header or null + * @deprecated please use {@link Sentry#getTraceparent()} instead. + * @return sentry trace header or null */ + @Deprecated + @SuppressWarnings("InlineMeSuggester") public static @Nullable SentryTraceHeader traceHeaders() { return getCurrentHub().traceHeaders(); } @@ -974,16 +978,33 @@ public interface OptionsConfiguration { * 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 sentryTrace "sentry-trace" header * @param baggageHeaders "baggage" headers + * @return a transaction context for starting a transaction or null if performance is disabled + */ + // return TransactionContext (if performance enabled) or null (if performance disabled) + public static @Nullable TransactionContext continueTrace( + final @Nullable String sentryTrace, final @Nullable List baggageHeaders) { + return getCurrentHub().continueTrace(sentryTrace, baggageHeaders); + } + + /** + * Returns the "sentry-trace" header that allows tracing across services. Can also be used in + * <meta> HTML tags. Also see {@link Sentry#getBaggage()}. + * + * @return sentry trace header or null */ - public static @Nullable PropagationContext continueTrace( - final @Nullable String sentryTraceHeader, final @Nullable List baggageHeaders) { - return getCurrentHub().continueTrace(sentryTraceHeader, baggageHeaders); + public static @Nullable SentryTraceHeader getTraceparent() { + return getCurrentHub().getTraceparent(); } - public static @Nullable BaggageHeader baggageHeader( - @Nullable List thirdPartyBaggageHeaders) { - return getCurrentHub().baggageHeader(thirdPartyBaggageHeaders); + /** + * Returns the "baggage" header that allows tracing across services. Can also be used in + * <meta> HTML tags. Also see {@link Sentry#getTraceparent()}. + * + * @return baggage header or null + */ + public static @Nullable BaggageHeader getBaggage() { + return getCurrentHub().getBaggage(); } } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 89629a7bb5..b3e99f17e1 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -659,8 +659,14 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint } // Set trace data from active span to connect events with transactions final ISpan span = scope.getSpan(); - if (event.getContexts().getTrace() == null && span != null) { - event.getContexts().setTrace(span.getSpanContext()); + if (event.getContexts().getTrace() == null) { + if (span == null) { + event + .getContexts() + .setTrace(TransactionContext.fromPropagationContext(scope.getPropagationContext())); + } else { + event.getContexts().setTrace(span.getSpanContext()); + } } event = processEvent(event, hint, scope.getEventProcessors()); diff --git a/sentry/src/main/java/io/sentry/TransactionContext.java b/sentry/src/main/java/io/sentry/TransactionContext.java index 3f24cfd480..cd6402e28b 100644 --- a/sentry/src/main/java/io/sentry/TransactionContext.java +++ b/sentry/src/main/java/io/sentry/TransactionContext.java @@ -3,13 +3,18 @@ import io.sentry.protocol.SentryId; import io.sentry.protocol.TransactionNameSource; import io.sentry.util.Objects; +import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; public final class TransactionContext extends SpanContext { - private final @NotNull String name; - private final @NotNull TransactionNameSource transactionNameSource; + private static final @NotNull String DEFAULT_NAME = ""; + private static final @NotNull TransactionNameSource DEFAULT_NAME_SOURCE = + TransactionNameSource.CUSTOM; + private static final @NotNull String DEFAULT_OPERATION = "default"; + private @NotNull String name; + private @NotNull TransactionNameSource transactionNameSource; private @Nullable TracesSamplingDecision parentSamplingDecision; private @Nullable Baggage baggage; private @NotNull Instrumenter instrumenter = Instrumenter.SENTRY; @@ -20,8 +25,11 @@ public final class TransactionContext extends SpanContext { * @param name - the transaction name * @param operation - the operation * @param sentryTrace - the sentry-trace header + * @deprecated use {@link Sentry#continueTrace(String, List)} and setters for name and operation + * here instead. * @return the transaction contexts */ + @Deprecated public static @NotNull TransactionContext fromSentryTrace( final @NotNull String name, final @NotNull String operation, @@ -30,21 +38,23 @@ public final class TransactionContext extends SpanContext { TracesSamplingDecision samplingDecision = parentSampled == null ? null : new TracesSamplingDecision(parentSampled); - return new TransactionContext( - name, - operation, - sentryTrace.getTraceId(), - new SpanId(), - TransactionNameSource.CUSTOM, - sentryTrace.getSpanId(), - samplingDecision, - null); + TransactionContext transactionContext = + new TransactionContext( + sentryTrace.getTraceId(), + new SpanId(), + sentryTrace.getSpanId(), + samplingDecision, + null); + + transactionContext.setName(name); + transactionContext.setTransactionNameSource(TransactionNameSource.CUSTOM); + transactionContext.setOperation(operation); + + return transactionContext; } + @ApiStatus.Internal public static TransactionContext fromPropagationContext( - final @NotNull String name, - final @NotNull TransactionNameSource transactionNameSource, - final @NotNull String operation, final @NotNull PropagationContext propagationContext) { @Nullable Boolean parentSampled = propagationContext.isSampled(); TracesSamplingDecision samplingDecision = @@ -65,11 +75,8 @@ public static TransactionContext fromPropagationContext( } return new TransactionContext( - name, - operation, propagationContext.getTraceId(), propagationContext.getSpanId(), - transactionNameSource, propagationContext.getParentSpanId(), samplingDecision, baggage); @@ -122,18 +129,15 @@ public TransactionContext( @ApiStatus.Internal public TransactionContext( - final @NotNull String name, - final @NotNull String operation, final @NotNull SentryId traceId, final @NotNull SpanId spanId, - final @NotNull TransactionNameSource transactionNameSource, final @Nullable SpanId parentSpanId, final @Nullable TracesSamplingDecision parentSamplingDecision, final @Nullable Baggage baggage) { - super(traceId, spanId, operation, parentSpanId, null); - this.name = Objects.requireNonNull(name, "name is required"); + super(traceId, spanId, DEFAULT_OPERATION, parentSpanId, null); + this.name = DEFAULT_NAME; this.parentSamplingDecision = parentSamplingDecision; - this.transactionNameSource = transactionNameSource; + this.transactionNameSource = DEFAULT_NAME_SOURCE; this.baggage = baggage; } @@ -188,4 +192,12 @@ public void setParentSampled( public void setInstrumenter(final @NotNull Instrumenter instrumenter) { this.instrumenter = instrumenter; } + + public void setName(final @NotNull String name) { + this.name = Objects.requireNonNull(name, "name is required"); + } + + public void setTransactionNameSource(final @NotNull TransactionNameSource transactionNameSource) { + this.transactionNameSource = transactionNameSource; + } } diff --git a/sentry/src/main/java/io/sentry/util/TracingUtils.java b/sentry/src/main/java/io/sentry/util/TracingUtils.java index 848cf04f65..3a66cff917 100644 --- a/sentry/src/main/java/io/sentry/util/TracingUtils.java +++ b/sentry/src/main/java/io/sentry/util/TracingUtils.java @@ -27,7 +27,7 @@ public static void startNewTrace(final @NotNull IHub hub) { @Nullable List thirdPartyBaggageHeaders, final @Nullable ISpan span) { final @NotNull SentryOptions sentryOptions = hub.getOptions(); - if (sentryOptions.isTraceSampling() && isAllowedToSendTo(requestUrl, sentryOptions)) { + if (sentryOptions.isTraceSampling() && shouldAttachTracingHeaders(requestUrl, sentryOptions)) { return trace(hub, thirdPartyBaggageHeaders, span); } diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 441d206a6d..9421c818ca 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -2061,6 +2061,7 @@ class SentryClientTest { whenever(scope.breadcrumbs).thenReturn(LinkedList()) whenever(scope.extras).thenReturn(emptyMap()) whenever(scope.contexts).thenReturn(Contexts()) + whenever(scope.propagationContext).thenReturn(PropagationContext()) val transactionEnd = object : TransactionEnd {} val transactionEndHint = HintUtils.createWithTypeCheckHint(transactionEnd) @@ -2076,6 +2077,168 @@ class SentryClientTest { ) } + @Test + fun `attaches trace context from span if none present yet`() { + val sut = fixture.getSut() + + // build up a running transaction + val spanContext = SpanContext("op.load") + val transaction = mock() + whenever(transaction.name).thenReturn("transaction") + whenever(transaction.spanContext).thenReturn(spanContext) + + // scope + val scope = mock() + whenever(scope.transaction).thenReturn(transaction) + whenever(scope.breadcrumbs).thenReturn(LinkedList()) + whenever(scope.extras).thenReturn(emptyMap()) + whenever(scope.contexts).thenReturn(Contexts()) + val scopePropagationContext = PropagationContext() + whenever(scope.propagationContext).thenReturn(scopePropagationContext) + whenever(scope.span).thenReturn(transaction) + + val sentryEvent = SentryEvent() + sut.captureEvent(sentryEvent, scope) + + verify(fixture.transport).send( + check { + assertEquals(1, it.items.count()) + }, + any() + ) + + assertEquals(spanContext.traceId, sentryEvent.contexts.trace!!.traceId) + assertEquals(spanContext.spanId, sentryEvent.contexts.trace!!.spanId) + assertNotEquals(scopePropagationContext.traceId, sentryEvent.contexts.trace!!.traceId) + assertNotEquals(scopePropagationContext.spanId, sentryEvent.contexts.trace!!.spanId) + } + + @Test + fun `attaches trace context from scope if none present yet and no span on scope`() { + val sut = fixture.getSut() + + // scope + val scope = mock() + whenever(scope.breadcrumbs).thenReturn(LinkedList()) + whenever(scope.extras).thenReturn(emptyMap()) + whenever(scope.contexts).thenReturn(Contexts()) + val scopePropagationContext = PropagationContext() + whenever(scope.propagationContext).thenReturn(scopePropagationContext) + + val sentryEvent = SentryEvent() + sut.captureEvent(sentryEvent, scope) + + verify(fixture.transport).send( + check { + assertEquals(1, it.items.count()) + }, + any() + ) + + assertEquals(scopePropagationContext.traceId, sentryEvent.contexts.trace!!.traceId) + assertEquals(scopePropagationContext.spanId, sentryEvent.contexts.trace!!.spanId) + } + + @Test + fun `keeps existing trace context if already present`() { + val sut = fixture.getSut() + + // build up a running transaction + val spanContext = SpanContext("op.load") + val transaction = mock() + whenever(transaction.name).thenReturn("transaction") + whenever(transaction.spanContext).thenReturn(spanContext) + + // scope + val scope = mock() + whenever(scope.transaction).thenReturn(transaction) + whenever(scope.breadcrumbs).thenReturn(LinkedList()) + whenever(scope.extras).thenReturn(emptyMap()) + whenever(scope.contexts).thenReturn(Contexts()) + val scopePropagationContext = PropagationContext() + whenever(scope.propagationContext).thenReturn(scopePropagationContext) + + val preExistingSpanContext = SpanContext("op.load") + + val sentryEvent = SentryEvent() + sentryEvent.contexts.trace = preExistingSpanContext + sut.captureEvent(sentryEvent, scope) + + verify(fixture.transport).send( + check { + assertEquals(1, it.items.count()) + }, + any() + ) + + assertEquals(preExistingSpanContext.traceId, sentryEvent.contexts.trace!!.traceId) + assertEquals(preExistingSpanContext.spanId, sentryEvent.contexts.trace!!.spanId) + assertNotEquals(spanContext.traceId, sentryEvent.contexts.trace!!.traceId) + assertNotEquals(spanContext.spanId, sentryEvent.contexts.trace!!.spanId) + assertNotEquals(scopePropagationContext.traceId, sentryEvent.contexts.trace!!.traceId) + assertNotEquals(scopePropagationContext.spanId, sentryEvent.contexts.trace!!.spanId) + } + + @Test + fun `uses propagation context on scope for trace header if no transaction is on scope`() { + val sut = fixture.getSut() + + // scope + val scope = mock() + whenever(scope.breadcrumbs).thenReturn(LinkedList()) + whenever(scope.extras).thenReturn(emptyMap()) + whenever(scope.contexts).thenReturn(Contexts()) + val scopePropagationContext = PropagationContext() + whenever(scope.propagationContext).thenReturn(scopePropagationContext) + + val sentryEvent = SentryEvent() + sut.captureEvent(sentryEvent, scope) + + verify(fixture.transport).send( + check { + assertNotNull(it.header.traceContext) + assertEquals(scopePropagationContext.traceId, it.header.traceContext!!.traceId) + }, + any() + ) + } + + @Test + fun `uses trace context on transaction for trace header if a transaction is on scope`() { + val sut = fixture.getSut() + + // build up a running transaction + val spanContext = SpanContext("op.load") + val transaction = mock() + whenever(transaction.name).thenReturn("transaction") + whenever(transaction.spanContext).thenReturn(spanContext) + val transactionTraceContext = TraceContext(SentryId(), "pubkey") + whenever(transaction.traceContext()).thenReturn(transactionTraceContext) + + // scope + val scope = mock() + whenever(scope.transaction).thenReturn(transaction) + whenever(scope.breadcrumbs).thenReturn(LinkedList()) + whenever(scope.extras).thenReturn(emptyMap()) + whenever(scope.contexts).thenReturn(Contexts()) + val scopePropagationContext = PropagationContext() + whenever(scope.propagationContext).thenReturn(scopePropagationContext) + + val preExistingSpanContext = SpanContext("op.load") + + val sentryEvent = SentryEvent() + sentryEvent.contexts.trace = preExistingSpanContext + sut.captureEvent(sentryEvent, scope) + + verify(fixture.transport).send( + check { + assertNotNull(it.header.traceContext) + assertEquals(transactionTraceContext.traceId, it.header.traceContext!!.traceId) + }, + any() + ) + } + private fun givenScopeWithStartedSession(errored: Boolean = false, crashed: Boolean = false): Scope { val scope = createScope(fixture.sentryOptions) scope.startSession() diff --git a/sentry/src/test/java/io/sentry/TransactionContextTest.kt b/sentry/src/test/java/io/sentry/TransactionContextTest.kt index dc55c6d501..6f5a0ead33 100644 --- a/sentry/src/test/java/io/sentry/TransactionContextTest.kt +++ b/sentry/src/test/java/io/sentry/TransactionContextTest.kt @@ -1,7 +1,6 @@ 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 @@ -38,7 +37,7 @@ class TransactionContextTest { 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) + val context = TransactionContext.fromPropagationContext(propagationContext) assertNull(context.sampled) assertNull(context.profileSampled) assertFalse(context.parentSampled!!) @@ -53,7 +52,7 @@ class TransactionContextTest { SentryTraceHeader(SentryId(), SpanId(), false).value, "sentry-trace_id=a,sentry-transaction=sentryTransaction" ) - val context = TransactionContext.fromPropagationContext("name", TransactionNameSource.CUSTOM, "op", propagationContext) + val context = TransactionContext.fromPropagationContext(propagationContext) assertNull(context.sampled) assertNull(context.profileSampled) assertFalse(context.parentSampled!!) @@ -68,7 +67,7 @@ class TransactionContextTest { 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) + val context = TransactionContext.fromPropagationContext(propagationContext) assertNull(context.sampled) assertNull(context.profileSampled) assertTrue(context.parentSampled!!) @@ -83,7 +82,7 @@ class TransactionContextTest { SentryTraceHeader(SentryId(), SpanId(), true).value, "sentry-trace_id=a,sentry-transaction=sentryTransaction" ) - val context = TransactionContext.fromPropagationContext("name", TransactionNameSource.CUSTOM, "op", propagationContext) + val context = TransactionContext.fromPropagationContext(propagationContext) assertNull(context.sampled) assertNull(context.profileSampled) assertTrue(context.parentSampled!!) From 0bb68866fb536cb5c4dbdfb8e7cd7e00dd72ce2a Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 22 Jun 2023 16:22:01 +0200 Subject: [PATCH 14/22] Start new trace in SentryGestureListener --- .../gestures/SentryGestureListener.java | 3 ++- .../SentryGestureListenerClickTest.kt | 21 +++++++++++++++++++ .../SentryGestureListenerScrollTest.kt | 21 +++++++++++++++++++ .../SentryGestureListenerTracingTest.kt | 4 ++++ 4 files changed, 48 insertions(+), 1 deletion(-) 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 88539454a0..f25cc72312 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 @@ -20,6 +20,7 @@ import io.sentry.android.core.SentryAndroidOptions; import io.sentry.internal.gestures.UiElement; import io.sentry.protocol.TransactionNameSource; +import io.sentry.util.TracingUtils; import java.lang.ref.WeakReference; import java.util.Collections; import java.util.Map; @@ -184,9 +185,9 @@ 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())) { + TracingUtils.startNewTrace(hub); return; } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerClickTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerClickTest.kt index b61f3805f0..3cd698e49f 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerClickTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerClickTest.kt @@ -11,11 +11,14 @@ import android.widget.CheckBox import android.widget.RadioButton import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryLevel.INFO import io.sentry.android.core.SentryAndroidOptions 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 @@ -36,6 +39,7 @@ class SentryGestureListenerClickTest { dsn = "https://key@sentry.io/proj" } val hub = mock() + val scope = mock() lateinit var target: View lateinit var invalidTarget: View @@ -79,6 +83,7 @@ class SentryGestureListenerClickTest { whenever(context.resources).thenReturn(resources) whenever(this.target.context).thenReturn(context) whenever(activity.window).thenReturn(window) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) return SentryGestureListener( activity, hub, @@ -228,4 +233,20 @@ class SentryGestureListenerClickTest { anyOrNull() ) } + + @Test + fun `creates new trace on click`() { + class LocalView(context: Context) : View(context) + + val event = mock() + val sut = fixture.getSut(event, attachViewsToRoot = false) + fixture.window.mockDecorView(event = event, touchWithinBounds = false) { + whenever(it.childCount).thenReturn(1) + whenever(it.getChildAt(0)).thenReturn(fixture.target) + } + + sut.onSingleTapUp(event) + + verify(fixture.scope).propagationContext = any() + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt index e00d22c73e..a2730bd045 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt @@ -12,14 +12,18 @@ import android.widget.ListAdapter import androidx.core.view.ScrollingView import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryLevel.INFO import io.sentry.android.core.SentryAndroidOptions import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.inOrder 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 @@ -39,6 +43,7 @@ class SentryGestureListenerScrollTest { gestureTargetLocators = listOf(AndroidViewGestureTargetLocator(true)) } val hub = mock() + val scope = mock() val firstEvent = mock() val eventsInBetween = listOf(mock(), mock(), mock()) @@ -69,6 +74,7 @@ class SentryGestureListenerScrollTest { endEvent.mockDirection(firstEvent, direction) } whenever(activity.window).thenReturn(window) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) return SentryGestureListener( activity, hub, @@ -145,6 +151,7 @@ class SentryGestureListenerScrollTest { }, anyOrNull() ) + verify(fixture.hub).configureScope(anyOrNull()) verify(fixture.hub).addBreadcrumb( check { assertEquals("ui.swipe", it.category) @@ -156,6 +163,7 @@ class SentryGestureListenerScrollTest { }, anyOrNull() ) + verify(fixture.hub).configureScope(anyOrNull()) } verifyNoMoreInteractions(fixture.hub) } @@ -182,6 +190,19 @@ class SentryGestureListenerScrollTest { verify(fixture.hub, never()).addBreadcrumb(any()) } + @Test + fun `starts a new trace on scroll`() { + val sut = fixture.getSut(direction = "left") + + sut.onDown(fixture.firstEvent) + fixture.eventsInBetween.forEach { + sut.onScroll(fixture.firstEvent, it, 10.0f, 0f) + } + sut.onUp(fixture.endEvent) + + verify(fixture.scope).propagationContext = any() + } + internal class ScrollableView : View(mock()), ScrollingView { override fun computeVerticalScrollOffset(): Int = 0 override fun computeVerticalScrollExtent(): Int = 0 diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerTracingTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerTracingTest.kt index bd31d98fda..5f216a660a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerTracingTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerTracingTest.kt @@ -11,6 +11,7 @@ import android.widget.AbsListView import android.widget.ListAdapter import io.sentry.IHub import io.sentry.Scope +import io.sentry.ScopeCallback import io.sentry.SentryTracer import io.sentry.SpanStatus import io.sentry.TransactionContext @@ -20,6 +21,7 @@ import io.sentry.protocol.TransactionNameSource import org.mockito.kotlin.any import org.mockito.kotlin.check import org.mockito.kotlin.clearInvocations +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -40,6 +42,7 @@ class SentryGestureListenerTracingTest { } val hub = mock() val event = mock() + val scope = mock() lateinit var target: View lateinit var transaction: SentryTracer @@ -79,6 +82,7 @@ class SentryGestureListenerTracingTest { whenever(hub.startTransaction(any(), any())) .thenReturn(this.transaction) + doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) return SentryGestureListener( activity, From 49b1699af2b5bb76f2f801768309e097a708ef9c Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 23 Jun 2023 12:19:51 +0200 Subject: [PATCH 15/22] More tests --- sentry/src/test/java/io/sentry/HubTest.kt | 75 +++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt index fcb0132d0d..96a44462d4 100644 --- a/sentry/src/test/java/io/sentry/HubTest.kt +++ b/sentry/src/test/java/io/sentry/HubTest.kt @@ -31,12 +31,14 @@ import java.io.File import java.nio.file.Files import java.util.Queue import java.util.UUID +import java.util.concurrent.atomic.AtomicReference import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertFalse +import kotlin.test.assertNotEquals import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -1725,6 +1727,79 @@ class HubTest { verify(hub).reportFullyDisplayed() } + @Test + fun `continueTrace creates propagation context from headers and returns transaction context if performance enabled`() { + val hub = generateHub() + val traceId = SentryId() + val parentSpanId = SpanId() + val transactionContext = hub.continueTrace("$traceId-$parentSpanId-1", listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=$traceId,sentry-transaction=HTTP%20GET")) + + hub.configureScope { scope -> + assertEquals(traceId, scope.propagationContext.traceId) + assertEquals(parentSpanId, scope.propagationContext.parentSpanId) + } + + assertEquals(traceId, transactionContext!!.traceId) + assertEquals(parentSpanId, transactionContext!!.parentSpanId) + } + + @Test + fun `continueTrace creates new propagation context if header invalid and returns transaction context if performance enabled`() { + val hub = generateHub() + val traceId = SentryId() + var propagationContextHolder = AtomicReference() + + hub.configureScope { propagationContextHolder.set(it.propagationContext) } + val propagationContextAtStart = propagationContextHolder.get()!! + + val transactionContext = hub.continueTrace("invalid", listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=$traceId,sentry-transaction=HTTP%20GET")) + + hub.configureScope { scope -> + assertNotEquals(propagationContextAtStart.traceId, scope.propagationContext.traceId) + assertNotEquals(propagationContextAtStart.parentSpanId, scope.propagationContext.parentSpanId) + assertNotEquals(propagationContextAtStart.spanId, scope.propagationContext.spanId) + + assertEquals(scope.propagationContext.traceId, transactionContext!!.traceId) + assertEquals(scope.propagationContext.parentSpanId, transactionContext!!.parentSpanId) + assertEquals(scope.propagationContext.spanId, transactionContext!!.spanId) + } + } + + @Test + fun `continueTrace creates propagation context from headers and returns null if performance disabled`() { + val hub = generateHub { it.enableTracing = false } + val traceId = SentryId() + val parentSpanId = SpanId() + val transactionContext = hub.continueTrace("$traceId-$parentSpanId-1", listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=$traceId,sentry-transaction=HTTP%20GET")) + + hub.configureScope { scope -> + assertEquals(traceId, scope.propagationContext.traceId) + assertEquals(parentSpanId, scope.propagationContext.parentSpanId) + } + + assertNull(transactionContext) + } + + @Test + fun `continueTrace creates new propagation context if header invalid and returns null if performance disabled`() { + val hub = generateHub { it.enableTracing = false } + val traceId = SentryId() + var propagationContextHolder = AtomicReference() + + hub.configureScope { propagationContextHolder.set(it.propagationContext) } + val propagationContextAtStart = propagationContextHolder.get()!! + + val transactionContext = hub.continueTrace("invalid", listOf("sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rate=1,sentry-trace_id=$traceId,sentry-transaction=HTTP%20GET")) + + hub.configureScope { scope -> + assertNotEquals(propagationContextAtStart.traceId, scope.propagationContext.traceId) + assertNotEquals(propagationContextAtStart.parentSpanId, scope.propagationContext.parentSpanId) + assertNotEquals(propagationContextAtStart.spanId, scope.propagationContext.spanId) + } + + assertNull(transactionContext) + } + private val dsnTest = "https://key@sentry.io/proj" private fun generateHub(optionsConfiguration: Sentry.OptionsConfiguration? = null): IHub { From e7e50bd86980f02dde54acbf76e0a0c058183da1 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 26 Jun 2023 10:34:25 +0200 Subject: [PATCH 16/22] Synchronize on scope for trace creation and usage from scope --- .../java/io/sentry/util/TracingUtils.java | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/sentry/src/main/java/io/sentry/util/TracingUtils.java b/sentry/src/main/java/io/sentry/util/TracingUtils.java index 3a66cff917..4b032a531c 100644 --- a/sentry/src/main/java/io/sentry/util/TracingUtils.java +++ b/sentry/src/main/java/io/sentry/util/TracingUtils.java @@ -17,7 +17,9 @@ public final class TracingUtils { public static void startNewTrace(final @NotNull IHub hub) { hub.configureScope( scope -> { - scope.setPropagationContext(new PropagationContext()); + synchronized (scope) { + scope.setPropagationContext(new PropagationContext()); + } }); } @@ -47,21 +49,23 @@ public static void startNewTrace(final @NotNull IHub hub) { final @NotNull TracingHeadersHolder returnValue = new TracingHeadersHolder(); 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); + synchronized (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); + } + + returnValue.headers = + new TracingHeaders( + new SentryTraceHeader( + propagationContext.getTraceId(), propagationContext.getSpanId(), null), + baggageHeader); } - - returnValue.headers = - new TracingHeaders( - new SentryTraceHeader( - propagationContext.getTraceId(), propagationContext.getSpanId(), null), - baggageHeader); }); return returnValue.headers; } From 4e5f6aaae6588bd3dc1ee51469606f0a32a1a41a Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 26 Jun 2023 13:47:46 +0200 Subject: [PATCH 17/22] Add withPropagationContext to Scope and synchronize using a private lock --- .../SentryGestureListenerClickTest.kt | 4 + .../SentryGestureListenerScrollTest.kt | 3 + sentry/api/sentry.api | 8 +- sentry/src/main/java/io/sentry/Baggage.java | 5 ++ .../java/io/sentry/PropagationContext.java | 10 ++- sentry/src/main/java/io/sentry/Scope.java | 22 ++++++ .../src/main/java/io/sentry/SentryClient.java | 5 +- .../java/io/sentry/util/TracingUtils.java | 73 ++++++++++--------- .../test/java/io/sentry/SentryClientTest.kt | 11 ++- 9 files changed, 101 insertions(+), 40 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerClickTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerClickTest.kt index 3cd698e49f..2fdbc9dc07 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerClickTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerClickTest.kt @@ -11,7 +11,9 @@ import android.widget.CheckBox import android.widget.RadioButton import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.PropagationContext import io.sentry.Scope +import io.sentry.Scope.IWithPropagationContext import io.sentry.ScopeCallback import io.sentry.SentryLevel.INFO import io.sentry.android.core.SentryAndroidOptions @@ -40,6 +42,7 @@ class SentryGestureListenerClickTest { } val hub = mock() val scope = mock() + val propagationContext = PropagationContext() lateinit var target: View lateinit var invalidTarget: View @@ -84,6 +87,7 @@ class SentryGestureListenerClickTest { whenever(this.target.context).thenReturn(context) whenever(activity.window).thenReturn(window) doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(propagationContext); propagationContext; }.whenever(scope).withPropagationContext(any()) return SentryGestureListener( activity, hub, diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt index a2730bd045..fca1723d54 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt @@ -12,6 +12,7 @@ import android.widget.ListAdapter import androidx.core.view.ScrollingView import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.PropagationContext import io.sentry.Scope import io.sentry.ScopeCallback import io.sentry.SentryLevel.INFO @@ -44,6 +45,7 @@ class SentryGestureListenerScrollTest { } val hub = mock() val scope = mock() + val propagationContext = PropagationContext() val firstEvent = mock() val eventsInBetween = listOf(mock(), mock(), mock()) @@ -75,6 +77,7 @@ class SentryGestureListenerScrollTest { } whenever(activity.window).thenReturn(window) doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(hub).configureScope(any()) + doAnswer { (it.arguments[0] as Scope.IWithPropagationContext).accept(propagationContext); propagationContext }.whenever(scope).withPropagationContext(any()) return SentryGestureListener( activity, hub, diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 2c1181890e..468e7e64de 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -30,6 +30,7 @@ public abstract interface class io/sentry/BackfillingEventProcessor : io/sentry/ } public final class io/sentry/Baggage { + public fun (Lio/sentry/Baggage;)V public fun (Lio/sentry/ILogger;)V public fun (Ljava/util/Map;Ljava/lang/String;ZLio/sentry/ILogger;)V public fun freeze ()V @@ -1196,9 +1197,14 @@ public final class io/sentry/Scope { public fun setTransaction (Lio/sentry/ITransaction;)V public fun setTransaction (Ljava/lang/String;)V public fun setUser (Lio/sentry/protocol/User;)V + public fun withPropagationContext (Lio/sentry/Scope$IWithPropagationContext;)Lio/sentry/PropagationContext; public fun withTransaction (Lio/sentry/Scope$IWithTransaction;)V } +public abstract interface class io/sentry/Scope$IWithPropagationContext { + public abstract fun accept (Lio/sentry/PropagationContext;)V +} + public abstract interface class io/sentry/Scope$IWithTransaction { public abstract fun accept (Lio/sentry/ITransaction;)V } @@ -4196,7 +4202,7 @@ public final class io/sentry/util/StringUtils { public final class io/sentry/util/TracingUtils { public fun ()V - public static fun maybeUpdateBaggage (Lio/sentry/Scope;Lio/sentry/SentryOptions;)V + public static fun maybeUpdateBaggage (Lio/sentry/Scope;Lio/sentry/SentryOptions;)Lio/sentry/PropagationContext; public static fun startNewTrace (Lio/sentry/IHub;)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; diff --git a/sentry/src/main/java/io/sentry/Baggage.java b/sentry/src/main/java/io/sentry/Baggage.java index e2ef0dc5fd..e2cca117da 100644 --- a/sentry/src/main/java/io/sentry/Baggage.java +++ b/sentry/src/main/java/io/sentry/Baggage.java @@ -130,6 +130,11 @@ public Baggage(final @NotNull ILogger logger) { this(new HashMap<>(), null, true, logger); } + @ApiStatus.Internal + public Baggage(final @NotNull Baggage baggage) { + this(baggage.keyValues, baggage.thirdPartyHeader, baggage.mutable, baggage.logger); + } + @ApiStatus.Internal public Baggage( final @NotNull Map keyValues, diff --git a/sentry/src/main/java/io/sentry/PropagationContext.java b/sentry/src/main/java/io/sentry/PropagationContext.java index 7f55460852..3ea960243f 100644 --- a/sentry/src/main/java/io/sentry/PropagationContext.java +++ b/sentry/src/main/java/io/sentry/PropagationContext.java @@ -65,10 +65,18 @@ public PropagationContext(final @NotNull PropagationContext propagationContext) propagationContext.getTraceId(), propagationContext.getSpanId(), propagationContext.getParentSpanId(), - propagationContext.getBaggage(), + cloneBaggage(propagationContext.getBaggage()), propagationContext.isSampled()); } + private static @Nullable Baggage cloneBaggage(final @Nullable Baggage baggage) { + if (baggage != null) { + return new Baggage(baggage); + } + + return null; + } + public PropagationContext( final @NotNull SentryId traceId, final @NotNull SpanId spanId, diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index e5d57578d4..41611795b3 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -65,6 +65,9 @@ public final class Scope { /** Transaction lock, Ops should be atomic */ private final @NotNull Object transactionLock = new Object(); + /** PropagationContext lock, Ops should be atomic */ + private final @NotNull Object propagationContextLock = new Object(); + /** Scope's contexts */ private @NotNull Contexts contexts = new Contexts(); @@ -814,6 +817,14 @@ public void setPropagationContext(final @NotNull PropagationContext propagationC return propagationContext; } + public @NotNull PropagationContext withPropagationContext( + final @NotNull IWithPropagationContext callback) { + synchronized (propagationContextLock) { + callback.accept(propagationContext); + return new PropagationContext(propagationContext); + } + } + /** the IWithTransaction callback */ @ApiStatus.Internal public interface IWithTransaction { @@ -825,4 +836,15 @@ public interface IWithTransaction { */ void accept(@Nullable ITransaction transaction); } + + /** the IWithPropagationContext callback */ + public interface IWithPropagationContext { + + /** + * The accept method of the callback + * + * @param propagationContext the current propagationContext + */ + void accept(@NotNull PropagationContext propagationContext); + } } diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java index 7f3c126002..97baa10bc1 100644 --- a/sentry/src/main/java/io/sentry/SentryClient.java +++ b/sentry/src/main/java/io/sentry/SentryClient.java @@ -177,8 +177,9 @@ private boolean shouldApplyScopeData( if (scope.getTransaction() != null) { traceContext = scope.getTransaction().traceContext(); } else { - TracingUtils.maybeUpdateBaggage(scope, options); - traceContext = scope.getPropagationContext().traceContext(); + final @NotNull PropagationContext propagationContext = + TracingUtils.maybeUpdateBaggage(scope, options); + traceContext = propagationContext.traceContext(); } } diff --git a/sentry/src/main/java/io/sentry/util/TracingUtils.java b/sentry/src/main/java/io/sentry/util/TracingUtils.java index 4b032a531c..fbb8ccb775 100644 --- a/sentry/src/main/java/io/sentry/util/TracingUtils.java +++ b/sentry/src/main/java/io/sentry/util/TracingUtils.java @@ -17,9 +17,10 @@ public final class TracingUtils { public static void startNewTrace(final @NotNull IHub hub) { hub.configureScope( scope -> { - synchronized (scope) { - scope.setPropagationContext(new PropagationContext()); - } + scope.withPropagationContext( + propagationContext -> { + scope.setPropagationContext(new PropagationContext()); + }); }); } @@ -46,43 +47,45 @@ public static void startNewTrace(final @NotNull IHub hub) { return new TracingHeaders( span.toSentryTrace(), span.toBaggageHeader(thirdPartyBaggageHeaders)); } else { - final @NotNull TracingHeadersHolder returnValue = new TracingHeadersHolder(); + final @NotNull PropagationContextHolder returnValue = new PropagationContextHolder(); hub.configureScope( (scope) -> { - synchronized (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); - } - - returnValue.headers = - new TracingHeaders( - new SentryTraceHeader( - propagationContext.getTraceId(), propagationContext.getSpanId(), null), - baggageHeader); - } + returnValue.propagationContext = maybeUpdateBaggage(scope, sentryOptions); }); - return returnValue.headers; + + if (returnValue.propagationContext != null) { + final @NotNull PropagationContext propagationContext = returnValue.propagationContext; + final @Nullable Baggage baggage = propagationContext.getBaggage(); + @Nullable BaggageHeader baggageHeader = null; + if (baggage != null) { + baggageHeader = + BaggageHeader.fromBaggageAndOutgoingHeader(baggage, thirdPartyBaggageHeaders); + } + + return new TracingHeaders( + new SentryTraceHeader( + propagationContext.getTraceId(), propagationContext.getSpanId(), null), + baggageHeader); + } + + return null; } } - public static void maybeUpdateBaggage( + public static @NotNull PropagationContext 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(); - } + return scope.withPropagationContext( + propagationContext -> { + @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 shouldAttachTracingHeaders( @@ -90,8 +93,8 @@ private static boolean shouldAttachTracingHeaders( return PropagationTargetsUtils.contain(sentryOptions.getTracePropagationTargets(), requestUrl); } - private static final class TracingHeadersHolder { - private @Nullable TracingHeaders headers = null; + private static final class PropagationContextHolder { + private @Nullable PropagationContext propagationContext = null; } public static final class TracingHeaders { diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt index 0741eb7aee..28e16e954d 100644 --- a/sentry/src/test/java/io/sentry/SentryClientTest.kt +++ b/sentry/src/test/java/io/sentry/SentryClientTest.kt @@ -1,5 +1,6 @@ package io.sentry +import io.sentry.Scope.IWithPropagationContext import io.sentry.clientreport.ClientReportTestHelper.Companion.assertClientReport import io.sentry.clientreport.DiscardReason import io.sentry.clientreport.DiscardedEvent @@ -2097,7 +2098,9 @@ class SentryClientTest { whenever(scope.breadcrumbs).thenReturn(LinkedList()) whenever(scope.extras).thenReturn(emptyMap()) whenever(scope.contexts).thenReturn(Contexts()) - whenever(scope.propagationContext).thenReturn(PropagationContext()) + val scopePropagationContext = PropagationContext() + whenever(scope.propagationContext).thenReturn(scopePropagationContext) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) val transactionEnd = object : TransactionEnd {} val transactionEndHint = HintUtils.createWithTypeCheckHint(transactionEnd) @@ -2132,6 +2135,7 @@ class SentryClientTest { val scopePropagationContext = PropagationContext() whenever(scope.propagationContext).thenReturn(scopePropagationContext) whenever(scope.span).thenReturn(transaction) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) val sentryEvent = SentryEvent() sut.captureEvent(sentryEvent, scope) @@ -2160,6 +2164,7 @@ class SentryClientTest { whenever(scope.contexts).thenReturn(Contexts()) val scopePropagationContext = PropagationContext() whenever(scope.propagationContext).thenReturn(scopePropagationContext) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) val sentryEvent = SentryEvent() sut.captureEvent(sentryEvent, scope) @@ -2193,6 +2198,7 @@ class SentryClientTest { whenever(scope.contexts).thenReturn(Contexts()) val scopePropagationContext = PropagationContext() whenever(scope.propagationContext).thenReturn(scopePropagationContext) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) val preExistingSpanContext = SpanContext("op.load") @@ -2225,7 +2231,9 @@ class SentryClientTest { whenever(scope.extras).thenReturn(emptyMap()) whenever(scope.contexts).thenReturn(Contexts()) val scopePropagationContext = PropagationContext() + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) whenever(scope.propagationContext).thenReturn(scopePropagationContext) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) val sentryEvent = SentryEvent() sut.captureEvent(sentryEvent, scope) @@ -2259,6 +2267,7 @@ class SentryClientTest { whenever(scope.contexts).thenReturn(Contexts()) val scopePropagationContext = PropagationContext() whenever(scope.propagationContext).thenReturn(scopePropagationContext) + doAnswer { (it.arguments[0] as IWithPropagationContext).accept(scopePropagationContext); scopePropagationContext }.whenever(scope).withPropagationContext(any()) val preExistingSpanContext = SpanContext("op.load") From 1ec1b5d7e4c8b3d4fa1f36ada0e967e90f141c0c Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 26 Jun 2023 15:22:18 +0200 Subject: [PATCH 18/22] Default to false for sampled decision of incoming trace without explicit sampling decision --- sentry/src/main/java/io/sentry/TransactionContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/TransactionContext.java b/sentry/src/main/java/io/sentry/TransactionContext.java index cd6402e28b..0885b9916a 100644 --- a/sentry/src/main/java/io/sentry/TransactionContext.java +++ b/sentry/src/main/java/io/sentry/TransactionContext.java @@ -66,7 +66,7 @@ public static TransactionContext fromPropagationContext( baggage.freeze(); Double sampleRate = baggage.getSampleRateDouble(); - Boolean sampled = parentSampled != null ? parentSampled.booleanValue() : true; + Boolean sampled = parentSampled != null ? parentSampled.booleanValue() : false; if (sampleRate != null) { samplingDecision = new TracesSamplingDecision(sampled, sampleRate); } else { From 29f9cd12d3726950e01695b3156bb8ef614d5c99 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 26 Jun 2023 16:40:37 +0200 Subject: [PATCH 19/22] Remove workaround --- .../io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java | 3 --- .../sentry/spring/boot/jakarta/SentryAutoConfigurationTest.kt | 2 +- .../java/io/sentry/spring/boot/SentryAutoConfiguration.java | 3 --- .../io/sentry/spring/boot/SentryAutoConfigurationTest.kt | 2 +- 4 files changed, 2 insertions(+), 8 deletions(-) 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 5c8c96be3b..2d27bd31ae 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 @@ -119,9 +119,6 @@ static class HubConfiguration { BuildConfig.SENTRY_SPRING_BOOT_JAKARTA_SDK_NAME + "/" + BuildConfig.VERSION_NAME); options.setSdkVersion(createSdkVersion(options)); addPackageAndIntegrationInfo(); - if (options.getTracesSampleRate() == null && options.getEnableTracing() == null) { - options.setTracesSampleRate(0.0); - } // Spring Boot sets ignored exceptions in runtime using reflection - where the generic // information is lost // its technically possible to set non-throwable class to `ignoredExceptionsForType` set 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 b686681b59..7914c10c44 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 @@ -225,7 +225,7 @@ class SentryAutoConfigurationTest { "sentry.dsn=http://key@localhost/proj" ).run { val options = it.getBean(SentryProperties::class.java) - assertThat(options.tracesSampleRate).isNotNull().isEqualTo(0.0) + assertThat(options.tracesSampleRate).isNull() } } 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 f7bc510875..80647c565b 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 @@ -119,9 +119,6 @@ static class HubConfiguration { BuildConfig.SENTRY_SPRING_BOOT_SDK_NAME + "/" + BuildConfig.VERSION_NAME); options.setSdkVersion(createSdkVersion(options)); addPackageAndIntegrationInfo(); - if (options.getTracesSampleRate() == null && options.getEnableTracing() == null) { - options.setTracesSampleRate(0.0); - } // Spring Boot sets ignored exceptions in runtime using reflection - where the generic // information is lost // its technically possible to set non-throwable class to `ignoredExceptionsForType` set 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 c87bcf674f..3452c3a093 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 @@ -225,7 +225,7 @@ class SentryAutoConfigurationTest { "sentry.dsn=http://key@localhost/proj" ).run { val options = it.getBean(SentryProperties::class.java) - assertThat(options.tracesSampleRate).isNotNull().isEqualTo(0.0) + assertThat(options.tracesSampleRate).isNull() } } From 67813f717c023c843404eec0d550a9e69635b8dc Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 27 Jun 2023 07:26:40 +0200 Subject: [PATCH 20/22] Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27fe3c3677..d933fa197c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ - Enhance regex patterns for native stackframes - Tracing headers (`sentry-trace` and `baggage`) are now attached and passed through even if performance is disabled ([#2788](https://github.com/getsentry/sentry-java/pull/2788)) +### Fixes + +- Remove code that set `tracesSampleRate` to `0.0` for Spring Boot if not set ([#2800](https://github.com/getsentry/sentry-java/pull/2800)) + ## 6.23.0 ### Features From cbfad4f35e97bed4849a34b57517c0b310e9292a Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 3 Jul 2023 07:30:25 +0200 Subject: [PATCH 21/22] changelog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 668f4d3d04..596cb91f65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,11 +5,13 @@ ### Features - Add manifest `AutoInit` to integrations list ([#2795](https://github.com/getsentry/sentry-java/pull/2795)) -- Tracing headers (`sentry-trace` and `baggage`) are now attached and passed through even if performance is disabled ([#2788](https://github.com/getsentry/sentry-java/pull/2788)) ### Fixes - Set `environment` from `SentryOptions` if none persisted in ANRv2 ([#2809](https://github.com/getsentry/sentry-java/pull/2809)) +- Remove code that set `tracesSampleRate` to `0.0` for Spring Boot if not set ([#2800](https://github.com/getsentry/sentry-java/pull/2800)) + - This used to enable performance but not send any transactions by default. + - Performance is now disabled by default. ### Dependencies From 09c8022744ba5d70abc21cd3f4ac5463af3766b2 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 3 Jul 2023 07:32:33 +0200 Subject: [PATCH 22/22] fix changelog --- CHANGELOG.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 596cb91f65..ef2fe5f106 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,10 +39,6 @@ - Enhance regex patterns for native stackframes - Tracing headers (`sentry-trace` and `baggage`) are now attached and passed through even if performance is disabled ([#2788](https://github.com/getsentry/sentry-java/pull/2788)) -### Fixes - -- Remove code that set `tracesSampleRate` to `0.0` for Spring Boot if not set ([#2800](https://github.com/getsentry/sentry-java/pull/2800)) - ## 6.23.0 ### Features