From 8734fe73ec34a073b663bd802d085e6f453bb00a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 2 Oct 2026 17:32:19 +0200 Subject: [PATCH] perf(android): Bound Nav3 argument payload size and report argument drop reasons Commit makes three refinements to how our Nav3 integration handles host-app provided arguments: 1. Limits the total argument characters sanitized from one back-stack update so large strings and stringified values cannot dominate navigation telemetry payloads or processing time. 2. Attaches a dropped reason marker when arguments are omitted for performance or safety reasons. 3. Reduces some of our argument limits in response to benchmark testing results. Commit also capitalizes nav context and back stack keys to match our usual convention for context keys. --- .../compose/navigation3/BackStackConverter.kt | 143 +++++++++++++----- .../compose/navigation3/BackStackObserver.kt | 28 ++-- .../compose/navigation3/SentryNavEffect.kt | 8 +- .../compose/navigation3/SentryNavOptions.kt | 2 +- .../navigation3/BackStackConverterTest.kt | 107 +++++++++++-- .../navigation3/BackStackObserverTest.kt | 53 +++++-- .../navigation3/SentryNavEffectTest.kt | 10 +- 7 files changed, 268 insertions(+), 83 deletions(-) diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackConverter.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackConverter.kt index b6d9424394..f567f7c8c5 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackConverter.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackConverter.kt @@ -2,6 +2,7 @@ package io.sentry.compose.navigation3 import io.sentry.ILogger import io.sentry.SentryLevel.WARNING +import io.sentry.compose.navigation3.ArgumentDropReason.Companion.ARGUMENT_DROP_REASON_KEY import io.sentry.compose.navigation3.NormalizedSentryBackStackEntry.Companion.UNKNOWN_ENTRY_NAME import io.sentry.util.ExceptionUtils import java.util.IdentityHashMap @@ -69,21 +70,33 @@ internal class BackStackConverter( // Back stack entry mappers are host app callbacks. ExceptionUtils.rethrowIfFatal(t) warningState.logMapperFailureWarning(logger, t) - return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME) + return NormalizedSentryBackStackEntry( + name = UNKNOWN_ENTRY_NAME, + argumentDropReason = ArgumentDropReason.MAPPING_FAILED, + ) } if (info == null) { - return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME) + return NormalizedSentryBackStackEntry(name = UNKNOWN_ENTRY_NAME) } - val arguments = info.arguments?.let(sanitizer::sanitizeEntry) ?: emptyMap() + val sanitizedArguments = + info.arguments?.let(sanitizer::sanitizeEntry) ?: SanitizedArguments(emptyMap()) val formattedName = NormalizedSentryBackStackEntry.formatName(info.name) return if (formattedName.isBlank()) { warningState.logInvalidNameWarning(logger) - NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME, arguments) + NormalizedSentryBackStackEntry( + name = UNKNOWN_ENTRY_NAME, + arguments = sanitizedArguments.values, + argumentDropReason = sanitizedArguments.dropReason, + ) } else { - NormalizedSentryBackStackEntry(formattedName, arguments) + NormalizedSentryBackStackEntry( + name = formattedName, + arguments = sanitizedArguments.values, + argumentDropReason = sanitizedArguments.dropReason, + ) } } @@ -109,6 +122,11 @@ internal class BackStackConverter( KEEP_LAST, } + internal data class SanitizedArguments( + val values: Map, + val dropReason: ArgumentDropReason? = null, + ) + /** * Sanitizes a back stack entry's arguments and writes them in a serializable form. It bounds * depth and total value count, and it rejects cyclic structures. @@ -124,7 +142,8 @@ internal class BackStackConverter( private val activeContainers = IdentityHashMap() private var remainingValues = MAX_ARGUMENT_COUNT - private var budgetExhausted = false + private var remainingCharacters = MAX_ARGUMENT_CHARACTERS + private var dropReason: ArgumentDropReason? = null /** * Sanitizes one entry's arguments, or returns an empty map to drop them, either because the @@ -132,24 +151,24 @@ internal class BackStackConverter( * value budget is spent (this entry and every older one). */ @Suppress("TooGenericExceptionCaught") - fun sanitizeEntry(raw: Map): Map { - if (budgetExhausted) { - return emptyMap() + fun sanitizeEntry(raw: Map): SanitizedArguments { + dropReason?.let { reason -> + return SanitizedArguments(emptyMap(), reason) } return try { - sanitizeMap(raw, depth = 0) + SanitizedArguments(sanitizeMap(raw, depth = 0)) } catch (drop: DropSubtree) { - if (drop.exhaustsBudget) { - budgetExhausted = true + if (drop.reason.exhaustsUpdateBudget) { + dropReason = drop.reason } logger.log(WARNING, drop.warning) - emptyMap() + SanitizedArguments(emptyMap(), drop.reason) } catch (t: Throwable) { // Extracted maps may invoke host app code while iterating or stringifying values. ExceptionUtils.rethrowIfFatal(t) logger.log(WARNING, STRUCTURE_WARNING, t) - emptyMap() + SanitizedArguments(emptyMap(), ArgumentDropReason.SANITIZATION_FAILED) } } @@ -158,7 +177,9 @@ internal class BackStackConverter( try { val sanitized = mutableMapOf() for ((key, childValue) in value) { - sanitized[key.toString()] = sanitizeValue(childValue, depth + 1) + val keyString = key.toString() + consumeCharacters(keyString.length) + sanitized[keyString] = sanitizeValue(childValue, depth + 1) } return sanitized } finally { @@ -185,17 +206,26 @@ internal class BackStackConverter( val collection = value?.asSanitizableCollectionOrNull() return when { - value == null || value is String || value is Number || value is Boolean -> value - value is CharSequence || value is Char -> value.toString() - value is Enum<*> -> value.name value is Map<*, *> -> sanitizeMap(value, depth) collection != null -> sanitizeCollection(collection, depth) + else -> sanitizeScalar(value) + } + } + + private fun sanitizeScalar(value: Any?): Any? = + when (value) { + null, + is Number, + is Boolean -> value + is String -> value.also { consumeCharacters(it.length) } + is CharSequence, + is Char -> value.toString().also { consumeCharacters(it.length) } + is Enum<*> -> value.name.also { consumeCharacters(it.length) } else -> { warningState.logUnsupportedValueWarning(value::class.simpleName, logger) - value.toString() + value.toString().also { consumeCharacters(it.length) } } } - } private fun Any.asSanitizableCollectionOrNull(): Collection<*>? = when (this) { @@ -218,16 +248,23 @@ internal class BackStackConverter( */ private fun visit(depth: Int) { if (depth > MAX_ARGUMENT_DEPTH) { - throw DropSubtree(STRUCTURE_WARNING, exhaustsBudget = false) + throw DropSubtree(STRUCTURE_WARNING, ArgumentDropReason.INVALID_STRUCTURE) } if (--remainingValues < 0) { - throw DropSubtree(BUDGET_WARNING, exhaustsBudget = true) + throw DropSubtree(MAX_COUNT_WARNING, ArgumentDropReason.MAX_COUNT) } } + private fun consumeCharacters(count: Int) { + if (count > remainingCharacters) { + throw DropSubtree(MAX_CHARACTER_WARNING, ArgumentDropReason.CHARACTER_LIMIT) + } + remainingCharacters -= count + } + private fun enter(container: Any) { if (activeContainers.put(container, Unit) != null) { - throw DropSubtree(STRUCTURE_WARNING, exhaustsBudget = false) + throw DropSubtree(STRUCTURE_WARNING, ArgumentDropReason.INVALID_STRUCTURE) } } @@ -239,11 +276,10 @@ internal class BackStackConverter( * Control-flow signal to abort sanitization of the current subtree. Internal to * [ArgumentSanitizer]. * - * [exhaustsBudget] distinguishes an entry-local drop (cycle or over-deep structure) from an - * update-wide one (the shared value budget is spent). Overrides [fillInStackTrace] to skip - * stack-trace capture. + * [reason] distinguishes entry-local drops from update-wide budget exhaustion. Overrides + * [fillInStackTrace] to skip stack-trace capture. */ - private class DropSubtree(val warning: String, val exhaustsBudget: Boolean) : + private class DropSubtree(val warning: String, val reason: ArgumentDropReason) : RuntimeException() { override fun fillInStackTrace(): Throwable = this } @@ -279,11 +315,22 @@ internal class BackStackConverter( * * Then /ProductDetail and /Home will have no arguments, but /Checkout will. */ - private const val MAX_ARGUMENT_COUNT = 500 + private const val MAX_ARGUMENT_COUNT = 200 + + /** + * Max number of characters visited while sanitizing all entries in a given back stack update. + * + * Caps payload size in the presence of large individual arguments. + */ + private const val MAX_ARGUMENT_CHARACTERS = 4_096 + + private const val MAX_CHARACTER_WARNING = + "Nav3 arguments exceeded the maximum total character count for one backstack update. Skipping " + + "arguments for this and older captured entries." - private const val BUDGET_WARNING = - "Nav3 arguments exceeded the maximum total value count for one backstack update. Skipping arguments " + - "for this and older captured entries." + private const val MAX_COUNT_WARNING = + "Nav3 arguments exceeded the maximum total count for one backstack update. Skipping arguments for " + + "this and older captured entries." private const val STRUCTURE_WARNING = "Nav3 argument sanitization failed (possibly a cyclic or deeply nested structure). Skipping arguments." @@ -348,6 +395,8 @@ internal data class NormalizedSentryBackStackEntry( val name: String, /** Sanitized [SentryBackStackEntry.arguments] (i.e., bounded in size and depth). */ val arguments: Map = emptyMap(), + /** The reason why the SDK dropped host-provided arguments. */ + val argumentDropReason: ArgumentDropReason? = null, ) { companion object { @@ -369,12 +418,21 @@ internal data class NormalizedSentryBackStackEntry( } } + /** Returns sanitized host arguments together with SDK-owned argument metadata. */ + fun argumentsWithMetadata(): Map { + val reason = argumentDropReason ?: return arguments + return buildMap { + putAll(arguments) + put(ARGUMENT_DROP_REASON_KEY, reason.serializedValue) + } + } + /** * Returns this entry in serialized form. E.g.: * ``` * { * "entry": "/ProductScreen" - * "arguments": { + * "entry_arguments": { * "product_id": 12345 * "promo_id:": "spring-marketing-drive-2026" * } @@ -383,11 +441,26 @@ internal data class NormalizedSentryBackStackEntry( */ fun serialize(): Map = buildMap { put("entry", name) - if (arguments.isNotEmpty()) { - put("arguments", arguments) - } + argumentsWithMetadata().takeIf { it.isNotEmpty() }?.let { put("entry_arguments", it) } } } internal fun List.serialize(): List> = map(NormalizedSentryBackStackEntry::serialize) + +/** Why host-provided arguments were unavailable in emitted navigation data. */ +internal enum class ArgumentDropReason( + val serializedValue: String, + val exhaustsUpdateBudget: Boolean = false, +) { + + CHARACTER_LIMIT("max_character_limit_exceeded", exhaustsUpdateBudget = true), + INVALID_STRUCTURE("invalid_structure"), + MAPPING_FAILED("mapping_failed"), + MAX_COUNT("max_argument_count_exceeded", exhaustsUpdateBudget = true), + SANITIZATION_FAILED("sanitization_failed"); + + companion object { + const val ARGUMENT_DROP_REASON_KEY = "dropped_by_sentry" + } +} diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt index 34a036a41f..9b687d0b25 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt @@ -208,7 +208,7 @@ internal class BackStackObserver( .start( scope, currentTop.name, - currentTop.arguments, + currentTop.argumentsWithMetadata(), ) ?.let { transaction -> navContext.updateTransaction(transaction, scope, currentBackStack) } } else { @@ -373,8 +373,8 @@ private class NavTransaction(private val scopes: IScopes) { private class NavContext(private val scopes: IScopes, private val options: SentryNavOptions) { private companion object { - private const val BACKSTACK_KEY = "backstack" - private const val NAVIGATION_CONTEXT_KEY = "navigation" + private const val BACKSTACK_KEY = "Back Stack" + private const val NAVIGATION_CONTEXT_KEY = "Navigation" } fun update(scope: IScope, backStackEntries: List) { @@ -455,17 +455,23 @@ private class NavBreadcrumbs(private val scopes: IScopes) { type = NAVIGATION_OP category = NAVIGATION_OP - fromEntry?.let { - data["from"] = it.name - if (it.arguments.isNotEmpty()) { - data["from_arguments"] = it.arguments - } + fromEntry?.let { entry -> + data["from"] = entry.name + entry + .argumentsWithMetadata() + .takeIf { it.isNotEmpty() } + ?.let { arguments -> + data["from_arguments"] = arguments + } } data["to"] = toEntry.name - if (toEntry.arguments.isNotEmpty()) { - data["to_arguments"] = toEntry.arguments - } + toEntry + .argumentsWithMetadata() + .takeIf { it.isNotEmpty() } + ?.let { arguments -> + data["to_arguments"] = arguments + } level = INFO } diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt index cdad850ca1..d041a03b68 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt @@ -108,8 +108,12 @@ internal fun SentryNavEffect( ) } - // The incoming back stack is mutable and shared with the host app; copy it so that BackStackKey - // and BackStackObserver are guaranteed to have the same (stable) view. + // Intentionally don't remember this copy. Snapshot-backed lists mutate in place, so + // remember(backStack) { backStack.toList() } would cache a stale copy. (The key reference + // retained by remember() and the backStack reference passed to this effect would point to the + // same instance, causing remember() to always return the originally copied list.) Making a fresh + // copy ensures a stable snapshot for the duration of each update and lets BackStackKey compare + // it with the previous one. val copy = backStack.toList() DisposableEffect(observer, BackStackKey(copy)) { diff --git a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt index 834bebd4f9..f463567f5e 100644 --- a/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt +++ b/sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt @@ -5,7 +5,7 @@ import org.jetbrains.annotations.ApiStatus // Keep the default low: every captured entry may require argument extraction and recursive // sanitization when navigation changes are observed. -private const val DEFAULT_MAX_CAPTURED_BACK_STACK_ENTRIES = 10 +private const val DEFAULT_MAX_CAPTURED_BACK_STACK_ENTRIES = 5 /** * Configuration info for a [SentryNavEffect]. diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt index 96b103a715..1e332c67bd 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackConverterTest.kt @@ -3,6 +3,7 @@ package io.sentry.compose.navigation3 import com.google.common.truth.Truth.assertThat import io.sentry.ILogger import io.sentry.SentryLevel.WARNING +import io.sentry.compose.navigation3.ArgumentDropReason.Companion.ARGUMENT_DROP_REASON_KEY import io.sentry.compose.navigation3.BackStackConverter.RetentionPolicy import io.sentry.compose.navigation3.NormalizedSentryBackStackEntry.Companion.UNKNOWN_ENTRY_NAME import java.util.AbstractCollection @@ -161,8 +162,14 @@ class BackStackConverterTest { assertThat(routes) .containsExactly( NormalizedSentryBackStackEntry("/SettingsScreen", mapOf("section" to "privacy")), - NormalizedSentryBackStackEntry("/ProfileScreen"), - NormalizedSentryBackStackEntry("/HomeScreen"), + NormalizedSentryBackStackEntry( + "/ProfileScreen", + argumentDropReason = ArgumentDropReason.MAX_COUNT, + ), + NormalizedSentryBackStackEntry( + "/HomeScreen", + argumentDropReason = ArgumentDropReason.MAX_COUNT, + ), ) .inOrder() } @@ -191,8 +198,14 @@ class BackStackConverterTest { assertThat(routes) .containsExactly( - NormalizedSentryBackStackEntry("/HomeScreen"), - NormalizedSentryBackStackEntry("/ProfileScreen"), + NormalizedSentryBackStackEntry( + "/HomeScreen", + argumentDropReason = ArgumentDropReason.MAX_COUNT, + ), + NormalizedSentryBackStackEntry( + "/ProfileScreen", + argumentDropReason = ArgumentDropReason.MAX_COUNT, + ), NormalizedSentryBackStackEntry("/SettingsScreen", mapOf("section" to "privacy")), ) .inOrder() @@ -222,8 +235,14 @@ class BackStackConverterTest { assertThat(routes) .containsExactly( NormalizedSentryBackStackEntry("/ProductScreen", mapOf("productId" to "sku-1")), - NormalizedSentryBackStackEntry("/ProfileScreen"), - NormalizedSentryBackStackEntry("/ProductScreen"), + NormalizedSentryBackStackEntry( + "/ProfileScreen", + argumentDropReason = ArgumentDropReason.MAX_COUNT, + ), + NormalizedSentryBackStackEntry( + "/ProductScreen", + argumentDropReason = ArgumentDropReason.MAX_COUNT, + ), ) .inOrder() } @@ -251,8 +270,14 @@ class BackStackConverterTest { assertThat(routes) .containsExactly( - NormalizedSentryBackStackEntry("/ProductScreen"), - NormalizedSentryBackStackEntry("/ProfileScreen"), + NormalizedSentryBackStackEntry( + "/ProductScreen", + argumentDropReason = ArgumentDropReason.MAX_COUNT, + ), + NormalizedSentryBackStackEntry( + "/ProfileScreen", + argumentDropReason = ArgumentDropReason.MAX_COUNT, + ), NormalizedSentryBackStackEntry("/ProductScreen", mapOf("productId" to "sku-1")), ) .inOrder() @@ -313,7 +338,12 @@ class BackStackConverterTest { val sut = getSut(entryMapper = { error("boom") }) assertThat(sut.convert(HomeScreen())) - .isEqualTo(NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME)) + .isEqualTo( + NormalizedSentryBackStackEntry( + UNKNOWN_ENTRY_NAME, + argumentDropReason = ArgumentDropReason.MAPPING_FAILED, + ) + ) verify(logger) .log( eq(WARNING), @@ -482,7 +512,10 @@ class BackStackConverterTest { val sut = getSut(entryMapper = { entry -> entryInfo(entry, mapOf("cyclic" to cyclic)) }) - assertThat(sut.convert(HomeScreen()).arguments).isEmpty() + val entry = sut.convert(HomeScreen()) + + assertThat(entry.arguments).isEmpty() + assertThat(entry.argumentDropReason).isEqualTo(ArgumentDropReason.INVALID_STRUCTURE) } @Test @@ -492,7 +525,10 @@ class BackStackConverterTest { val sut = getSut(entryMapper = { entry -> entryInfo(entry, mapOf("nested" to nested)) }) - assertThat(sut.convert(ProfileScreen("123")).arguments).isEmpty() + val entry = sut.convert(ProfileScreen("123")) + + assertThat(entry.arguments).isEmpty() + assertThat(entry.argumentDropReason).isEqualTo(ArgumentDropReason.INVALID_STRUCTURE) } @Test @@ -500,7 +536,35 @@ class BackStackConverterTest { val sut = getSut(entryMapper = { entry -> entryInfo(entry, mapOf("values" to List(1_001) { it })) }) - assertThat(sut.convert(HomeScreen()).arguments).isEmpty() + val entry = sut.convert(HomeScreen()) + + assertThat(entry.arguments).isEmpty() + assertThat(entry.argumentDropReason).isEqualTo(ArgumentDropReason.MAX_COUNT) + } + + @Test + fun `convert drops arguments when character budget is exceeded`() { + val sut = + getSut(entryMapper = { entry -> entryInfo(entry, mapOf("value" to "x".repeat(4_097))) }) + + val entry = sut.convert(HomeScreen()) + + assertThat(entry.arguments).isEmpty() + assertThat(entry.argumentDropReason).isEqualTo(ArgumentDropReason.CHARACTER_LIMIT) + } + + @Test + fun `convert reports sanitization failures`() { + class ThrowingValue { + override fun toString(): String = error("boom") + } + + val sut = getSut(entryMapper = { entry -> entryInfo(entry, mapOf("value" to ThrowingValue())) }) + + val converted = sut.convert(HomeScreen()) + + assertThat(converted.arguments).isEmpty() + assertThat(converted.argumentDropReason).isEqualTo(ArgumentDropReason.SANITIZATION_FAILED) } @Test @@ -555,9 +619,26 @@ class BackStackConverterTest { assertThat(routes.map(NormalizedSentryBackStackEntry::serialize)) .containsExactly( - mapOf("entry" to "/SettingsScreen", "arguments" to mapOf("section" to "privacy")), + mapOf("entry" to "/SettingsScreen", "entry_arguments" to mapOf("section" to "privacy")), mapOf("entry" to "/ProfileScreen"), ) .inOrder() } + + @Test + fun `serialize includes the reason arguments were dropped`() { + val entry = + NormalizedSentryBackStackEntry( + name = "/HomeScreen", + argumentDropReason = ArgumentDropReason.MAX_COUNT, + ) + + assertThat(entry.serialize()) + .isEqualTo( + mapOf( + "entry" to "/HomeScreen", + "entry_arguments" to mapOf(ARGUMENT_DROP_REASON_KEY to "max_argument_count_exceeded"), + ) + ) + } } diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt index d0fc171b93..0c7cdc9f53 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/BackStackObserverTest.kt @@ -15,6 +15,7 @@ import io.sentry.SentryOptions import io.sentry.SentryTracer import io.sentry.TransactionContext import io.sentry.TransactionOptions +import io.sentry.compose.navigation3.ArgumentDropReason.Companion.ARGUMENT_DROP_REASON_KEY import io.sentry.protocol.App import io.sentry.protocol.TransactionNameSource import org.junit.Test @@ -272,8 +273,11 @@ class BackStackObserverTest { assertThat(fixture.scope.navigationBackStack()) .isEqualTo( listOf( - mapOf("entry" to "/ProfileScreen", "arguments" to mapOf("userId" to "123")), - mapOf("entry" to "/HomeScreen"), + mapOf("entry" to "/ProfileScreen", "entry_arguments" to mapOf("userId" to "123")), + mapOf( + "entry" to "/HomeScreen", + "entry_arguments" to mapOf(ARGUMENT_DROP_REASON_KEY to "max_argument_count_exceeded"), + ), ) ) assertThat(fixture.startedTransactions.single().getData("arguments")) @@ -333,14 +337,14 @@ class BackStackObserverTest { config = ObserverConfig(captureBackStack = true, maxCapturedBackStackEntries = 0) ) fixture.scope.setContexts( - "navigation", - mapOf("backstack" to listOf(mapOf("entry" to "/Stale"))), + NAVIGATION_CONTEXT_KEY, + mapOf(BACKSTACK_KEY to listOf(mapOf("entry" to "/Stale"))), ) sut.onBackStackChanged(listOf(HomeScreen())) // Doesn't emit a back stack... - assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() + assertThat(fixture.scope.contexts.containsKey(NAVIGATION_CONTEXT_KEY)).isFalse() // ...but continues to emit all other Sentry data. assertThat(fixture.breadcrumbs.single().data["to"]).isEqualTo("/HomeScreen") @@ -354,14 +358,14 @@ class BackStackObserverTest { val fixture = Fixture() val sut = fixture.getSut(config = ObserverConfig(captureBackStack = false)) fixture.scope.setContexts( - "navigation", - mapOf("backstack" to listOf(mapOf("entry" to "/Stale"))), + NAVIGATION_CONTEXT_KEY, + mapOf(BACKSTACK_KEY to listOf(mapOf("entry" to "/Stale"))), ) sut.onBackStackChanged(listOf(HomeScreen())) // Doesn't emit a back stack... - assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() + assertThat(fixture.scope.contexts.containsKey(NAVIGATION_CONTEXT_KEY)).isFalse() // ...but continues to emit all other Sentry data. assertThat(fixture.breadcrumbs.single().data["to"]).isEqualTo("/HomeScreen") @@ -457,7 +461,7 @@ class BackStackObserverTest { assertThat(transaction.navigationBackStack()) .isEqualTo( listOf( - mapOf("entry" to "/ProfileScreen", "arguments" to mapOf("userId" to "123")), + mapOf("entry" to "/ProfileScreen", "entry_arguments" to mapOf("userId" to "123")), mapOf("entry" to "/HomeScreen"), ) ) @@ -602,7 +606,7 @@ class BackStackObserverTest { assertThat(fixture.scope.transaction).isNull() assertThat(fixture.scope.screen).isNull() assertThat(fixture.scope.contexts.app?.viewNames).isNull() - assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() + assertThat(fixture.scope.contexts.containsKey(NAVIGATION_CONTEXT_KEY)).isFalse() assertThat(fixture.breadcrumbs).hasSize(1) } @@ -672,6 +676,8 @@ class BackStackObserverTest { val cartTransaction = fixture.startedTransactions.last() assertThat(cartTransaction.isFinished).isFalse() assertThat(cartTransaction.name).isEqualTo(NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME) + assertThat(cartTransaction.getData("arguments")) + .isEqualTo(mapOf(ARGUMENT_DROP_REASON_KEY to "mapping_failed")) assertThat(fixture.scope.transaction).isSameInstanceAs(cartTransaction) assertThat(fixture.scope.screen).isEqualTo(NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME) assertThat(fixture.scope.contexts.app?.viewNames) @@ -683,11 +689,16 @@ class BackStackObserverTest { NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME, "to", NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME, + "to_arguments", + mapOf(ARGUMENT_DROP_REASON_KEY to "mapping_failed"), ) assertThat(fixture.scope.navigationBackStack()) .isEqualTo( listOf( - mapOf("entry" to NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME), + mapOf( + "entry" to NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME, + "entry_arguments" to mapOf(ARGUMENT_DROP_REASON_KEY to "mapping_failed"), + ), mapOf("entry" to NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME), mapOf("entry" to "/home"), ) @@ -707,12 +718,22 @@ class BackStackObserverTest { assertThat(fixture.scope.contexts.app?.viewNames).isEqualTo(listOf("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/settings")) assertThat(fixture.breadcrumbs).hasSize(4) assertThat(fixture.breadcrumbs.last().data) - .containsExactly("from", NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME, "to", "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/settings") + .containsExactly( + "from", + NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME, + "from_arguments", + mapOf(ARGUMENT_DROP_REASON_KEY to "mapping_failed"), + "to", + "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/settings", + ) assertThat(fixture.scope.navigationBackStack()) .isEqualTo( listOf( mapOf("entry" to "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/settings"), - mapOf("entry" to NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME), + mapOf( + "entry" to NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME, + "entry_arguments" to mapOf(ARGUMENT_DROP_REASON_KEY to "mapping_failed"), + ), mapOf("entry" to NormalizedSentryBackStackEntry.UNKNOWN_ENTRY_NAME), mapOf("entry" to "/home"), ) @@ -733,7 +754,7 @@ class BackStackObserverTest { assertThat(fixture.scope.transaction).isNull() assertThat(fixture.scope.screen).isNull() assertThat(fixture.scope.contexts.app?.viewNames).isNull() - assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() + assertThat(fixture.scope.contexts.containsKey(NAVIGATION_CONTEXT_KEY)).isFalse() } private fun IScope.navigationBackStack(): List>? { @@ -751,7 +772,7 @@ class BackStackObserverTest { } private companion object { - const val NAVIGATION_CONTEXT_KEY = "navigation" - const val BACKSTACK_KEY = "backstack" + const val NAVIGATION_CONTEXT_KEY = "Navigation" + const val BACKSTACK_KEY = "Back Stack" } } diff --git a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt index 384226f683..93c1cde567 100644 --- a/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt +++ b/sentry-android-navigation3/src/test/kotlin/io/sentry/compose/navigation3/SentryNavEffectTest.kt @@ -440,7 +440,7 @@ class SentryNavEffectTest { assertThat(fixture.scope.navigationBackStack()) .isEqualTo( listOf( - mapOf("entry" to "/ProfileRoute", "arguments" to mapOf("userId" to "123")), + mapOf("entry" to "/ProfileRoute", "entry_arguments" to mapOf("userId" to "123")), mapOf("entry" to "/HomeRoute"), ) ) @@ -510,7 +510,7 @@ class SentryNavEffectTest { assertThat(fixture.transactions.last().isFinished).isFalse() assertThat(fixture.scope.transaction).isSameInstanceAs(fixture.transactions.last()) assertThat(fixture.scope.screen).isEqualTo("/HomeRoute") - assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() + assertThat(fixture.scope.contexts.containsKey(NAVIGATION_CONTEXT_KEY)).isFalse() } @Test @@ -539,7 +539,7 @@ class SentryNavEffectTest { assertThat(fixture.scope.transaction).isNull() assertThat(fixture.scope.screen).isNull() assertThat(fixture.scope.contexts.app?.viewNames).isNull() - assertThat(fixture.scope.contexts.containsKey("navigation")).isFalse() + assertThat(fixture.scope.contexts.containsKey(NAVIGATION_CONTEXT_KEY)).isFalse() } private fun IScope.navigationBackStack(): List>? { @@ -557,7 +557,7 @@ class SentryNavEffectTest { } private companion object { - const val NAVIGATION_CONTEXT_KEY = "navigation" - const val BACKSTACK_KEY = "backstack" + const val NAVIGATION_CONTEXT_KEY = "Navigation" + const val BACKSTACK_KEY = "Back Stack" } }