diff --git a/.cursor/rules/scopes.mdc b/.cursor/rules/scopes.mdc index e054755d4f..6da9baeec3 100644 --- a/.cursor/rules/scopes.mdc +++ b/.cursor/rules/scopes.mdc @@ -90,15 +90,20 @@ For JVM Backend applications (servers) we discourage enabling `globalHubMode` si ### Enabled -If `globalHubMode` is enabled, the SDK avoids forking scopes. +If `globalHubMode` is enabled, the SDK avoids forking scopes _implicitly_. This means, retrieving current scopes on a thread where specific scopes do not exist yet for the thread, the root scopes are not forked but returned directly. -The SDK also doesn't fork scopes when `Sentry.pushScope`, `Sentry.pushIsolation`, `Sentry.withScope` or `Sentry.withIsolationScope` are executed. +`Sentry.pushScope`, `Sentry.pushIsolationScope` and `Sentry.popScope` are no-ops. +They are unbalanced API: the caller may never restore the previous scopes, or restore them on a different thread, which would corrupt the globally shared scopes. + +`Sentry.withScope` and `Sentry.withIsolationScope` do fork, also when `globalHubMode` is enabled. +They are balanced by construction and always restore the previous scopes before returning, so they cannot corrupt the shared scopes. +This matches the cross SDK spec ("fork the current scope, invoke callback, discard the fork when done") and what the SDK did before major version 8. The suppression of forking via `globalHubMode` only applies when using `Sentry` static API or `ScopesAdapter`. In case the `Scopes` instance is accessed directly, forking will happen as if `globalHubMode` is disabled. -However, while it's possible to use `Sentry.setCurrentScopes` it does not have any effect due to `Sentry.getCurrentScopes` directly returning `rootScopes` if `globalHubMode` is enabled. -This means the forked scopes have to be managed manually, e.g. by keeping a reference and accessing Sentry API via the reference instead of using static API. +`Sentry.setCurrentScopes`, and thus `Scopes.makeCurrent`, does have an effect when `globalHubMode` is enabled, but only for scopes that descend from the current `rootScopes`. +`Sentry.getCurrentScopes` ignores all other scopes, because scopes left over from a closed or re-initialized SDK would otherwise be read back from a thread local that the SDK cannot clean up. `ScopesAdapter` makes use of the static `Sentry` API internally. It allows us to access the correct scopes for the current context without passing it along explicitly. It also makes testing easier. diff --git a/CHANGELOG.md b/CHANGELOG.md index 878238076f..b326556855 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ - Prevent duplicated breadcrumbs on tombstone-merged native crash events ([#5888](https://github.com/getsentry/sentry-java/pull/5888)) - Prevent a class of Session Replay deadlocks by confining lifecycle state changes to Android's main thread ([#5965](https://github.com/getsentry/sentry-java/pull/5965)) - Symbolicate tombstone native frames for libraries loaded directly from APKs ([#5992](https://github.com/getsentry/sentry-java/pull/5992)) +- Apply `Sentry.withScope` and `Sentry.withIsolationScope` data to events captured inside the callback when `globalHubMode` is enabled ([#6004](https://github.com/getsentry/sentry-java/pull/6004)) + - `globalHubMode` is enabled by default on Android, where tags, extras, contexts and level set inside the callback were silently dropped + - Scopes that are explicitly made current, e.g. via `Sentry.setCurrentScopes` or the `SentryContext` coroutine integration, are now also honoured when `globalHubMode` is enabled + - `Sentry.pushScope`, `Sentry.pushIsolationScope` and `Sentry.popScope` remain no-ops when `globalHubMode` is enabled ### Features diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 266aa39e79..03288ae00a 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -114,15 +114,24 @@ private Sentry() {} @ApiStatus.Internal @SuppressWarnings("deprecation") public static @NotNull IScopes getCurrentScopes(final boolean ensureForked) { + // read the volatile rootScopes once, so a concurrent Sentry.init cannot make the check below + // disagree with what we return + final @NotNull IScopes root = rootScopes; + @Nullable IScopes scopes = getScopesStorage().get(); if (globalHubMode) { - return rootScopes; + // in global hub mode we never fork implicitly, but scopes that have explicitly been made + // current (e.g. by withScope) must still be honoured. Anything that did not originate from + // the present rootScopes is stale (SDK closed or re-initialized) and gets ignored. + if (scopes != null && !scopes.isNoOp() && root.isAncestorOf(scopes)) { + return scopes; + } + return root; } - @Nullable IScopes scopes = getScopesStorage().get(); if (scopes == null || scopes.isNoOp()) { if (!ensureForked) { return NoOpScopes.getInstance(); } else { - scopes = rootScopes.forkedScopes("getCurrentScopes"); + scopes = root.forkedScopes("getCurrentScopes"); getScopesStorage().set(scopes); } } @@ -1060,7 +1069,13 @@ public static void removeExtra(final @Nullable String key) { return getCurrentScopes().getLastEventId(); } - /** Pushes a new scope while inheriting the current scope's data. */ + /** + * Pushes a new scope while inheriting the current scope's data. + * + *
This is a no-op in global hub mode, as the caller may never pop the scope again, or pop it + * on a different thread, which would corrupt the globally shared scopes. Use {@link + * Sentry#withScope(ScopeCallback)} if you need forking that also works in global hub mode. + */ public static @NotNull ISentryLifecycleToken pushScope() { // pushScope is no-op in global hub mode if (!globalHubMode) { @@ -1069,7 +1084,12 @@ public static void removeExtra(final @Nullable String key) { return NoOpScopesLifecycleToken.getInstance(); } - /** Pushes a new isolation and current scope while inheriting the current scope's data. */ + /** + * Pushes a new isolation and current scope while inheriting the current scope's data. + * + *
This is a no-op in global hub mode, for the same reason as {@link Sentry#pushScope()}. Use + * {@link Sentry#withIsolationScope(ScopeCallback)} instead. + */ public static @NotNull ISentryLifecycleToken pushIsolationScope() { // pushScope is no-op in global hub mode if (!globalHubMode) { @@ -1093,7 +1113,10 @@ public static void popScope() { } /** - * Runs the callback with a new current scope which gets dropped at the end + * Runs the callback with a new current scope which gets dropped at the end. + * + *
Unlike {@link Sentry#pushScope()} this also forks in global hub mode, since the previous + * scopes are always restored once the callback returns. * * @param callback the callback */ @@ -1105,6 +1128,9 @@ public static void withScope(final @NotNull ScopeCallback callback) { * Runs the callback with a new isolation scope which gets dropped at the end. Current scope is * also forked. * + *
Unlike {@link Sentry#pushIsolationScope()} this also forks in global hub mode, since the
+ * previous scopes are always restored once the callback returns.
+ *
* @param callback the callback
*/
public static void withIsolationScope(final @NotNull ScopeCallback callback) {
diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt
index 98cda8e9d8..86b40cdc8c 100644
--- a/sentry/src/test/java/io/sentry/SentryTest.kt
+++ b/sentry/src/test/java/io/sentry/SentryTest.kt
@@ -1,5 +1,6 @@
package io.sentry
+import com.google.common.truth.Truth.assertThat
import io.sentry.SentryFeedbackOptions.IFormHandler
import io.sentry.SentryOptions.ProfilesSamplerCallback
import io.sentry.SentryOptions.TracesSamplerCallback
@@ -1434,6 +1435,93 @@ class SentryTest {
assertNotSame(s1, s2)
}
+ private fun initCapturingEvents(globalHubMode: Boolean): MutableList