fix(compose): Stop SentryTraced from reusing stale parent spans

CHANGELOG.md

@@ -4,7 +4,8 @@
 
 ### Fixes
 
-- Fix typos in Spring GraphQL integration names (`GrahQL` to `GraphQL`)
+- `SentryTraced` now checks for its owning transaction dynamically rather than once per app process. The latter caused `SentryTraced` spans to be dropped process-wide once the original transaction finished ([#6057](https://github.com/getsentry/sentry-java/pull/6057))
+- Fix typos in Spring GraphQL integration names (`GrahQL` to `GraphQL`) ([#6061](https://github.com/getsentry/sentry-java/pull/6061))
 
 ## 8.55.0
 

sentry-compose/build.gradle.kts

@@ -62,11 +62,14 @@ kotlin {
     }
     getByName("androidUnitTest") {
       dependencies {
+        implementation(libs.androidx.compose.foundation)
+        implementation(libs.androidx.compose.foundation.layout)
         implementation(libs.androidx.compose.ui.test.junit4)
         implementation(libs.androidx.navigation.compose)
         implementation(libs.androidx.test.ext.junit)
         implementation(libs.androidx.test.rules)
         implementation(libs.androidx.test.runner)
+        implementation(libs.google.truth)
         implementation(libs.kotlin.test.junit)
         implementation(libs.mockito.inline)
         implementation(libs.mockito.kotlin)

sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt

@@ -4,87 +4,62 @@ import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.BoxScope
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.SideEffect
-import androidx.compose.runtime.compositionLocalOf
 import androidx.compose.runtime.remember
 import androidx.compose.ui.ExperimentalComposeUiApi
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.draw.drawWithContent
 import io.sentry.ISpan
+import io.sentry.Instrumenter
+import io.sentry.NoOpSpan
 import io.sentry.Sentry
 import io.sentry.SentryDate
 import io.sentry.SpanOptions
 import io.sentry.compose.SentryModifier.sentryTag
+import java.lang.ref.WeakReference
+import java.util.WeakHashMap
 
-private const val OP_PARENT_COMPOSITION = "ui.compose.composition"
-private const val OP_COMPOSE = "ui.compose"
+private const val DESCRIPTION_COMPOSITION_BUCKET = "Jetpack Compose Initial Composition"
+private const val OP_COMPOSITION_BUCKET = "ui.compose.composition"
+private const val OP_COMPOSITION_SPAN = "ui.compose"
 
-private const val OP_PARENT_RENDER = "ui.compose.rendering"
-private const val OP_RENDER = "ui.render"
+private const val DESCRIPTION_RENDER_BUCKET = "Jetpack Compose Initial Render"
+private const val OP_RENDER_BUCKET = "ui.compose.rendering"
+private const val OP_RENDER_SPAN = "ui.render"
 
 private const val OP_TRACE_ORIGIN = "auto.ui.jetpack_compose"
 
-private val localSentryCompositionParentSpan = compositionLocalOf {
-  getRootSpan()
-    // Create a single parent span to own composition spans emitted by all SentryTraced composables
-    // during the root's lifetime.
-    ?.startChild(
-      OP_PARENT_COMPOSITION,
-      "Jetpack Compose Initial Composition",
-      SpanOptions().apply {
-        isTrimStart = true
-        isTrimEnd = true
-        isIdle = true
-      },
-    )
-    ?.apply { spanContext.origin = OP_TRACE_ORIGIN }
-}
-
-private val localSentryRenderingParentSpan = compositionLocalOf {
-  getRootSpan()
-    // Create a single parent span to own render spans emitted by all SentryTraced composables
-    // during the root's lifetime.
-    ?.startChild(
-      OP_PARENT_RENDER,
-      "Jetpack Compose Initial Render",
-      SpanOptions().apply {
-        isTrimStart = true
-        isTrimEnd = true
-        isIdle = true
-      },
-    )
-    ?.apply { spanContext.origin = OP_TRACE_ORIGIN }
-}
-
-/**
- * A substitute for Compose's `MutableState` that doesn't register itself with the snapshot system,
- * so mutating [value] never triggers recomposition.
- */
-private class MutableRef<T>(var value: T)
-
 /**
- * Creates a single span for tracking the time required to compose the wrapped [content], and a span
- * for its initial draw.
+ * Creates a span for the initial composition of the wrapped [content], and a span for its initial
+ * rendering, each of which lives under a shared "bucket" span (see "Span organization" below).
  *
  * Spans are approximate and include work performed by any composables [content] invokes. Abandoned
  * recompositions are ignored.
  *
- * Spans live under a set of parents shared by all `SentryTraced` composables. Every `SentryTraced`
- * contributes at most one `ui.compose` child and one `ui.render` child per parent lifetime:
+ * **Span organization**
+ *
+ * All spans produced are rooted under an owner span defined by the environment `SentryTraced` runs
+ * in. `SentryTraced` composables with the same owner share two common "bucket" spans
+ * (`ui.compose.composition` and `ui.compose.rendering`). Each `SentryTraced` in the group emits at
+ * most one `ui.compose` span to the composition bucket and one `ui.render` span to the render
+ * bucket.
+ *
+ * The end result looks something like this:
  * ```
- * Root span
+ * Owner span
  * │
  * ├─ ui.compose.composition  "Jetpack Compose Initial Composition"
+ * │   ├─ ui.compose   "marketing_banner"
  * │   ├─ ui.compose   "product_info"
  * │   └─ ui.compose   "add_to_cart_button"
  * │
  * └─ ui.compose.rendering    "Jetpack Compose Initial Render"
+ *     ├─ ui.render    "marketing_banner"
  *     ├─ ui.render    "product_info"
  *     └─ ui.render    "add_to_cart_button"
  * ```
  *
- * Here `ui.compose.composition` and `ui.compose.rendering` are the shared parents. A `SentryTraced`
- * generates the "product_info" spans, and a separate `SentryTraced` generates the
- * "add_to_cart_button" spans.
+ * (Here, there were three `SentryTraced` composables in the owner group. One emitted
+ * "marketing_banner" spans, another "product_info" spans, and another "add_to_cart_button" spans.)
  */
 @ExperimentalComposeUiApi
 @Composable
@@ -95,31 +70,31 @@ public fun SentryTraced(
   content: @Composable BoxScope.() -> Unit,
 ) {
   val baseModifier = if (enableUserInteractionTracing) modifier.sentryTag(tag) else modifier
+  val scopes = Sentry.getCurrentScopes()
+  val ownerSpan = scopes.transaction ?: NoOpSpan.getInstance()
 
-  val parentCompositionSpan = localSentryCompositionParentSpan.current
-  val parentRenderingSpan = localSentryRenderingParentSpan.current
-
-  val alreadyComposed = remember(parentCompositionSpan) { MutableRef(false) }
-  val alreadyRendered = remember(parentRenderingSpan) { MutableRef(false) }
-  val dateProvider = Sentry.getCurrentScopes().options.dateProvider
+  val alreadyComposed = remember(ownerSpan) { MutableRef(false) }
+  val alreadyRendered = remember(ownerSpan) { MutableRef(false) }
+  val shouldRecordSpans = !ownerSpan.dropsChildSpans
 
-  // Only record spans if we have a parent for them.
+  val dateProvider = scopes.options.dateProvider
   val compositionStart =
-    if (!alreadyComposed.value) parentCompositionSpan?.let { dateProvider.now() } else null
+    if (shouldRecordSpans && !alreadyComposed.value) dateProvider.now() else null
 
   Box(
     modifier =
       baseModifier.drawWithContent {
-        if (alreadyRendered.value || parentRenderingSpan == null) {
+        if (!shouldRecordSpans || alreadyRendered.value) {
           drawContent()
-        } else {
-          val renderStart = dateProvider.now()
-          drawContent()
-          val renderEnd = dateProvider.now()
-
-          alreadyRendered.value = true
-          recordRenderSpan(parentRenderingSpan, tag, renderStart, renderEnd)
+          return@drawWithContent
         }
+
+        val renderStart = dateProvider.now()
+        drawContent()
+        val renderEnd = dateProvider.now()
+
+        alreadyRendered.value = true
+        recordRenderSpan(ownerSpan, tag, renderStart, renderEnd)
       },
     propagateMinConstraints = true,
   ) {
@@ -130,44 +105,156 @@ public fun SentryTraced(
     val compositionEnd = dateProvider.now()
 
     SideEffect {
-      recordCompositionSpan(
-        parentSpan = parentCompositionSpan,
-        tag = tag,
-        startTimestamp = compositionStart,
-        endTimestamp = compositionEnd,
-      )
-
       alreadyComposed.value = true
+      recordCompositionSpan(ownerSpan, tag, compositionStart, compositionEnd)
     }
   }
 }
 
-private fun getRootSpan(): ISpan? {
-  var rootSpan: ISpan? = null
-  Sentry.configureScope { rootSpan = it.transaction }
-  return rootSpan
-}
-
+/**
+ * Creates a [OP_COMPOSITION_SPAN] under the [ownerSpan]'s composition bucket.
+ *
+ * If the owner doesn't yet have a composition bucket, this method creates one for its own use and
+ * for use by other `SentryTraced` composables in the same owner group.
+ */
 private fun recordCompositionSpan(
-  parentSpan: ISpan?,
+  ownerSpan: ISpan,
   tag: String,
   startTimestamp: SentryDate,
   endTimestamp: SentryDate,
 ) {
-  parentSpan?.startChild(OP_COMPOSE, tag, startTimestamp)?.apply {
+  val bucketSpan = BucketSpans.getOrCreateCompositionSpan(ownerSpan, startTimestamp) ?: return
+
+  bucketSpan.startChild(OP_COMPOSITION_SPAN, tag, startTimestamp).apply {
     spanContext.origin = OP_TRACE_ORIGIN
     finish(null, endTimestamp)
   }
 }
 
+/**
+ * Creates a [OP_RENDER_SPAN] under the [ownerSpan]'s render bucket.
+ *
+ * If the owner doesn't yet have a render bucket, this method creates one for its own use and for
+ * use by other `SentryTraced` composables in the same owner group.
+ */
 private fun recordRenderSpan(
-  parentSpan: ISpan?,
+  ownerSpan: ISpan,
   tag: String,
   startTimestamp: SentryDate,
   endTimestamp: SentryDate,
 ) {
-  parentSpan?.startChild(OP_RENDER, tag, startTimestamp)?.apply {
+  val bucketSpan = BucketSpans.getOrCreateRenderSpan(ownerSpan, startTimestamp) ?: return
+
+  bucketSpan.startChild(OP_RENDER_SPAN, tag, startTimestamp).apply {
     spanContext.origin = OP_TRACE_ORIGIN
     finish(null, endTimestamp)
   }
 }
+
+/**
+ * Returns true if spans parented under the receiver will be dropped (and therefore aren't worth
+ * creating in the first place).
+ */
+private val ISpan.dropsChildSpans: Boolean
+  // NoOp spans return false for isFinished, so we check for no-op status directly.
+  get() = this.isFinished || this.isNoOp
+
+/**
+ * Manages the creation of [OP_COMPOSITION_BUCKET] and [OP_RENDER_BUCKET] spans as owner spans
+ * rotate over time. It does so for all [SentryTraced] instances throughout the app process.
+ * (Process-wide logic and state lives in the companion object; per-`SentryTraced` state is
+ * implemented by the instance properties.)
+ *
+ * Under the hood this class tracks which bucket spans have been created for which owner span, so it
+ * knows when new bucket spans need to be created. But it doesn't own the lifecycle of either and
+ * holds only weak references.
+ *
+ * **Not threadsafe:** Access must be confined to Compose UI-thread callbacks.
+ */
+private class BucketSpans {
+
+  // Bucket spans must be weakly held because spans keep a reference to their owning transaction. If
+  // the owning transaction is the owner span or an ancestor of it, a strong reference here would
+  // interfere with cleanup of the corresponding ownerSpanToBucketSpans entry.
+  private var compositionBucketSpan: WeakReference<ISpan>? = null
+  private var renderBucketSpan: WeakReference<ISpan>? = null
+
+  companion object {
+
+    private val ownerSpanToBucketSpans = WeakHashMap<ISpan, BucketSpans>()
+
+    fun getOrCreateCompositionSpan(ownerSpan: ISpan, startTimestamp: SentryDate): ISpan? =
+      getFor(ownerSpan).getOrCreateCompositionSpan(ownerSpan, startTimestamp)
+
+    fun getOrCreateRenderSpan(ownerSpan: ISpan, startTimestamp: SentryDate): ISpan? =
+      getFor(ownerSpan).getOrCreateRenderSpan(ownerSpan, startTimestamp)
+
+    private fun getFor(ownerSpan: ISpan): BucketSpans =
+      ownerSpanToBucketSpans.getOrPut(ownerSpan) { BucketSpans() }
+  }
+
+  private fun getOrCreateCompositionSpan(ownerSpan: ISpan, startTimestamp: SentryDate): ISpan? =
+    getOrCreate(
+      ownerSpan = ownerSpan,
+      startTimestamp = startTimestamp,
+      cached = compositionBucketSpan,
+      operation = OP_COMPOSITION_BUCKET,
+      description = DESCRIPTION_COMPOSITION_BUCKET,
+    ) {
+      compositionBucketSpan = it
+    }
+
+  private fun getOrCreateRenderSpan(ownerSpan: ISpan, startTimestamp: SentryDate): ISpan? =
+    getOrCreate(
+      ownerSpan = ownerSpan,
+      startTimestamp = startTimestamp,
+      cached = renderBucketSpan,
+      operation = OP_RENDER_BUCKET,
+      description = DESCRIPTION_RENDER_BUCKET,
+    ) {
+      renderBucketSpan = it
+    }
+
+  private fun getOrCreate(
+    ownerSpan: ISpan,
+    startTimestamp: SentryDate,
+    cached: WeakReference<ISpan>?,
+    operation: String,
+    description: String,
+    setCached: (WeakReference<ISpan>) -> Unit,
+  ): ISpan? {
+    cached
+      ?.get()
+      ?.takeUnless { it.dropsChildSpans }
+      ?.let {
+        return it
+      }
+
+    val bucketSpan =
+      ownerSpan.startChild(
+        operation,
+        description,
+        startTimestamp,
+        Instrumenter.SENTRY,
+        SpanOptions().apply {
+          isTrimStart = true
+          isTrimEnd = true
+          isIdle = true
+        },
+      )
+
+    if (bucketSpan.dropsChildSpans) {
+      return null
+    }
+
+    bucketSpan.spanContext.origin = OP_TRACE_ORIGIN
+    setCached(WeakReference(bucketSpan))
+    return bucketSpan
+  }
+}
+
+/**
+ * A substitute for Compose's `MutableState` that doesn't register itself with the snapshot system,
+ * so mutating [value] won't trigger recomposition.
+ */
+private class MutableRef<T>(var value: T)