Merge 62cb91a into bf4a7bf

adinauer · web-flow · commit 8e897f0943a1 · 2024-04-15T12:37:01.000Z
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java
@@ -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";
@@ -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";
@@ -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);
diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryTaskDecorator.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/SentryTaskDecorator.java
@@ -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");
 
diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/ReactorUtils.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/webflux/ReactorUtils.java
@@ -9,8 +9,8 @@
 import reactor.util.context.Context;
 
 @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 {
 
   /**
diff --git a/sentry-spring/src/main/java/io/sentry/spring/SentryTaskDecorator.java b/sentry-spring/src/main/java/io/sentry/spring/SentryTaskDecorator.java
@@ -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");
 
diff --git a/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java b/sentry-spring/src/main/java/io/sentry/spring/webflux/SentryWebFilter.java
@@ -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);
     }
diff --git a/sentry/src/main/java/io/sentry/CombinedScopeView.java b/sentry/src/main/java/io/sentry/CombinedScopeView.java
@@ -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);
@@ -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));
   }
@@ -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);
   }
 
@@ -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);
   }
 
@@ -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;
     }
@@ -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());
@@ -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();
   }
 
@@ -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?
     final @Nullable ISentryClient current = scope.getClient();
     if (!(current instanceof NoOpSentryClient)) {
       return current;
diff --git a/sentry/src/main/java/io/sentry/DefaultScopesStorage.java b/sentry/src/main/java/io/sentry/DefaultScopesStorage.java
@@ -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();
   }
diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java
@@ -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();
   }
 
diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java
@@ -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<>());
 
@@ -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;
diff --git a/sentry/src/main/java/io/sentry/ScopeType.java b/sentry/src/main/java/io/sentry/ScopeType.java
@@ -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;
 }
diff --git a/sentry/src/main/java/io/sentry/Scopes.java b/sentry/src/main/java/io/sentry/Scopes.java
@@ -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;
@@ -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;
@@ -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;
@@ -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);
   }
 
@@ -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();
@@ -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:
@@ -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
@@ -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()) {
@@ -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()) {
@@ -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) {
@@ -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"));
   }
 
@@ -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;
@@ -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);
diff --git a/sentry/src/main/java/io/sentry/ScopesAdapter.java b/sentry/src/main/java/io/sentry/ScopesAdapter.java
@@ -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();
   }
 
diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java
@@ -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 */
@@ -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");
@@ -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();
diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java
@@ -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;
 
diff --git a/sentry/src/main/java/io/sentry/SentryWrapper.java b/sentry/src/main/java/io/sentry/SentryWrapper.java

Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,6 @@ public Mono<Void> filter(`
`51`	`51`	`final @NotNull ServerWebExchange serverWebExchange,`
`52`	`52`	`final @NotNull WebFilterChain webFilterChain) {`
`53`	`53`	`@NotNull IScopes requestHub = Sentry.forkedRootScopes("request.webflux");`
`54`		`- // TODO do not push / pop, use fork instead`
`55`	`54`	`if (!requestHub.isEnabled()) {`
`56`	`55`	`return webFilterChain.filter(serverWebExchange);`
`57`	`56`	`}`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ public ISentryLifecycleToken set(@Nullable IScopes scopes) {`
`21`	`21`
`22`	`22`	`@Override`
`23`	`23`	`public void close() {`
`24`		`- // TODO prevent further storing? would this cause problems if singleton, closed and`
	`24`	`+ // TODO [HSM] prevent further storing? would this cause problems if singleton, closed and`
`25`	`25`	`// re-initialized?`
`26`	`26`	`currentScopes.remove();`
`27`	`27`	`}`
Original file line number	Diff line number	Diff line change
`@@ -210,7 +210,7 @@ public void flush(long timeoutMillis) {`
`210`	`210`
`211`	`211`	`@Override`
`212`	`212`	`public @NotNull ISentryLifecycleToken makeCurrent() {`
`213`		`- // TODO this wouldn't do anything since it replaced the current with the same Scopes`
	`213`	`+ // TODO [HSM] this wouldn't do anything since it replaced the current with the same Scopes`
`214`	`214`	`return NoOpScopesStorage.NoOpScopesLifecycleToken.getInstance();`
`215`	`215`	`}`
`216`	`216`
Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,6 @@ public enum ScopeType {`
`5`	`5`	`ISOLATION,`
`6`	`6`	`GLOBAL,`
`7`	`7`
`8`		`- // TODO do we need a combined as well so configureScope`
	`8`	`+ // TODO [HSM] do we need a combined as well so configureScope`
`9`	`9`	`COMBINED;`
`10`	`10`	`}`