Skip to content

Commit d698dce

Browse files
0xadam-brown0xadam-brown
committed
fix(compose): Prevent dangling spans in SentryTraced + distinguish between completed vs abandoned compositions
Commit repairs a few defects in the implementation of SentryTraced while aiming to maintain (near) parity with the previous approach. In particular it: - Prevents dangling spans due to abandoned composition or exceptions thrown by drawContent(), both by finishing spans in `finally` blocks. - Improves composition measurement and distinguishes between successful vs abandoned composition by measuring from RememberObserver callbacks, rather than the end of the composable body. - Sets the origin for the render span, as that had been previously omitted.
1 parent 2fd0a9d commit d698dce

3 files changed

Lines changed: 171 additions & 16 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
### Fixes
1515

16+
- Prevent `SentryTraced` from producing dangling spans if recomposition is abandoned or drawing fails; add origin info to first-render span; and tag composition spans with composition outcome
1617
- Keep dropped tombstone and ANR events dropped, instead of reporting the same app exit again at every app start ([#6002](https://github.com/getsentry/sentry-java/pull/6002))
1718
- Apply `Sentry.withScope` and `Sentry.withIsolationScope` data to events captured inside the callback when `globalHubMode` is enabled ([#6004](https://github.com/getsentry/sentry-java/pull/6004))
1819
- `globalHubMode` is enabled by default on Android, where tags, extras, contexts and level set inside the callback were silently dropped

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

Lines changed: 78 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@ import androidx.compose.foundation.layout.Box
44
import androidx.compose.foundation.layout.BoxScope
55
import androidx.compose.runtime.Composable
66
import androidx.compose.runtime.Immutable
7+
import androidx.compose.runtime.RememberObserver
78
import androidx.compose.runtime.compositionLocalOf
89
import androidx.compose.runtime.remember
910
import androidx.compose.ui.ExperimentalComposeUiApi
1011
import androidx.compose.ui.Modifier
1112
import androidx.compose.ui.draw.drawWithContent
1213
import io.sentry.ISpan
1314
import io.sentry.Sentry
15+
import io.sentry.SentryDate
1416
import io.sentry.SpanOptions
1517
import io.sentry.compose.SentryModifier.sentryTag
1618

@@ -22,14 +24,63 @@ private const val OP_RENDER = "ui.render"
2224

2325
private const val OP_TRACE_ORIGIN = "auto.ui.jetpack_compose"
2426

25-
@Immutable private class ImmutableHolder<T>(var item: T)
27+
private const val TAG_KEY_COMPOSITION_RESULT = "composition.result"
28+
private const val TAG_VALUE_COMPOSITION_ABANDONED = "abandoned"
29+
private const val TAG_VALUE_COMPOSITION_SUCCESSFUL = "success"
30+
31+
@Immutable internal class ImmutableHolder<T>(var item: T)
32+
33+
/**
34+
* A [RememberObserver] that records an [OP_COMPOSE] span whenever composition succeeds or is
35+
* abandoned.
36+
*
37+
* Span start time is pegged to the provided [startTimestamp].
38+
*/
39+
internal class CompositionSpanRecorder(
40+
private val parentSpan: ISpan?,
41+
private val tag: String,
42+
private val startTimestamp: SentryDate,
43+
) : RememberObserver {
44+
45+
override fun onRemembered() {
46+
recordCompositionSpan(resultTag = TAG_VALUE_COMPOSITION_SUCCESSFUL)
47+
}
48+
49+
override fun onForgotten() = Unit
50+
51+
override fun onAbandoned() {
52+
recordCompositionSpan(resultTag = TAG_VALUE_COMPOSITION_ABANDONED)
53+
}
54+
55+
private fun recordCompositionSpan(resultTag: String) {
56+
val endTimestamp = Sentry.getCurrentScopes().options.dateProvider.now()
57+
58+
parentSpan
59+
?.startChild(
60+
OP_COMPOSE,
61+
tag,
62+
SpanOptions().apply { this.startTimestamp = this@CompositionSpanRecorder.startTimestamp },
63+
)
64+
?.apply {
65+
spanContext.origin = OP_TRACE_ORIGIN
66+
setTag(TAG_KEY_COMPOSITION_RESULT, resultTag)
67+
finish(null, endTimestamp)
68+
}
69+
}
70+
}
2671

2772
private fun getRootSpan(): ISpan? {
2873
var rootSpan: ISpan? = null
2974
Sentry.configureScope { rootSpan = it.transaction }
3075
return rootSpan
3176
}
3277

78+
private fun startRenderSpan(
79+
parentRenderingSpan: ISpan?,
80+
tag: String,
81+
): ISpan? =
82+
parentRenderingSpan?.startChild(OP_RENDER, tag)?.apply { spanContext.origin = OP_TRACE_ORIGIN }
83+
3384
private val localSentryCompositionParentSpan = compositionLocalOf {
3485
ImmutableHolder(
3586
getRootSpan()
@@ -62,6 +113,14 @@ private val localSentryRenderingParentSpan = compositionLocalOf {
62113
)
63114
}
64115

116+
/**
117+
* Creates separate spans for the time required to compose the wrapped [content] and for its first
118+
* draw.
119+
*
120+
* Spans are approximate and include work performed by any composables [content] invokes.
121+
* Time-to-compose spans are produced for every composition attempt, and
122+
* [a result tag][TAG_KEY_COMPOSITION_RESULT] indicates whether the composition succeeded.
123+
*/
65124
@ExperimentalComposeUiApi
66125
@Composable
67126
public fun SentryTraced(
@@ -70,32 +129,35 @@ public fun SentryTraced(
70129
enableUserInteractionTracing: Boolean = true,
71130
content: @Composable BoxScope.() -> Unit,
72131
) {
73-
val parentCompositionSpan = localSentryCompositionParentSpan.current
74-
val parentRenderingSpan = localSentryRenderingParentSpan.current
75-
val compositionSpan =
76-
parentCompositionSpan.item?.startChild(OP_COMPOSE, tag)?.apply {
77-
spanContext.origin = OP_TRACE_ORIGIN
78-
}
79-
val firstRendered = remember { ImmutableHolder(false) }
80-
132+
val alreadyRendered = remember { ImmutableHolder(false) }
81133
val baseModifier = if (enableUserInteractionTracing) modifier.sentryTag(tag) else modifier
82134

135+
val parentCompositionSpan = localSentryCompositionParentSpan.current.item
136+
val parentRenderingSpan = localSentryRenderingParentSpan.current.item
137+
138+
val compositionStartTimestamp = Sentry.getCurrentScopes().options.dateProvider.now()
139+
remember(parentCompositionSpan, tag, compositionStartTimestamp) {
140+
CompositionSpanRecorder(parentCompositionSpan, tag, compositionStartTimestamp)
141+
}
142+
83143
Box(
84144
modifier =
85145
baseModifier.drawWithContent {
86146
val renderSpan =
87-
if (!firstRendered.item) {
88-
parentRenderingSpan.item?.startChild(OP_RENDER, tag)
89-
} else {
147+
if (alreadyRendered.item) {
90148
null
149+
} else {
150+
startRenderSpan(parentRenderingSpan, tag).also { alreadyRendered.item = true }
91151
}
92-
drawContent()
93-
firstRendered.item = true
94-
renderSpan?.finish()
152+
153+
try {
154+
drawContent()
155+
} finally {
156+
renderSpan?.finish()
157+
}
95158
},
96159
propagateMinConstraints = true,
97160
) {
98161
content()
99162
}
100-
compositionSpan?.finish()
101163
}

‎sentry-compose/src/androidUnitTest/kotlin/io/sentry/compose/SentryComposeTracingTest.kt‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package io.sentry.compose
2+
3+
import io.sentry.ISpan
4+
import io.sentry.NoOpTransportFactory
5+
import io.sentry.Sentry
6+
import io.sentry.SentryDate
7+
import io.sentry.SentryDateProvider
8+
import io.sentry.SentryLongDate
9+
import io.sentry.SpanContext
10+
import io.sentry.SpanOptions
11+
import kotlin.test.assertEquals
12+
import org.junit.After
13+
import org.junit.Test
14+
import org.mockito.kotlin.any
15+
import org.mockito.kotlin.argumentCaptor
16+
import org.mockito.kotlin.eq
17+
import org.mockito.kotlin.mock
18+
import org.mockito.kotlin.verify
19+
import org.mockito.kotlin.whenever
20+
21+
class SentryComposeTracingTest {
22+
23+
@After
24+
fun tearDown() {
25+
Sentry.close()
26+
}
27+
28+
@Test
29+
fun `onRemembered records successful composition span`() {
30+
val startTimestamp = SentryLongDate(10)
31+
val endTimestamp = SentryLongDate(20)
32+
initSentry(endTimestamp)
33+
34+
val parentSpan = mock<ISpan>()
35+
val childSpan = mock<ISpan>()
36+
val childSpanContext = SpanContext("child")
37+
whenever(childSpan.spanContext).thenReturn(childSpanContext)
38+
whenever(parentSpan.startChild(eq("ui.compose"), eq("tag"), any<SpanOptions>()))
39+
.thenReturn(childSpan)
40+
41+
CompositionSpanRecorder(parentSpan, "tag", startTimestamp).onRemembered()
42+
43+
val optionsCaptor = argumentCaptor<SpanOptions>()
44+
verify(parentSpan).startChild(eq("ui.compose"), eq("tag"), optionsCaptor.capture())
45+
assertEquals(
46+
startTimestamp.nanoTimestamp(),
47+
optionsCaptor.firstValue.startTimestamp?.nanoTimestamp(),
48+
)
49+
assertEquals("auto.ui.jetpack_compose", childSpanContext.origin)
50+
verify(childSpan).setTag("composition.result", "success")
51+
verify(childSpan).finish(null, endTimestamp)
52+
}
53+
54+
@Test
55+
fun `onAbandoned records abandoned composition span`() {
56+
val startTimestamp = SentryLongDate(30)
57+
val endTimestamp = SentryLongDate(40)
58+
initSentry(endTimestamp)
59+
60+
val parentSpan = mock<ISpan>()
61+
val childSpan = mock<ISpan>()
62+
val childSpanContext = SpanContext("child")
63+
whenever(childSpan.spanContext).thenReturn(childSpanContext)
64+
whenever(parentSpan.startChild(eq("ui.compose"), eq("tag"), any<SpanOptions>()))
65+
.thenReturn(childSpan)
66+
67+
CompositionSpanRecorder(parentSpan, "tag", startTimestamp).onAbandoned()
68+
69+
val optionsCaptor = argumentCaptor<SpanOptions>()
70+
verify(parentSpan).startChild(eq("ui.compose"), eq("tag"), optionsCaptor.capture())
71+
assertEquals(
72+
startTimestamp.nanoTimestamp(),
73+
optionsCaptor.firstValue.startTimestamp?.nanoTimestamp(),
74+
)
75+
assertEquals("auto.ui.jetpack_compose", childSpanContext.origin)
76+
verify(childSpan).setTag("composition.result", "abandoned")
77+
verify(childSpan).finish(null, endTimestamp)
78+
}
79+
80+
private fun initSentry(vararg dates: SentryDate) {
81+
val iterator = dates.iterator()
82+
val last = dates.last()
83+
84+
Sentry.init { options ->
85+
options.dsn = "https://public@example.com/1"
86+
options.setTransportFactory(NoOpTransportFactory.getInstance())
87+
options.dateProvider = SentryDateProvider {
88+
if (iterator.hasNext()) iterator.next() else last
89+
}
90+
}
91+
}
92+
}

0 commit comments

Comments
 (0)