Skip to content

Commit 8734fe7

Browse files
committed
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.
1 parent 65c166a commit 8734fe7

7 files changed

Lines changed: 268 additions & 83 deletions

File tree

‎sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackConverter.kt‎

Lines changed: 108 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package io.sentry.compose.navigation3
22

33
import io.sentry.ILogger
44
import io.sentry.SentryLevel.WARNING
5+
import io.sentry.compose.navigation3.ArgumentDropReason.Companion.ARGUMENT_DROP_REASON_KEY
56
import io.sentry.compose.navigation3.NormalizedSentryBackStackEntry.Companion.UNKNOWN_ENTRY_NAME
67
import io.sentry.util.ExceptionUtils
78
import java.util.IdentityHashMap
@@ -69,21 +70,33 @@ internal class BackStackConverter<T : Any>(
6970
// Back stack entry mappers are host app callbacks.
7071
ExceptionUtils.rethrowIfFatal(t)
7172
warningState.logMapperFailureWarning(logger, t)
72-
return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME)
73+
return NormalizedSentryBackStackEntry(
74+
name = UNKNOWN_ENTRY_NAME,
75+
argumentDropReason = ArgumentDropReason.MAPPING_FAILED,
76+
)
7377
}
7478

7579
if (info == null) {
76-
return NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME)
80+
return NormalizedSentryBackStackEntry(name = UNKNOWN_ENTRY_NAME)
7781
}
7882

79-
val arguments = info.arguments?.let(sanitizer::sanitizeEntry) ?: emptyMap()
83+
val sanitizedArguments =
84+
info.arguments?.let(sanitizer::sanitizeEntry) ?: SanitizedArguments(emptyMap())
8085
val formattedName = NormalizedSentryBackStackEntry.formatName(info.name)
8186

8287
return if (formattedName.isBlank()) {
8388
warningState.logInvalidNameWarning(logger)
84-
NormalizedSentryBackStackEntry(UNKNOWN_ENTRY_NAME, arguments)
89+
NormalizedSentryBackStackEntry(
90+
name = UNKNOWN_ENTRY_NAME,
91+
arguments = sanitizedArguments.values,
92+
argumentDropReason = sanitizedArguments.dropReason,
93+
)
8594
} else {
86-
NormalizedSentryBackStackEntry(formattedName, arguments)
95+
NormalizedSentryBackStackEntry(
96+
name = formattedName,
97+
arguments = sanitizedArguments.values,
98+
argumentDropReason = sanitizedArguments.dropReason,
99+
)
87100
}
88101
}
89102

@@ -109,6 +122,11 @@ internal class BackStackConverter<T : Any>(
109122
KEEP_LAST,
110123
}
111124

125+
internal data class SanitizedArguments(
126+
val values: Map<String, Any?>,
127+
val dropReason: ArgumentDropReason? = null,
128+
)
129+
112130
/**
113131
* Sanitizes a back stack entry's arguments and writes them in a serializable form. It bounds
114132
* depth and total value count, and it rejects cyclic structures.
@@ -124,32 +142,33 @@ internal class BackStackConverter<T : Any>(
124142

125143
private val activeContainers = IdentityHashMap<Any, Unit>()
126144
private var remainingValues = MAX_ARGUMENT_COUNT
127-
private var budgetExhausted = false
145+
private var remainingCharacters = MAX_ARGUMENT_CHARACTERS
146+
private var dropReason: ArgumentDropReason? = null
128147

129148
/**
130149
* Sanitizes one entry's arguments, or returns an empty map to drop them, either because the
131150
* structure is cyclic or too deeply nested (this entry only), or because the shared per-update
132151
* value budget is spent (this entry and every older one).
133152
*/
134153
@Suppress("TooGenericExceptionCaught")
135-
fun sanitizeEntry(raw: Map<String, Any?>): Map<String, Any?> {
136-
if (budgetExhausted) {
137-
return emptyMap()
154+
fun sanitizeEntry(raw: Map<String, Any?>): SanitizedArguments {
155+
dropReason?.let { reason ->
156+
return SanitizedArguments(emptyMap(), reason)
138157
}
139158

140159
return try {
141-
sanitizeMap(raw, depth = 0)
160+
SanitizedArguments(sanitizeMap(raw, depth = 0))
142161
} catch (drop: DropSubtree) {
143-
if (drop.exhaustsBudget) {
144-
budgetExhausted = true
162+
if (drop.reason.exhaustsUpdateBudget) {
163+
dropReason = drop.reason
145164
}
146165
logger.log(WARNING, drop.warning)
147-
emptyMap()
166+
SanitizedArguments(emptyMap(), drop.reason)
148167
} catch (t: Throwable) {
149168
// Extracted maps may invoke host app code while iterating or stringifying values.
150169
ExceptionUtils.rethrowIfFatal(t)
151170
logger.log(WARNING, STRUCTURE_WARNING, t)
152-
emptyMap()
171+
SanitizedArguments(emptyMap(), ArgumentDropReason.SANITIZATION_FAILED)
153172
}
154173
}
155174

@@ -158,7 +177,9 @@ internal class BackStackConverter<T : Any>(
158177
try {
159178
val sanitized = mutableMapOf<String, Any?>()
160179
for ((key, childValue) in value) {
161-
sanitized[key.toString()] = sanitizeValue(childValue, depth + 1)
180+
val keyString = key.toString()
181+
consumeCharacters(keyString.length)
182+
sanitized[keyString] = sanitizeValue(childValue, depth + 1)
162183
}
163184
return sanitized
164185
} finally {
@@ -185,17 +206,26 @@ internal class BackStackConverter<T : Any>(
185206
val collection = value?.asSanitizableCollectionOrNull()
186207

187208
return when {
188-
value == null || value is String || value is Number || value is Boolean -> value
189-
value is CharSequence || value is Char -> value.toString()
190-
value is Enum<*> -> value.name
191209
value is Map<*, *> -> sanitizeMap(value, depth)
192210
collection != null -> sanitizeCollection(collection, depth)
211+
else -> sanitizeScalar(value)
212+
}
213+
}
214+
215+
private fun sanitizeScalar(value: Any?): Any? =
216+
when (value) {
217+
null,
218+
is Number,
219+
is Boolean -> value
220+
is String -> value.also { consumeCharacters(it.length) }
221+
is CharSequence,
222+
is Char -> value.toString().also { consumeCharacters(it.length) }
223+
is Enum<*> -> value.name.also { consumeCharacters(it.length) }
193224
else -> {
194225
warningState.logUnsupportedValueWarning(value::class.simpleName, logger)
195-
value.toString()
226+
value.toString().also { consumeCharacters(it.length) }
196227
}
197228
}
198-
}
199229

200230
private fun Any.asSanitizableCollectionOrNull(): Collection<*>? =
201231
when (this) {
@@ -218,16 +248,23 @@ internal class BackStackConverter<T : Any>(
218248
*/
219249
private fun visit(depth: Int) {
220250
if (depth > MAX_ARGUMENT_DEPTH) {
221-
throw DropSubtree(STRUCTURE_WARNING, exhaustsBudget = false)
251+
throw DropSubtree(STRUCTURE_WARNING, ArgumentDropReason.INVALID_STRUCTURE)
222252
}
223253
if (--remainingValues < 0) {
224-
throw DropSubtree(BUDGET_WARNING, exhaustsBudget = true)
254+
throw DropSubtree(MAX_COUNT_WARNING, ArgumentDropReason.MAX_COUNT)
225255
}
226256
}
227257

258+
private fun consumeCharacters(count: Int) {
259+
if (count > remainingCharacters) {
260+
throw DropSubtree(MAX_CHARACTER_WARNING, ArgumentDropReason.CHARACTER_LIMIT)
261+
}
262+
remainingCharacters -= count
263+
}
264+
228265
private fun enter(container: Any) {
229266
if (activeContainers.put(container, Unit) != null) {
230-
throw DropSubtree(STRUCTURE_WARNING, exhaustsBudget = false)
267+
throw DropSubtree(STRUCTURE_WARNING, ArgumentDropReason.INVALID_STRUCTURE)
231268
}
232269
}
233270

@@ -239,11 +276,10 @@ internal class BackStackConverter<T : Any>(
239276
* Control-flow signal to abort sanitization of the current subtree. Internal to
240277
* [ArgumentSanitizer].
241278
*
242-
* [exhaustsBudget] distinguishes an entry-local drop (cycle or over-deep structure) from an
243-
* update-wide one (the shared value budget is spent). Overrides [fillInStackTrace] to skip
244-
* stack-trace capture.
279+
* [reason] distinguishes entry-local drops from update-wide budget exhaustion. Overrides
280+
* [fillInStackTrace] to skip stack-trace capture.
245281
*/
246-
private class DropSubtree(val warning: String, val exhaustsBudget: Boolean) :
282+
private class DropSubtree(val warning: String, val reason: ArgumentDropReason) :
247283
RuntimeException() {
248284
override fun fillInStackTrace(): Throwable = this
249285
}
@@ -279,11 +315,22 @@ internal class BackStackConverter<T : Any>(
279315
*
280316
* Then /ProductDetail and /Home will have no arguments, but /Checkout will.
281317
*/
282-
private const val MAX_ARGUMENT_COUNT = 500
318+
private const val MAX_ARGUMENT_COUNT = 200
319+
320+
/**
321+
* Max number of characters visited while sanitizing all entries in a given back stack update.
322+
*
323+
* Caps payload size in the presence of large individual arguments.
324+
*/
325+
private const val MAX_ARGUMENT_CHARACTERS = 4_096
326+
327+
private const val MAX_CHARACTER_WARNING =
328+
"Nav3 arguments exceeded the maximum total character count for one backstack update. Skipping " +
329+
"arguments for this and older captured entries."
283330

284-
private const val BUDGET_WARNING =
285-
"Nav3 arguments exceeded the maximum total value count for one backstack update. Skipping arguments " +
286-
"for this and older captured entries."
331+
private const val MAX_COUNT_WARNING =
332+
"Nav3 arguments exceeded the maximum total count for one backstack update. Skipping arguments for " +
333+
"this and older captured entries."
287334

288335
private const val STRUCTURE_WARNING =
289336
"Nav3 argument sanitization failed (possibly a cyclic or deeply nested structure). Skipping arguments."
@@ -348,6 +395,8 @@ internal data class NormalizedSentryBackStackEntry(
348395
val name: String,
349396
/** Sanitized [SentryBackStackEntry.arguments] (i.e., bounded in size and depth). */
350397
val arguments: Map<String, Any?> = emptyMap(),
398+
/** The reason why the SDK dropped host-provided arguments. */
399+
val argumentDropReason: ArgumentDropReason? = null,
351400
) {
352401

353402
companion object {
@@ -369,12 +418,21 @@ internal data class NormalizedSentryBackStackEntry(
369418
}
370419
}
371420

421+
/** Returns sanitized host arguments together with SDK-owned argument metadata. */
422+
fun argumentsWithMetadata(): Map<String, Any?> {
423+
val reason = argumentDropReason ?: return arguments
424+
return buildMap {
425+
putAll(arguments)
426+
put(ARGUMENT_DROP_REASON_KEY, reason.serializedValue)
427+
}
428+
}
429+
372430
/**
373431
* Returns this entry in serialized form. E.g.:
374432
* ```
375433
* {
376434
* "entry": "/ProductScreen"
377-
* "arguments": {
435+
* "entry_arguments": {
378436
* "product_id": 12345
379437
* "promo_id:": "spring-marketing-drive-2026"
380438
* }
@@ -383,11 +441,26 @@ internal data class NormalizedSentryBackStackEntry(
383441
*/
384442
fun serialize(): Map<String, Any?> = buildMap {
385443
put("entry", name)
386-
if (arguments.isNotEmpty()) {
387-
put("arguments", arguments)
388-
}
444+
argumentsWithMetadata().takeIf { it.isNotEmpty() }?.let { put("entry_arguments", it) }
389445
}
390446
}
391447

392448
internal fun List<NormalizedSentryBackStackEntry>.serialize(): List<Map<String, Any?>> =
393449
map(NormalizedSentryBackStackEntry::serialize)
450+
451+
/** Why host-provided arguments were unavailable in emitted navigation data. */
452+
internal enum class ArgumentDropReason(
453+
val serializedValue: String,
454+
val exhaustsUpdateBudget: Boolean = false,
455+
) {
456+
457+
CHARACTER_LIMIT("max_character_limit_exceeded", exhaustsUpdateBudget = true),
458+
INVALID_STRUCTURE("invalid_structure"),
459+
MAPPING_FAILED("mapping_failed"),
460+
MAX_COUNT("max_argument_count_exceeded", exhaustsUpdateBudget = true),
461+
SANITIZATION_FAILED("sanitization_failed");
462+
463+
companion object {
464+
const val ARGUMENT_DROP_REASON_KEY = "dropped_by_sentry"
465+
}
466+
}

‎sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/BackStackObserver.kt‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ internal class BackStackObserver<T : Any>(
208208
.start(
209209
scope,
210210
currentTop.name,
211-
currentTop.arguments,
211+
currentTop.argumentsWithMetadata(),
212212
)
213213
?.let { transaction -> navContext.updateTransaction(transaction, scope, currentBackStack) }
214214
} else {
@@ -373,8 +373,8 @@ private class NavTransaction(private val scopes: IScopes) {
373373
private class NavContext(private val scopes: IScopes, private val options: SentryNavOptions) {
374374

375375
private companion object {
376-
private const val BACKSTACK_KEY = "backstack"
377-
private const val NAVIGATION_CONTEXT_KEY = "navigation"
376+
private const val BACKSTACK_KEY = "Back Stack"
377+
private const val NAVIGATION_CONTEXT_KEY = "Navigation"
378378
}
379379

380380
fun update(scope: IScope, backStackEntries: List<NormalizedSentryBackStackEntry>) {
@@ -455,17 +455,23 @@ private class NavBreadcrumbs(private val scopes: IScopes) {
455455
type = NAVIGATION_OP
456456
category = NAVIGATION_OP
457457

458-
fromEntry?.let {
459-
data["from"] = it.name
460-
if (it.arguments.isNotEmpty()) {
461-
data["from_arguments"] = it.arguments
462-
}
458+
fromEntry?.let { entry ->
459+
data["from"] = entry.name
460+
entry
461+
.argumentsWithMetadata()
462+
.takeIf { it.isNotEmpty() }
463+
?.let { arguments ->
464+
data["from_arguments"] = arguments
465+
}
463466
}
464467

465468
data["to"] = toEntry.name
466-
if (toEntry.arguments.isNotEmpty()) {
467-
data["to_arguments"] = toEntry.arguments
468-
}
469+
toEntry
470+
.argumentsWithMetadata()
471+
.takeIf { it.isNotEmpty() }
472+
?.let { arguments ->
473+
data["to_arguments"] = arguments
474+
}
469475

470476
level = INFO
471477
}

‎sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavEffect.kt‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,12 @@ internal fun <T : Any> SentryNavEffect(
108108
)
109109
}
110110

111-
// The incoming back stack is mutable and shared with the host app; copy it so that BackStackKey
112-
// and BackStackObserver are guaranteed to have the same (stable) view.
111+
// Intentionally don't remember this copy. Snapshot-backed lists mutate in place, so
112+
// remember(backStack) { backStack.toList() } would cache a stale copy. (The key reference
113+
// retained by remember() and the backStack reference passed to this effect would point to the
114+
// same instance, causing remember() to always return the originally copied list.) Making a fresh
115+
// copy ensures a stable snapshot for the duration of each update and lets BackStackKey compare
116+
// it with the previous one.
113117
val copy = backStack.toList()
114118

115119
DisposableEffect(observer, BackStackKey(copy)) {

‎sentry-android-navigation3/src/main/kotlin/io/sentry/compose/navigation3/SentryNavOptions.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import org.jetbrains.annotations.ApiStatus
55

66
// Keep the default low: every captured entry may require argument extraction and recursive
77
// sanitization when navigation changes are observed.
8-
private const val DEFAULT_MAX_CAPTURED_BACK_STACK_ENTRIES = 10
8+
private const val DEFAULT_MAX_CAPTURED_BACK_STACK_ENTRIES = 5
99

1010
/**
1111
* Configuration info for a [SentryNavEffect].

0 commit comments

Comments
 (0)