Skip to content

Commit

Permalink
Merge b64e688 into b01298b
Browse files Browse the repository at this point in the history
  • Loading branch information
adinauer authored Apr 12, 2024
2 parents b01298b + b64e688 commit ddf2c22
Show file tree
Hide file tree
Showing 26 changed files with 159 additions and 181 deletions.
8 changes: 4 additions & 4 deletions sentry-spring-jakarta/api/sentry-spring-jakarta.api
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,10 @@ public final class io/sentry/spring/jakarta/webflux/ReactorUtils {
public fun <init> ()V
public static fun withSentry (Lreactor/core/publisher/Flux;)Lreactor/core/publisher/Flux;
public static fun withSentry (Lreactor/core/publisher/Mono;)Lreactor/core/publisher/Mono;
public static fun withSentryHub (Lreactor/core/publisher/Flux;Lio/sentry/IScopes;)Lreactor/core/publisher/Flux;
public static fun withSentryHub (Lreactor/core/publisher/Mono;Lio/sentry/IScopes;)Lreactor/core/publisher/Mono;
public static fun withSentryNewMainHubClone (Lreactor/core/publisher/Flux;)Lreactor/core/publisher/Flux;
public static fun withSentryNewMainHubClone (Lreactor/core/publisher/Mono;)Lreactor/core/publisher/Mono;
public static fun withSentryForkedRoots (Lreactor/core/publisher/Flux;)Lreactor/core/publisher/Flux;
public static fun withSentryForkedRoots (Lreactor/core/publisher/Mono;)Lreactor/core/publisher/Mono;
public static fun withSentryScopes (Lreactor/core/publisher/Flux;Lio/sentry/IScopes;)Lreactor/core/publisher/Flux;
public static fun withSentryScopes (Lreactor/core/publisher/Mono;Lio/sentry/IScopes;)Lreactor/core/publisher/Mono;
}

public final class io/sentry/spring/jakarta/webflux/SentryReactorThreadLocalAccessor : io/micrometer/context/ThreadLocalAccessor {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import io.sentry.EventProcessor;
import io.sentry.Hint;
import io.sentry.IScopes;
import io.sentry.ISentryLifecycleToken;
import io.sentry.ScopesAdapter;
import io.sentry.SentryEvent;
import io.sentry.SentryLevel;
Expand Down Expand Up @@ -60,7 +61,7 @@ protected void doFilterInternal(
if (scopes.isEnabled()) {
// request may qualify for caching request body, if so resolve cached request
final HttpServletRequest request = resolveHttpServletRequest(servletRequest);
scopes.pushScope();
final @NotNull ISentryLifecycleToken lifecycleToken = scopes.pushIsolationScope();
try {
final Hint hint = new Hint();
hint.set(SPRING_REQUEST_FILTER_REQUEST, servletRequest);
Expand All @@ -70,7 +71,7 @@ protected void doFilterInternal(
configureScope(request);
filterChain.doFilter(request, response);
} finally {
scopes.popScope();
lifecycleToken.close();
}
} else {
filterChain.doFilter(servletRequest, response);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.sentry.spring.jakarta;

import io.sentry.IScopes;
import io.sentry.ISentryLifecycleToken;
import io.sentry.Sentry;
import java.util.concurrent.Callable;
import org.jetbrains.annotations.NotNull;
Expand All @@ -14,18 +15,13 @@
*/
public final class SentryTaskDecorator implements TaskDecorator {
@Override
@SuppressWarnings("deprecation")
// TODO should there also be a SentryIsolatedTaskDecorator or similar that uses forkedScopes()?
public @NotNull Runnable decorate(final @NotNull Runnable runnable) {
// TODO fork
final IScopes newHub = Sentry.getCurrentScopes().clone();
final IScopes newScopes = Sentry.getCurrentScopes().forkedCurrentScope("spring.taskDecorator");

return () -> {
final IScopes oldState = Sentry.getCurrentScopes();
Sentry.setCurrentScopes(newHub);
try {
try (final @NotNull ISentryLifecycleToken ignored = newScopes.makeCurrent()) {
runnable.run();
} finally {
Sentry.setCurrentScopes(oldState);
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.jakewharton.nopen.annotation.Open;
import io.sentry.IScopes;
import io.sentry.ISentryLifecycleToken;
import io.sentry.ITransaction;
import io.sentry.ScopesAdapter;
import io.sentry.SpanStatus;
Expand Down Expand Up @@ -68,7 +69,7 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
} else {
operation = "bean";
}
scopes.pushScope();
final @NotNull ISentryLifecycleToken lifecycleToken = scopes.pushIsolationScope();
final TransactionOptions transactionOptions = new TransactionOptions();
transactionOptions.setBindToScope(true);
final ITransaction transaction =
Expand All @@ -86,7 +87,7 @@ public Object invoke(final @NotNull MethodInvocation invocation) throws Throwabl
throw e;
} finally {
transaction.finish();
scopes.popScope();
lifecycleToken.close();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,19 +68,13 @@ protected void doFinally(
if (transaction != null) {
finishTransaction(serverWebExchange, transaction);
}
if (requestHub.isEnabled()) {
// TODO close lifecycle token instead of popscope
requestHub.popScope();
}
Sentry.setCurrentScopes(NoOpScopes.getInstance());
}

protected void doFirst(
final @NotNull ServerWebExchange serverWebExchange, final @NotNull IScopes requestHub) {
if (requestHub.isEnabled()) {
serverWebExchange.getAttributes().put(SENTRY_SCOPES_KEY, requestHub);
// TODO fork instead
requestHub.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 @@ -8,8 +8,9 @@
import reactor.core.publisher.Mono;
import reactor.util.context.Context;

// TODO deprecate and replace with "withSentryScopes" etc.
@ApiStatus.Experimental
// TODO do we keep old methods around and deprecate them?
// TODO do we need to offer isolated variants?
public final class ReactorUtils {

/**
Expand All @@ -20,27 +21,23 @@ public final class ReactorUtils {
* 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
@SuppressWarnings("deprecation")
public static <T> Mono<T> withSentry(final @NotNull Mono<T> mono) {
final @NotNull IScopes oldHub = Sentry.getCurrentScopes();
// TODO fork
final @NotNull IScopes clonedHub = oldHub.clone();
return withSentryHub(mono, clonedHub);
final @NotNull IScopes oldScopes = Sentry.getCurrentScopes();
final @NotNull IScopes forkedScopes = oldScopes.forkedCurrentScope("reactor.withSentry");
return withSentryScopes(mono, forkedScopes);
}

/**
* Writes a new Sentry {@link IScopes} cloned from the main hub to the {@link Context} and uses
* Writes a new Sentry {@link IScopes} cloned from the main scopes to the {@link Context} and uses
* {@link io.micrometer.context.ThreadLocalAccessor} to propagate it.
*
* <p>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 IScopes hub = Sentry.cloneMainHub();
return withSentryHub(mono, hub);
public static <T> Mono<T> withSentryForkedRoots(final @NotNull Mono<T> mono) {
final @NotNull IScopes scopes = Sentry.forkedRootScopes("reactor");
return withSentryScopes(mono, scopes);
}

/**
Expand All @@ -51,17 +48,16 @@ public static <T> Mono<T> withSentryNewMainHubClone(final @NotNull Mono<T> mono)
* 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 IScopes hub) {
public static <T> Mono<T> withSentryScopes(
final @NotNull Mono<T> mono, final @NotNull IScopes scopes) {
/**
* 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.
* WARNING: Cannot set the scopes as current. It would be used by others to clone again causing
* shared scopes and thus leading to issues like unrelated breadcrumbs showing up in events.
*/
// Sentry.setCurrentHub(clonedHub);
// Sentry.setCurrentScopes(forkedScopes);

return Mono.deferContextual(ctx -> mono)
.contextWrite(Context.of(SentryReactorThreadLocalAccessor.KEY, hub));
.contextWrite(Context.of(SentryReactorThreadLocalAccessor.KEY, scopes));
}

/**
Expand All @@ -72,28 +68,24 @@ public static <T> Mono<T> withSentryHub(final @NotNull Mono<T> mono, final @NotN
* 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
@SuppressWarnings("deprecation")
public static <T> Flux<T> withSentry(final @NotNull Flux<T> flux) {
final @NotNull IScopes oldHub = Sentry.getCurrentScopes();
// TODO fork
final @NotNull IScopes clonedHub = oldHub.clone();
final @NotNull IScopes oldScopes = Sentry.getCurrentScopes();
final @NotNull IScopes forkedScopes = oldScopes.forkedCurrentScope("reactor.withSentry");

return withSentryHub(flux, clonedHub);
return withSentryScopes(flux, forkedScopes);
}

/**
* Writes a new Sentry {@link IScopes} cloned from the main hub to the {@link Context} and uses
* Writes a new Sentry {@link IScopes} cloned from the main scopes to the {@link Context} and uses
* {@link io.micrometer.context.ThreadLocalAccessor} to propagate it.
*
* <p>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 IScopes hub = Sentry.cloneMainHub();
return withSentryHub(flux, hub);
public static <T> Flux<T> withSentryForkedRoots(final @NotNull Flux<T> flux) {
final @NotNull IScopes scopes = Sentry.forkedRootScopes("reactor");
return withSentryScopes(flux, scopes);
}

/**
Expand All @@ -104,16 +96,15 @@ public static <T> Flux<T> withSentryNewMainHubClone(final @NotNull Flux<T> flux)
* 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 IScopes hub) {
public static <T> Flux<T> withSentryScopes(
final @NotNull Flux<T> flux, final @NotNull IScopes scopes) {
/**
* 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.
* WARNING: Cannot set the scopes as current. It would be used by others to clone again causing
* shared scopes and thus leading to issues like unrelated breadcrumbs showing up in events.
*/
// Sentry.setCurrentHub(clonedHub);
// Sentry.setCurrentScopes(forkedScopes);

return Flux.deferContextual(ctx -> flux)
.contextWrite(Context.of(SentryReactorThreadLocalAccessor.KEY, hub));
.contextWrite(Context.of(SentryReactorThreadLocalAccessor.KEY, scopes));
}
}
Original file line number Diff line number Diff line change
@@ -1,30 +1,25 @@
package io.sentry.spring.jakarta.webflux;

import io.sentry.IScopes;
import io.sentry.ISentryLifecycleToken;
import io.sentry.Sentry;
import java.util.function.Function;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;

/**
* Hook meant to used with {@link reactor.core.scheduler.Schedulers#onScheduleHook(String,
* Function)} to configure Reactor to copy correct hub into the operating thread.
* Function)} to configure Reactor to copy correct scopes into the operating thread.
*/
@ApiStatus.Experimental
public final class SentryScheduleHook implements Function<Runnable, Runnable> {
@Override
@SuppressWarnings("deprecation")
public Runnable apply(final @NotNull Runnable runnable) {
// TODO fork instead
final IScopes newHub = Sentry.getCurrentScopes().clone();
final IScopes newScopes = Sentry.getCurrentScopes().forkedCurrentScope("spring.scheduleHook");

return () -> {
final IScopes oldState = Sentry.getCurrentScopes();
Sentry.setCurrentScopes(newHub);
try {
try (final @NotNull ISentryLifecycleToken ignored = newScopes.makeCurrent()) {
runnable.run();
} finally {
Sentry.setCurrentScopes(oldState);
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public SentryWebExceptionHandler(final @NotNull IScopes scopes) {
serverWebExchange.getAttributeOrDefault(SentryWebFilter.SENTRY_SCOPES_KEY, null);
final @NotNull IScopes scopesToUse = requestScopes != null ? requestScopes : scopes;

return ReactorUtils.withSentryHub(
return ReactorUtils.withSentryScopes(
Mono.just(ex)
.map(
it -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public SentryWebFilter(final @NotNull IScopes scopes) {
public Mono<Void> filter(
final @NotNull ServerWebExchange serverWebExchange,
final @NotNull WebFilterChain webFilterChain) {
@NotNull IScopes requestScopes = Sentry.cloneMainHub();
@NotNull IScopes requestScopes = Sentry.forkedRootScopes("request.webflux");
final ServerHttpRequest request = serverWebExchange.getRequest();
final @Nullable ITransaction transaction = maybeStartTransaction(requestScopes, request);
if (transaction != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public Mono<Void> filter(
final @NotNull ServerWebExchange serverWebExchange,
final @NotNull WebFilterChain webFilterChain) {
final @NotNull TransactionContainer transactionContainer = new TransactionContainer();
return ReactorUtils.withSentryNewMainHubClone(
return ReactorUtils.withSentryForkedRoots(
webFilterChain
.filter(serverWebExchange)
.doFinally(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package io.sentry.spring.jakarta
import io.sentry.Breadcrumb
import io.sentry.IScope
import io.sentry.IScopes
import io.sentry.ISentryLifecycleToken
import io.sentry.Scope
import io.sentry.ScopeCallback
import io.sentry.SentryOptions
Expand Down Expand Up @@ -39,6 +40,7 @@ class SentrySpringFilterTest {
private class Fixture {
val scopes = mock<IScopes>()
val response = MockHttpServletResponse()
val lifecycleToken = mock<ISentryLifecycleToken>()
val chain = mock<FilterChain>()
lateinit var scope: IScope
lateinit var request: HttpServletRequest
Expand All @@ -47,6 +49,7 @@ class SentrySpringFilterTest {
scope = Scope(options)
whenever(scopes.options).thenReturn(options)
whenever(scopes.isEnabled).thenReturn(true)
whenever(scopes.pushIsolationScope()).thenReturn(lifecycleToken)
doAnswer { (it.arguments[0] as ScopeCallback).run(scope) }.whenever(scopes).configureScope(any())
this.request = request
?: MockHttpServletRequest().apply {
Expand All @@ -64,7 +67,7 @@ class SentrySpringFilterTest {
val listener = fixture.getSut()
listener.doFilter(fixture.request, fixture.response, fixture.chain)

verify(fixture.scopes).pushScope()
verify(fixture.scopes).pushIsolationScope()
}

@Test
Expand All @@ -87,7 +90,7 @@ class SentrySpringFilterTest {
val listener = fixture.getSut()
listener.doFilter(fixture.request, fixture.response, fixture.chain)

verify(fixture.scopes).popScope()
verify(fixture.lifecycleToken).close()
}

@Test
Expand All @@ -99,7 +102,7 @@ class SentrySpringFilterTest {
listener.doFilter(fixture.request, fixture.response, fixture.chain)
fail()
} catch (e: Exception) {
verify(fixture.scopes).popScope()
verify(fixture.lifecycleToken).close()
}
}

Expand Down
Loading

0 comments on commit ddf2c22

Please sign in to comment.