From 14ba7f7e14e057e85a2eb424ae8b6ce159b0c77c Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 10 Mar 2023 09:56:02 +0100 Subject: [PATCH 1/4] Implement performance for WebFlux --- .../src/main/resources/application.properties | 1 + .../src/main/resources/application.properties | 1 + .../sentry-spring-boot-starter-jakarta.api | 1 + .../SentryWebfluxAutoConfiguration.java | 15 ++ .../api/sentry-spring-boot-starter.api | 1 + .../boot/SentryWebfluxAutoConfiguration.java | 15 ++ .../api/sentry-spring-jakarta.api | 5 + .../webflux/SentryWebTracingFilter.java | 123 +++++++++ .../webflux/SentryWebfluxIntegrationTest.kt | 21 ++ sentry-spring/api/sentry-spring.api | 6 + .../spring/webflux/SentryWebFilter.java | 2 + .../webflux/SentryWebTracingFilter.java | 122 +++++++++ .../webflux/SentryWebFluxTracingFilterTest.kt | 251 ++++++++++++++++++ .../webflux/SentryWebfluxIntegrationTest.kt | 21 ++ 14 files changed, 585 insertions(+) create mode 100644 sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebTracingFilter.java create mode 100644 sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebTracingFilter.java create mode 100644 sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt diff --git a/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/src/main/resources/application.properties index 2be0db3b20..c7fb66417b 100644 --- a/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/src/main/resources/application.properties @@ -8,3 +8,4 @@ sentry.max-breadcrumbs=150 sentry.logging.minimum-event-level=info sentry.logging.minimum-breadcrumb-level=debug sentry.reactive.thread-local-accessor-enabled=true +sentry.enable-tracing=false diff --git a/sentry-samples/sentry-samples-spring-boot-webflux/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-webflux/src/main/resources/application.properties index a7107b70f2..a9a9f0f79e 100644 --- a/sentry-samples/sentry-samples-spring-boot-webflux/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-webflux/src/main/resources/application.properties @@ -7,3 +7,4 @@ sentry.max-breadcrumbs=150 # Logback integration configuration options sentry.logging.minimum-event-level=info sentry.logging.minimum-breadcrumb-level=debug +sentry.enable-tracing=true diff --git a/sentry-spring-boot-starter-jakarta/api/sentry-spring-boot-starter-jakarta.api b/sentry-spring-boot-starter-jakarta/api/sentry-spring-boot-starter-jakarta.api index 8674f4ada7..228c31320f 100644 --- a/sentry-spring-boot-starter-jakarta/api/sentry-spring-boot-starter-jakarta.api +++ b/sentry-spring-boot-starter-jakarta/api/sentry-spring-boot-starter-jakarta.api @@ -51,5 +51,6 @@ public class io/sentry/spring/boot/jakarta/SentryProperties$Reactive { public class io/sentry/spring/boot/jakarta/SentryWebfluxAutoConfiguration { public fun ()V public fun sentryWebExceptionHandler (Lio/sentry/IHub;)Lio/sentry/spring/jakarta/webflux/SentryWebExceptionHandler; + public fun sentryWebTracingFilter ()Lio/sentry/spring/jakarta/webflux/SentryWebTracingFilter; } diff --git a/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryWebfluxAutoConfiguration.java b/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryWebfluxAutoConfiguration.java index 3b677d29a2..127de0a0ba 100644 --- a/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryWebfluxAutoConfiguration.java +++ b/sentry-spring-boot-starter-jakarta/src/main/java/io/sentry/spring/boot/jakarta/SentryWebfluxAutoConfiguration.java @@ -6,6 +6,7 @@ import io.sentry.spring.jakarta.webflux.SentryWebExceptionHandler; import io.sentry.spring.jakarta.webflux.SentryWebFilter; import io.sentry.spring.jakarta.webflux.SentryWebFilterWithThreadLocalAccessor; +import io.sentry.spring.jakarta.webflux.SentryWebTracingFilter; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.springframework.boot.ApplicationRunner; @@ -13,12 +14,15 @@ import org.springframework.boot.autoconfigure.condition.AnyNestedCondition; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; import reactor.core.publisher.Hooks; import reactor.core.scheduler.Schedulers; @@ -30,6 +34,7 @@ @Open @ApiStatus.Experimental public class SentryWebfluxAutoConfiguration { + private static final int SENTRY_SPRING_FILTER_PRECEDENCE = Ordered.HIGHEST_PRECEDENCE; @Configuration(proxyBeanMethods = false) @Conditional(SentryThreadLocalAccessorCondition.class) @@ -43,6 +48,7 @@ static class SentryWebfluxFilterThreadLocalAccessorConfiguration { * ThreadLocalAccessor to propagate the Sentry hub. */ @Bean + @Order(SENTRY_SPRING_FILTER_PRECEDENCE) public @NotNull SentryWebFilterWithThreadLocalAccessor sentryWebFilterWithContextPropagation( final @NotNull IHub hub) { Hooks.enableAutomaticContextPropagation(); @@ -65,11 +71,20 @@ static class SentryWebfluxFilterConfiguration { /** Configures a filter that sets up Sentry {@link io.sentry.Scope} for each request. */ @Bean + @Order(SENTRY_SPRING_FILTER_PRECEDENCE) public @NotNull SentryWebFilter sentryWebFilter(final @NotNull IHub hub) { return new SentryWebFilter(hub); } } + @Bean + @Order(SENTRY_SPRING_FILTER_PRECEDENCE + 1) + @Conditional(SentryAutoConfiguration.SentryTracingCondition.class) + @ConditionalOnMissingBean(name = "sentryWebTracingFilter") + public @NotNull SentryWebTracingFilter sentryWebTracingFilter() { + return new SentryWebTracingFilter(); + } + /** Configures exception handler that handles unhandled exceptions and sends them to Sentry. */ @Bean public @NotNull SentryWebExceptionHandler sentryWebExceptionHandler(final @NotNull IHub hub) { diff --git a/sentry-spring-boot-starter/api/sentry-spring-boot-starter.api b/sentry-spring-boot-starter/api/sentry-spring-boot-starter.api index 570a2f57c9..1d4bfb4b21 100644 --- a/sentry-spring-boot-starter/api/sentry-spring-boot-starter.api +++ b/sentry-spring-boot-starter/api/sentry-spring-boot-starter.api @@ -45,5 +45,6 @@ public class io/sentry/spring/boot/SentryWebfluxAutoConfiguration { public fun sentryScheduleHookApplicationRunner ()Lorg/springframework/boot/ApplicationRunner; public fun sentryWebExceptionHandler (Lio/sentry/IHub;)Lio/sentry/spring/webflux/SentryWebExceptionHandler; public fun sentryWebFilter (Lio/sentry/IHub;)Lio/sentry/spring/webflux/SentryWebFilter; + public fun sentryWebTracingFilter ()Lio/sentry/spring/webflux/SentryWebTracingFilter; } diff --git a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryWebfluxAutoConfiguration.java b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryWebfluxAutoConfiguration.java index 7a5998a29a..5fb8ee0068 100644 --- a/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryWebfluxAutoConfiguration.java +++ b/sentry-spring-boot-starter/src/main/java/io/sentry/spring/boot/SentryWebfluxAutoConfiguration.java @@ -5,14 +5,19 @@ import io.sentry.spring.webflux.SentryScheduleHook; import io.sentry.spring.webflux.SentryWebExceptionHandler; import io.sentry.spring.webflux.SentryWebFilter; +import io.sentry.spring.webflux.SentryWebTracingFilter; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.springframework.boot.ApplicationRunner; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; +import org.springframework.core.Ordered; +import org.springframework.core.annotation.Order; import reactor.core.scheduler.Schedulers; /** Configures Sentry integration for Spring Webflux and Project Reactor. */ @@ -23,6 +28,7 @@ @Open @ApiStatus.Experimental public class SentryWebfluxAutoConfiguration { + private static final int SENTRY_SPRING_FILTER_PRECEDENCE = Ordered.HIGHEST_PRECEDENCE; /** Configures hook that sets correct hub on the executing thread. */ @Bean @@ -34,10 +40,19 @@ public class SentryWebfluxAutoConfiguration { /** Configures a filter that sets up Sentry {@link io.sentry.Scope} for each request. */ @Bean + @Order(SENTRY_SPRING_FILTER_PRECEDENCE) public @NotNull SentryWebFilter sentryWebFilter(final @NotNull IHub hub) { return new SentryWebFilter(hub); } + @Bean + @Order(SENTRY_SPRING_FILTER_PRECEDENCE + 1) + @Conditional(SentryAutoConfiguration.SentryTracingCondition.class) + @ConditionalOnMissingBean(name = "sentryWebTracingFilter") + public @NotNull SentryWebTracingFilter sentryWebTracingFilter() { + return new SentryWebTracingFilter(); + } + /** Configures exception handler that handles unhandled exceptions and sends them to Sentry. */ @Bean public @NotNull SentryWebExceptionHandler sentryWebExceptionHandler(final @NotNull IHub hub) { diff --git a/sentry-spring-jakarta/api/sentry-spring-jakarta.api b/sentry-spring-jakarta/api/sentry-spring-jakarta.api index 0a8b5e964a..9de61a26c4 100644 --- a/sentry-spring-jakarta/api/sentry-spring-jakarta.api +++ b/sentry-spring-jakarta/api/sentry-spring-jakarta.api @@ -214,3 +214,8 @@ public final class io/sentry/spring/jakarta/webflux/SentryWebFilterWithThreadLoc public fun filter (Lorg/springframework/web/server/ServerWebExchange;Lorg/springframework/web/server/WebFilterChain;)Lreactor/core/publisher/Mono; } +public class io/sentry/spring/jakarta/webflux/SentryWebTracingFilter : org/springframework/web/server/WebFilter { + public fun ()V + public fun filter (Lorg/springframework/web/server/ServerWebExchange;Lorg/springframework/web/server/WebFilterChain;)Lreactor/core/publisher/Mono; +} + diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebTracingFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebTracingFilter.java new file mode 100644 index 0000000000..a222e67a50 --- /dev/null +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebTracingFilter.java @@ -0,0 +1,123 @@ +package io.sentry.spring.jakarta.webflux; + +import static io.sentry.spring.jakarta.webflux.AbstractSentryWebFilter.SENTRY_HUB_KEY; + +import com.jakewharton.nopen.annotation.Open; +import io.sentry.Baggage; +import io.sentry.BaggageHeader; +import io.sentry.CustomSamplingContext; +import io.sentry.IHub; +import io.sentry.ITransaction; +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 java.util.List; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatusCode; +import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.WebFilter; +import org.springframework.web.server.WebFilterChain; +import reactor.core.publisher.Mono; + +@Open +@ApiStatus.Experimental +public class SentryWebTracingFilter implements WebFilter { + + private static final String TRANSACTION_OP = "http.server"; + + @Override + public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { + final @Nullable Object hubObject = exchange.getAttributes().getOrDefault(SENTRY_HUB_KEY, null); + final @NotNull IHub hub = hubObject == null ? Sentry.getCurrentHub() : (IHub) hubObject; + final @NotNull ServerHttpRequest request = exchange.getRequest(); + + if (hub.isEnabled() && shouldTraceRequest(hub, request)) { + final @NotNull ITransaction transaction = startTransaction(hub, request); + + return chain + .filter(exchange) + .doFinally( + __ -> { + String transactionName = TransactionNameProvider.provideTransactionName(exchange); + if (transactionName != null) { + transaction.setName(transactionName, TransactionNameSource.ROUTE); + transaction.setOperation(TRANSACTION_OP); + } + if (transaction.getStatus() == null) { + final @Nullable ServerHttpResponse response = exchange.getResponse(); + if (response != null) { + final @Nullable HttpStatusCode statusCode = response.getStatusCode(); + if (statusCode != null) { + transaction.setStatus(SpanStatus.fromHttpStatusCode(statusCode.value())); + } + } + } + transaction.finish(); + }) + .doOnError( + e -> { + transaction.setStatus(SpanStatus.INTERNAL_ERROR); + transaction.setThrowable(e); + }); + } else { + return chain.filter(exchange); + } + } + + private boolean shouldTraceRequest( + final @NotNull IHub hub, final @NotNull ServerHttpRequest request) { + return hub.getOptions().isTraceOptionsRequests() + || !HttpMethod.OPTIONS.equals(request.getMethod()); + } + + 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 String name = request.getMethod() + " " + request.getURI().getPath(); + final @NotNull CustomSamplingContext customSamplingContext = new CustomSamplingContext(); + customSamplingContext.set("request", request); + + final TransactionOptions transactionOptions = new TransactionOptions(); + 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()); + } + } + + return hub.startTransaction( + new TransactionContext(name, TransactionNameSource.URL, TRANSACTION_OP), + transactionOptions); + } +} diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebfluxIntegrationTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebfluxIntegrationTest.kt index 2abb45b913..603071e134 100644 --- a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebfluxIntegrationTest.kt +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebfluxIntegrationTest.kt @@ -5,6 +5,7 @@ import io.sentry.IHub import io.sentry.ITransportFactory import io.sentry.Sentry import io.sentry.checkEvent +import io.sentry.checkTransaction import io.sentry.transport.ITransport import org.assertj.core.api.Assertions.assertThat import org.awaitility.kotlin.await @@ -122,6 +123,22 @@ class SentryWebfluxIntegrationTest { ) } } + + @Test + fun `sends transaction`() { + testClient.get() + .uri("http://localhost:$port/hello?param=value#top") + .exchange() + .expectStatus() + .isOk + + verify(transport).send( + checkTransaction { event -> + assertEquals("GET /hello", event.transaction) + }, + anyOrNull() + ) + } } @SpringBootApplication(exclude = [ReactiveSecurityAutoConfiguration::class, SecurityAutoConfiguration::class]) @@ -148,6 +165,9 @@ open class App { @Bean open fun sentryWebExceptionHandler(hub: IHub) = SentryWebExceptionHandler(hub) + @Bean + open fun sentryTracingFilter(hub: IHub) = SentryWebTracingFilter() + @Bean open fun sentryScheduleHookRegistrar() = ApplicationRunner { Schedulers.onScheduleHook("sentry", SentryScheduleHook()) @@ -159,6 +179,7 @@ open class App { it.dsn = "http://key@localhost/proj" it.setDebug(true) it.setTransportFactory(transportFactory) + it.enableTracing = true } } } diff --git a/sentry-spring/api/sentry-spring.api b/sentry-spring/api/sentry-spring.api index 7eedecec7b..d7683e11eb 100644 --- a/sentry-spring/api/sentry-spring.api +++ b/sentry-spring/api/sentry-spring.api @@ -177,7 +177,13 @@ public final class io/sentry/spring/webflux/SentryWebExceptionHandler : org/spri } public final class io/sentry/spring/webflux/SentryWebFilter : org/springframework/web/server/WebFilter { + public static final field SENTRY_HUB_KEY Ljava/lang/String; public fun (Lio/sentry/IHub;)V public fun filter (Lorg/springframework/web/server/ServerWebExchange;Lorg/springframework/web/server/WebFilterChain;)Lreactor/core/publisher/Mono; } +public class io/sentry/spring/webflux/SentryWebTracingFilter : org/springframework/web/server/WebFilter { + public fun ()V + public fun filter (Lorg/springframework/web/server/ServerWebExchange;Lorg/springframework/web/server/WebFilterChain;)Lreactor/core/publisher/Mono; +} + 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 640fd62816..06e4e133c2 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 @@ -21,6 +21,7 @@ /** Manages {@link io.sentry.Scope} in Webflux request processing. */ @ApiStatus.Experimental public final class SentryWebFilter implements WebFilter { + public static final String SENTRY_HUB_KEY = "sentry-hub"; private final @NotNull SentryRequestResolver sentryRequestResolver; @@ -43,6 +44,7 @@ public Mono filter( }) .doFirst( () -> { + serverWebExchange.getAttributes().put(SENTRY_HUB_KEY, requestHub); Sentry.setCurrentHub(requestHub); requestHub.pushScope(); final ServerHttpRequest request = serverWebExchange.getRequest(); diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebTracingFilter.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebTracingFilter.java new file mode 100644 index 0000000000..68384e879c --- /dev/null +++ b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebTracingFilter.java @@ -0,0 +1,122 @@ +package io.sentry.spring.webflux; + +import static io.sentry.spring.webflux.SentryWebFilter.SENTRY_HUB_KEY; + +import com.jakewharton.nopen.annotation.Open; +import io.sentry.Baggage; +import io.sentry.BaggageHeader; +import io.sentry.CustomSamplingContext; +import io.sentry.IHub; +import io.sentry.ITransaction; +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 java.util.List; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.server.WebFilter; +import org.springframework.web.server.WebFilterChain; +import reactor.core.publisher.Mono; + +@Open +@ApiStatus.Experimental +public class SentryWebTracingFilter implements WebFilter { + + private static final String TRANSACTION_OP = "http.server"; + + @Override + public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { + final @Nullable Object hubObject = exchange.getAttributes().getOrDefault(SENTRY_HUB_KEY, null); + final @NotNull IHub hub = hubObject == null ? Sentry.getCurrentHub() : (IHub) hubObject; + final @NotNull ServerHttpRequest request = exchange.getRequest(); + + if (hub.isEnabled() && shouldTraceRequest(hub, request)) { + final @NotNull ITransaction transaction = startTransaction(hub, request); + + return chain + .filter(exchange) + .doFinally( + __ -> { + String transactionName = TransactionNameProvider.provideTransactionName(exchange); + if (transactionName != null) { + transaction.setName(transactionName, TransactionNameSource.ROUTE); + transaction.setOperation(TRANSACTION_OP); + } + if (transaction.getStatus() == null) { + final @Nullable ServerHttpResponse response = exchange.getResponse(); + if (response != null) { + final @Nullable Integer rawStatusCode = response.getRawStatusCode(); + if (rawStatusCode != null) { + transaction.setStatus(SpanStatus.fromHttpStatusCode(rawStatusCode)); + } + } + } + transaction.finish(); + }) + .doOnError( + e -> { + transaction.setStatus(SpanStatus.INTERNAL_ERROR); + transaction.setThrowable(e); + }); + } else { + return chain.filter(exchange); + } + } + + private boolean shouldTraceRequest( + final @NotNull IHub hub, final @NotNull ServerHttpRequest request) { + return hub.getOptions().isTraceOptionsRequests() + || !HttpMethod.OPTIONS.equals(request.getMethod()); + } + + 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 String name = request.getMethod() + " " + request.getURI().getPath(); + final @NotNull CustomSamplingContext customSamplingContext = new CustomSamplingContext(); + customSamplingContext.set("request", request); + + final TransactionOptions transactionOptions = new TransactionOptions(); + 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()); + } + } + + return hub.startTransaction( + new TransactionContext(name, TransactionNameSource.URL, TRANSACTION_OP), + transactionOptions); + } +} 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 new file mode 100644 index 0000000000..72f8d068b5 --- /dev/null +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt @@ -0,0 +1,251 @@ +package io.sentry.spring.webflux + +import io.sentry.IHub +import io.sentry.SentryOptions +import io.sentry.SentryTracer +import io.sentry.SpanId +import io.sentry.SpanStatus +import io.sentry.TraceContext +import io.sentry.TransactionContext +import io.sentry.TransactionOptions +import io.sentry.protocol.SentryId +import io.sentry.protocol.TransactionNameSource +import io.sentry.spring.webflux.SentryWebFilter.SENTRY_HUB_KEY +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.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoMoreInteractions +import org.mockito.kotlin.whenever +import org.springframework.http.HttpMethod +import org.springframework.http.HttpStatus +import org.springframework.http.server.reactive.ServerHttpRequest +import org.springframework.mock.http.server.reactive.MockServerHttpRequest +import org.springframework.mock.web.server.MockServerWebExchange +import org.springframework.web.reactive.HandlerMapping +import org.springframework.web.server.WebFilterChain +import org.springframework.web.util.pattern.PathPatternParser +import reactor.core.publisher.Mono +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue +import kotlin.test.fail + +class SentryWebFluxTracingFilterTest { + private class Fixture { + val hub = mock() + lateinit var request: MockServerHttpRequest + lateinit var exchange: MockServerWebExchange + val chain = mock() + val options = SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + enableTracing = true + } + + init { + whenever(hub.options).thenReturn(options) + } + + fun getSut(isEnabled: Boolean = true, status: HttpStatus = HttpStatus.OK, sentryTraceHeader: String? = null, method: HttpMethod = HttpMethod.POST): SentryWebTracingFilter { + 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) } + } + request = requestBuilder.build() + exchange = MockServerWebExchange.builder(request).build() + exchange.attributes.put(SENTRY_HUB_KEY, hub) + exchange.attributes.put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, PathPatternParser().parse("/product/{id}")) + exchange.response.statusCode = status + 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() }) + return SentryWebTracingFilter() + } + } + + private val fixture = Fixture() + + @Test + fun `creates transaction around the request`() { + val filter = fixture.getSut() + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.hub).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 ServerHttpRequest) + assertTrue(it.isBindToScope) + } + ) + verify(fixture.chain).filter(fixture.exchange) + verify(fixture.hub).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() + ) + } + + @Test + fun `sets correct span status based on the response status`() { + val filter = fixture.getSut(status = HttpStatus.INTERNAL_SERVER_ERROR) + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.hub).captureTransaction( + check { + assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.INTERNAL_ERROR) + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `does not set span status for response status that dont match predefined span statuses`() { + val filter = fixture.getSut(status = HttpStatus.FOUND) + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.hub).captureTransaction( + check { + assertThat(it.contexts.trace!!.status).isNull() + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `when sentry trace is not present, transaction does not have parentSpanId set`() { + val filter = fixture.getSut() + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.hub).captureTransaction( + check { + assertThat(it.contexts.trace!!.parentSpanId).isNull() + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `when sentry trace is present, transaction has parentSpanId set`() { + val parentSpanId = SpanId() + val filter = fixture.getSut(sentryTraceHeader = "${SentryId()}-$parentSpanId-1") + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.hub).captureTransaction( + check { + assertThat(it.contexts.trace!!.parentSpanId).isEqualTo(parentSpanId) + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `when hub is disabled, components are not invoked`() { + val filter = fixture.getSut(isEnabled = false) + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.chain).filter(fixture.exchange) + + verify(fixture.hub).isEnabled + verifyNoMoreInteractions(fixture.hub) + } + + @Test + fun `sets status to internal server error when chain throws exception`() { + val filter = fixture.getSut() + whenever(fixture.chain.filter(any())).thenReturn(Mono.error(RuntimeException("error"))) + + try { + filter.filter(fixture.exchange, fixture.chain).block() + fail("filter is expected to rethrow exception") + } catch (_: Exception) { + } + verify(fixture.hub).captureTransaction( + check { + assertThat(it.status).isEqualTo(SpanStatus.INTERNAL_ERROR) + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `does not track OPTIONS request with traceOptionsRequests=false`() { + val filter = fixture.getSut(method = HttpMethod.OPTIONS) + fixture.options.isTraceOptionsRequests = false + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.chain).filter(fixture.exchange) + + verify(fixture.hub).isEnabled + verify(fixture.hub).options + verifyNoMoreInteractions(fixture.hub) + } + + @Test + fun `tracks OPTIONS request with traceOptionsRequests=true`() { + val filter = fixture.getSut(method = HttpMethod.OPTIONS) + fixture.options.isTraceOptionsRequests = true + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.chain).filter(fixture.exchange) + + verify(fixture.hub).captureTransaction( + check { + assertThat(it.contexts.trace!!.parentSpanId).isNull() + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `tracks POST request with traceOptionsRequests=false`() { + val filter = fixture.getSut(method = HttpMethod.POST) + fixture.options.isTraceOptionsRequests = false + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.chain).filter(fixture.exchange) + + verify(fixture.hub).captureTransaction( + check { + assertThat(it.contexts.trace!!.parentSpanId).isNull() + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } +} diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebfluxIntegrationTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebfluxIntegrationTest.kt index 1ef8cc0f2a..6ac8582941 100644 --- a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebfluxIntegrationTest.kt +++ b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebfluxIntegrationTest.kt @@ -5,6 +5,7 @@ import io.sentry.IHub import io.sentry.ITransportFactory import io.sentry.Sentry import io.sentry.checkEvent +import io.sentry.checkTransaction import io.sentry.transport.ITransport import org.assertj.core.api.Assertions.assertThat import org.awaitility.kotlin.await @@ -122,6 +123,22 @@ class SentryWebfluxIntegrationTest { ) } } + + @Test + fun `sends transaction`() { + testClient.get() + .uri("http://localhost:$port/hello?param=value#top") + .exchange() + .expectStatus() + .isOk + + verify(transport).send( + checkTransaction { event -> + assertEquals("GET /hello", event.transaction) + }, + anyOrNull() + ) + } } @SpringBootApplication(exclude = [ReactiveSecurityAutoConfiguration::class, SecurityAutoConfiguration::class]) @@ -145,6 +162,9 @@ open class App { @Bean open fun sentryFilter(hub: IHub) = SentryWebFilter(hub) + @Bean + open fun sentryTracingFilter(hub: IHub) = SentryWebTracingFilter() + @Bean open fun sentryWebExceptionHandler(hub: IHub) = SentryWebExceptionHandler(hub) @@ -159,6 +179,7 @@ open class App { it.dsn = "http://key@localhost/proj" it.setDebug(true) it.setTransportFactory(transportFactory) + it.enableTracing = true } } } From 418b521123e3fc041eeb07de439d729a3cf4c45e Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 10 Mar 2023 10:05:21 +0100 Subject: [PATCH 2/4] Add changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3baa5d589..4916c70b76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Features - Improve versatility of exception resolver component for Spring with more flexible API for consumers. ([#2577](https://github.com/getsentry/sentry-java/pull/2577)) +- Automatic performance instrumentation for WebFlux ([#2597](https://github.com/getsentry/sentry-java/pull/2597)) + - You can enable it by adding `sentry.enable-tracing=true` to your `application.properties` ### Fixes From 134f7a9c7b15ba76e68ce286888097e1a4b8d19f Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Fri, 10 Mar 2023 10:16:27 +0100 Subject: [PATCH 3/4] Duplicate test into jakarta module --- .../webflux/SentryWebFluxTracingFilterTest.kt | 251 ++++++++++++++++++ 1 file changed, 251 insertions(+) create mode 100644 sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt 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 new file mode 100644 index 0000000000..3944df69e7 --- /dev/null +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt @@ -0,0 +1,251 @@ +package io.sentry.spring.jakarta.webflux + +import io.sentry.IHub +import io.sentry.SentryOptions +import io.sentry.SentryTracer +import io.sentry.SpanId +import io.sentry.SpanStatus +import io.sentry.TraceContext +import io.sentry.TransactionContext +import io.sentry.TransactionOptions +import io.sentry.protocol.SentryId +import io.sentry.protocol.TransactionNameSource +import io.sentry.spring.jakarta.webflux.AbstractSentryWebFilter.SENTRY_HUB_KEY +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.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoMoreInteractions +import org.mockito.kotlin.whenever +import org.springframework.http.HttpMethod +import org.springframework.http.HttpStatus +import org.springframework.http.server.reactive.ServerHttpRequest +import org.springframework.mock.http.server.reactive.MockServerHttpRequest +import org.springframework.mock.web.server.MockServerWebExchange +import org.springframework.web.reactive.HandlerMapping +import org.springframework.web.server.WebFilterChain +import org.springframework.web.util.pattern.PathPatternParser +import reactor.core.publisher.Mono +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue +import kotlin.test.fail + +class SentryWebFluxTracingFilterTest { + private class Fixture { + val hub = mock() + lateinit var request: MockServerHttpRequest + lateinit var exchange: MockServerWebExchange + val chain = mock() + val options = SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + enableTracing = true + } + + init { + whenever(hub.options).thenReturn(options) + } + + fun getSut(isEnabled: Boolean = true, status: HttpStatus = HttpStatus.OK, sentryTraceHeader: String? = null, method: HttpMethod = HttpMethod.POST): SentryWebTracingFilter { + 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) } + } + request = requestBuilder.build() + exchange = MockServerWebExchange.builder(request).build() + exchange.attributes.put(SENTRY_HUB_KEY, hub) + exchange.attributes.put(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE, PathPatternParser().parse("/product/{id}")) + exchange.response.statusCode = status + 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() }) + return SentryWebTracingFilter() + } + } + + private val fixture = Fixture() + + @Test + fun `creates transaction around the request`() { + val filter = fixture.getSut() + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.hub).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 ServerHttpRequest) + assertTrue(it.isBindToScope) + } + ) + verify(fixture.chain).filter(fixture.exchange) + verify(fixture.hub).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() + ) + } + + @Test + fun `sets correct span status based on the response status`() { + val filter = fixture.getSut(status = HttpStatus.INTERNAL_SERVER_ERROR) + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.hub).captureTransaction( + check { + assertThat(it.contexts.trace!!.status).isEqualTo(SpanStatus.INTERNAL_ERROR) + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `does not set span status for response status that dont match predefined span statuses`() { + val filter = fixture.getSut(status = HttpStatus.FOUND) + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.hub).captureTransaction( + check { + assertThat(it.contexts.trace!!.status).isNull() + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `when sentry trace is not present, transaction does not have parentSpanId set`() { + val filter = fixture.getSut() + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.hub).captureTransaction( + check { + assertThat(it.contexts.trace!!.parentSpanId).isNull() + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `when sentry trace is present, transaction has parentSpanId set`() { + val parentSpanId = SpanId() + val filter = fixture.getSut(sentryTraceHeader = "${SentryId()}-$parentSpanId-1") + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.hub).captureTransaction( + check { + assertThat(it.contexts.trace!!.parentSpanId).isEqualTo(parentSpanId) + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `when hub is disabled, components are not invoked`() { + val filter = fixture.getSut(isEnabled = false) + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.chain).filter(fixture.exchange) + + verify(fixture.hub).isEnabled + verifyNoMoreInteractions(fixture.hub) + } + + @Test + fun `sets status to internal server error when chain throws exception`() { + val filter = fixture.getSut() + whenever(fixture.chain.filter(any())).thenReturn(Mono.error(RuntimeException("error"))) + + try { + filter.filter(fixture.exchange, fixture.chain).block() + fail("filter is expected to rethrow exception") + } catch (_: Exception) { + } + verify(fixture.hub).captureTransaction( + check { + assertThat(it.status).isEqualTo(SpanStatus.INTERNAL_ERROR) + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `does not track OPTIONS request with traceOptionsRequests=false`() { + val filter = fixture.getSut(method = HttpMethod.OPTIONS) + fixture.options.isTraceOptionsRequests = false + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.chain).filter(fixture.exchange) + + verify(fixture.hub).isEnabled + verify(fixture.hub).options + verifyNoMoreInteractions(fixture.hub) + } + + @Test + fun `tracks OPTIONS request with traceOptionsRequests=true`() { + val filter = fixture.getSut(method = HttpMethod.OPTIONS) + fixture.options.isTraceOptionsRequests = true + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.chain).filter(fixture.exchange) + + verify(fixture.hub).captureTransaction( + check { + assertThat(it.contexts.trace!!.parentSpanId).isNull() + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } + + @Test + fun `tracks POST request with traceOptionsRequests=false`() { + val filter = fixture.getSut(method = HttpMethod.POST) + fixture.options.isTraceOptionsRequests = false + + filter.filter(fixture.exchange, fixture.chain).block() + + verify(fixture.chain).filter(fixture.exchange) + + verify(fixture.hub).captureTransaction( + check { + assertThat(it.contexts.trace!!.parentSpanId).isNull() + }, + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + } +} From 330abacc6278e2a4d9523a74d704244f4325ef6f Mon Sep 17 00:00:00 2001 From: Alexander Dinauer Date: Mon, 13 Mar 2023 14:33:00 +0100 Subject: [PATCH 4/4] Enable performance for jakarta webflux sample --- .../src/main/resources/application.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/src/main/resources/application.properties b/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/src/main/resources/application.properties index c7fb66417b..cded2b5e60 100644 --- a/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/src/main/resources/application.properties +++ b/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/src/main/resources/application.properties @@ -8,4 +8,4 @@ sentry.max-breadcrumbs=150 sentry.logging.minimum-event-level=info sentry.logging.minimum-breadcrumb-level=debug sentry.reactive.thread-local-accessor-enabled=true -sentry.enable-tracing=false +sentry.enable-tracing=true