Skip to content

Commit 0b2fa67

Browse files
authored
fix(compose): Stop SentryTraced from reusing stale parent spans (#6057)
Fix the stale parent problem that causes spans from all SentryTraced instances for the entire app process to be dropped once the initial active transaction finishes. Prior to this commit, SentryTraced used process-wide composition locals to bind the transaction active whenever the first SentryTraced for an app process entered the composition, and to reuse it for all SentryTraced instances thereafter. That meant all SentryTraced spans for the entire app would be dropped for the lifetime of the app process once the initial transaction finished. Oof. This commit corrects that behavior by having each SentryTraced composable request the current active transaction and update the generation of spans accordingly. Because we're no longer relying on a single transaction + parent span pair, SentryTraced now needs to manage the creation of possibly multiple parent span pairs, as the owning transaction updates. That logic lives in the new ParentSpans class.
1 parent c597f23 commit 0b2fa67

4 files changed

Lines changed: 621 additions & 85 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44

55
### Fixes
66

7-
- Fix typos in Spring GraphQL integration names (`GrahQL` to `GraphQL`)
7+
- `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))
8+
- Fix typos in Spring GraphQL integration names (`GrahQL` to `GraphQL`) ([#6061](https://github.com/getsentry/sentry-java/pull/6061))
89

910
## 8.55.0
1011

‎sentry-compose/build.gradle.kts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,14 @@ kotlin {
6262
}
6363
getByName("androidUnitTest") {
6464
dependencies {
65+
implementation(libs.androidx.compose.foundation)
66+
implementation(libs.androidx.compose.foundation.layout)
6567
implementation(libs.androidx.compose.ui.test.junit4)
6668
implementation(libs.androidx.navigation.compose)
6769
implementation(libs.androidx.test.ext.junit)
6870
implementation(libs.androidx.test.rules)
6971
implementation(libs.androidx.test.runner)
72+
implementation(libs.google.truth)
7073
implementation(libs.kotlin.test.junit)
7174
implementation(libs.mockito.inline)
7275
implementation(libs.mockito.kotlin)

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

Lines changed: 171 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -4,87 +4,62 @@ import androidx.compose.foundation.layout.Box
44
import androidx.compose.foundation.layout.BoxScope
55
import androidx.compose.runtime.Composable
66
import androidx.compose.runtime.SideEffect
7-
import androidx.compose.runtime.compositionLocalOf
87
import androidx.compose.runtime.remember
98
import androidx.compose.ui.ExperimentalComposeUiApi
109
import androidx.compose.ui.Modifier
1110
import androidx.compose.ui.draw.drawWithContent
1211
import io.sentry.ISpan
12+
import io.sentry.Instrumenter
13+
import io.sentry.NoOpSpan
1314
import io.sentry.Sentry
1415
import io.sentry.SentryDate
1516
import io.sentry.SpanOptions
1617
import io.sentry.compose.SentryModifier.sentryTag
18+
import java.lang.ref.WeakReference
19+
import java.util.WeakHashMap
1720

18-
private const val OP_PARENT_COMPOSITION = "ui.compose.composition"
19-
private const val OP_COMPOSE = "ui.compose"
21+
private const val DESCRIPTION_COMPOSITION_BUCKET = "Jetpack Compose Initial Composition"
22+
private const val OP_COMPOSITION_BUCKET = "ui.compose.composition"
23+
private const val OP_COMPOSITION_SPAN = "ui.compose"
2024

21-
private const val OP_PARENT_RENDER = "ui.compose.rendering"
22-
private const val OP_RENDER = "ui.render"
25+
private const val DESCRIPTION_RENDER_BUCKET = "Jetpack Compose Initial Render"
26+
private const val OP_RENDER_BUCKET = "ui.compose.rendering"
27+
private const val OP_RENDER_SPAN = "ui.render"
2328

2429
private const val OP_TRACE_ORIGIN = "auto.ui.jetpack_compose"
2530

26-
private val localSentryCompositionParentSpan = compositionLocalOf {
27-
getRootSpan()
28-
// Create a single parent span to own composition spans emitted by all SentryTraced composables
29-
// during the root's lifetime.
30-
?.startChild(
31-
OP_PARENT_COMPOSITION,
32-
"Jetpack Compose Initial Composition",
33-
SpanOptions().apply {
34-
isTrimStart = true
35-
isTrimEnd = true
36-
isIdle = true
37-
},
38-
)
39-
?.apply { spanContext.origin = OP_TRACE_ORIGIN }
40-
}
41-
42-
private val localSentryRenderingParentSpan = compositionLocalOf {
43-
getRootSpan()
44-
// Create a single parent span to own render spans emitted by all SentryTraced composables
45-
// during the root's lifetime.
46-
?.startChild(
47-
OP_PARENT_RENDER,
48-
"Jetpack Compose Initial Render",
49-
SpanOptions().apply {
50-
isTrimStart = true
51-
isTrimEnd = true
52-
isIdle = true
53-
},
54-
)
55-
?.apply { spanContext.origin = OP_TRACE_ORIGIN }
56-
}
57-
58-
/**
59-
* A substitute for Compose's `MutableState` that doesn't register itself with the snapshot system,
60-
* so mutating [value] never triggers recomposition.
61-
*/
62-
private class MutableRef<T>(var value: T)
63-
6431
/**
65-
* Creates a single span for tracking the time required to compose the wrapped [content], and a span
66-
* for its initial draw.
32+
* Creates a span for the initial composition of the wrapped [content], and a span for its initial
33+
* rendering, each of which lives under a shared "bucket" span (see "Span organization" below).
6734
*
6835
* Spans are approximate and include work performed by any composables [content] invokes. Abandoned
6936
* recompositions are ignored.
7037
*
71-
* Spans live under a set of parents shared by all `SentryTraced` composables. Every `SentryTraced`
72-
* contributes at most one `ui.compose` child and one `ui.render` child per parent lifetime:
38+
* **Span organization**
39+
*
40+
* All spans produced are rooted under an owner span defined by the environment `SentryTraced` runs
41+
* in. `SentryTraced` composables with the same owner share two common "bucket" spans
42+
* (`ui.compose.composition` and `ui.compose.rendering`). Each `SentryTraced` in the group emits at
43+
* most one `ui.compose` span to the composition bucket and one `ui.render` span to the render
44+
* bucket.
45+
*
46+
* The end result looks something like this:
7347
* ```
74-
* Root span
48+
* Owner span
7549
* │
7650
* ├─ ui.compose.composition "Jetpack Compose Initial Composition"
51+
* │ ├─ ui.compose "marketing_banner"
7752
* │ ├─ ui.compose "product_info"
7853
* │ └─ ui.compose "add_to_cart_button"
7954
* │
8055
* └─ ui.compose.rendering "Jetpack Compose Initial Render"
56+
* ├─ ui.render "marketing_banner"
8157
* ├─ ui.render "product_info"
8258
* └─ ui.render "add_to_cart_button"
8359
* ```
8460
*
85-
* Here `ui.compose.composition` and `ui.compose.rendering` are the shared parents. A `SentryTraced`
86-
* generates the "product_info" spans, and a separate `SentryTraced` generates the
87-
* "add_to_cart_button" spans.
61+
* (Here, there were three `SentryTraced` composables in the owner group. One emitted
62+
* "marketing_banner" spans, another "product_info" spans, and another "add_to_cart_button" spans.)
8863
*/
8964
@ExperimentalComposeUiApi
9065
@Composable
@@ -95,31 +70,31 @@ public fun SentryTraced(
9570
content: @Composable BoxScope.() -> Unit,
9671
) {
9772
val baseModifier = if (enableUserInteractionTracing) modifier.sentryTag(tag) else modifier
73+
val scopes = Sentry.getCurrentScopes()
74+
val ownerSpan = scopes.transaction ?: NoOpSpan.getInstance()
9875

99-
val parentCompositionSpan = localSentryCompositionParentSpan.current
100-
val parentRenderingSpan = localSentryRenderingParentSpan.current
101-
102-
val alreadyComposed = remember(parentCompositionSpan) { MutableRef(false) }
103-
val alreadyRendered = remember(parentRenderingSpan) { MutableRef(false) }
104-
val dateProvider = Sentry.getCurrentScopes().options.dateProvider
76+
val alreadyComposed = remember(ownerSpan) { MutableRef(false) }
77+
val alreadyRendered = remember(ownerSpan) { MutableRef(false) }
78+
val shouldRecordSpans = !ownerSpan.dropsChildSpans
10579

106-
// Only record spans if we have a parent for them.
80+
val dateProvider = scopes.options.dateProvider
10781
val compositionStart =
108-
if (!alreadyComposed.value) parentCompositionSpan?.let { dateProvider.now() } else null
82+
if (shouldRecordSpans && !alreadyComposed.value) dateProvider.now() else null
10983

11084
Box(
11185
modifier =
11286
baseModifier.drawWithContent {
113-
if (alreadyRendered.value || parentRenderingSpan == null) {
87+
if (!shouldRecordSpans || alreadyRendered.value) {
11488
drawContent()
115-
} else {
116-
val renderStart = dateProvider.now()
117-
drawContent()
118-
val renderEnd = dateProvider.now()
119-
120-
alreadyRendered.value = true
121-
recordRenderSpan(parentRenderingSpan, tag, renderStart, renderEnd)
89+
return@drawWithContent
12290
}
91+
92+
val renderStart = dateProvider.now()
93+
drawContent()
94+
val renderEnd = dateProvider.now()
95+
96+
alreadyRendered.value = true
97+
recordRenderSpan(ownerSpan, tag, renderStart, renderEnd)
12398
},
12499
propagateMinConstraints = true,
125100
) {
@@ -130,44 +105,156 @@ public fun SentryTraced(
130105
val compositionEnd = dateProvider.now()
131106

132107
SideEffect {
133-
recordCompositionSpan(
134-
parentSpan = parentCompositionSpan,
135-
tag = tag,
136-
startTimestamp = compositionStart,
137-
endTimestamp = compositionEnd,
138-
)
139-
140108
alreadyComposed.value = true
109+
recordCompositionSpan(ownerSpan, tag, compositionStart, compositionEnd)
141110
}
142111
}
143112
}
144113

145-
private fun getRootSpan(): ISpan? {
146-
var rootSpan: ISpan? = null
147-
Sentry.configureScope { rootSpan = it.transaction }
148-
return rootSpan
149-
}
150-
114+
/**
115+
* Creates a [OP_COMPOSITION_SPAN] under the [ownerSpan]'s composition bucket.
116+
*
117+
* If the owner doesn't yet have a composition bucket, this method creates one for its own use and
118+
* for use by other `SentryTraced` composables in the same owner group.
119+
*/
151120
private fun recordCompositionSpan(
152-
parentSpan: ISpan?,
121+
ownerSpan: ISpan,
153122
tag: String,
154123
startTimestamp: SentryDate,
155124
endTimestamp: SentryDate,
156125
) {
157-
parentSpan?.startChild(OP_COMPOSE, tag, startTimestamp)?.apply {
126+
val bucketSpan = BucketSpans.getOrCreateCompositionSpan(ownerSpan, startTimestamp) ?: return
127+
128+
bucketSpan.startChild(OP_COMPOSITION_SPAN, tag, startTimestamp).apply {
158129
spanContext.origin = OP_TRACE_ORIGIN
159130
finish(null, endTimestamp)
160131
}
161132
}
162133

134+
/**
135+
* Creates a [OP_RENDER_SPAN] under the [ownerSpan]'s render bucket.
136+
*
137+
* If the owner doesn't yet have a render bucket, this method creates one for its own use and for
138+
* use by other `SentryTraced` composables in the same owner group.
139+
*/
163140
private fun recordRenderSpan(
164-
parentSpan: ISpan?,
141+
ownerSpan: ISpan,
165142
tag: String,
166143
startTimestamp: SentryDate,
167144
endTimestamp: SentryDate,
168145
) {
169-
parentSpan?.startChild(OP_RENDER, tag, startTimestamp)?.apply {
146+
val bucketSpan = BucketSpans.getOrCreateRenderSpan(ownerSpan, startTimestamp) ?: return
147+
148+
bucketSpan.startChild(OP_RENDER_SPAN, tag, startTimestamp).apply {
170149
spanContext.origin = OP_TRACE_ORIGIN
171150
finish(null, endTimestamp)
172151
}
173152
}
153+
154+
/**
155+
* Returns true if spans parented under the receiver will be dropped (and therefore aren't worth
156+
* creating in the first place).
157+
*/
158+
private val ISpan.dropsChildSpans: Boolean
159+
// NoOp spans return false for isFinished, so we check for no-op status directly.
160+
get() = this.isFinished || this.isNoOp
161+
162+
/**
163+
* Manages the creation of [OP_COMPOSITION_BUCKET] and [OP_RENDER_BUCKET] spans as owner spans
164+
* rotate over time. It does so for all [SentryTraced] instances throughout the app process.
165+
* (Process-wide logic and state lives in the companion object; per-`SentryTraced` state is
166+
* implemented by the instance properties.)
167+
*
168+
* Under the hood this class tracks which bucket spans have been created for which owner span, so it
169+
* knows when new bucket spans need to be created. But it doesn't own the lifecycle of either and
170+
* holds only weak references.
171+
*
172+
* **Not threadsafe:** Access must be confined to Compose UI-thread callbacks.
173+
*/
174+
private class BucketSpans {
175+
176+
// Bucket spans must be weakly held because spans keep a reference to their owning transaction. If
177+
// the owning transaction is the owner span or an ancestor of it, a strong reference here would
178+
// interfere with cleanup of the corresponding ownerSpanToBucketSpans entry.
179+
private var compositionBucketSpan: WeakReference<ISpan>? = null
180+
private var renderBucketSpan: WeakReference<ISpan>? = null
181+
182+
companion object {
183+
184+
private val ownerSpanToBucketSpans = WeakHashMap<ISpan, BucketSpans>()
185+
186+
fun getOrCreateCompositionSpan(ownerSpan: ISpan, startTimestamp: SentryDate): ISpan? =
187+
getFor(ownerSpan).getOrCreateCompositionSpan(ownerSpan, startTimestamp)
188+
189+
fun getOrCreateRenderSpan(ownerSpan: ISpan, startTimestamp: SentryDate): ISpan? =
190+
getFor(ownerSpan).getOrCreateRenderSpan(ownerSpan, startTimestamp)
191+
192+
private fun getFor(ownerSpan: ISpan): BucketSpans =
193+
ownerSpanToBucketSpans.getOrPut(ownerSpan) { BucketSpans() }
194+
}
195+
196+
private fun getOrCreateCompositionSpan(ownerSpan: ISpan, startTimestamp: SentryDate): ISpan? =
197+
getOrCreate(
198+
ownerSpan = ownerSpan,
199+
startTimestamp = startTimestamp,
200+
cached = compositionBucketSpan,
201+
operation = OP_COMPOSITION_BUCKET,
202+
description = DESCRIPTION_COMPOSITION_BUCKET,
203+
) {
204+
compositionBucketSpan = it
205+
}
206+
207+
private fun getOrCreateRenderSpan(ownerSpan: ISpan, startTimestamp: SentryDate): ISpan? =
208+
getOrCreate(
209+
ownerSpan = ownerSpan,
210+
startTimestamp = startTimestamp,
211+
cached = renderBucketSpan,
212+
operation = OP_RENDER_BUCKET,
213+
description = DESCRIPTION_RENDER_BUCKET,
214+
) {
215+
renderBucketSpan = it
216+
}
217+
218+
private fun getOrCreate(
219+
ownerSpan: ISpan,
220+
startTimestamp: SentryDate,
221+
cached: WeakReference<ISpan>?,
222+
operation: String,
223+
description: String,
224+
setCached: (WeakReference<ISpan>) -> Unit,
225+
): ISpan? {
226+
cached
227+
?.get()
228+
?.takeUnless { it.dropsChildSpans }
229+
?.let {
230+
return it
231+
}
232+
233+
val bucketSpan =
234+
ownerSpan.startChild(
235+
operation,
236+
description,
237+
startTimestamp,
238+
Instrumenter.SENTRY,
239+
SpanOptions().apply {
240+
isTrimStart = true
241+
isTrimEnd = true
242+
isIdle = true
243+
},
244+
)
245+
246+
if (bucketSpan.dropsChildSpans) {
247+
return null
248+
}
249+
250+
bucketSpan.spanContext.origin = OP_TRACE_ORIGIN
251+
setCached(WeakReference(bucketSpan))
252+
return bucketSpan
253+
}
254+
}
255+
256+
/**
257+
* A substitute for Compose's `MutableState` that doesn't register itself with the snapshot system,
258+
* so mutating [value] won't trigger recomposition.
259+
*/
260+
private class MutableRef<T>(var value: T)

0 commit comments

Comments
 (0)