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)) + } + } }