Repository navigation
feat(nav3): Introduce BackStackEntryMappers for Nav3 + prepare for SAGP auto-instrumentation - #6273
Draft
0xadam-brown wants to merge 1 commit into
Draft
0xadam-brown wants to merge 1 commit into
0xadam-brown wants to merge 1 commit into
Conversation
Add BackStackEntryMapper.serialName(), which names @serializable back stack entries after their kotlinx.serialization serial names, and make it the default mapper for SentryNavEffect. Previously, every app had to write a mapper. Serial names are compile-time constants, so they survive R8 minification without keep rules. Only classes annotated @serializable are named; other entries fall back to /unknown. Custom mappers can delegate the entries they don't handle, so moving to a custom mapper doesn't rename routes. BackStackEntryMapper becomes contravariant so one mapper serves back stacks of any type. kotlinx.serialization is a compileOnly dependency. The lookup only runs after a runtime presence check, and the AAR ships R8 rules so apps without the library still build and minified apps still detect it.
Contributor
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Features
- Introduce BackStackEntryMappers for Nav3 + prepare for SAGP auto-instrumentation ([#6273](https://github.com/getsentry/sentry-java/pull/6273))If none of the above apply, you can opt out of this check by adding |
📲 Install BuildsAndroid
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Description
Introduce sentry-java changes needed to support Nav3 auto-instrumentation via the Sentry Android Gradle Plugin.
Those changes have the happy side effect of introducing some helpful out-of-the-box implementations of
BackStackEntryMapperso most developers won't have to implement their own.See "Motivation and Context" below for some helpful details.
💡 Motivation and Context
The basic idea:
[1] The new
SentryNavAutoInstrumentation.Effectacts as a stable API for SAGP code that lets it avoid having to care about Compose compiler plugin modifications to SentryNavEffect's parameters.[2]
Effectalso lets us expose aBackStackMappingTyperather than the actualBackStackEntryMapperexposed by SentryNavEffect.[3] That mapping type can be configured externally, either by a new
SentryOptions.dataCollection.navigationParams-esque boolean, or by a toggle exposed via an AndroidManifest.[4] The flexibility is [3] means we don't have to wait on folks to approve (or deny) our request to add a new boolean to
SentryOptions.dataCollection, since those have to be coordinated across SDKs and formally specified.A helpful addition:
The spec for the recently introduced Data Collection API requires that all sensitive denylist values be scrubbed from HTTP headers, cookies, and URL params (link). This PR subjects arguments serialized via the new
BackStackEntryMapper.serializeNameWithArguments()to similar scrubbing.Note that
serialName()andserialNameWithArguments()fall back to "/unknown" if a@SerialNameisn't available, rather than our original idea of falling back to the entry's class name with the help of this KEEP rule:That fallback would help NavKey types that aren't @serializable. Turns out they're very uncommon, as a search against GitHub shows (about 23,200 hits for "NavKey" but the API returns at most the first 1,000, which came from 749 repos):
@SerializableNavKeytypesSo about 99% of
NavKeytypes are@Serializable, and about 95% of repos make everyNavKeytype serializable. And so I've refrained from adding the KEEP rule in order to avoid interfering with R8 on behalf of what looks like a de minimis corner case. Chime in if you think differently!💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps