diff --git a/CHANGELOG.md b/CHANGELOG.md index cfd2e8d0f78..560bcc53815 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ ## Unreleased +### Features + +- Add `LocalSentryScopes` to `sentry-compose`, allowing `SentryTraced` to trace against a custom `IScopes` instance instead of always defaulting to `Sentry.getCurrentScopes()` ([#5838](https://github.com/getsentry/sentry-java/pull/5838)) + - Example usage: + ```kotlin + val scopes = Scopes(options) + CompositionLocalProvider(LocalSentryScopes provides scopes) { + // this uses custom scopes now + SentryTraced(tag = "custom") { Box {} } + } + ``` + ### Improvements - Skip building Android manifest metadata debug log messages when debug logging is disabled, reducing allocations during SDK init ([#5790](https://github.com/getsentry/sentry-java/pull/5790)) diff --git a/sentry-compose/api/android/sentry-compose.api b/sentry-compose/api/android/sentry-compose.api index 3ae9af627b1..52327934b6c 100644 --- a/sentry-compose/api/android/sentry-compose.api +++ b/sentry-compose/api/android/sentry-compose.api @@ -12,6 +12,7 @@ public final class io/sentry/compose/SentryComposeHelperKt { public final class io/sentry/compose/SentryComposeTracingKt { public static final fun SentryTraced (Ljava/lang/String;Landroidx/compose/ui/Modifier;ZLkotlin/jvm/functions/Function3;Landroidx/compose/runtime/Composer;II)V + public static final fun getLocalSentryScopes ()Landroidx/compose/runtime/ProvidableCompositionLocal; } public final class io/sentry/compose/SentryModifier { diff --git a/sentry-compose/build.gradle.kts b/sentry-compose/build.gradle.kts index 4ebd9349662..2bc706562a6 100644 --- a/sentry-compose/build.gradle.kts +++ b/sentry-compose/build.gradle.kts @@ -68,9 +68,11 @@ kotlin { implementation(libs.androidx.test.rules) implementation(libs.androidx.test.runner) implementation(libs.kotlin.test.junit) + implementation(libs.androidx.compose.foundation) implementation(libs.mockito.inline) implementation(libs.mockito.kotlin) implementation(libs.roboelectric) + implementation(projects.sentryTestSupport) } } } diff --git a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt index ca923e5d2c9..3ccf3ed5010 100644 --- a/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt +++ b/sentry-compose/src/androidMain/kotlin/io/sentry/compose/SentryComposeTracing.kt @@ -4,11 +4,14 @@ import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.BoxScope import androidx.compose.runtime.Composable import androidx.compose.runtime.Immutable -import androidx.compose.runtime.compositionLocalOf +import androidx.compose.runtime.ProvidableCompositionLocal +import androidx.compose.runtime.RememberObserver import androidx.compose.runtime.remember +import androidx.compose.runtime.staticCompositionLocalOf import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier import androidx.compose.ui.draw.drawWithContent +import io.sentry.IScopes import io.sentry.ISpan import io.sentry.Sentry import io.sentry.SpanOptions @@ -24,15 +27,24 @@ private const val OP_TRACE_ORIGIN = "auto.ui.jetpack_compose" @Immutable private class ImmutableHolder(var item: T) -private fun getRootSpan(): ISpan? { +// Defaults to null rather than eagerly resolving Sentry.getCurrentScopes(): a CompositionLocal's +// default value is computed at most once for the process lifetime, so a non-null default would +// permanently cache whatever scopes were current on first read (e.g. a NoOp scopes if read before +// Sentry.init()). SentryTraced instead falls back to Sentry.getCurrentScopes() on every call when +// no value has been explicitly provided, so it always reflects the current scopes. +public val LocalSentryScopes: ProvidableCompositionLocal = staticCompositionLocalOf { + null +} + +private fun getRootSpan(scopes: IScopes): ISpan? { var rootSpan: ISpan? = null - Sentry.configureScope { rootSpan = it.transaction } + scopes.configureScope { rootSpan = it.transaction } return rootSpan } -private val localSentryCompositionParentSpan = compositionLocalOf { +private fun createCompositionParentSpan(scopes: IScopes): ImmutableHolder = ImmutableHolder( - getRootSpan() + getRootSpan(scopes) ?.startChild( OP_PARENT_COMPOSITION, "Jetpack Compose Initial Composition", @@ -44,11 +56,10 @@ private val localSentryCompositionParentSpan = compositionLocalOf { ) ?.apply { spanContext.origin = OP_TRACE_ORIGIN } ) -} -private val localSentryRenderingParentSpan = compositionLocalOf { +private fun createRenderingParentSpan(scopes: IScopes): ImmutableHolder = ImmutableHolder( - getRootSpan() + getRootSpan(scopes) ?.startChild( OP_PARENT_RENDER, "Jetpack Compose Initial Render", @@ -60,8 +71,104 @@ private val localSentryRenderingParentSpan = compositionLocalOf { ) ?.apply { spanContext.origin = OP_TRACE_ORIGIN } ) + +private class RootSpans { + // Entries are cleaned up deterministically via reference counting (see retain/release) rather + // than relying on GC to reclaim a weakly-keyed map: a value here (ImmutableHolder -> Span) + // strongly references the IScopes that created it (Span keeps its Scopes alive), so a + // WeakHashMap keyed on IScopes would never actually evict its own key, and a value wrapped in a + // WeakReference could be collected between recompositions even while a SentryTraced call under + // that scopes is still in composition, silently breaking root span sharing. + val compositionSpans = HashMap>() + val renderingSpans = HashMap>() + private val refCounts = HashMap() + + fun retain(scopes: IScopes) { + refCounts[scopes] = (refCounts[scopes] ?: 0) + 1 + } + + fun release(scopes: IScopes) { + val remaining = (refCounts[scopes] ?: 1) - 1 + if (remaining <= 0) { + refCounts.remove(scopes) + // Deliberately not finishing these spans here: Compose can deactivate and later reactivate + // the same retained instance (e.g. LazyColumn item reuse) without necessarily re-mounting a + // new SentryTraced in between, so a refcount reaching zero doesn't reliably mean the scopes + // is done for good. Finishing eagerly would risk silently dropping compose/render spans + // started while briefly deactivated. These are idle spans, so they still get finished + // automatically once the whole transaction finishes, same as before this cache existed; the + // trade-off is a possible extra root span if the same scopes is genuinely reused much later. + compositionSpans.remove(scopes) + renderingSpans.remove(scopes) + } else { + refCounts[scopes] = remaining + } + } +} + +// Retains scopes in the constructor so it always runs synchronously during composition, before +// any effect-phase work for this frame (including another SentryTraced call's dispose): Compose +// runs the dispose of an outgoing node before the effects of an incoming one in the same +// recomposition, so retaining from an effect could let a shared entry's refcount hit zero (and +// get evicted) between an old and a new SentryTraced call sharing the same scopes. Releasing from +// both onForgotten and onAbandoned (rather than a DisposableEffect) also covers a composition +// that never commits (e.g. an exception elsewhere during composition), which would otherwise +// leave the retain in place with no matching release. +// +// A single instance can also go through onForgotten/onRemembered more than once without the +// constructor ever running again: reusable content (e.g. LazyColumn item reuse) deactivates by +// forgetting all remembered objects and later reactivates existing ones by calling onRemembered +// directly. isRetained tracks whether this instance currently holds a retain so onRemembered can +// re-retain on reactivation, while still not double-retaining right after construction +// (onRemembered +// always fires once for a freshly remembered object too). +private class ScopesRetention(private val rootSpans: RootSpans, private val scopes: IScopes) : + RememberObserver { + private var isRetained = false + + init { + retain() + } + + private fun retain() { + rootSpans.retain(scopes) + isRetained = true + } + + override fun onRemembered() { + if (!isRetained) retain() + } + + override fun onForgotten() { + if (isRetained) { + rootSpans.release(scopes) + isRetained = false + } + } + + override fun onAbandoned() { + if (isRetained) { + rootSpans.release(scopes) + isRetained = false + } + } } +private fun getOrCreateParentSpan( + map: MutableMap>, + scopes: IScopes, + create: (IScopes) -> ImmutableHolder, +): ImmutableHolder = + // Only cache the holder once it actually contains a span; a null result (no transaction bound + // to the scopes yet) is recomputed on the next call so a later transaction is still picked up. + map[scopes] ?: create(scopes).also { if (it.item != null) map[scopes] = it } + +// Cached once per Composition and shared by every SentryTraced call within it, mirroring the +// old eagerly-computed `compositionLocalOf { ... }` default (which Compose resolves once and +// reuses for every `.current` read that has no ancestor Provider, sibling or not). Keyed per +// IScopes so distinct custom scopes each get their own root span instead of colliding. +private val LocalRootSpans = staticCompositionLocalOf { RootSpans() } + @ExperimentalComposeUiApi @Composable public fun SentryTraced( @@ -70,8 +177,14 @@ public fun SentryTraced( enableUserInteractionTracing: Boolean = true, content: @Composable BoxScope.() -> Unit, ) { - val parentCompositionSpan = localSentryCompositionParentSpan.current - val parentRenderingSpan = localSentryRenderingParentSpan.current + val scopes = LocalSentryScopes.current ?: Sentry.getCurrentScopes() + val rootSpans = LocalRootSpans.current + remember(rootSpans, scopes) { ScopesRetention(rootSpans, scopes) } + val parentCompositionSpan = + getOrCreateParentSpan(rootSpans.compositionSpans, scopes, ::createCompositionParentSpan) + val parentRenderingSpan = + getOrCreateParentSpan(rootSpans.renderingSpans, scopes, ::createRenderingParentSpan) + val compositionSpan = parentCompositionSpan.item?.startChild(OP_COMPOSE, tag)?.apply { spanContext.origin = OP_TRACE_ORIGIN diff --git a/sentry-compose/src/androidUnitTest/kotlin/io/sentry/compose/SentryTracedTest.kt b/sentry-compose/src/androidUnitTest/kotlin/io/sentry/compose/SentryTracedTest.kt new file mode 100644 index 00000000000..5243195789e --- /dev/null +++ b/sentry-compose/src/androidUnitTest/kotlin/io/sentry/compose/SentryTracedTest.kt @@ -0,0 +1,176 @@ +package io.sentry.compose + +import android.app.Application +import android.content.ComponentName +import androidx.activity.ComponentActivity +import androidx.compose.foundation.layout.Box +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.getValue +import androidx.compose.runtime.key +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.ui.ExperimentalComposeUiApi +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.IScopes +import io.sentry.ITransaction +import io.sentry.Sentry +import io.sentry.SentryOptions +import io.sentry.TransactionOptions +import io.sentry.test.createTestScopes +import kotlin.test.assertEquals +import org.junit.After +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TestWatcher +import org.junit.runner.Description +import org.junit.runner.RunWith +import org.robolectric.Shadows +import org.robolectric.annotation.Config + +@OptIn(ExperimentalComposeUiApi::class) +@RunWith(AndroidJUnit4::class) +@Config(sdk = [30]) +class SentryTracedTest { + // workaround for robolectric tests with composeRule + // from https://github.com/robolectric/robolectric/pull/4736#issuecomment-1831034882 + @get:Rule(order = 1) + val addActivityToRobolectricRule = + object : TestWatcher() { + override fun starting(description: Description?) { + super.starting(description) + val appContext: Application = ApplicationProvider.getApplicationContext() + Shadows.shadowOf(appContext.packageManager) + .addActivityIfNotPresent( + ComponentName(appContext.packageName, ComponentActivity::class.java.name) + ) + } + } + + @get:Rule(order = 2) val rule = createAndroidComposeRule() + + @After + fun tearDown() { + Sentry.close() + } + + private fun newTracingScopes(): IScopes = + createTestScopes( + SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + tracesSampleRate = 1.0 + } + ) + + private fun IScopes.startBoundTransaction(name: String): ITransaction = + startTransaction(name, "test", TransactionOptions().apply { isBindToScope = true }) + + @Test + fun `SentryTraced creates its root span on the scopes provided via LocalSentryScopes`() { + val scopes = newTracingScopes() + val tx = scopes.startBoundTransaction("custom-scopes-tx") + + rule.setContent { + CompositionLocalProvider(LocalSentryScopes provides scopes) { + SentryTraced(tag = "custom") { Box {} } + } + } + rule.waitForIdle() + + assertEquals(1, tx.spans.count { it.operation == "ui.compose.composition" }) + assertEquals(1, tx.spans.count { it.operation == "ui.compose" }) + } + + @Test + fun `sibling SentryTraced composables under the same scopes share one root span`() { + val scopes = newTracingScopes() + val tx = scopes.startBoundTransaction("custom-scopes-tx") + + rule.setContent { + CompositionLocalProvider(LocalSentryScopes provides scopes) { + SentryTraced(tag = "first") { Box {} } + SentryTraced(tag = "second") { Box {} } + } + } + rule.waitForIdle() + + assertEquals(1, tx.spans.count { it.operation == "ui.compose.composition" }) + assertEquals(2, tx.spans.count { it.operation == "ui.compose" }) + } + + @Test + fun `SentryTraced composables under different scopes do not interfere with each other`() { + val scopesA = newTracingScopes() + val txA = scopesA.startBoundTransaction("scopes-a-tx") + val scopesB = newTracingScopes() + val txB = scopesB.startBoundTransaction("scopes-b-tx") + + rule.setContent { + CompositionLocalProvider(LocalSentryScopes provides scopesA) { + SentryTraced(tag = "a") { Box {} } + } + CompositionLocalProvider(LocalSentryScopes provides scopesB) { + SentryTraced(tag = "b") { Box {} } + } + } + rule.waitForIdle() + + assertEquals(1, txA.spans.count { it.operation == "ui.compose.composition" }) + assertEquals(1, txA.spans.count { it.operation == "ui.compose" }) + assertEquals(1, txB.spans.count { it.operation == "ui.compose.composition" }) + assertEquals(1, txB.spans.count { it.operation == "ui.compose" }) + } + + @Test + fun `repeatedly replacing a traced composable under the same scopes keeps sharing the root span`() { + // Each swap disposes the outgoing keyed composable and mounts a new one under the same + // scopes within a single recomposition. Compose dispatches the outgoing composable's + // onForgotten before the incoming one's onRemembered, so a second swap is needed to surface + // a root span cache that was cleared out from under a still-live retain. + val scopes = newTracingScopes() + val tx = scopes.startBoundTransaction("custom-scopes-tx") + var step by mutableStateOf(0) + + rule.setContent { + CompositionLocalProvider(LocalSentryScopes provides scopes) { + key(step) { SentryTraced(tag = "step-$step") { Box {} } } + } + } + rule.waitForIdle() + + step = 1 + rule.waitForIdle() + + step = 2 + rule.waitForIdle() + + assertEquals(1, tx.spans.count { it.operation == "ui.compose.composition" }) + } + + @Test + fun `fully unmounting and remounting under the same scopes creates a fresh root span`() { + // Confirms the cache entry is actually dropped once the last consumer of a scopes leaves + // composition (the fix for the original RootSpans leak), rather than being reused forever. + val scopes = newTracingScopes() + val tx = scopes.startBoundTransaction("custom-scopes-tx") + var mounted by mutableStateOf(true) + + rule.setContent { + if (mounted) { + CompositionLocalProvider(LocalSentryScopes provides scopes) { + SentryTraced(tag = "first") { Box {} } + } + } + } + rule.waitForIdle() + + mounted = false + rule.waitForIdle() + + mounted = true + rule.waitForIdle() + + assertEquals(2, tx.spans.count { it.operation == "ui.compose.composition" }) + } +}