Implement pushScope popScope and withScope for Scopes

adinauer · adinauer · commit 98da9ff6e4a1 · 2024-04-04T16:28:30.000+02:00
diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt
@@ -252,10 +252,10 @@ class SentryWebFluxTracingFilterTest {
             verify(fixture.scopes, times(3)).isEnabled
             verify(fixture.scopes, times(2)).options
             verify(fixture.scopes).continueTrace(anyOrNull(), anyOrNull())
-            verify(fixture.scopes).pushScope()
+            verify(fixture.scopes).pushScope() // TODO don't
             verify(fixture.scopes).addBreadcrumb(any<Breadcrumb>(), any<Hint>())
             verify(fixture.scopes).configureScope(any<ScopeCallback>())
-            verify(fixture.scopes).popScope()
+            verify(fixture.scopes).popScope() // TODO don't
             verifyNoMoreInteractions(fixture.scopes)
         }
     }
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
@@ -81,7 +81,7 @@ isTracingEnabled && shouldTraceRequest(requestHub, request)
               if (transaction != null) {
                 finishTransaction(serverWebExchange, transaction);
               }
-              requestHub.popScope();
+              requestHub.popScope(); // TODO don't
               // TODO token based cleanup instead?
               Sentry.setCurrentScopes(NoOpScopes.getInstance());
             })
@@ -96,7 +96,7 @@ isTracingEnabled && shouldTraceRequest(requestHub, request)
             () -> {
               serverWebExchange.getAttributes().put(SENTRY_SCOPES_KEY, requestHub);
               Sentry.setCurrentScopes(requestHub);
-              requestHub.pushScope();
+              requestHub.pushScope(); // TODO don't
               final ServerHttpResponse response = serverWebExchange.getResponse();
 
               final Hint hint = new Hint();
diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt
@@ -253,10 +253,10 @@ class SentryWebFluxTracingFilterTest {
             verify(fixture.scopes).isEnabled
             verify(fixture.scopes, times(2)).options
             verify(fixture.scopes).continueTrace(anyOrNull(), anyOrNull())
-            verify(fixture.scopes).pushScope()
+            verify(fixture.scopes).pushScope() // TODO don't
             verify(fixture.scopes).addBreadcrumb(any<Breadcrumb>(), any<Hint>())
             verify(fixture.scopes).configureScope(any<ScopeCallback>())
-            verify(fixture.scopes).popScope()
+            verify(fixture.scopes).popScope() // TODO don't
             verifyNoMoreInteractions(fixture.scopes)
         }
     }
diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api
@@ -456,7 +456,7 @@ public final class io/sentry/Hub : io/sentry/IHub, io/sentry/metrics/MetricsApi$
 	public fun isHealthy ()Z
 	public fun metrics ()Lio/sentry/metrics/MetricsApi;
 	public fun popScope ()V
-	public fun pushScope ()V
+	public fun pushScope ()Lio/sentry/ISentryLifecycleToken;
 	public fun removeExtra (Ljava/lang/String;)V
 	public fun removeTag (Ljava/lang/String;)V
 	public fun reportFullyDisplayed ()V
@@ -510,7 +510,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub {
 	public fun isHealthy ()Z
 	public fun metrics ()Lio/sentry/metrics/MetricsApi;
 	public fun popScope ()V
-	public fun pushScope ()V
+	public fun pushScope ()Lio/sentry/ISentryLifecycleToken;
 	public fun removeExtra (Ljava/lang/String;)V
 	public fun removeTag (Ljava/lang/String;)V
 	public fun reportFullyDisplayed ()V
@@ -563,7 +563,7 @@ public final class io/sentry/HubScopesWrapper : io/sentry/IHub {
 	public fun isHealthy ()Z
 	public fun metrics ()Lio/sentry/metrics/MetricsApi;
 	public fun popScope ()V
-	public fun pushScope ()V
+	public fun pushScope ()Lio/sentry/ISentryLifecycleToken;
 	public fun removeExtra (Ljava/lang/String;)V
 	public fun removeTag (Ljava/lang/String;)V
 	public fun reportFullyDisplayed ()V
@@ -780,7 +780,7 @@ public abstract interface class io/sentry/IScopes {
 	public fun isNoOp ()Z
 	public abstract fun metrics ()Lio/sentry/metrics/MetricsApi;
 	public abstract fun popScope ()V
-	public abstract fun pushScope ()V
+	public abstract fun pushScope ()Lio/sentry/ISentryLifecycleToken;
 	public abstract fun removeExtra (Ljava/lang/String;)V
 	public abstract fun removeTag (Ljava/lang/String;)V
 	public fun reportFullDisplayed ()V
@@ -1270,7 +1270,7 @@ public final class io/sentry/NoOpHub : io/sentry/IHub {
 	public fun isNoOp ()Z
 	public fun metrics ()Lio/sentry/metrics/MetricsApi;
 	public fun popScope ()V
-	public fun pushScope ()V
+	public fun pushScope ()Lio/sentry/ISentryLifecycleToken;
 	public fun removeExtra (Ljava/lang/String;)V
 	public fun removeTag (Ljava/lang/String;)V
 	public fun reportFullyDisplayed ()V
@@ -1394,7 +1394,7 @@ public final class io/sentry/NoOpScopes : io/sentry/IScopes {
 	public fun isNoOp ()Z
 	public fun metrics ()Lio/sentry/metrics/MetricsApi;
 	public fun popScope ()V
-	public fun pushScope ()V
+	public fun pushScope ()Lio/sentry/ISentryLifecycleToken;
 	public fun removeExtra (Ljava/lang/String;)V
 	public fun removeTag (Ljava/lang/String;)V
 	public fun reportFullyDisplayed ()V
@@ -1866,9 +1866,10 @@ public final class io/sentry/Scopes : io/sentry/IScopes, io/sentry/metrics/Metri
 	public fun isCrashedLastRun ()Ljava/lang/Boolean;
 	public fun isEnabled ()Z
 	public fun isHealthy ()Z
+	public fun makeCurrent ()Lio/sentry/ISentryLifecycleToken;
 	public fun metrics ()Lio/sentry/metrics/MetricsApi;
 	public fun popScope ()V
-	public fun pushScope ()V
+	public fun pushScope ()Lio/sentry/ISentryLifecycleToken;
 	public fun removeExtra (Ljava/lang/String;)V
 	public fun removeTag (Ljava/lang/String;)V
 	public fun reportFullyDisplayed ()V
@@ -1922,7 +1923,7 @@ public final class io/sentry/ScopesAdapter : io/sentry/IScopes {
 	public fun isHealthy ()Z
 	public fun metrics ()Lio/sentry/metrics/MetricsApi;
 	public fun popScope ()V
-	public fun pushScope ()V
+	public fun pushScope ()Lio/sentry/ISentryLifecycleToken;
 	public fun removeExtra (Ljava/lang/String;)V
 	public fun removeTag (Ljava/lang/String;)V
 	public fun reportFullyDisplayed ()V
@@ -2017,13 +2018,13 @@ public final class io/sentry/Sentry {
 	public static fun isHealthy ()Z
 	public static fun metrics ()Lio/sentry/metrics/MetricsApi;
 	public static fun popScope ()V
-	public static fun pushScope ()V
+	public static fun pushScope ()Lio/sentry/ISentryLifecycleToken;
 	public static fun removeExtra (Ljava/lang/String;)V
 	public static fun removeTag (Ljava/lang/String;)V
 	public static fun reportFullDisplayed ()V
 	public static fun reportFullyDisplayed ()V
 	public static fun setCurrentHub (Lio/sentry/IHub;)V
-	public static fun setCurrentScopes (Lio/sentry/IScopes;)V
+	public static fun setCurrentScopes (Lio/sentry/IScopes;)Lio/sentry/ISentryLifecycleToken;
 	public static fun setExtra (Ljava/lang/String;Ljava/lang/String;)V
 	public static fun setFingerprint (Ljava/util/List;)V
 	public static fun setLevel (Lio/sentry/SentryLevel;)V
diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java
@@ -526,7 +526,7 @@ public void removeExtra(final @NotNull String key) {
   }
 
   @Override
-  public void pushScope() {
+  public @NotNull ISentryLifecycleToken pushScope() {
     if (!isEnabled()) {
       options
           .getLogger()
@@ -536,6 +536,7 @@ public void pushScope() {
       final StackItem newItem = new StackItem(options, item.getClient(), item.getScope().clone());
       stack.push(newItem);
     }
+    return NoOpScopesStorage.NoOpScopesLifecycleToken.getInstance();
   }
 
   @Override
diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java
@@ -154,8 +154,8 @@ public void removeExtra(@NotNull String key) {
   }
 
   @Override
-  public void pushScope() {
-    Sentry.pushScope();
+  public @NotNull ISentryLifecycleToken pushScope() {
+    return Sentry.pushScope();
   }
 
   @Override
diff --git a/sentry/src/main/java/io/sentry/HubScopesWrapper.java b/sentry/src/main/java/io/sentry/HubScopesWrapper.java
@@ -149,8 +149,8 @@ public void removeExtra(@NotNull String key) {
   }
 
   @Override
-  public void pushScope() {
-    scopes.pushScope();
+  public @NotNull ISentryLifecycleToken pushScope() {
+    return scopes.pushScope();
   }
 
   @Override
diff --git a/sentry/src/main/java/io/sentry/IScopes.java b/sentry/src/main/java/io/sentry/IScopes.java
@@ -306,7 +306,8 @@ default void addBreadcrumb(@NotNull String message, @NotNull String category) {
   SentryId getLastEventId();
 
   /** Pushes a new scope while inheriting the current scope's data. */
-  void pushScope();
+  @NotNull
+  ISentryLifecycleToken pushScope();
 
   /** Removes the first scope */
   void popScope();
diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java
@@ -124,7 +124,9 @@ public void removeExtra(@NotNull String key) {}
   }
 
   @Override
-  public void pushScope() {}
+  public @NotNull ISentryLifecycleToken pushScope() {
+    return NoOpScopesStorage.NoOpScopesLifecycleToken.getInstance();
+  }
 
   @Override
   public void popScope() {}
diff --git a/sentry/src/main/java/io/sentry/NoOpScopes.java b/sentry/src/main/java/io/sentry/NoOpScopes.java
@@ -122,7 +122,9 @@ public void removeExtra(@NotNull String key) {}
   }
 
   @Override
-  public void pushScope() {}
+  public @NotNull ISentryLifecycleToken pushScope() {
+    return NoOpScopesStorage.NoOpScopesLifecycleToken.getInstance();
+  }
 
   @Override
   public void popScope() {}
diff --git a/sentry/src/main/java/io/sentry/Scopes.java b/sentry/src/main/java/io/sentry/Scopes.java
@@ -595,20 +595,21 @@ private void updateLastEventId(final @NotNull SentryId lastEventId) {
   // TODO needs to be deprecated because there's no more stack
   // TODO needs to return a lifecycle token
   @Override
-  public void pushScope() {
+  public ISentryLifecycleToken pushScope() {
     if (!isEnabled()) {
       options
           .getLogger()
           .log(SentryLevel.WARNING, "Instance is disabled and this 'pushScope' call is a no-op.");
+      return NoOpScopesStorage.NoOpScopesLifecycleToken.getInstance();
     } else {
-      //      Scopes scopes = this.forkedScopes("pushScope");
-      //      return scopes.makeCurrent();
+      Scopes scopes = this.forkedCurrentScope("pushScope");
+      return scopes.makeCurrent();
     }
   }
 
-  //  public SentryLifecycleToken makeCurrent() {
-  //    // TODO store.set(this);
-  //  }
+  public ISentryLifecycleToken makeCurrent() {
+    return Sentry.setCurrentScopes(this);
+  }
 
   // TODO needs to be deprecated because there's no more stack
   @Override
@@ -618,8 +619,11 @@ public void popScope() {
           .getLogger()
           .log(SentryLevel.WARNING, "Instance is disabled and this 'popScope' call is a no-op.");
     } else {
-      // TODO how to remove fork?
-      // TODO getParentScopes().makeCurrent()?
+      final @Nullable Scopes parent = getParent();
+      if (parent != null) {
+        // TODO this is never closed
+        parent.makeCurrent();
+      }
     }
   }
 
@@ -634,7 +638,7 @@ public void withScope(final @NotNull ScopeCallback callback) {
       }
 
     } else {
-      Scopes forkedScopes = forkedScopes("withScope");
+      Scopes forkedScopes = forkedCurrentScope("withScope");
       // TODO should forkedScopes be made current inside callback?
       // TODO forkedScopes.makeCurrent()?
       try {
@@ -705,7 +709,8 @@ public void flush(long timeoutMillis) {
     if (!isEnabled()) {
       options.getLogger().log(SentryLevel.WARNING, "Disabled Hub cloned.");
     }
-    return new HubScopesWrapper(forkedScopes("scopes clone"));
+    // TODO should this fork isolation scope as well?
+    return new HubScopesWrapper(forkedCurrentScope("scopes clone"));
   }
 
   @ApiStatus.Internal
diff --git a/sentry/src/main/java/io/sentry/ScopesAdapter.java b/sentry/src/main/java/io/sentry/ScopesAdapter.java
@@ -150,8 +150,8 @@ public void removeExtra(@NotNull String key) {
   }
 
   @Override
-  public void pushScope() {
-    Sentry.pushScope();
+  public @NotNull ISentryLifecycleToken pushScope() {
+    return Sentry.pushScope();
   }
 
   @Override
diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java
@@ -813,12 +813,13 @@ public static void removeExtra(final @NotNull String key) {
   }
 
   /** Pushes a new scope while inheriting the current scope's data. */
-  public static void pushScope() {
+  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
-      getCurrentScopes().pushScope();
+      return getCurrentScopes().pushScope();
     }
+    return NoOpScopesStorage.NoOpScopesLifecycleToken.getInstance();
   }
 
   /** Removes the first scope */
diff --git a/sentry/src/test/java/io/sentry/NoOpHubTest.kt b/sentry/src/test/java/io/sentry/NoOpHubTest.kt
@@ -71,7 +71,9 @@ class NoOpHubTest {
     }
 
     @Test
-    fun `pushScope is no op`() = sut.pushScope()
+    fun `pushScope is no op`() {
+        sut.pushScope()
+    }
 
     @Test
     fun `popScope is no op`() = sut.popScope()

Original file line number	Diff line number	Diff line change
`@@ -252,10 +252,10 @@ class SentryWebFluxTracingFilterTest {`
`252`	`252`	`verify(fixture.scopes, times(3)).isEnabled`
`253`	`253`	`verify(fixture.scopes, times(2)).options`
`254`	`254`	`verify(fixture.scopes).continueTrace(anyOrNull(), anyOrNull())`
`255`		`- verify(fixture.scopes).pushScope()`
	`255`	`+ verify(fixture.scopes).pushScope() // TODO don't`
`256`	`256`	`verify(fixture.scopes).addBreadcrumb(any<Breadcrumb>(), any<Hint>())`
`257`	`257`	`verify(fixture.scopes).configureScope(any<ScopeCallback>())`
`258`		`- verify(fixture.scopes).popScope()`
	`258`	`+ verify(fixture.scopes).popScope() // TODO don't`
`259`	`259`	`verifyNoMoreInteractions(fixture.scopes)`
`260`	`260`	`}`
`261`	`261`	`}`
Original file line number	Diff line number	Diff line change
`@@ -253,10 +253,10 @@ class SentryWebFluxTracingFilterTest {`
`253`	`253`	`verify(fixture.scopes).isEnabled`
`254`	`254`	`verify(fixture.scopes, times(2)).options`
`255`	`255`	`verify(fixture.scopes).continueTrace(anyOrNull(), anyOrNull())`
`256`		`- verify(fixture.scopes).pushScope()`
	`256`	`+ verify(fixture.scopes).pushScope() // TODO don't`
`257`	`257`	`verify(fixture.scopes).addBreadcrumb(any<Breadcrumb>(), any<Hint>())`
`258`	`258`	`verify(fixture.scopes).configureScope(any<ScopeCallback>())`
`259`		`- verify(fixture.scopes).popScope()`
	`259`	`+ verify(fixture.scopes).popScope() // TODO don't`
`260`	`260`	`verifyNoMoreInteractions(fixture.scopes)`
`261`	`261`	`}`
`262`	`262`	`}`
Original file line number	Diff line number	Diff line change
`@@ -526,7 +526,7 @@ public void removeExtra(final @NotNull String key) {`
`526`	`526`	`}`
`527`	`527`
`528`	`528`	`@Override`
`529`		`- public void pushScope() {`
	`529`	`+ public @NotNull ISentryLifecycleToken pushScope() {`
`530`	`530`	`if (!isEnabled()) {`
`531`	`531`	`options`
`532`	`532`	`.getLogger()`
`@@ -536,6 +536,7 @@ public void pushScope() {`
`536`	`536`	`final StackItem newItem = new StackItem(options, item.getClient(), item.getScope().clone());`
`537`	`537`	`stack.push(newItem);`
`538`	`538`	`}`
	`539`	`+ return NoOpScopesStorage.NoOpScopesLifecycleToken.getInstance();`
`539`	`540`	`}`
`540`	`541`
`541`	`542`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -154,8 +154,8 @@ public void removeExtra(@NotNull String key) {`
`154`	`154`	`}`
`155`	`155`
`156`	`156`	`@Override`
`157`		`- public void pushScope() {`
`158`		`- Sentry.pushScope();`
	`157`	`+ public @NotNull ISentryLifecycleToken pushScope() {`
	`158`	`+ return Sentry.pushScope();`
`159`	`159`	`}`
`160`	`160`
`161`	`161`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -149,8 +149,8 @@ public void removeExtra(@NotNull String key) {`
`149`	`149`	`}`
`150`	`150`
`151`	`151`	`@Override`
`152`		`- public void pushScope() {`
`153`		`- scopes.pushScope();`
	`152`	`+ public @NotNull ISentryLifecycleToken pushScope() {`
	`153`	`+ return scopes.pushScope();`
`154`	`154`	`}`
`155`	`155`
`156`	`156`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -124,7 +124,9 @@ public void removeExtra(@NotNull String key) {}`
`124`	`124`	`}`
`125`	`125`
`126`	`126`	`@Override`
`127`		`- public void pushScope() {}`
	`127`	`+ public @NotNull ISentryLifecycleToken pushScope() {`
	`128`	`+ return NoOpScopesStorage.NoOpScopesLifecycleToken.getInstance();`
	`129`	`+ }`
`128`	`130`
`129`	`131`	`@Override`
`130`	`132`	`public void popScope() {}`
Original file line number	Diff line number	Diff line change
`@@ -122,7 +122,9 @@ public void removeExtra(@NotNull String key) {}`
`122`	`122`	`}`
`123`	`123`
`124`	`124`	`@Override`
`125`		`- public void pushScope() {}`
	`125`	`+ public @NotNull ISentryLifecycleToken pushScope() {`
	`126`	`+ return NoOpScopesStorage.NoOpScopesLifecycleToken.getInstance();`
	`127`	`+ }`
`126`	`128`
`127`	`129`	`@Override`
`128`	`130`	`public void popScope() {}`