diff --git a/CHANGELOG.md b/CHANGELOG.md index d93d346464..1b20c9b910 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,10 +16,12 @@ - Enable the feature by setting `sentry.reactive.thread-local-accessor-enabled=true` - This is still considered experimental. Once we have enough feedback we may turn this on by default. - Checkout the sample here: https://github.com/getsentry/sentry-java/tree/main/sentry-samples/sentry-samples-spring-boot-webflux-jakarta + - A new hub is now cloned from the main hub for every request ### Fixes - Leave `inApp` flag for stack frames undecided in SDK if unsure and let ingestion decide instead ([#2547](https://github.com/getsentry/sentry-java/pull/2547)) +- Use the same hub in WebFlux exception handler as we do in WebFilter ([#2566](https://github.com/getsentry/sentry-java/pull/2566)) ## 6.14.0 diff --git a/sentry-spring-jakarta/build.gradle.kts b/sentry-spring-jakarta/build.gradle.kts index 89a0513ce4..457a15b063 100644 --- a/sentry-spring-jakarta/build.gradle.kts +++ b/sentry-spring-jakarta/build.gradle.kts @@ -70,6 +70,7 @@ dependencies { testImplementation(Config.Libs.springBoot3StarterWebflux) testImplementation(Config.Libs.springBoot3StarterSecurity) testImplementation(Config.Libs.springBoot3StarterAop) + testImplementation(Config.Libs.contextPropagation) testImplementation(Config.TestLibs.awaitility) } diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/ReactorUtils.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/ReactorUtils.java index 956307221e..e2c7f8c962 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/ReactorUtils.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/ReactorUtils.java @@ -13,48 +13,101 @@ public final class ReactorUtils { /** - * Writes the Sentry {@link IHub} to the {@link Context} and uses {@link io.micrometer.context.ThreadLocalAccessor} to propagate it. + * Writes the current Sentry {@link IHub} to the {@link Context} and uses {@link io.micrometer.context.ThreadLocalAccessor} to propagate it. * * This requires * - reactor.core.publisher.Hooks#enableAutomaticContextPropagation() to be enabled - * - having `io.micrometer:context-propagation:1.0.2` or newer as dependency - * - having `io.projectreactor:reactor-core:3.5.3` or newer as dependency + * - having `io.micrometer:context-propagation:1.0.2+` (provided by Spring Boot 3.0.3+) + * - having `io.projectreactor:reactor-core:3.5.3+` (provided by Spring Boot 3.0.3+) */ @ApiStatus.Experimental - public static Mono withSentry(Mono mono) { + public static Mono withSentry(final @NotNull Mono mono) { final @NotNull IHub oldHub = Sentry.getCurrentHub(); final @NotNull IHub clonedHub = oldHub.clone(); + return withSentryHub(mono, clonedHub); + } + + /** + * Writes a new Sentry {@link IHub} cloned from the main hub to the {@link Context} and uses {@link io.micrometer.context.ThreadLocalAccessor} to propagate it. + * + * This requires + * - reactor.core.publisher.Hooks#enableAutomaticContextPropagation() to be enabled + * - having `io.micrometer:context-propagation:1.0.2+` (provided by Spring Boot 3.0.3+) + * - having `io.projectreactor:reactor-core:3.5.3+` (provided by Spring Boot 3.0.3+) + */ + @ApiStatus.Experimental + public static Mono withSentryNewMainHubClone(final @NotNull Mono mono) { + final @NotNull IHub hub = Sentry.cloneMainHub(); + return withSentryHub(mono, hub); + } + /** + * Writes the given Sentry {@link IHub} to the {@link Context} and uses {@link io.micrometer.context.ThreadLocalAccessor} to propagate it. + * + * This requires + * - reactor.core.publisher.Hooks#enableAutomaticContextPropagation() to be enabled + * - having `io.micrometer:context-propagation:1.0.2+` (provided by Spring Boot 3.0.3+) + * - having `io.projectreactor:reactor-core:3.5.3+` (provided by Spring Boot 3.0.3+) + */ + @ApiStatus.Experimental + public static Mono withSentryHub(final @NotNull Mono mono, final @NotNull IHub hub) { /** - * WARNING: Cannot set the clonedHub as current hub. + * WARNING: Cannot set the hub as current. * It would be used by others to clone again causing shared hubs and scopes and thus * leading to issues like unrelated breadcrumbs showing up in events. */ // Sentry.setCurrentHub(clonedHub); - return Mono.deferContextual(ctx -> mono).contextWrite(Context.of(SentryReactorThreadLocalAccessor.KEY, clonedHub)); + return Mono.deferContextual(ctx -> mono).contextWrite(Context.of(SentryReactorThreadLocalAccessor.KEY, hub)); } /** - * Writes the Sentry {@link IHub} to the {@link Context} and uses {@link io.micrometer.context.ThreadLocalAccessor} to propagate it. + * Writes the current Sentry {@link IHub} to the {@link Context} and uses {@link io.micrometer.context.ThreadLocalAccessor} to propagate it. * * This requires * - reactor.core.publisher.Hooks#enableAutomaticContextPropagation() to be enabled - * - having `io.micrometer:context-propagation:1.0.2` or newer as dependency - * - having `io.projectreactor:reactor-core:3.5.3` or newer as dependency + * - having `io.micrometer:context-propagation:1.0.2+` (provided by Spring Boot 3.0.3+) + * - having `io.projectreactor:reactor-core:3.5.3+` (provided by Spring Boot 3.0.3+) */ @ApiStatus.Experimental - public static Flux withSentry(Flux flux) { + public static Flux withSentry(final @NotNull Flux flux) { final @NotNull IHub oldHub = Sentry.getCurrentHub(); final @NotNull IHub clonedHub = oldHub.clone(); + return withSentryHub(flux, clonedHub); + } + + /** + * Writes a new Sentry {@link IHub} cloned from the main hub to the {@link Context} and uses {@link io.micrometer.context.ThreadLocalAccessor} to propagate it. + * + * This requires + * - reactor.core.publisher.Hooks#enableAutomaticContextPropagation() to be enabled + * - having `io.micrometer:context-propagation:1.0.2+` (provided by Spring Boot 3.0.3+) + * - having `io.projectreactor:reactor-core:3.5.3+` (provided by Spring Boot 3.0.3+) + */ + @ApiStatus.Experimental + public static Flux withSentryNewMainHubClone(final @NotNull Flux flux) { + final @NotNull IHub hub = Sentry.cloneMainHub(); + return withSentryHub(flux, hub); + } + + /** + * Writes the given Sentry {@link IHub} to the {@link Context} and uses {@link io.micrometer.context.ThreadLocalAccessor} to propagate it. + * + * This requires + * - reactor.core.publisher.Hooks#enableAutomaticContextPropagation() to be enabled + * - having `io.micrometer:context-propagation:1.0.2+` (provided by Spring Boot 3.0.3+) + * - having `io.projectreactor:reactor-core:3.5.3+` (provided by Spring Boot 3.0.3+) + */ + @ApiStatus.Experimental + public static Flux withSentryHub(final @NotNull Flux flux, final @NotNull IHub hub) { /** - * WARNING: Cannot set the clonedHub as current hub. + * WARNING: Cannot set the hub as current. * It would be used by others to clone again causing shared hubs and scopes and thus * leading to issues like unrelated breadcrumbs showing up in events. */ // Sentry.setCurrentHub(clonedHub); - return Flux.deferContextual(ctx -> flux).contextWrite(Context.of(SentryReactorThreadLocalAccessor.KEY, clonedHub)); + return Flux.deferContextual(ctx -> flux).contextWrite(Context.of(SentryReactorThreadLocalAccessor.KEY, hub)); } } diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebExceptionHandler.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebExceptionHandler.java index ccd07d8914..8f9189ef2d 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebExceptionHandler.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebExceptionHandler.java @@ -12,6 +12,7 @@ import io.sentry.util.Objects; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.springframework.core.annotation.Order; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; @@ -33,22 +34,29 @@ public SentryWebExceptionHandler(final @NotNull IHub hub) { @Override public @NotNull Mono handle( final @NotNull ServerWebExchange serverWebExchange, final @NotNull Throwable ex) { - if (!(ex instanceof ResponseStatusException)) { - final Mechanism mechanism = new Mechanism(); - mechanism.setType("SentryWebExceptionHandler"); - mechanism.setHandled(false); - final Throwable throwable = - new ExceptionMechanismException(mechanism, ex, Thread.currentThread()); - final SentryEvent event = new SentryEvent(throwable); - event.setLevel(SentryLevel.FATAL); - event.setTransaction(TransactionNameProvider.provideTransactionName(serverWebExchange)); - - final Hint hint = new Hint(); - hint.set(WEBFLUX_EXCEPTION_HANDLER_REQUEST, serverWebExchange.getRequest()); - hint.set(WEBFLUX_EXCEPTION_HANDLER_RESPONSE, serverWebExchange.getResponse()); - - hub.captureEvent(event, hint); - } - return Mono.error(ex); + final @Nullable IHub requestHub = serverWebExchange.getAttributeOrDefault(SentryWebFilter.SENTRY_HUB_KEY, null); + final @NotNull IHub hubToUse = requestHub != null ? requestHub : hub; + + return ReactorUtils.withSentryHub(Mono.just(ex) + .map(it -> { + if (!(ex instanceof ResponseStatusException)) { + final Mechanism mechanism = new Mechanism(); + mechanism.setType("SentryWebExceptionHandler"); + mechanism.setHandled(false); + final Throwable throwable = + new ExceptionMechanismException(mechanism, ex, Thread.currentThread()); + final SentryEvent event = new SentryEvent(throwable); + event.setLevel(SentryLevel.FATAL); + event.setTransaction(TransactionNameProvider.provideTransactionName(serverWebExchange)); + + final Hint hint = new Hint(); + hint.set(WEBFLUX_EXCEPTION_HANDLER_REQUEST, serverWebExchange.getRequest()); + hint.set(WEBFLUX_EXCEPTION_HANDLER_RESPONSE, serverWebExchange.getResponse()); + + hub.captureEvent(event, hint); + } + + return it; + }), hubToUse).flatMap(it -> Mono.error(ex)); } } diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilter.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilter.java index 56f7444973..78e9d4f624 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilter.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilter.java @@ -25,6 +25,7 @@ public class SentryWebFilter implements WebFilter { private final @NotNull IHub hub; private final @NotNull SentryRequestResolver sentryRequestResolver; + public static final String SENTRY_HUB_KEY = "sentry-hub"; public SentryWebFilter(final @NotNull IHub hub) { this.hub = Objects.requireNonNull(hub, "hub is required"); @@ -43,6 +44,7 @@ public Mono filter( }) .doFirst( () -> { + serverWebExchange.getAttributes().put(SENTRY_HUB_KEY, Sentry.getCurrentHub()); hub.pushScope(); final ServerHttpRequest request = serverWebExchange.getRequest(); final ServerHttpResponse response = serverWebExchange.getResponse(); diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilterWithThreadLocalAccessor.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilterWithThreadLocalAccessor.java index 66e4be2b62..419378c25c 100644 --- a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilterWithThreadLocalAccessor.java +++ b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilterWithThreadLocalAccessor.java @@ -20,6 +20,6 @@ public SentryWebFilterWithThreadLocalAccessor(final @NotNull IHub hub) { public Mono filter( final @NotNull ServerWebExchange serverWebExchange, final @NotNull WebFilterChain webFilterChain) { - return ReactorUtils.withSentry(super.filter(serverWebExchange, webFilterChain)); + return ReactorUtils.withSentryNewMainHubClone(super.filter(serverWebExchange, webFilterChain)); } } diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/ReactorUtilsTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/ReactorUtilsTest.kt new file mode 100644 index 0000000000..d9571b581d --- /dev/null +++ b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/ReactorUtilsTest.kt @@ -0,0 +1,116 @@ +package io.sentry.spring.jakarta.webflux + +import io.sentry.IHub +import io.sentry.NoOpHub +import io.sentry.Sentry +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotSame +import kotlin.test.assertSame +import reactor.core.publisher.Mono +import reactor.core.scheduler.Schedulers +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import reactor.core.publisher.Flux +import reactor.core.publisher.Hooks + +class ReactorUtilsTest { + + @BeforeTest + fun setup() { + Hooks.enableAutomaticContextPropagation() + } + + @AfterTest + fun teardown() { + Sentry.setCurrentHub(NoOpHub.getInstance()); + } + + @Test + fun `propagates hub inside mono`() { + val hubToUse = mock() + var hubInside: IHub? = null + val mono = ReactorUtils.withSentryHub( + Mono.just("hello") + .publishOn(Schedulers.boundedElastic()) + .map { it -> + hubInside = Sentry.getCurrentHub() + it + }, + hubToUse + ) + + assertEquals("hello", mono.block()) + assertSame(hubToUse, hubInside) + } + @Test + fun `propagates hub inside flux`() { + val hubToUse = mock() + var hubInside: IHub? = null + val flux = ReactorUtils.withSentryHub( + Flux.just("hello") + .publishOn(Schedulers.boundedElastic()) + .map { it -> + hubInside = Sentry.getCurrentHub() + it + }, + hubToUse + ) + + assertEquals("hello", flux.blockFirst()) + assertSame(hubToUse, hubInside) + } + + @Test + fun `without reactive utils hub is not propagated to mono`() { + val hubToUse = mock() + var hubInside: IHub? = null + val mono = Mono.just("hello") + .publishOn(Schedulers.boundedElastic()) + .map { it -> + hubInside = Sentry.getCurrentHub() + it + } + + assertEquals("hello", mono.block()) + assertNotSame(hubToUse, hubInside) + } + + @Test + fun `without reactive utils hub is not propagated to flux`() { + val hubToUse = mock() + var hubInside: IHub? = null + val flux = Flux.just("hello") + .publishOn(Schedulers.boundedElastic()) + .map { it -> + hubInside = Sentry.getCurrentHub() + it + } + + assertEquals("hello", flux.blockFirst()) + assertNotSame(hubToUse, hubInside) + } + + @Test + fun `clones hub for mono`() { + val mockHub = mock() + whenever(mockHub.clone()).thenReturn(mock()) + Sentry.setCurrentHub(mockHub) + ReactorUtils.withSentry(Mono.just("hello")).block() + + verify(mockHub).clone() + } + + @Test + fun `clones hub for flux`() { + val mockHub = mock() + whenever(mockHub.clone()).thenReturn(mock()) + Sentry.setCurrentHub(mockHub) + ReactorUtils.withSentry(Flux.just("hello")).blockFirst() + + verify(mockHub).clone() + } +} diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index e87d693ab7..c724f9277d 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1161,6 +1161,7 @@ public final class io/sentry/Sentry { public static fun captureMessage (Ljava/lang/String;Lio/sentry/SentryLevel;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; public static fun captureUserFeedback (Lio/sentry/UserFeedback;)V public static fun clearBreadcrumbs ()V + public static fun cloneMainHub ()Lio/sentry/IHub; public static fun close ()V public static fun configureScope (Lio/sentry/ScopeCallback;)V public static fun endSession ()V diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 2f8d29209d..c7043ee993 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -58,6 +58,20 @@ private Sentry() {} return hub; } + /** + * Returns a new hub which is cloned from the mainHub. + * + * @return the hub + */ + @ApiStatus.Internal + @ApiStatus.Experimental + public static @NotNull IHub cloneMainHub() { + if (globalHubMode) { + return mainHub; + } + return mainHub.clone(); + } + @ApiStatus.Internal // exposed for the coroutines integration in SentryContext public static void setCurrentHub(final @NotNull IHub hub) { currentHub.set(hub);