Merge a9dadc4 into 55b103c

markushi · web-flow · commit 3d502fb4fa61 · 2023-08-11T08:32:24.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ Breaking changes:
 - Apollo v2 BeforeSpanCallback now allows returning null ([#2890](https://github.com/getsentry/sentry-java/pull/2890))
 - Automatic user interaction tracking: every click now starts a new automatic transaction ([#2891](https://github.com/getsentry/sentry-java/pull/2891))
   - Previously performing a click on the same UI widget twice would keep the existing transaction running, the new behavior now better aligns with other SDKs
+- Android: If global hub mode is enabled (default on Android), Sentry.getSpan() returns the root span instead of the latest span ([#2855](https://github.com/getsentry/sentry-java/pull/2855))
 
 ### Fixes
 
diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt
@@ -37,7 +37,7 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req
         val method: String = request.method
 
         // We start the call span that will contain all the others
-        callRootSpan = hub.span?.startChild("http.client", "$method $url")
+        callRootSpan = hub.transaction?.startChild("http.client", "$method $url")
         callRootSpan?.spanContext?.origin = TRACE_ORIGIN
         urlDetails.applyToSpan(callRootSpan)
 
diff --git a/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt b/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt
@@ -78,7 +78,7 @@ class SentryOkHttpInterceptor(
             isFromEventListener = true
         } else {
             // read the span from the bound scope
-            span = hub.span?.startChild("http.client", "$method $url")
+            span = hub.transaction?.startChild("http.client", "$method $url")
             isFromEventListener = false
         }
 
diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api
@@ -364,6 +364,7 @@ public final class io/sentry/Hub : io/sentry/IHub {
 	public fun getOptions ()Lio/sentry/SentryOptions;
 	public fun getSpan ()Lio/sentry/ISpan;
 	public fun getTraceparent ()Lio/sentry/SentryTraceHeader;
+	public fun getTransaction ()Lio/sentry/ITransaction;
 	public fun isCrashedLastRun ()Ljava/lang/Boolean;
 	public fun isEnabled ()Z
 	public fun popScope ()V
@@ -411,6 +412,7 @@ public final class io/sentry/HubAdapter : io/sentry/IHub {
 	public fun getOptions ()Lio/sentry/SentryOptions;
 	public fun getSpan ()Lio/sentry/ISpan;
 	public fun getTraceparent ()Lio/sentry/SentryTraceHeader;
+	public fun getTransaction ()Lio/sentry/ITransaction;
 	public fun isCrashedLastRun ()Ljava/lang/Boolean;
 	public fun isEnabled ()Z
 	public fun popScope ()V
@@ -483,6 +485,7 @@ public abstract interface class io/sentry/IHub {
 	public abstract fun getOptions ()Lio/sentry/SentryOptions;
 	public abstract fun getSpan ()Lio/sentry/ISpan;
 	public abstract fun getTraceparent ()Lio/sentry/SentryTraceHeader;
+	public abstract fun getTransaction ()Lio/sentry/ITransaction;
 	public abstract fun isCrashedLastRun ()Ljava/lang/Boolean;
 	public abstract fun isEnabled ()Z
 	public abstract fun popScope ()V
@@ -869,6 +872,7 @@ public final class io/sentry/NoOpHub : io/sentry/IHub {
 	public fun getOptions ()Lio/sentry/SentryOptions;
 	public fun getSpan ()Lio/sentry/ISpan;
 	public fun getTraceparent ()Lio/sentry/SentryTraceHeader;
+	public fun getTransaction ()Lio/sentry/ITransaction;
 	public fun isCrashedLastRun ()Ljava/lang/Boolean;
 	public fun isEnabled ()Z
 	public fun popScope ()V
diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java
@@ -762,6 +762,22 @@ public void flush(long timeoutMillis) {
     return span;
   }
 
+  @Override
+  @ApiStatus.Internal
+  public @Nullable ITransaction getTransaction() {
+    ITransaction span = null;
+    if (!isEnabled()) {
+      options
+          .getLogger()
+          .log(
+              SentryLevel.WARNING,
+              "Instance is disabled and this 'getTransaction' call is a no-op.");
+    } else {
+      span = stack.peek().getScope().getTransaction();
+    }
+    return span;
+  }
+
   @Override
   @ApiStatus.Internal
   public void setSpanContext(
diff --git a/sentry/src/main/java/io/sentry/HubAdapter.java b/sentry/src/main/java/io/sentry/HubAdapter.java
@@ -221,6 +221,12 @@ public void setSpanContext(
     return Sentry.getCurrentHub().getSpan();
   }
 
+  @Override
+  @ApiStatus.Internal
+  public @Nullable ITransaction getTransaction() {
+    return Sentry.getCurrentHub().getTransaction();
+  }
+
   @Override
   public @NotNull SentryOptions getOptions() {
     return Sentry.getCurrentHub().getOptions();
diff --git a/sentry/src/main/java/io/sentry/IHub.java b/sentry/src/main/java/io/sentry/IHub.java
@@ -554,6 +554,15 @@ void setSpanContext(
   @Nullable
   ISpan getSpan();
 
+  /**
+   * Returns the transaction.
+   *
+   * @return the transaction or null when no active transaction is running.
+   */
+  @ApiStatus.Internal
+  @Nullable
+  ITransaction getTransaction();
+
   /**
    * Gets the {@link SentryOptions} attached to current scope.
    *
diff --git a/sentry/src/main/java/io/sentry/NoOpHub.java b/sentry/src/main/java/io/sentry/NoOpHub.java
@@ -179,6 +179,11 @@ public void setSpanContext(
     return null;
   }
 
+  @Override
+  public @Nullable ITransaction getTransaction() {
+    return null;
+  }
+
   @Override
   public @NotNull SentryOptions getOptions() {
     return emptyOptions;
diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java
@@ -15,6 +15,7 @@
 import io.sentry.transport.NoOpEnvelopeCache;
 import io.sentry.util.DebugMetaPropertiesApplier;
 import io.sentry.util.FileUtils;
+import io.sentry.util.Platform;
 import io.sentry.util.thread.IMainThreadChecker;
 import io.sentry.util.thread.MainThreadChecker;
 import io.sentry.util.thread.NoOpMainThreadChecker;
@@ -918,10 +919,16 @@ public static void endSession() {
   /**
    * Gets the current active transaction or span.
    *
-   * @return the active span or null when no active transaction is running
+   * @return the active span or null when no active transaction is running. In case of
+   *     globalHubMode=true, always the active transaction is returned, rather than the last active
+   *     span.
    */
   public static @Nullable ISpan getSpan() {
-    return getCurrentHub().getSpan();
+    if (globalHubMode && Platform.isAndroid()) {
+      return getCurrentHub().getTransaction();
+    } else {
+      return getCurrentHub().getSpan();
+    }
   }
 
   /**
diff --git a/sentry/src/main/java/io/sentry/instrumentation/file/FileIOSpanManager.java b/sentry/src/main/java/io/sentry/instrumentation/file/FileIOSpanManager.java
@@ -29,7 +29,7 @@ final class FileIOSpanManager {
   private final @NotNull SentryStackTraceFactory stackTraceFactory;
 
   static @Nullable ISpan startSpan(final @NotNull IHub hub, final @NotNull String op) {
-    final ISpan parent = hub.getSpan();
+    final ISpan parent = hub.getTransaction();
     return parent != null ? parent.startChild(op) : null;
   }
 
diff --git a/sentry/src/main/java/io/sentry/util/Platform.java b/sentry/src/main/java/io/sentry/util/Platform.java
@@ -6,7 +6,7 @@
 
 @ApiStatus.Internal
 public final class Platform {
-  private static boolean isAndroid;
+  static boolean isAndroid;
   static boolean isJavaNinePlus;
 
   static {
diff --git a/sentry/src/test/java/io/sentry/HubAdapterTest.kt b/sentry/src/test/java/io/sentry/HubAdapterTest.kt
@@ -226,6 +226,11 @@ class HubAdapterTest {
         verify(hub).span
     }
 
+    @Test fun `getTransaction calls Hub`() {
+        HubAdapter.getInstance().transaction
+        verify(hub).transaction
+    }
+
     @Test fun `getOptions calls Hub`() {
         HubAdapter.getInstance().options
         verify(hub).options
diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt
@@ -1607,15 +1607,32 @@ class HubTest {
     @Test
     fun `when there is no active transaction, getSpan returns null`() {
         val hub = generateHub()
-        assertNull(hub.getSpan())
+        assertNull(hub.span)
     }
 
     @Test
-    fun `when there is active transaction bound to the scope, getSpan returns active transaction`() {
+    fun `when there is no active transaction, getTransaction returns null`() {
+        val hub = generateHub()
+        assertNull(hub.transaction)
+    }
+
+    @Test
+    fun `when there is active transaction bound to the scope, getTransaction and getSpan return active transaction`() {
         val hub = generateHub()
         val tx = hub.startTransaction("aTransaction", "op")
-        hub.configureScope { it.setTransaction(tx) }
-        assertEquals(tx, hub.getSpan())
+        hub.configureScope { it.transaction = tx }
+
+        assertEquals(tx, hub.transaction)
+        assertEquals(tx, hub.span)
+    }
+
+    @Test
+    fun `when there is a transaction but the hub is closed, getTransaction returns null`() {
+        val hub = generateHub()
+        hub.startTransaction("name", "op")
+        hub.close()
+
+        assertNull(hub.transaction)
     }
 
     @Test
@@ -1625,6 +1642,8 @@ class HubTest {
         hub.configureScope { it.setTransaction(tx) }
         hub.configureScope { it.setTransaction(tx) }
         val span = tx.startChild("op")
+
+        assertEquals(tx, hub.transaction)
         assertEquals(span, hub.span)
     }
     // endregion
diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt
@@ -9,6 +9,7 @@ import io.sentry.internal.modules.IModulesLoader
 import io.sentry.protocol.SdkVersion
 import io.sentry.protocol.SentryId
 import io.sentry.test.ImmediateExecutorService
+import io.sentry.util.PlatformTestManipulator
 import io.sentry.util.thread.IMainThreadChecker
 import io.sentry.util.thread.MainThreadChecker
 import org.awaitility.kotlin.await
@@ -723,6 +724,65 @@ class SentryTest {
         assertFalse(previousSessionFile.exists())
     }
 
+    @Test
+    fun `getSpan calls hub getSpan`() {
+        val hub = mock<IHub>()
+        Sentry.init({
+            it.dsn = dsn
+        }, false)
+        Sentry.setCurrentHub(hub)
+        Sentry.getSpan()
+        verify(hub).span
+    }
+
+    @Test
+    fun `getSpan calls returns root span if globalhub mode is enabled on Android`() {
+        PlatformTestManipulator.pretendIsAndroid(true)
+        Sentry.init({
+            it.dsn = dsn
+            it.enableTracing = true
+            it.sampleRate = 1.0
+        }, true)
+
+        val transaction = Sentry.startTransaction("name", "op-root", true)
+        transaction.startChild("op-child")
+
+        val span = Sentry.getSpan()!!
+        assertEquals("op-root", span.operation)
+        PlatformTestManipulator.pretendIsAndroid(false)
+    }
+
+    @Test
+    fun `getSpan calls returns child span if globalhub mode is enabled, but the platform is not Android`() {
+        PlatformTestManipulator.pretendIsAndroid(false)
+        Sentry.init({
+            it.dsn = dsn
+            it.enableTracing = true
+            it.sampleRate = 1.0
+        }, false)
+
+        val transaction = Sentry.startTransaction("name", "op-root", true)
+        transaction.startChild("op-child")
+
+        val span = Sentry.getSpan()!!
+        assertEquals("op-child", span.operation)
+    }
+
+    @Test
+    fun `getSpan calls returns child span if globalhub mode is disabled`() {
+        Sentry.init({
+            it.dsn = dsn
+            it.enableTracing = true
+            it.sampleRate = 1.0
+        }, false)
+
+        val transaction = Sentry.startTransaction("name", "op-root", true)
+        transaction.startChild("op-child")
+
+        val span = Sentry.getSpan()!!
+        assertEquals("op-child", span.operation)
+    }
+
     private class InMemoryOptionsObserver : IOptionsObserver {
         var release: String? = null
             private set
diff --git a/sentry/src/test/java/io/sentry/util/PlatformTestManipulator.kt b/sentry/src/test/java/io/sentry/util/PlatformTestManipulator.kt
@@ -6,5 +6,9 @@ class PlatformTestManipulator {
         fun pretendJavaNinePlus(isJavaNinePlus: Boolean) {
             Platform.isJavaNinePlus = isJavaNinePlus
         }
+
+        fun pretendIsAndroid(isAndroid: Boolean) {
+            Platform.isAndroid = isAndroid
+        }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -78,7 +78,7 @@ class SentryOkHttpInterceptor(`
`78`	`78`	`isFromEventListener = true`
`79`	`79`	`} else {`
`80`	`80`	`// read the span from the bound scope`
`81`		`- span = hub.span?.startChild("http.client", "$method $url")`
	`81`	`+ span = hub.transaction?.startChild("http.client", "$method $url")`
`82`	`82`	`isFromEventListener = false`
`83`	`83`	`}`
`84`	`84`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ final class FileIOSpanManager {`
`29`	`29`	`private final @NotNull SentryStackTraceFactory stackTraceFactory;`
`30`	`30`
`31`	`31`	`static @Nullable ISpan startSpan(final @NotNull IHub hub, final @NotNull String op) {`
`32`		`- final ISpan parent = hub.getSpan();`
	`32`	`+ final ISpan parent = hub.getTransaction();`
`33`	`33`	`return parent != null ? parent.startChild(op) : null;`
`34`	`34`	`}`
`35`	`35`