Skip to content

Commit

Permalink
Clone hub from main hub for every WebFlux request (#2567)
Browse files Browse the repository at this point in the history
  • Loading branch information
adinauer authored Mar 1, 2023
1 parent bd5f90e commit d3f277b
Show file tree
Hide file tree
Showing 9 changed files with 227 additions and 30 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions sentry-spring-jakarta/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 <T> Mono<T> withSentry(Mono<T> mono) {
public static <T> Mono<T> withSentry(final @NotNull Mono<T> 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 <T> Mono<T> withSentryNewMainHubClone(final @NotNull Mono<T> 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 <T> Mono<T> withSentryHub(final @NotNull Mono<T> 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 <T> Flux<T> withSentry(Flux<T> flux) {
public static <T> Flux<T> withSentry(final @NotNull Flux<T> 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 <T> Flux<T> withSentryNewMainHubClone(final @NotNull Flux<T> 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 <T> Flux<T> withSentryHub(final @NotNull Flux<T> 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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,22 +34,29 @@ public SentryWebExceptionHandler(final @NotNull IHub hub) {
@Override
public @NotNull Mono<Void> 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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -43,6 +44,7 @@ public Mono<Void> filter(
})
.doFirst(
() -> {
serverWebExchange.getAttributes().put(SENTRY_HUB_KEY, Sentry.getCurrentHub());
hub.pushScope();
final ServerHttpRequest request = serverWebExchange.getRequest();
final ServerHttpResponse response = serverWebExchange.getResponse();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ public SentryWebFilterWithThreadLocalAccessor(final @NotNull IHub hub) {
public Mono<Void> filter(
final @NotNull ServerWebExchange serverWebExchange,
final @NotNull WebFilterChain webFilterChain) {
return ReactorUtils.withSentry(super.filter(serverWebExchange, webFilterChain));
return ReactorUtils.withSentryNewMainHubClone(super.filter(serverWebExchange, webFilterChain));
}
}
Original file line number Diff line number Diff line change
@@ -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<IHub>()
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<IHub>()
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<IHub>()
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<IHub>()
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<IHub>()
whenever(mockHub.clone()).thenReturn(mock<IHub>())
Sentry.setCurrentHub(mockHub)
ReactorUtils.withSentry(Mono.just("hello")).block()

verify(mockHub).clone()
}

@Test
fun `clones hub for flux`() {
val mockHub = mock<IHub>()
whenever(mockHub.clone()).thenReturn(mock<IHub>())
Sentry.setCurrentHub(mockHub)
ReactorUtils.withSentry(Flux.just("hello")).blockFirst()

verify(mockHub).clone()
}
}
1 change: 1 addition & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions sentry/src/main/java/io/sentry/Sentry.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit d3f277b

Please sign in to comment.