Skip to content

Hubs/Scopes Merge 33 - No longer replace global scope #3362

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.

Sign up for GitHub

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

Merged
merged 39 commits into from
May 2, 2024
Merged

Hubs/Scopes Merge 33 - No longer replace global scope #3362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 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
d06fc50
Use new API in servlet integrations
adinauer Apr 12, 2024
f0af5c3
Use new API for kotlin coroutines and wrapers for Supplier/Callable
adinauer Apr 12, 2024
2f02001
Discussion TODOs
adinauer Apr 12, 2024
bf4a7bf
Fix breadcrumb ordering
adinauer Apr 15, 2024
62cb91a
Mark TODOS with [HSM]
adinauer Apr 15, 2024
b1630ea
Add getGlobalScope and forkedRootScopes to IScopes
adinauer Apr 16, 2024
136b9ce
Fix EventProcessor ordering on scopes
adinauer Apr 16, 2024
94d54ef
Reuse code in Scopes
adinauer Apr 16, 2024
017599d
No longer replace global scope
adinauer Apr 16, 2024
0b5a5a0
Merge branch '8.x.x' into feat/hsm-33-no-longer-replace-global-scope
adinauer May 2, 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
1 change: 1 addition & 0 deletions sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public final class InternalSentrySdk {
@Nullable
public static IScope getCurrentScope() {
final @NotNull AtomicReference<IScope> scopeRef = new AtomicReference<>();
// TODO [HSM] should this retrieve combined scope?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way, if a user adds, e.g. tags to global scope, cross platform SDKs will also receive the tag.

ScopesAdapter.getInstance()
.configureScope(
scope -> {
Expand Down
4 changes: 4 additions & 0 deletions sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
/** Sentry initialization class */
public final class SentryAndroid {

static {
Sentry.getGlobalScope().replaceOptions(new SentryAndroidOptions());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe @romtsn can recall exactly, but I think creating options can be expensive. There's also SentryOptions.empty(), maybe that's better?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, might be a good idea.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even the empty() one will instantiate some of the options fields (like EnvelopeReader or JsonSerializer), so ideally we don't do that, but I guess there's no way around it. We should just probably fix the root cause here eventually and lazily initialize those fields #2541

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC we discussed that this is not required, as any integrations / processors will access the SentryAndroidOptions only after Sentry.init() which replaces the options already.

}

// SystemClock.uptimeMillis() isn't affected by phone provider or clock changes.
private static final long sdkInitMillis = SystemClock.uptimeMillis();

Expand Down
7 changes: 4 additions & 3 deletions sentry/api/sentry.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ public final class io/sentry/CombinedScopeView : io/sentry/IScope {
public fun removeContexts (Ljava/lang/String;)V
public fun removeExtra (Ljava/lang/String;)V
public fun removeTag (Ljava/lang/String;)V
public fun replaceOptions (Lio/sentry/SentryOptions;)V
public fun setContexts (Ljava/lang/String;Ljava/lang/Boolean;)V
public fun setContexts (Ljava/lang/String;Ljava/lang/Character;)V
public fun setContexts (Ljava/lang/String;Ljava/lang/Number;)V
Expand Down Expand Up @@ -805,7 +806,7 @@ public abstract interface class io/sentry/IScope {
public abstract fun removeContexts (Ljava/lang/String;)V
public abstract fun removeExtra (Ljava/lang/String;)V
public abstract fun removeTag (Ljava/lang/String;)V
public abstract fun setClient (Lio/sentry/ISentryClient;)V
public abstract fun replaceOptions (Lio/sentry/SentryOptions;)V
public abstract fun setContexts (Ljava/lang/String;Ljava/lang/Boolean;)V
public abstract fun setContexts (Ljava/lang/String;Ljava/lang/Character;)V
public abstract fun setContexts (Ljava/lang/String;Ljava/lang/Number;)V
Expand Down Expand Up @@ -1467,7 +1468,7 @@ public final class io/sentry/NoOpScope : io/sentry/IScope {
public fun removeContexts (Ljava/lang/String;)V
public fun removeExtra (Ljava/lang/String;)V
public fun removeTag (Ljava/lang/String;)V
public fun setClient (Lio/sentry/ISentryClient;)V
public fun replaceOptions (Lio/sentry/SentryOptions;)V
public fun setContexts (Ljava/lang/String;Ljava/lang/Boolean;)V
public fun setContexts (Ljava/lang/String;Ljava/lang/Character;)V
public fun setContexts (Ljava/lang/String;Ljava/lang/Number;)V
Expand Down Expand Up @@ -1911,7 +1912,7 @@ public final class io/sentry/Scope : io/sentry/IScope {
public fun removeContexts (Ljava/lang/String;)V
public fun removeExtra (Ljava/lang/String;)V
public fun removeTag (Ljava/lang/String;)V
public fun setClient (Lio/sentry/ISentryClient;)V
public fun replaceOptions (Lio/sentry/SentryOptions;)V
public fun setContexts (Ljava/lang/String;Ljava/lang/Boolean;)V
public fun setContexts (Ljava/lang/String;Ljava/lang/Character;)V
public fun setContexts (Ljava/lang/String;Ljava/lang/Number;)V
Expand Down
9 changes: 8 additions & 1 deletion sentry/src/main/java/io/sentry/CombinedScopeView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.Queue;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

Expand Down Expand Up @@ -395,7 +396,7 @@ public void withTransaction(Scope.@NotNull IWithTransaction callback) {

@Override
public @NotNull SentryOptions getOptions() {
return scope.getOptions();
return globalScope.getOptions();
}

@Override
Expand Down Expand Up @@ -474,4 +475,10 @@ public void setSpanContext(
@NotNull Throwable throwable, @NotNull ISpan span, @NotNull String transactionName) {
globalScope.setSpanContext(throwable, span, transactionName);
}

@ApiStatus.Internal
@Override
public void replaceOptions(@NotNull SentryOptions options) {
globalScope.replaceOptions(options);
}
}
3 changes: 3 additions & 0 deletions sentry/src/main/java/io/sentry/IScope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,4 +396,7 @@ void setSpanContext(
final @NotNull Throwable throwable,
final @NotNull ISpan span,
final @NotNull String transactionName);

@ApiStatus.Internal
void replaceOptions(final @NotNull SentryOptions options);
}
3 changes: 3 additions & 0 deletions sentry/src/main/java/io/sentry/NoOpScope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,4 +276,7 @@ public void assignTraceContext(@NotNull SentryEvent event) {}
@Override
public void setSpanContext(
@NotNull Throwable throwable, @NotNull ISpan span, @NotNull String transactionName) {}

@Override
public void replaceOptions(@NotNull SentryOptions options) {}
}
21 changes: 19 additions & 2 deletions sentry/src/main/java/io/sentry/Scope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public final class Scope implements IScope {
private @NotNull List<String> fingerprint = new ArrayList<>();

/** Scope's breadcrumb queue */
private final @NotNull Queue<Breadcrumb> breadcrumbs;
private volatile @NotNull Queue<Breadcrumb> breadcrumbs;

/** Scope's tags */
private @NotNull Map<String, @NotNull String> tags = new ConcurrentHashMap<>();
Expand All @@ -66,7 +66,7 @@ public final class Scope implements IScope {
private @NotNull List<EventProcessorAndOrder> eventProcessors = new CopyOnWriteArrayList<>();

/** Scope's SentryOptions */
private final @NotNull SentryOptions options;
private volatile @NotNull SentryOptions options;

// TODO Consider: Scope clone doesn't clone sessions

Expand Down Expand Up @@ -1038,6 +1038,23 @@ public void setSpanContext(
}
}

@ApiStatus.Internal
@Override
public void replaceOptions(final @NotNull SentryOptions options) {
// TODO [HSM] check if already enabled and noop in that case?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be used after Sentry.init since only some options are actually updated.

// if (!isEnabled()) {}
this.options = options;
final Queue<Breadcrumb> oldBreadcrumbs = breadcrumbs;
breadcrumbs = createBreadcrumbsList(options.getMaxBreadcrumbs());
for (Breadcrumb breadcrumb : oldBreadcrumbs) {
/*
this should trigger beforeBreadcrumb
and notify observers for breadcrumbs added before options where customized in Sentry.init
*/
addBreadcrumb(breadcrumb);
}
}

/** The IWithTransaction callback */
@ApiStatus.Internal
public interface IWithTransaction {
Expand Down
Loading