Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hubs/Scopes Merge 24 - Use new API in Spring integrations #3348

Merged
merged 30 commits into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
305baf5
replace hub with scopes
adinauer Mar 27, 2024
95f5e1b
Add Scopes
adinauer Apr 2, 2024
27f2398
Introduce `IScopes` interface.
adinauer Apr 2, 2024
ce3c14f
Replace `IHub` with `IScopes` in core
adinauer Apr 2, 2024
ce615f4
Replace `IHub` with `IScopes` in android core
adinauer Apr 2, 2024
22ddc00
Replace `IHub` with `IScopes` in android integrations
adinauer Apr 2, 2024
305c217
Replace `IHub` with `IScopes` in apollo integrations
adinauer Apr 2, 2024
da927bc
Replace `IHub` with `IScopes` in okhttp integration
adinauer Apr 2, 2024
8279276
Replace `IHub` with `IScopes` in graphql integration
adinauer Apr 2, 2024
9bfc086
Replace `IHub` with `IScopes` in logging integrations
adinauer Apr 2, 2024
b998e50
Replace `IHub` with `IScopes` in more integrations
adinauer Apr 2, 2024
739827a
Replace `IHub` with `IScopes` in OTel integration
adinauer Apr 2, 2024
69f2d63
Replace `IHub` with `IScopes` in Spring 5 / Spring Boot 2 integrations
adinauer Apr 2, 2024
792d482
Replace `IHub` with `IScopes` in Spring 6 / Spring Boot 3 integrations
adinauer Apr 2, 2024
9bcbce6
Replace `IHub` with `IScopes` in samples
adinauer Apr 2, 2024
3f25a4b
Merge branch 'feat/hsm-13-replacements-in-samples' into feat/hubs-sco…
adinauer Apr 2, 2024
d6fb40a
gitscopes -> github
adinauer Apr 2, 2024
7752bcc
Replace ThreadLocal with ScopesStorage
adinauer Apr 4, 2024
1e329c5
Move client and throwable to span map to scope
adinauer Apr 4, 2024
b0d89ae
Add global scope
adinauer Apr 4, 2024
cdd414a
use global scope in Scopes
adinauer Apr 4, 2024
98da9ff
Implement pushScope popScope and withScope for Scopes
adinauer Apr 4, 2024
2d26033
Add pushIsolationScope; add fork methods to ISCope
adinauer Apr 12, 2024
bbb6700
Use separate scopes for current, isolation and global scope; rename m…
adinauer Apr 12, 2024
c714b21
Allow controlling which scope configureScope uses
adinauer Apr 12, 2024
a474402
Combine scopes
adinauer Apr 12, 2024
ae93e33
Use new API for CRONS integrations
adinauer Apr 12, 2024
b01298b
Add lifecycle helper
adinauer Apr 12, 2024
b64e688
Change spring integrations to use new API
adinauer Apr 12, 2024
855ebfe
Merge branch '8.x.x' into feat/hsm-24-spring-integrations
adinauer Apr 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
lbloder marked this conversation as resolved.
Show resolved Hide resolved
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()?
lbloder marked this conversation as resolved.
Show resolved Hide resolved
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();
lbloder marked this conversation as resolved.
Show resolved Hide resolved
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
lbloder marked this conversation as resolved.
Show resolved Hide resolved
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()) {
lbloder marked this conversation as resolved.
Show resolved Hide resolved
// 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 @@ -10,6 +10,8 @@

// TODO deprecate and replace with "withSentryScopes" etc.
@ApiStatus.Experimental
// TODO do we keep old methods around and deprecate them?
lbloder marked this conversation as resolved.
Show resolved Hide resolved
// TODO do we need to offer isolated variants?
public final class ReactorUtils {

/**
Expand All @@ -20,27 +22,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 +49,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 +69,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 +97,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,26 @@
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
Loading