Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 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
5b8902f
Merge branch '8.x.x' into feat/hsm-29-todos
adinauer Apr 23, 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
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ final class ManifestMetadataReader {
static final String SDK_NAME = "io.sentry.sdk.name";
static final String SDK_VERSION = "io.sentry.sdk.version";

// TODO: remove on 6.x in favor of SESSION_AUTO_TRACKING_ENABLE
// TODO [MAJOR]: remove on 6.x in favor of SESSION_AUTO_TRACKING_ENABLE
static final String SESSION_TRACKING_ENABLE = "io.sentry.session-tracking.enable";

static final String AUTO_SESSION_TRACKING_ENABLE = "io.sentry.auto-session-tracking.enable";
Expand Down Expand Up @@ -70,7 +70,7 @@ final class ManifestMetadataReader {

@ApiStatus.Experimental static final String TRACE_SAMPLING = "io.sentry.traces.trace-sampling";

// TODO: remove in favor of TRACE_PROPAGATION_TARGETS
// TODO [MAJOR]: remove in favor of TRACE_PROPAGATION_TARGETS
@Deprecated static final String TRACING_ORIGINS = "io.sentry.traces.tracing-origins";

static final String TRACE_PROPAGATION_TARGETS = "io.sentry.traces.trace-propagation-targets";
Expand Down Expand Up @@ -323,7 +323,7 @@ static void applyMetadata(
List<String> tracePropagationTargets =
readList(metadata, logger, TRACE_PROPAGATION_TARGETS);

// TODO remove once TRACING_ORIGINS have been removed
// TODO [MAJOR] remove once TRACING_ORIGINS have been removed
if (!metadata.containsKey(TRACE_PROPAGATION_TARGETS)
&& (tracePropagationTargets == null || tracePropagationTargets.isEmpty())) {
tracePropagationTargets = readList(metadata, logger, TRACING_ORIGINS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
*/
public final class SentryTaskDecorator implements TaskDecorator {
@Override
// TODO should there also be a SentryIsolatedTaskDecorator or similar that uses forkedScopes()?
// TODO [HSM] should there also be a SentryIsolatedTaskDecorator or similar that uses
// forkedScopes()?
public @NotNull Runnable decorate(final @NotNull Runnable runnable) {
final IScopes newScopes = Sentry.getCurrentScopes().forkedCurrentScope("spring.taskDecorator");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@

// 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?
// TODO [HSM] do we keep old methods around and deprecate them?
// TODO [HSM] do we need to offer isolated variants?
public final class ReactorUtils {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
*/
public final class SentryTaskDecorator implements TaskDecorator {
@Override
// TODO should there also be a SentryIsolatedTaskDecorator or similar that uses forkedScopes()?
// TODO [HSM] should there also be a SentryIsolatedTaskDecorator or similar that uses
// forkedScopes()?
public @NotNull Runnable decorate(final @NotNull Runnable runnable) {
final IScopes newHub = Sentry.getCurrentScopes().forkedCurrentScope("spring.taskDecorator");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ public Mono<Void> filter(
final @NotNull ServerWebExchange serverWebExchange,
final @NotNull WebFilterChain webFilterChain) {
@NotNull IScopes requestHub = Sentry.forkedRootScopes("request.webflux");
// TODO do not push / pop, use fork instead
if (!requestHub.isEnabled()) {
return webFilterChain.filter(serverWebExchange);
}
Expand Down
17 changes: 8 additions & 9 deletions sentry/src/main/java/io/sentry/CombinedScopeView.java
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ public void setFingerprint(@NotNull List<String> fingerprint) {
allBreadcrumbs.addAll(scope.getBreadcrumbs());
Collections.sort(allBreadcrumbs);

// TODO test oldest are removed first
final @NotNull Queue<Breadcrumb> breadcrumbs =
createBreadcrumbsList(scope.getOptions().getMaxBreadcrumbs());
breadcrumbs.addAll(allBreadcrumbs);
Expand All @@ -178,7 +177,7 @@ public void setFingerprint(@NotNull List<String> fingerprint) {
* @param maxBreadcrumb the max number of breadcrumbs
* @return the breadcrumbs queue
*/
// TODO copied from Scope, should reuse instead
// TODO [HSM] copied from Scope, should reuse instead
private @NotNull Queue<Breadcrumb> createBreadcrumbsList(final int maxBreadcrumb) {
return SynchronizedQueue.synchronizedQueue(new CircularFifoQueue<>(maxBreadcrumb));
}
Expand Down Expand Up @@ -237,7 +236,7 @@ public void setTag(@NotNull String key, @NotNull String value) {

@Override
public void removeTag(@NotNull String key) {
// TODO should this go to all scopes?
// TODO [HSM] should this go to all scopes?
getDefaultWriteScope().removeTag(key);
}

Expand All @@ -257,7 +256,7 @@ public void setExtra(@NotNull String key, @NotNull String value) {

@Override
public void removeExtra(@NotNull String key) {
// TODO should this go to all scopes?
// TODO [HSM] should this go to all scopes?
getDefaultWriteScope().removeExtra(key);
}

Expand Down Expand Up @@ -307,12 +306,12 @@ public void setContexts(@NotNull String key, @NotNull Character value) {

@Override
public void removeContexts(@NotNull String key) {
// TODO should this go to all scopes?
// TODO [HSM] should this go to all scopes?
getDefaultWriteScope().removeContexts(key);
}

private @NotNull IScope getDefaultWriteScope() {
// TODO use Scopes.getSpecificScope?
// TODO [HSM] use Scopes.getSpecificScope?
if (ScopeType.CURRENT.equals(getOptions().getDefaultScopeType())) {
return scope;
}
Expand Down Expand Up @@ -343,7 +342,7 @@ public void clearAttachments() {

@Override
public @NotNull List<EventProcessor> getEventProcessors() {
// TODO mechanism for ordering event processors
// TODO [HSM] mechanism for ordering event processors
final @NotNull List<EventProcessor> allEventProcessors = new CopyOnWriteArrayList<>();
allEventProcessors.addAll(globalScope.getEventProcessors());
allEventProcessors.addAll(isolationScope.getEventProcessors());
Expand Down Expand Up @@ -412,7 +411,7 @@ public void setPropagationContext(@NotNull PropagationContext propagationContext

@Override
public @NotNull IScope clone() {
// TODO just return a new CombinedScopeView with forked scope?
// TODO [HSM] just return a new CombinedScopeView with forked scope?
return getDefaultWriteScope().clone();
}

Expand All @@ -435,7 +434,7 @@ public void bindClient(@NotNull ISentryClient client) {

@Override
public @NotNull ISentryClient getClient() {
// TODO checking for noop here doesn't allow disabling via client, is that ok?
// TODO [HSM] checking for noop here doesn't allow disabling via client, is that ok?
Copy link
Member Author

Choose a reason for hiding this comment

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

Should a customer want to disable Sentry this way, they can provide their own implementation of ISentryClient that is noop but not detected as such here.

final @Nullable ISentryClient current = scope.getClient();
if (!(current instanceof NoOpSentryClient)) {
return current;
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/DefaultScopesStorage.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public ISentryLifecycleToken set(@Nullable IScopes scopes) {

@Override
public void close() {
// TODO prevent further storing? would this cause problems if singleton, closed and
// TODO [HSM] prevent further storing? would this cause problems if singleton, closed and
// re-initialized?
currentScopes.remove();
}
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/HubAdapter.java
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public void flush(long timeoutMillis) {

@Override
public @NotNull ISentryLifecycleToken makeCurrent() {
// TODO this wouldn't do anything since it replaced the current with the same Scopes
// TODO [HSM] this wouldn't do anything since it replaced the current with the same Scopes
return NoOpScopesStorage.NoOpScopesLifecycleToken.getInstance();
}

Expand Down
6 changes: 3 additions & 3 deletions sentry/src/main/java/io/sentry/Scope.java
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ public final class Scope implements IScope {

private @NotNull ISentryClient client = NoOpSentryClient.getInstance();

// TODO intended only for global scope
// TODO test for memory leak
// TODO [HSM] intended only for global scope
// TODO [HSM] test for memory leak
private final @NotNull Map<Throwable, Pair<WeakReference<ISpan>, String>> throwableToSpan =
Collections.synchronizedMap(new WeakHashMap<>());

Expand All @@ -114,7 +114,7 @@ private Scope(final @NotNull Scope scope) {
this.options = scope.options;
this.level = scope.level;
this.client = scope.client;
// TODO should we do this? didn't do it for Hub
// TODO [HSM] should we do this? didn't do it for Hub
this.lastEventId = scope.getLastEventId();

final User userRef = scope.user;
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/ScopeType.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ public enum ScopeType {
ISOLATION,
GLOBAL,

// TODO do we need a combined as well so configureScope
// TODO [HSM] do we need a combined as well so configureScope
COMBINED;
}
52 changes: 16 additions & 36 deletions sentry/src/main/java/io/sentry/Scopes.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ public final class Scopes implements IScopes, MetricsApi.IMetricsInterface {

private final @NotNull IScope scope;
private final @NotNull IScope isolationScope;
// TODO just for debugging

@SuppressWarnings("UnusedVariable")
private final @Nullable Scopes parentScopes;

private final @NotNull String creator;

// TODO should this be set on all scopes (global, isolation, current)?
// TODO [HSM] should this be set on all scopes (global, isolation, current)?
private final @NotNull SentryOptions options;
private volatile boolean isEnabled;
private final @NotNull TracesSampler tracesSampler;
Expand Down Expand Up @@ -82,12 +82,12 @@ private Scopes(
return isolationScope;
}

// TODO add to IScopes interface?
// TODO [HSM] add to IScopes interface?
public @Nullable Scopes getParent() {
return parentScopes;
}

// TODO add to IScopes interface?
// TODO [HSM] add to IScopes interface?
public boolean isAncestorOf(final @Nullable Scopes otherScopes) {
if (otherScopes == null) {
return false;
Expand Down Expand Up @@ -115,7 +115,7 @@ public boolean isAncestorOf(final @Nullable Scopes otherScopes) {
return new Scopes(scope.clone(), isolationScope, this, options, creator);
}

// TODO always read from root scope?
// TODO [HSM] always read from root scope?
@Override
public boolean isEnabled() {
return isEnabled;
Expand Down Expand Up @@ -363,7 +363,7 @@ public void endSession() {
}

private IScope getCombinedScopeView() {
// TODO create in ctor?
// TODO [HSM] create in ctor?
return new CombinedScopeView(getGlobalScope(), isolationScope, scope);
}

Expand Down Expand Up @@ -393,7 +393,7 @@ public void close(final boolean isRestarting) {
}
}

// TODO which scopes do we call this on? isolation and current scope?
// TODO [HSM] which scopes do we call this on? isolation and current scope?
configureScope(scope -> scope.clear());
options.getTransactionProfiler().close();
options.getTransactionPerformanceCollector().close();
Expand Down Expand Up @@ -429,7 +429,7 @@ public void addBreadcrumb(final @NotNull Breadcrumb breadcrumb, final @Nullable
}

private IScope getSpecificScope(final @Nullable ScopeType scopeType) {
// TODO extract and reuse
// TODO [HSM] extract and reuse
if (scopeType != null) {
switch (scopeType) {
case CURRENT:
Expand Down Expand Up @@ -582,11 +582,9 @@ private void updateLastEventId(final @NotNull SentryId lastEventId) {
getCombinedScopeView().setLastEventId(lastEventId);
}

// TODO add to IScopes interface
// TODO [HSM] add to IScopes interface
public @NotNull IScope getGlobalScope() {
// TODO should be:
return Sentry.getGlobalScope();
// return scope;
}

@Override
Expand Down Expand Up @@ -627,7 +625,7 @@ public ISentryLifecycleToken pushIsolationScope() {
return Sentry.setCurrentScopes(this);
}

// TODO needs to be deprecated because there's no more stack
// TODO [HSM] needs to be deprecated because there's no more stack
@Override
public void popScope() {
if (!isEnabled()) {
Expand All @@ -637,13 +635,13 @@ public void popScope() {
} else {
final @Nullable Scopes parent = getParent();
if (parent != null) {
// TODO this is never closed
// TODO [HSM] this is never closed
parent.makeCurrent();
}
}
}

// TODO lots of testing required to see how ThreadLocal is affected
// TODO [HSM] lots of testing required to see how ThreadLocal is affected
@Override
public void withScope(final @NotNull ScopeCallback callback) {
if (!isEnabled()) {
Expand All @@ -655,8 +653,8 @@ public void withScope(final @NotNull ScopeCallback callback) {

} else {
final @NotNull IScopes forkedScopes = forkedCurrentScope("withScope");
// TODO should forkedScopes be made current inside callback?
// TODO forkedScopes.makeCurrent()?
// TODO [HSM] should forkedScopes be made current inside callback?
// TODO [HSM] forkedScopes.makeCurrent()?
try {
callback.run(forkedScopes.getScope());
} catch (Throwable e) {
Expand Down Expand Up @@ -726,7 +724,7 @@ public void flush(long timeoutMillis) {
if (!isEnabled()) {
options.getLogger().log(SentryLevel.WARNING, "Disabled Hub cloned.");
}
// TODO should this fork isolation scope as well?
// TODO [HSM] should this fork isolation scope as well?
return new HubScopesWrapper(forkedCurrentScope("scopes clone"));
}

Expand Down Expand Up @@ -876,24 +874,6 @@ public void setSpanContext(
getCombinedScopeView().setSpanContext(throwable, span, transactionName);
}

// // TODO this seems unused
// @Nullable
// SpanContext getSpanContext(final @NotNull Throwable throwable) {
// Objects.requireNonNull(throwable, "throwable is required");
// final Throwable rootCause = ExceptionUtils.findRootCause(throwable);
// final Pair<WeakReference<ISpan>, String> pair = this.throwableToSpan.get(rootCause);
// if (pair != null) {
// final WeakReference<ISpan> spanWeakRef = pair.getFirst();
// if (spanWeakRef != null) {
// final ISpan span = spanWeakRef.get();
// if (span != null) {
// return span.getSpanContext();
// }
// }
// }
// return null;
// }

@Override
public @Nullable ISpan getSpan() {
ISpan span = null;
Expand Down Expand Up @@ -947,7 +927,7 @@ public void reportFullyDisplayed() {
@NotNull
PropagationContext propagationContext =
PropagationContext.fromHeaders(getOptions().getLogger(), sentryTrace, baggageHeaders);
// TODO should this go on isolation scope?
// TODO [HSM] should this go on isolation scope?
configureScope(
(scope) -> {
scope.setPropagationContext(propagationContext);
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/ScopesAdapter.java
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ public void flush(long timeoutMillis) {

@Override
public @NotNull ISentryLifecycleToken makeCurrent() {
// TODO this wouldn't do anything since it replaced the current with the same Scopes
// TODO [HSM] this wouldn't do anything since it replaced the current with the same Scopes
return NoOpScopesStorage.NoOpScopesLifecycleToken.getInstance();
}

Expand Down
5 changes: 2 additions & 3 deletions sentry/src/main/java/io/sentry/Sentry.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ private Sentry() {}

/** The root Scopes or NoOp if Sentry is disabled. */
private static volatile @NotNull IScopes rootScopes = NoOpScopes.getInstance();
// TODO cannot pass options here
// TODO [HSM] cannot pass options here
private static volatile @NotNull IScope globalScope = new Scope(new SentryOptions());

/** Default value for globalHubMode is false */
Expand Down Expand Up @@ -271,7 +271,7 @@ private static synchronized void init(
final IScopes scopes = getCurrentScopes();
final IScope rootScope = new Scope(options);
final IScope rootIsolationScope = new Scope(options);
// TODO shouldn't replace global scope
// TODO [HSM] shouldn't replace global scope
globalScope = new Scope(options);
globalScope.bindClient(new SentryClient(options));
rootScopes = new Scopes(rootScope, rootIsolationScope, options, "Sentry.init");
Expand Down Expand Up @@ -819,7 +819,6 @@ public static void removeExtra(final @NotNull String key) {
public static @NotNull ISentryLifecycleToken pushScope() {
// pushScope is no-op in global hub mode
if (!globalHubMode) {
// TODO this might have to behave differently from Scopes.pushScope
return getCurrentScopes().pushScope();
}
return NoOpScopesStorage.NoOpScopesLifecycleToken.getInstance();
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/SentryOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ public class SentryOptions {

private @NotNull IMainThreadChecker mainThreadChecker = NoOpMainThreadChecker.getInstance();

// TODO this should default to false on the next major
// TODO [MAJOR] this should default to false on the next major
/** Whether OPTIONS requests should be traced. */
private boolean traceOptionsRequests = true;

Expand Down
Loading