From c67085f9e741d1c380c8247017ab8c48ab490273 Mon Sep 17 00:00:00 2001 From: Isak W Date: Thu, 12 Dec 2024 10:06:17 +0100 Subject: [PATCH 1/7] Add support for async dispatch requests Keeps the transaction open until the response is committed. --- .../jakarta/tracing/SentryTracingFilter.java | 66 +++++++++++------- .../spring/tracing/SentryTracingFilter.java | 67 +++++++++++-------- 2 files changed, 80 insertions(+), 53 deletions(-) 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 ed91f85a294..e32635ccea4 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 @@ -36,6 +36,7 @@ public class SentryTracingFilter extends OncePerRequestFilter { private static final String TRANSACTION_OP = "http.server"; private static final String TRACE_ORIGIN = "auto.http.spring_jakarta.webmvc"; + private static final String TRANSACTION_ATTR = SentryTracingFilter.class.getName() + ".transaction"; private final @NotNull TransactionNameProvider transactionNameProvider; private final @NotNull IHub hub; @@ -52,6 +53,11 @@ public SentryTracingFilter() { this(HubAdapter.getInstance()); } + @Override + protected boolean shouldNotFilterAsyncDispatch() { + return false; + } + /** * Creates filter that resolves transaction name using transaction name provider given by * parameter. @@ -77,14 +83,8 @@ protected void doFilterInternal( final @NotNull FilterChain filterChain) throws ServletException, IOException { 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 TransactionContext transactionContext = - hub.continueTrace(sentryTraceHeader, baggageHeader); if (hub.getOptions().isTracingEnabled() && shouldTraceRequest(httpRequest)) { - doFilterWithTransaction(httpRequest, httpResponse, filterChain, transactionContext); + doFilterWithTransaction(httpRequest, httpResponse, filterChain); } else { filterChain.doFilter(httpRequest, httpResponse); } @@ -96,12 +96,24 @@ protected void doFilterInternal( private void doFilterWithTransaction( HttpServletRequest httpRequest, HttpServletResponse httpResponse, - FilterChain filterChain, - final @Nullable TransactionContext transactionContext) + FilterChain filterChain) throws IOException, ServletException { - // at this stage we are not able to get real transaction name - final ITransaction transaction = startTransaction(httpRequest, transactionContext); - transaction.getSpanContext().setOrigin(TRACE_ORIGIN); + final ITransaction transaction; + if (isAsyncDispatch(httpRequest)) { + transaction = (ITransaction) httpRequest.getAttribute(TRANSACTION_ATTR); + } else { + final @Nullable String sentryTraceHeader = + httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); + final @Nullable List baggageHeader = + Collections.list(httpRequest.getHeaders(BaggageHeader.BAGGAGE_HEADER)); + final @Nullable TransactionContext transactionContext = + hub.continueTrace(sentryTraceHeader, baggageHeader); + + // at this stage we are not able to get real transaction name + transaction = startTransaction(httpRequest, transactionContext); + transaction.getSpanContext().setOrigin(TRACE_ORIGIN); + httpRequest.setAttribute(TRANSACTION_ATTR, transaction); + } try { filterChain.doFilter(httpRequest, httpResponse); @@ -110,21 +122,23 @@ private void doFilterWithTransaction( 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())); + if (!isAsyncStarted(httpRequest)) { + // 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(); } - transaction.finish(); } } } 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 5af329f6004..66af0a258eb 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 @@ -33,6 +33,7 @@ public class SentryTracingFilter extends OncePerRequestFilter { private static final String TRANSACTION_OP = "http.server"; private static final String TRACE_ORIGIN = "auto.http.spring.webmvc"; + private static final String TRANSACTION_ATTR = SentryTracingFilter.class.getName() + ".transaction"; private final @NotNull TransactionNameProvider transactionNameProvider; private final @NotNull IHub hub; @@ -67,6 +68,11 @@ public SentryTracingFilter(final @NotNull IHub hub) { this(hub, new SpringMvcTransactionNameProvider()); } + @Override + protected boolean shouldNotFilterAsyncDispatch() { + return false; + } + @Override protected void doFilterInternal( final @NotNull HttpServletRequest httpRequest, @@ -75,15 +81,8 @@ protected void doFilterInternal( throws ServletException, IOException { 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 TransactionContext transactionContext = - hub.continueTrace(sentryTraceHeader, baggageHeader); - if (hub.getOptions().isTracingEnabled() && shouldTraceRequest(httpRequest)) { - doFilterWithTransaction(httpRequest, httpResponse, filterChain, transactionContext); + doFilterWithTransaction(httpRequest, httpResponse, filterChain); } else { filterChain.doFilter(httpRequest, httpResponse); } @@ -95,12 +94,24 @@ protected void doFilterInternal( private void doFilterWithTransaction( HttpServletRequest httpRequest, HttpServletResponse httpResponse, - FilterChain filterChain, - final @Nullable TransactionContext transactionContext) + FilterChain filterChain) throws IOException, ServletException { - // at this stage we are not able to get real transaction name - final ITransaction transaction = startTransaction(httpRequest, transactionContext); - transaction.getSpanContext().setOrigin(TRACE_ORIGIN); + final ITransaction transaction; + if (isAsyncDispatch(httpRequest)) { + transaction = (ITransaction) httpRequest.getAttribute(TRANSACTION_ATTR); + } else { + final @Nullable String sentryTraceHeader = + httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); + final @Nullable List baggageHeader = + Collections.list(httpRequest.getHeaders(BaggageHeader.BAGGAGE_HEADER)); + final @Nullable TransactionContext transactionContext = + hub.continueTrace(sentryTraceHeader, baggageHeader); + + // at this stage we are not able to get real transaction name + transaction = startTransaction(httpRequest, transactionContext); + transaction.getSpanContext().setOrigin(TRACE_ORIGIN); + httpRequest.setAttribute(TRANSACTION_ATTR, transaction); + } try { filterChain.doFilter(httpRequest, httpResponse); @@ -109,21 +120,23 @@ private void doFilterWithTransaction( 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())); + if (!isAsyncStarted(httpRequest)) { + // 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(); } - transaction.finish(); } } } From 402fcc941c94cfa5800c2174332bae535afca909 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Tue, 18 Feb 2025 07:25:43 +0100 Subject: [PATCH 2/7] fix distributed tracing for TwP --- .../jakarta/tracing/SentryTracingFilter.java | 78 ++++++++++++++----- 1 file changed, 59 insertions(+), 19 deletions(-) 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 4c57e12baaf..dde43d2f0ef 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 @@ -37,8 +37,7 @@ public class SentryTracingFilter extends OncePerRequestFilter { private static final String TRANSACTION_OP = "http.server"; private static final String TRACE_ORIGIN = "auto.http.spring_jakarta.webmvc"; - private static final String TRANSACTION_ATTR = - SentryTracingFilter.class.getName() + ".transaction"; + private static final String TRANSACTION_ATTR = "sentry.transaction"; private final @NotNull TransactionNameProvider transactionNameProvider; private final @NotNull IScopes scopes; @@ -86,12 +85,14 @@ protected void doFilterInternal( final @NotNull FilterChain filterChain) throws ServletException, IOException { if (scopes.isEnabled() && !isIgnored()) { - final @Nullable String sentryTraceHeader = - httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); - final @Nullable List baggageHeader = - Collections.list(httpRequest.getHeaders(BaggageHeader.BAGGAGE_HEADER)); - final @Nullable TransactionContext transactionContext = - scopes.continueTrace(sentryTraceHeader, baggageHeader); + @Nullable TransactionContext transactionContext = null; + if (shouldContinueTrace(httpRequest)) { + final @Nullable String sentryTraceHeader = + httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); + final @Nullable List baggageHeader = + Collections.list(httpRequest.getHeaders(BaggageHeader.BAGGAGE_HEADER)); + transactionContext = scopes.continueTrace(sentryTraceHeader, baggageHeader); + } if (scopes.getOptions().isTracingEnabled() && shouldTraceRequest(httpRequest)) { doFilterWithTransaction(httpRequest, httpResponse, filterChain, transactionContext); } else { @@ -112,23 +113,19 @@ private void doFilterWithTransaction( FilterChain filterChain, final @Nullable TransactionContext transactionContext) throws IOException, ServletException { - @Nullable ITransaction transaction = null; - if (isAsyncDispatch(httpRequest)) { - transaction = (ITransaction) httpRequest.getAttribute(TRANSACTION_ATTR); - } else { - // at this stage we are not able to get real transaction name - transaction = startTransaction(httpRequest, transactionContext); - httpRequest.setAttribute(TRANSACTION_ATTR, transaction); - } + final @Nullable ITransaction transaction = + getOrStartTransaction(httpRequest, transactionContext); try { filterChain.doFilter(httpRequest, httpResponse); } catch (Throwable e) { - // exceptions that are not handled by Spring - transaction.setStatus(SpanStatus.INTERNAL_ERROR); + if (transaction != null) { + // exceptions that are not handled by Spring + transaction.setStatus(SpanStatus.INTERNAL_ERROR); + } throw e; } finally { - if (!isAsyncStarted(httpRequest)) { + if (shouldFinishTransaction(httpRequest) && transaction != null) { // after all filters run, templated path pattern is available in request attribute final String transactionName = transactionNameProvider.provideTransactionName(httpRequest); final TransactionNameSource transactionNameSource = @@ -149,6 +146,49 @@ private void doFilterWithTransaction( } } + private ITransaction getOrStartTransaction( + final @NotNull HttpServletRequest httpRequest, + final @Nullable TransactionContext transactionContext) { + if (isAsyncDispatch(httpRequest)) { + // second invocation of this filter for the same async request already has the transaction + // in the attributes + return (ITransaction) httpRequest.getAttribute(TRANSACTION_ATTR); + } else { + // at this stage we are not able to get real transaction name + final @NotNull ITransaction transaction = startTransaction(httpRequest, transactionContext); + if (shouldStoreTransactionForAsyncProcessing()) { + httpRequest.setAttribute(TRANSACTION_ATTR, transaction); + } + return transaction; + } + } + + /** + * Returns false if an async request is being dispatched (first invocation of the filter for the + * same async request). + * + *

Returns true if not an async request or this is the second invocation of the filter for the + * same async request + */ + private boolean shouldContinueTrace(HttpServletRequest httpRequest) { + return !isAsyncDispatch(httpRequest); + } + + private boolean shouldStoreTransactionForAsyncProcessing() { + return true; + } + + /** + * Returns false if async request handling has only been started but not yet finished (first + * invocation of this filter for the same async request). + * + *

Returns true if not an async request or async request handling has finished (second + * invocation of this filter for the same async request) + */ + private boolean shouldFinishTransaction(HttpServletRequest httpRequest) { + return !isAsyncStarted(httpRequest); + } + private boolean shouldTraceRequest(final @NotNull HttpServletRequest request) { return scopes.getOptions().isTraceOptionsRequests() || !HttpMethod.OPTIONS.name().equals(request.getMethod()); From 14a0181529ed06dd349a227c67d314906cc996d9 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 20 Feb 2025 11:59:02 +0100 Subject: [PATCH 3/7] add option to opt into async handling --- .../api/sentry-spring-boot-jakarta.api | 2 ++ .../boot/jakarta/SentryAutoConfiguration.java | 9 ++++-- .../spring/boot/jakarta/SentryProperties.java | 12 +++++++ .../api/sentry-spring-jakarta.api | 1 + .../jakarta/tracing/SentryTracingFilter.java | 32 +++++++++++++++---- 5 files changed, 47 insertions(+), 9 deletions(-) diff --git a/sentry-spring-boot-jakarta/api/sentry-spring-boot-jakarta.api b/sentry-spring-boot-jakarta/api/sentry-spring-boot-jakarta.api index ac89da6d379..cec2df120b5 100644 --- a/sentry-spring-boot-jakarta/api/sentry-spring-boot-jakarta.api +++ b/sentry-spring-boot-jakarta/api/sentry-spring-boot-jakarta.api @@ -32,10 +32,12 @@ public class io/sentry/spring/boot/jakarta/SentryProperties : io/sentry/SentryOp public fun getReactive ()Lio/sentry/spring/boot/jakarta/SentryProperties$Reactive; public fun getUserFilterOrder ()Ljava/lang/Integer; public fun isEnableAotCompatibility ()Z + public fun isKeepTransactionsOpenForAsyncResponses ()Z public fun isUseGitCommitIdAsRelease ()Z public fun setEnableAotCompatibility (Z)V public fun setExceptionResolverOrder (I)V public fun setGraphql (Lio/sentry/spring/boot/jakarta/SentryProperties$Graphql;)V + public fun setKeepTransactionsOpenForAsyncResponses (Z)V public fun setLogging (Lio/sentry/spring/boot/jakarta/SentryProperties$Logging;)V public fun setReactive (Lio/sentry/spring/boot/jakarta/SentryProperties$Reactive;)V public fun setUseGitCommitIdAsRelease (Z)V diff --git a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java index 54800b58310..a20e251757e 100644 --- a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java +++ b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryAutoConfiguration.java @@ -310,9 +310,14 @@ static class SentrySecurityConfiguration { @ConditionalOnMissingBean(name = "sentryTracingFilter") public FilterRegistrationBean sentryTracingFilter( final @NotNull IScopes scopes, - final @NotNull TransactionNameProvider transactionNameProvider) { + final @NotNull TransactionNameProvider transactionNameProvider, + final @NotNull SentryProperties sentryProperties) { FilterRegistrationBean filter = - new FilterRegistrationBean<>(new SentryTracingFilter(scopes, transactionNameProvider)); + new FilterRegistrationBean<>( + new SentryTracingFilter( + scopes, + transactionNameProvider, + sentryProperties.isKeepTransactionsOpenForAsyncResponses())); filter.setOrder(SENTRY_SPRING_FILTER_PRECEDENCE + 1); // must run after SentrySpringFilter return filter; } diff --git a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java index 80ea79932ca..b0778134bd4 100644 --- a/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java +++ b/sentry-spring-boot-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryProperties.java @@ -5,6 +5,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.slf4j.event.Level; @@ -28,6 +29,8 @@ public class SentryProperties extends SentryOptions { */ private @Nullable Integer userFilterOrder; + @ApiStatus.Experimental private boolean keepTransactionsOpenForAsyncResponses = false; + /** Logging framework integration properties. */ private @NotNull Logging logging = new Logging(); @@ -104,6 +107,15 @@ public void setEnableAotCompatibility(boolean enableAotCompatibility) { this.enableAotCompatibility = enableAotCompatibility; } + public boolean isKeepTransactionsOpenForAsyncResponses() { + return keepTransactionsOpenForAsyncResponses; + } + + public void setKeepTransactionsOpenForAsyncResponses( + boolean keepTransactionsOpenForAsyncResponses) { + this.keepTransactionsOpenForAsyncResponses = keepTransactionsOpenForAsyncResponses; + } + public @NotNull Graphql getGraphql() { return graphql; } diff --git a/sentry-spring-jakarta/api/sentry-spring-jakarta.api b/sentry-spring-jakarta/api/sentry-spring-jakarta.api index db0c3fa456e..48aa8f2b0e9 100644 --- a/sentry-spring-jakarta/api/sentry-spring-jakarta.api +++ b/sentry-spring-jakarta/api/sentry-spring-jakarta.api @@ -257,6 +257,7 @@ public class io/sentry/spring/jakarta/tracing/SentryTracingFilter : org/springfr public fun ()V public fun (Lio/sentry/IScopes;)V public fun (Lio/sentry/IScopes;Lio/sentry/spring/jakarta/tracing/TransactionNameProvider;)V + public fun (Lio/sentry/IScopes;Lio/sentry/spring/jakarta/tracing/TransactionNameProvider;Z)V protected fun doFilterInternal (Ljakarta/servlet/http/HttpServletRequest;Ljakarta/servlet/http/HttpServletResponse;Ljakarta/servlet/FilterChain;)V protected fun shouldNotFilterAsyncDispatch ()Z } 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 dde43d2f0ef..f2559a01860 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 @@ -41,6 +41,7 @@ public class SentryTracingFilter extends OncePerRequestFilter { private final @NotNull TransactionNameProvider transactionNameProvider; private final @NotNull IScopes scopes; + private final boolean isAsyncSupportEnabled; /** * Creates filter that resolves transaction name using {@link SpringMvcTransactionNameProvider}. @@ -54,9 +55,17 @@ public SentryTracingFilter() { this(ScopesAdapter.getInstance()); } - @Override - protected boolean shouldNotFilterAsyncDispatch() { - return false; + /** + * Creates filter that resolves transaction name using transaction name provider given by + * parameter. + * + * @param scopes - the scopes + * @param transactionNameProvider - transaction name provider. + */ + public SentryTracingFilter( + final @NotNull IScopes scopes, + final @NotNull TransactionNameProvider transactionNameProvider) { + this(scopes, transactionNameProvider, false); } /** @@ -65,19 +74,28 @@ protected boolean shouldNotFilterAsyncDispatch() { * * @param scopes - the scopes * @param transactionNameProvider - transaction name provider. + * @param isAsyncSupportEnabled - whether transactions should be kept open until async handling is + * done */ public SentryTracingFilter( final @NotNull IScopes scopes, - final @NotNull TransactionNameProvider transactionNameProvider) { + final @NotNull TransactionNameProvider transactionNameProvider, + final boolean isAsyncSupportEnabled) { this.scopes = Objects.requireNonNull(scopes, "Scopes are required"); this.transactionNameProvider = Objects.requireNonNull(transactionNameProvider, "transactionNameProvider is required"); + this.isAsyncSupportEnabled = isAsyncSupportEnabled; } public SentryTracingFilter(final @NotNull IScopes scopes) { this(scopes, new SpringMvcTransactionNameProvider()); } + @Override + protected boolean shouldNotFilterAsyncDispatch() { + return !isAsyncSupportEnabled; + } + @Override protected void doFilterInternal( final @NotNull HttpServletRequest httpRequest, @@ -171,11 +189,11 @@ private ITransaction getOrStartTransaction( * same async request */ private boolean shouldContinueTrace(HttpServletRequest httpRequest) { - return !isAsyncDispatch(httpRequest); + return !isAsyncSupportEnabled || !isAsyncDispatch(httpRequest); } private boolean shouldStoreTransactionForAsyncProcessing() { - return true; + return isAsyncSupportEnabled; } /** @@ -186,7 +204,7 @@ private boolean shouldStoreTransactionForAsyncProcessing() { * invocation of this filter for the same async request) */ private boolean shouldFinishTransaction(HttpServletRequest httpRequest) { - return !isAsyncStarted(httpRequest); + return !isAsyncSupportEnabled || !isAsyncStarted(httpRequest); } private boolean shouldTraceRequest(final @NotNull HttpServletRequest request) { From ec9f830db85faba2e740e957856c5b0e4b61f985 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Thu, 20 Feb 2025 12:00:04 +0100 Subject: [PATCH 4/7] Copy changes to Spring Boot 2 --- sentry-spring-boot/api/sentry-spring-boot.api | 2 + .../spring/boot/SentryAutoConfiguration.java | 9 +- .../sentry/spring/boot/SentryProperties.java | 12 +++ sentry-spring/api/sentry-spring.api | 1 + .../spring/tracing/SentryTracingFilter.java | 98 +++++++++++++++---- 5 files changed, 100 insertions(+), 22 deletions(-) diff --git a/sentry-spring-boot/api/sentry-spring-boot.api b/sentry-spring-boot/api/sentry-spring-boot.api index 79b72bfb39f..b3ce0896e38 100644 --- a/sentry-spring-boot/api/sentry-spring-boot.api +++ b/sentry-spring-boot/api/sentry-spring-boot.api @@ -30,9 +30,11 @@ public class io/sentry/spring/boot/SentryProperties : io/sentry/SentryOptions { public fun getGraphql ()Lio/sentry/spring/boot/SentryProperties$Graphql; public fun getLogging ()Lio/sentry/spring/boot/SentryProperties$Logging; public fun getUserFilterOrder ()Ljava/lang/Integer; + public fun isKeepTransactionsOpenForAsyncResponses ()Z public fun isUseGitCommitIdAsRelease ()Z public fun setExceptionResolverOrder (I)V public fun setGraphql (Lio/sentry/spring/boot/SentryProperties$Graphql;)V + public fun setKeepTransactionsOpenForAsyncResponses (Z)V public fun setLogging (Lio/sentry/spring/boot/SentryProperties$Logging;)V public fun setUseGitCommitIdAsRelease (Z)V public fun setUserFilterOrder (Ljava/lang/Integer;)V diff --git a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java index 7476b756dd6..f2f93440d08 100644 --- a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java +++ b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryAutoConfiguration.java @@ -295,9 +295,14 @@ static class SentrySecurityConfiguration { @ConditionalOnMissingBean(name = "sentryTracingFilter") public FilterRegistrationBean sentryTracingFilter( final @NotNull IScopes scopes, - final @NotNull TransactionNameProvider transactionNameProvider) { + final @NotNull TransactionNameProvider transactionNameProvider, + final @NotNull SentryProperties sentryProperties) { FilterRegistrationBean filter = - new FilterRegistrationBean<>(new SentryTracingFilter(scopes, transactionNameProvider)); + new FilterRegistrationBean<>( + new SentryTracingFilter( + scopes, + transactionNameProvider, + sentryProperties.isKeepTransactionsOpenForAsyncResponses())); filter.setOrder(SENTRY_SPRING_FILTER_PRECEDENCE + 1); // must run after SentrySpringFilter return filter; } diff --git a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java index 334e36f4024..e40700b6b63 100644 --- a/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java +++ b/sentry-spring-boot/src/main/java/io/sentry/spring/boot/SentryProperties.java @@ -5,6 +5,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.slf4j.event.Level; @@ -28,6 +29,8 @@ public class SentryProperties extends SentryOptions { */ private @Nullable Integer userFilterOrder; + @ApiStatus.Experimental private boolean keepTransactionsOpenForAsyncResponses = false; + /** Logging framework integration properties. */ private @NotNull Logging logging = new Logging(); @@ -70,6 +73,15 @@ public void setUserFilterOrder(final @Nullable Integer userFilterOrder) { this.userFilterOrder = userFilterOrder; } + public boolean isKeepTransactionsOpenForAsyncResponses() { + return keepTransactionsOpenForAsyncResponses; + } + + public void setKeepTransactionsOpenForAsyncResponses( + boolean keepTransactionsOpenForAsyncResponses) { + this.keepTransactionsOpenForAsyncResponses = keepTransactionsOpenForAsyncResponses; + } + public @NotNull Logging getLogging() { return logging; } diff --git a/sentry-spring/api/sentry-spring.api b/sentry-spring/api/sentry-spring.api index 85150716fa6..07880430c28 100644 --- a/sentry-spring/api/sentry-spring.api +++ b/sentry-spring/api/sentry-spring.api @@ -248,6 +248,7 @@ public class io/sentry/spring/tracing/SentryTracingFilter : org/springframework/ public fun ()V public fun (Lio/sentry/IScopes;)V public fun (Lio/sentry/IScopes;Lio/sentry/spring/tracing/TransactionNameProvider;)V + public fun (Lio/sentry/IScopes;Lio/sentry/spring/tracing/TransactionNameProvider;Z)V protected fun doFilterInternal (Ljavax/servlet/http/HttpServletRequest;Ljavax/servlet/http/HttpServletResponse;Ljavax/servlet/FilterChain;)V protected fun shouldNotFilterAsyncDispatch ()Z } 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 d438ba89bd8..5310c5d7c22 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 @@ -34,11 +34,11 @@ public class SentryTracingFilter extends OncePerRequestFilter { private static final String TRANSACTION_OP = "http.server"; private static final String TRACE_ORIGIN = "auto.http.spring.webmvc"; - private static final String TRANSACTION_ATTR = - SentryTracingFilter.class.getName() + ".transaction"; + private static final String TRANSACTION_ATTR = "sentry.transaction"; private final @NotNull TransactionNameProvider transactionNameProvider; private final @NotNull IScopes scopes; + private final boolean isAsyncSupportEnabled; /** * Creates filter that resolves transaction name using {@link SpringMvcTransactionNameProvider}. @@ -62,9 +62,26 @@ public SentryTracingFilter() { public SentryTracingFilter( final @NotNull IScopes scopes, final @NotNull TransactionNameProvider transactionNameProvider) { + this(scopes, transactionNameProvider, false); + } + + /** + * Creates filter that resolves transaction name using transaction name provider given by + * parameter. + * + * @param scopes - the scopes + * @param transactionNameProvider - transaction name provider. + * @param isAsyncSupportEnabled - whether transactions should be kept open until async handling is + * done + */ + public SentryTracingFilter( + final @NotNull IScopes scopes, + final @NotNull TransactionNameProvider transactionNameProvider, + final boolean isAsyncSupportEnabled) { this.scopes = Objects.requireNonNull(scopes, "Scopes are required"); this.transactionNameProvider = Objects.requireNonNull(transactionNameProvider, "transactionNameProvider is required"); + this.isAsyncSupportEnabled = isAsyncSupportEnabled; } public SentryTracingFilter(final @NotNull IScopes scopes) { @@ -73,7 +90,7 @@ public SentryTracingFilter(final @NotNull IScopes scopes) { @Override protected boolean shouldNotFilterAsyncDispatch() { - return false; + return !isAsyncSupportEnabled; } @Override @@ -84,12 +101,14 @@ protected void doFilterInternal( throws ServletException, IOException { if (scopes.isEnabled() && !isIgnored()) { - final @Nullable String sentryTraceHeader = - httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); - final @Nullable List baggageHeader = - Collections.list(httpRequest.getHeaders(BaggageHeader.BAGGAGE_HEADER)); - final @Nullable TransactionContext transactionContext = - scopes.continueTrace(sentryTraceHeader, baggageHeader); + @Nullable TransactionContext transactionContext = null; + if (shouldContinueTrace(httpRequest)) { + final @Nullable String sentryTraceHeader = + httpRequest.getHeader(SentryTraceHeader.SENTRY_TRACE_HEADER); + final @Nullable List baggageHeader = + Collections.list(httpRequest.getHeaders(BaggageHeader.BAGGAGE_HEADER)); + transactionContext = scopes.continueTrace(sentryTraceHeader, baggageHeader); + } if (scopes.getOptions().isTracingEnabled() && shouldTraceRequest(httpRequest)) { doFilterWithTransaction(httpRequest, httpResponse, filterChain, transactionContext); @@ -111,23 +130,19 @@ private void doFilterWithTransaction( FilterChain filterChain, final @Nullable TransactionContext transactionContext) throws IOException, ServletException { - @Nullable ITransaction transaction; - if (isAsyncDispatch(httpRequest)) { - transaction = (ITransaction) httpRequest.getAttribute(TRANSACTION_ATTR); - } else { - // at this stage we are not able to get real transaction name - transaction = startTransaction(httpRequest, transactionContext); - httpRequest.setAttribute(TRANSACTION_ATTR, transaction); - } + final @Nullable ITransaction transaction = + getOrStartTransaction(httpRequest, transactionContext); try { filterChain.doFilter(httpRequest, httpResponse); } catch (Throwable e) { - // exceptions that are not handled by Spring - transaction.setStatus(SpanStatus.INTERNAL_ERROR); + if (transaction != null) { + // exceptions that are not handled by Spring + transaction.setStatus(SpanStatus.INTERNAL_ERROR); + } throw e; } finally { - if (!isAsyncStarted(httpRequest)) { + if (shouldFinishTransaction(httpRequest) && transaction != null) { // after all filters run, templated path pattern is available in request attribute final String transactionName = transactionNameProvider.provideTransactionName(httpRequest); final TransactionNameSource transactionNameSource = @@ -148,6 +163,49 @@ private void doFilterWithTransaction( } } + private ITransaction getOrStartTransaction( + final @NotNull HttpServletRequest httpRequest, + final @Nullable TransactionContext transactionContext) { + if (isAsyncDispatch(httpRequest)) { + // second invocation of this filter for the same async request already has the transaction + // in the attributes + return (ITransaction) httpRequest.getAttribute(TRANSACTION_ATTR); + } else { + // at this stage we are not able to get real transaction name + final @NotNull ITransaction transaction = startTransaction(httpRequest, transactionContext); + if (shouldStoreTransactionForAsyncProcessing()) { + httpRequest.setAttribute(TRANSACTION_ATTR, transaction); + } + return transaction; + } + } + + /** + * Returns false if an async request is being dispatched (first invocation of the filter for the + * same async request). + * + *

Returns true if not an async request or this is the second invocation of the filter for the + * same async request + */ + private boolean shouldContinueTrace(HttpServletRequest httpRequest) { + return !isAsyncSupportEnabled || !isAsyncDispatch(httpRequest); + } + + private boolean shouldStoreTransactionForAsyncProcessing() { + return isAsyncSupportEnabled; + } + + /** + * Returns false if async request handling has only been started but not yet finished (first + * invocation of this filter for the same async request). + * + *

Returns true if not an async request or async request handling has finished (second + * invocation of this filter for the same async request) + */ + private boolean shouldFinishTransaction(HttpServletRequest httpRequest) { + return !isAsyncSupportEnabled || !isAsyncStarted(httpRequest); + } + private boolean shouldTraceRequest(final @NotNull HttpServletRequest request) { return scopes.getOptions().isTraceOptionsRequests() || !HttpMethod.OPTIONS.name().equals(request.getMethod()); From a8ad5f11d8a1deb37ee33f2f09474c5afc67db56 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 24 Feb 2025 16:25:15 +0100 Subject: [PATCH 5/7] add tests; fix comments --- .../jakarta/tracing/SentryTracingFilter.java | 4 +- .../tracing/SentryTracingFilterTest.kt | 107 +++++++++++++++++- .../spring/tracing/SentryTracingFilter.java | 4 +- .../spring/tracing/SentryTracingFilterTest.kt | 107 +++++++++++++++++- 4 files changed, 214 insertions(+), 8 deletions(-) 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 f2559a01860..2046199aa7a 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 @@ -182,10 +182,10 @@ private ITransaction getOrStartTransaction( } /** - * Returns false if an async request is being dispatched (first invocation of the filter for the + * Returns false if an async request is being dispatched (second invocation of the filter for the * same async request). * - *

Returns true if not an async request or this is the second invocation of the filter for the + *

Returns true if not an async request or this is the first invocation of the filter for the * same async request */ private boolean shouldContinueTrace(HttpServletRequest httpRequest) { 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 ff0020ddfeb..14bf0969035 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 @@ -13,12 +13,15 @@ import io.sentry.TransactionOptions import io.sentry.protocol.SentryId import io.sentry.protocol.SentryTransaction import io.sentry.protocol.TransactionNameSource +import jakarta.servlet.DispatcherType import jakarta.servlet.FilterChain import jakarta.servlet.http.HttpServletRequest import org.assertj.core.api.Assertions.assertThat +import org.mockito.Mockito import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -29,6 +32,8 @@ import org.mockito.kotlin.whenever import org.springframework.http.HttpMethod import org.springframework.mock.web.MockHttpServletRequest import org.springframework.mock.web.MockHttpServletResponse +import org.springframework.web.context.request.async.AsyncWebRequest +import org.springframework.web.context.request.async.WebAsyncUtils import org.springframework.web.servlet.HandlerMapping import kotlin.test.Test import kotlin.test.assertEquals @@ -47,13 +52,14 @@ class SentryTracingFilterTest { dsn = "https://key@sentry.io/proj" tracesSampleRate = 1.0 } + val asyncRequest = mock() val logger = mock() init { whenever(scopes.options).thenReturn(options) } - fun getSut(isEnabled: Boolean = true, status: Int = 200, sentryTraceHeader: String? = null, baggageHeaders: List? = null): SentryTracingFilter { + fun getSut(isEnabled: Boolean = true, status: Int = 200, sentryTraceHeader: String? = null, baggageHeaders: List? = null, isAsyncSupportEnabled: Boolean = false): SentryTracingFilter { request.requestURI = "/product/12" request.method = "POST" request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/product/{id}") @@ -70,7 +76,7 @@ class SentryTracingFilterTest { whenever(scopes.startTransaction(any(), check { assertTrue(it.isBindToScope) })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, scopes) } whenever(scopes.isEnabled).thenReturn(isEnabled) whenever(scopes.continueTrace(any(), any())).thenAnswer { TransactionContext.fromPropagationContext(PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?)) } - return SentryTracingFilter(scopes, transactionNameProvider) + return SentryTracingFilter(scopes, transactionNameProvider, isAsyncSupportEnabled) } } @@ -307,4 +313,101 @@ class SentryTracingFilterTest { anyOrNull() ) } + + @Test + fun `creates transaction around async request`() { + val sentryTrace = "f9118105af4a2d42b4124532cd1065ff-424cffc8f94feeee-1" + val baggage = listOf("baggage: sentry-environment=production,sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rand=0.456789,sentry-sample_rate=0.5,sentry-sampled=true,sentry-trace_id=df71f5972f754b4c85af13ff5c07017d") + val filter = fixture.getSut(sentryTraceHeader = sentryTrace, baggageHeaders = baggage, isAsyncSupportEnabled = true) + + val asyncChain = mock() + doAnswer { + val request = it.arguments.first() as MockHttpServletRequest + whenever(fixture.asyncRequest.isAsyncStarted).thenReturn(true) + WebAsyncUtils.getAsyncManager(request).setAsyncWebRequest(fixture.asyncRequest) + }.whenever(asyncChain).doFilter(any(), any()) + + filter.doFilter(fixture.request, fixture.response, asyncChain) + + verify(fixture.scopes).continueTrace(eq(sentryTrace), eq(baggage)) + verify(fixture.scopes).startTransaction( + check { + assertEquals("POST /product/12", it.name) + assertEquals(TransactionNameSource.URL, it.transactionNameSource) + assertEquals("http.server", it.operation) + }, + check { + assertNotNull(it.customSamplingContext?.get("request")) + assertTrue(it.customSamplingContext?.get("request") is HttpServletRequest) + assertTrue(it.isBindToScope) + assertThat(it.origin).isEqualTo("auto.http.spring_jakarta.webmvc") + } + ) + verify(asyncChain).doFilter(fixture.request, fixture.response) + verify(fixture.scopes, never()).captureTransaction( + any(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + + Mockito.clearInvocations(fixture.scopes) + + fixture.request.dispatcherType = DispatcherType.ASYNC + whenever(fixture.asyncRequest.isAsyncStarted).thenReturn(false) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.scopes, never()).startTransaction(anyOrNull(), anyOrNull()) + + verify(fixture.chain).doFilter(fixture.request, fixture.response) + + verify(fixture.scopes).captureTransaction( + check { + assertThat(it.transaction).isEqualTo("POST /product/{id}") + assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.OK) + assertThat(it.contexts.trace!!.operation).isEqualTo("http.server") + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + verify(fixture.scopes, never()).continueTrace(anyOrNull(), anyOrNull()) + } + + @Test + fun `creates and finishes transaction immediately for async request if handling disabled`() { + val sentryTrace = "f9118105af4a2d42b4124532cd1065ff-424cffc8f94feeee-1" + val baggage = listOf("baggage: sentry-environment=production,sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rand=0.456789,sentry-sample_rate=0.5,sentry-sampled=true,sentry-trace_id=df71f5972f754b4c85af13ff5c07017d") + val filter = fixture.getSut(sentryTraceHeader = sentryTrace, baggageHeaders = baggage, isAsyncSupportEnabled = false) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.scopes).startTransaction( + check { + assertEquals("POST /product/12", it.name) + assertEquals(TransactionNameSource.URL, it.transactionNameSource) + assertEquals("http.server", it.operation) + }, + check { + assertNotNull(it.customSamplingContext?.get("request")) + assertTrue(it.customSamplingContext?.get("request") is HttpServletRequest) + assertTrue(it.isBindToScope) + assertThat(it.origin).isEqualTo("auto.http.spring_jakarta.webmvc") + } + ) + verify(fixture.scopes).continueTrace(eq(sentryTrace), eq(baggage)) + verify(fixture.chain).doFilter(fixture.request, fixture.response) + + verify(fixture.scopes).captureTransaction( + check { + assertThat(it.transaction).isEqualTo("POST /product/{id}") + assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.OK) + assertThat(it.contexts.trace!!.operation).isEqualTo("http.server") + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } } 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 5310c5d7c22..035e84359ca 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 @@ -181,10 +181,10 @@ private ITransaction getOrStartTransaction( } /** - * Returns false if an async request is being dispatched (first invocation of the filter for the + * Returns false if an async request is being dispatched (second invocation of the filter for the * same async request). * - *

Returns true if not an async request or this is the second invocation of the filter for the + *

Returns true if not an async request or this is the first invocation of the filter for the * same async request */ private boolean shouldContinueTrace(HttpServletRequest httpRequest) { 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 538ac7c8cc6..9270d9b0c8a 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 @@ -14,9 +14,11 @@ 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.Mockito import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -27,7 +29,10 @@ import org.mockito.kotlin.whenever import org.springframework.http.HttpMethod import org.springframework.mock.web.MockHttpServletRequest import org.springframework.mock.web.MockHttpServletResponse +import org.springframework.web.context.request.async.AsyncWebRequest +import org.springframework.web.context.request.async.WebAsyncUtils import org.springframework.web.servlet.HandlerMapping +import javax.servlet.DispatcherType import javax.servlet.FilterChain import javax.servlet.http.HttpServletRequest import kotlin.test.Test @@ -48,12 +53,13 @@ class SentryTracingFilterTest { tracesSampleRate = 1.0 } val logger = mock() + val asyncRequest = mock() init { whenever(scopes.options).thenReturn(options) } - fun getSut(isEnabled: Boolean = true, status: Int = 200, sentryTraceHeader: String? = null, baggageHeaders: List? = null): SentryTracingFilter { + fun getSut(isEnabled: Boolean = true, status: Int = 200, sentryTraceHeader: String? = null, baggageHeaders: List? = null, isAsyncSupportEnabled: Boolean = false): SentryTracingFilter { request.requestURI = "/product/12" request.method = "POST" request.setAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, "/product/{id}") @@ -70,7 +76,7 @@ class SentryTracingFilterTest { whenever(scopes.startTransaction(any(), check { assertTrue(it.isBindToScope) })).thenAnswer { SentryTracer(it.arguments[0] as TransactionContext, scopes) } whenever(scopes.isEnabled).thenReturn(isEnabled) whenever(scopes.continueTrace(any(), any())).thenAnswer { TransactionContext.fromPropagationContext(PropagationContext.fromHeaders(logger, it.arguments[0] as String?, it.arguments[1] as List?)) } - return SentryTracingFilter(scopes, transactionNameProvider) + return SentryTracingFilter(scopes, transactionNameProvider, isAsyncSupportEnabled) } } @@ -307,4 +313,101 @@ class SentryTracingFilterTest { anyOrNull() ) } + + @Test + fun `creates transaction around async request`() { + val sentryTrace = "f9118105af4a2d42b4124532cd1065ff-424cffc8f94feeee-1" + val baggage = listOf("baggage: sentry-environment=production,sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rand=0.456789,sentry-sample_rate=0.5,sentry-sampled=true,sentry-trace_id=df71f5972f754b4c85af13ff5c07017d") + val filter = fixture.getSut(sentryTraceHeader = sentryTrace, baggageHeaders = baggage, isAsyncSupportEnabled = true) + + val asyncChain = mock() + doAnswer { + val request = it.arguments.first() as MockHttpServletRequest + whenever(fixture.asyncRequest.isAsyncStarted).thenReturn(true) + WebAsyncUtils.getAsyncManager(request).setAsyncWebRequest(fixture.asyncRequest) + }.whenever(asyncChain).doFilter(any(), any()) + + filter.doFilter(fixture.request, fixture.response, asyncChain) + + verify(fixture.scopes).continueTrace(eq(sentryTrace), eq(baggage)) + verify(fixture.scopes).startTransaction( + check { + assertEquals("POST /product/12", it.name) + assertEquals(TransactionNameSource.URL, it.transactionNameSource) + assertEquals("http.server", it.operation) + }, + check { + assertNotNull(it.customSamplingContext?.get("request")) + assertTrue(it.customSamplingContext?.get("request") is HttpServletRequest) + assertTrue(it.isBindToScope) + assertThat(it.origin).isEqualTo("auto.http.spring.webmvc") + } + ) + verify(asyncChain).doFilter(fixture.request, fixture.response) + verify(fixture.scopes, never()).captureTransaction( + any(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + + Mockito.clearInvocations(fixture.scopes) + + fixture.request.dispatcherType = DispatcherType.ASYNC + whenever(fixture.asyncRequest.isAsyncStarted).thenReturn(false) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.scopes, never()).startTransaction(anyOrNull(), anyOrNull()) + + verify(fixture.chain).doFilter(fixture.request, fixture.response) + + verify(fixture.scopes).captureTransaction( + check { + assertThat(it.transaction).isEqualTo("POST /product/{id}") + assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.OK) + assertThat(it.contexts.trace!!.operation).isEqualTo("http.server") + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + verify(fixture.scopes, never()).continueTrace(anyOrNull(), anyOrNull()) + } + + @Test + fun `creates and finishes transaction immediately for async request if handling disabled`() { + val sentryTrace = "f9118105af4a2d42b4124532cd1065ff-424cffc8f94feeee-1" + val baggage = listOf("baggage: sentry-environment=production,sentry-public_key=502f25099c204a2fbf4cb16edc5975d1,sentry-sample_rand=0.456789,sentry-sample_rate=0.5,sentry-sampled=true,sentry-trace_id=df71f5972f754b4c85af13ff5c07017d") + val filter = fixture.getSut(sentryTraceHeader = sentryTrace, baggageHeaders = baggage, isAsyncSupportEnabled = false) + + filter.doFilter(fixture.request, fixture.response, fixture.chain) + + verify(fixture.scopes).startTransaction( + check { + assertEquals("POST /product/12", it.name) + assertEquals(TransactionNameSource.URL, it.transactionNameSource) + assertEquals("http.server", it.operation) + }, + check { + assertNotNull(it.customSamplingContext?.get("request")) + assertTrue(it.customSamplingContext?.get("request") is HttpServletRequest) + assertTrue(it.isBindToScope) + assertThat(it.origin).isEqualTo("auto.http.spring.webmvc") + } + ) + verify(fixture.scopes).continueTrace(eq(sentryTrace), eq(baggage)) + verify(fixture.chain).doFilter(fixture.request, fixture.response) + + verify(fixture.scopes).captureTransaction( + check { + assertThat(it.transaction).isEqualTo("POST /product/{id}") + assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.OK) + assertThat(it.contexts.trace!!.operation).isEqualTo("http.server") + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } } From dff9b24fe1c0b9cbc3b024fa4bf6f359920de09f Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 24 Feb 2025 16:31:20 +0100 Subject: [PATCH 6/7] additional comment --- .../io/sentry/spring/jakarta/tracing/SentryTracingFilter.java | 3 +++ .../java/io/sentry/spring/tracing/SentryTracingFilter.java | 3 +++ 2 files changed, 6 insertions(+) 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 2046199aa7a..f740ce4e612 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 @@ -202,6 +202,9 @@ private boolean shouldStoreTransactionForAsyncProcessing() { * *

Returns true if not an async request or async request handling has finished (second * invocation of this filter for the same async request) + * + *

Note: isAsyncStarted changes its return value after filterChain.doFilter() of the first + * async invocation */ private boolean shouldFinishTransaction(HttpServletRequest httpRequest) { return !isAsyncSupportEnabled || !isAsyncStarted(httpRequest); 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 035e84359ca..6c83eb2fb4f 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 @@ -201,6 +201,9 @@ private boolean shouldStoreTransactionForAsyncProcessing() { * *

Returns true if not an async request or async request handling has finished (second * invocation of this filter for the same async request) + * + *

Note: isAsyncStarted changes its return value after filterChain.doFilter() of the first + * async invocation */ private boolean shouldFinishTransaction(HttpServletRequest httpRequest) { return !isAsyncSupportEnabled || !isAsyncStarted(httpRequest); From f1c8dcac2175b1c4555d30aa14b251418aaf44e9 Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Wed, 26 Feb 2025 11:33:58 +0100 Subject: [PATCH 7/7] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b620355f2d3..44a32be975d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ - Added `enableTraceIdGeneration` to the AndroidOptions. This allows Hybrid SDKs to "freeze" and control the trace and connect errors on different layers of the application ([4188](https://github.com/getsentry/sentry-java/pull/4188)) - Move to a single NetworkCallback listener to reduce number of IPC calls on Android ([#4164](https://github.com/getsentry/sentry-java/pull/4164)) - Add GraphQL Apollo Kotlin 4 integration ([#4166](https://github.com/getsentry/sentry-java/pull/4166)) +- Add support for async dispatch requests to Spring Boot 2 and 3 ([#3983](https://github.com/getsentry/sentry-java/pull/3983)) + - To enable it, please set `sentry.keep-transactions-open-for-async-responses=true` in `application.properties` or `sentry.keepTransactionsOpenForAsyncResponses: true` in `application.yml` ### Fixes