Use ThreadLocalAccessor in Spring Boot 3 WebFlux sample (#2569)

getsentry · Mar 1, 2023 · 5b12e27 · 5b12e27
1 parent 9d43bb9
commit 5b12e27
Show file tree

Hide file tree

Showing 18 changed files with 308 additions and 39 deletions.
diff --git 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/...ring-boot-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/TodoController.java b/...ring-boot-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/TodoController.java
@@ -1,10 +1,14 @@
 package io.sentry.samples.spring.boot.jakarta;
 
+import io.sentry.spring.jakarta.webflux.ReactorUtils;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.PathVariable;
 import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.client.RestTemplate;
 import org.springframework.web.reactive.function.client.WebClient;
+import reactor.core.publisher.Hooks;
+import reactor.core.publisher.Mono;
+import reactor.core.scheduler.Schedulers;
 
 @RestController
 public class TodoController {
@@ -24,11 +28,18 @@ Todo todo(@PathVariable Long id) {
 
   @GetMapping("/todo-webclient/{id}")
   Todo todoWebClient(@PathVariable Long id) {
-    return webClient
-        .get()
-        .uri("https://jsonplaceholder.typicode.com/todos/{id}", id)
-        .retrieve()
-        .bodyToMono(Todo.class)
+    Hooks.enableAutomaticContextPropagation();
+    return ReactorUtils.withSentry(
+            Mono.just(true)
+                .publishOn(Schedulers.boundedElastic())
+                .flatMap(
+                    x ->
+                        webClient
+                            .get()
+                            .uri("https://jsonplaceholder.typicode.com/todos/{id}", id)
+                            .retrieve()
+                            .bodyToMono(Todo.class)
+                            .map(response -> response)))
         .block();
   }
 }
diff --git a/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/build.gradle.kts b/sentry-samples/sentry-samples-spring-boot-webflux-jakarta/build.gradle.kts
@@ -19,10 +19,12 @@ repositories {
 
 dependencies {
     implementation(Config.Libs.springBoot3StarterWebflux)
+    implementation(Config.Libs.contextPropagation)
     implementation(Config.Libs.kotlinReflect)
     implementation(kotlin(Config.kotlinStdLib, KotlinCompilerVersion.VERSION))
     implementation(projects.sentrySpringBootStarterJakarta)
     implementation(projects.sentryLogback)
+
     testImplementation(Config.Libs.springBoot3StarterTest) {
         exclude(group = "org.junit.vintage", module = "junit-vintage-engine")
     }

diff --git a/...io/sentry/samples/spring/boot/Person.java → ...y/samples/spring/boot/jakarta/Person.java b/...io/sentry/samples/spring/boot/Person.java → ...y/samples/spring/boot/jakarta/Person.java
@@ -1,4 +1,4 @@
-package io.sentry.samples.spring.boot;
+package io.sentry.samples.spring.boot.jakarta;
 
 public class Person {
   private final String firstName;

diff --git a/...samples/spring/boot/PersonController.java → ...spring/boot/jakarta/PersonController.java b/...samples/spring/boot/PersonController.java → ...spring/boot/jakarta/PersonController.java
@@ -1,4 +1,4 @@
-package io.sentry.samples.spring.boot;
+package io.sentry.samples.spring.boot.jakarta;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;

diff --git a/...ry/samples/spring/boot/PersonService.java → ...es/spring/boot/jakarta/PersonService.java b/...ry/samples/spring/boot/PersonService.java → ...es/spring/boot/jakarta/PersonService.java
@@ -1,4 +1,4 @@
-package io.sentry.samples.spring.boot;
+package io.sentry.samples.spring.boot.jakarta;
 
 import io.sentry.Sentry;
 import java.time.Duration;

diff --git a/...es/spring/boot/SentryDemoApplication.java → ...g/boot/jakarta/SentryDemoApplication.java b/...es/spring/boot/SentryDemoApplication.java → ...g/boot/jakarta/SentryDemoApplication.java
@@ -1,11 +1,18 @@
-package io.sentry.samples.spring.boot;
+package io.sentry.samples.spring.boot.jakarta;
 
 import org.springframework.boot.SpringApplication;
 import org.springframework.boot.autoconfigure.SpringBootApplication;
+import org.springframework.context.annotation.Bean;
+import org.springframework.web.reactive.function.client.WebClient;
 
 @SpringBootApplication
 public class SentryDemoApplication {
   public static void main(String[] args) {
     SpringApplication.run(SentryDemoApplication.class, args);
   }
+
+  @Bean
+  WebClient webClient(WebClient.Builder builder) {
+    return builder.build();
+  }
 }
diff --git a/...spring-boot-webflux-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/Todo.java b/...spring-boot-webflux-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/Todo.java
@@ -0,0 +1,25 @@
+package io.sentry.samples.spring.boot.jakarta;
+
+public class Todo {
+  private final Long id;
+  private final String title;
+  private final boolean completed;
+
+  public Todo(Long id, String title, boolean completed) {
+    this.id = id;
+    this.title = title;
+    this.completed = completed;
+  }
+
+  public Long getId() {
+    return id;
+  }
+
+  public String getTitle() {
+    return title;
+  }
+
+  public boolean isCompleted() {
+    return completed;
+  }
+}
diff --git a/...t-webflux-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/TodoController.java b/...t-webflux-jakarta/src/main/java/io/sentry/samples/spring/boot/jakarta/TodoController.java
@@ -0,0 +1,26 @@
+package io.sentry.samples.spring.boot.jakarta;
+
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.PathVariable;
+import org.springframework.web.bind.annotation.RestController;
+import org.springframework.web.reactive.function.client.WebClient;
+import reactor.core.publisher.Mono;
+
+@RestController
+public class TodoController {
+  private final WebClient webClient;
+
+  public TodoController(WebClient webClient) {
+    this.webClient = webClient;
+  }
+
+  @GetMapping("/todo-webclient/{id}")
+  Mono<Todo> todoWebClient(@PathVariable Long id) {
+    return webClient
+        .get()
+        .uri("https://jsonplaceholder.typicode.com/todos/{id}", id)
+        .retrieve()
+        .bodyToMono(Todo.class)
+        .map(response -> response);
+  }
+}
diff --git a/...ples/sentry-samples-spring-boot-webflux-jakarta/src/main/resources/application.properties b/...ples/sentry-samples-spring-boot-webflux-jakarta/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.reactive.thread-local-accessor-enabled=true
diff --git 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
@@ -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));
   }
 }
diff --git a/...ing-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/SentryWebExceptionHandler.java b/...ing-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<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));
   }
 }
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
@@ -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<Void> 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/...rc/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilterWithThreadLocalAccessor.java b/...rc/main/java/io/sentry/spring/jakarta/webflux/SentryWebFilterWithThreadLocalAccessor.java
@@ -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));
   }
 }