feat(tracing): Wrap Expo Router push, replace, navigate, back, dismiss in addition to prefetch#6221
Draft
alwx wants to merge 5 commits into
Draft
feat(tracing): Wrap Expo Router push, replace, navigate, back, dismiss in addition to prefetch#6221alwx wants to merge 5 commits into
push, replace, navigate, back, dismiss in addition to prefetch#6221alwx wants to merge 5 commits into
Conversation
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
Contributor
|
push, replace, navigate, back, dismiss in addition to prefetchpush, replace, navigate, back, dismiss in addition to prefetch
…nd sendDefaultPii
Expo Router `push`/`replace`/`navigate`/`back`/`dismiss` (and `prefetch`)
wrappers previously emitted the raw `href` and route `params` on both
the navigation breadcrumb and span attributes regardless of the client
options. Dynamic segments like `/users/123` and parameter values such
as `{ id: '123' }` can carry user identifiers and other PII, so the
existing `reactnavigation.ts` integration already gates them behind
`sendDefaultPii`. The new Expo Router wrappers should follow the same
contract.
This change:
- Reads `sendDefaultPii` from the client options and only attaches
`href` (raw string or stringified object form) and `params` to the
breadcrumb `data` and span attributes when it is enabled.
- Keeps the non-PII fields — `method`, `pathname` (route template),
and `route.name` — unconditional so navigations remain visible and
groupable in Sentry without leaking user data.
- Applies the same guard to the `prefetch` span's `route.href`
attribute, which was newly added in this PR.
- Adds dedicated `sendDefaultPii` test cases and updates the existing
default-off assertions for all wrapped methods.
- Adds a changelog entry for #6221.
push, replace, navigate, back, dismiss in addition to prefetchpush, replace, navigate, back, dismiss in addition to prefetch
| SPAN_ORIGIN_AUTO_EXPO_ROUTER_NAVIGATION, | ||
| SPAN_ORIGIN_AUTO_EXPO_ROUTER_PREFETCH, | ||
| } from '../../src/js/tracing/origin'; | ||
| import { |
Contributor
There was a problem hiding this comment.
No test covers reactnavigation.ts consuming pendingExpoRouterNavigation to tag idle spans
The PR's core feature — attributing idle navigation spans back to the Expo Router call via navigation.method — is exercised in expoRouter.test.ts only for the setter side; no test in reactnavigation.test.ts (or anywhere else) verifies that reactnavigation.ts actually reads and applies the pending value to the resulting span.
Evidence
reactnavigation.ts:456callsconsumePendingExpoRouterNavigation()and setsnavigation.methodonlatestNavigationSpan.grepforpendingExpoRouterandnavigation.methodacross all test files returns only hits insideexpoRouter.test.ts.reactnavigation.test.tshas zero matches for either symbol.- The hand-off path (set in
wrapNavigationMethod→ consumed inreactnavigationidle span handler) has no integration-level test, so a regression in the consumer would go undetected.
Also found at 2 additional locations
packages/core/src/js/tracing/reactnavigation.ts:33packages/core/src/js/tracing/reactnavigation.ts:455
Identified by Warden code-review · 4UA-DHD
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.
📢 Type of change
📜 Description
Extends the Expo Router instrumentation beyond
prefetchso that programmatic navigation calls produce breadcrumbs and spans, and the resulting idle navigation transaction can be attributed back to the call that triggered it.What changed:
wrapExpoRouternow wrapspush,replace,navigate,back, anddismissin addition toprefetch.navigationbreadcrumb (category: 'navigation',data.method,data.pathname),navigation.<method>span tagged withsentry.origin = auto.navigation.expo_router,navigation.method, androute.name,pendingExpoRouterNavigation) that the next React Navigation idle navigation span consumes to set its ownnavigation.methodattribute, attributing the transaction back to the originating call.__sentryWrappedflag guards against double-wrapping ifwrapExpoRouteris called multiple times on the same router.reactnavigation.ts: rawhrefand routeparams(which can encode user identifiers like/users/123or{ id: '123' }) are only attached to breadcrumbs and span attributes whensendDefaultPiiis enabled.method,pathname(the route template), androute.nameremain unconditional so navigations stay visible and groupable.💡 Motivation and Context
Fixes #6158.
Previously only
prefetchwas instrumented, so any programmaticrouter.push(...)/router.replace(...)/router.back()etc. was invisible in Sentry — no breadcrumb trail, no span, and the resulting React Navigation idle transaction had no way to know which call site initiated it. This PR closes that gap while keeping the existingprefetchbehavior intact and aligning PII handling with the rest of the navigation tracing.💚 How did you test it?
packages/core/test/tracing/expoRouter.test.tscovering:push/replace/navigateviadescribe.each— span and breadcrumb shape with string and object hrefs, error reporting, and the pending-navigation hand-off,back()anddismiss(count)with no/optional args,href, noparams) and dedicatedsendDefaultPii: truecases that verifyhrefandparamsreappear on both the breadcrumb and the span.yarn jest— 113 suites / 1481 tests passing.yarn lint:lernaclean.yarn api-report:checkup to date.📝 Checklist
sendDefaultPiiis enabled🔮 Next steps