feat(tracing): Correlate deep links with the navigation they trigger#6264
Open
alwx wants to merge 5 commits into
Open
feat(tracing): Correlate deep links with the navigation they trigger#6264alwx wants to merge 5 commits into
alwx wants to merge 5 commits into
Conversation
Bridges `deeplinkIntegration` and `reactNavigationIntegration` so the idle
navigation span started after a deep link arrival is tagged with the link
that caused it. Previously the two timelines were unconnected — the
breadcrumb recorded `Linking.getInitialURL()` / the `'url'` event, but the
resulting navigation span had no way to know it had been triggered by a
deep link, making it impossible to measure the gap between "URL received"
and "navigation dispatched".
Approach mirrors the existing `pendingExpoRouterNavigation` hand-off:
* New `tracing/pendingDeepLink.ts` stores the raw URL plus a wall-clock
receive timestamp. `consumePendingDeepLink(maxAgeMs)` discards values
older than `routeChangeTimeoutMs` (default 1000ms) and clears the slot
in all cases so a stale link cannot leak onto a later, unrelated nav.
* `deeplinkIntegration` now calls `setPendingDeepLink` alongside its
existing breadcrumb. The raw URL is stored — sanitization is deferred
to attach time so `sendDefaultPii` is read at the right moment.
* `reactNavigationIntegration` consumes the pending value in two places:
- In `startIdleNavigationSpan`, covering the warm-open path (link
arrives, then navigation dispatches).
- In `updateLatestNavigationSpanWithCurrentRoute`, covering the cold
start path where `getInitialURL()` resolves *after* the initial
idle span has already been started in `afterAllSetup`.
Attachment is idempotent per span via a `deepLinkAppliedToLatestSpan`
flag, reset on discard and after each successful route change.
When attached, the span gets:
* `navigation.trigger`: `'deeplink'`
* `deeplink.url`: sanitized via the existing `sanitizeDeepLinkUrl`
(now exported from `integrations/deeplink.ts`), or raw when
`sendDefaultPii` is enabled
* `deeplink.received_at`: ms elapsed between URL receipt and the
moment the span is annotated — captures the dispatch delay
Tests cover warm open, cold-start ordering (span starts before pending
is set), single-consume semantics, PII gating, stale rejection, and the
"no deep link" baseline. Out of scope (and noted in the issue): wiring
the native TTID fallback start time to deep-link arrival instead of
navigation dispatch — that requires native-bridge plumbing.
Fixes #6159
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
Plus 4 more 🤖 This preview updates automatically when you update the PR. |
Contributor
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Addresses three findings from Cursor Bugbot and one from Sentry's review bot on #6264: * **Late deep link never tags span** (Cursor, medium). When Expo Router auto-handles `Linking.getInitialURL()` and finishes the initial navigation *before* our integration's own `getInitialURL().then(...)` chain resolves, the URL was never attributed to the resulting span. Fix: `pendingDeepLink` now supports a synchronous listener that the navigation integration registers in `afterAllSetup`. When a link arrives, the listener tags the in-flight (`latestNavigationSpan`) or most-recent still-recording (`lastIdleNavSpan`) span directly. If nothing live exists, the link falls through to the slot for the next dispatched navigation. A WeakSet guards against double-tagging. * **Same-route path skips deeplink retry** (Cursor, low). The early return in `updateLatestNavigationSpanWithCurrentRoute` for matching route keys (legitimate when deep-linking to the screen you're already on) bypassed the attribution call. Fix: consume + tag in that branch too, before the span reference is dropped. * **Early consume loses pending link** (Cursor, medium). Consuming the pending value inside `startIdleNavigationSpan` meant a later discard (noop / timeout / empty route) silently wasted the link — the real link-driven navigation that followed would not be attributed. Fix: removed the eager consume on dispatch. Pending values are now only consumed in `updateLatestNavigationSpanWithCurrentRoute` (post route mount) or by the listener (live span). Discards never touch the slot. * **`deeplink.received_at` semantically misleading** (Sentry, medium). The attribute stored a duration but its `_at` suffix conventionally denotes a timestamp. Renamed to `deeplink.dispatch_delay_ms`, which accurately reflects the measured gap between URL receipt and span annotation. Plus: changelog entry now references PR #6264 (Danger gate).
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
antonis
reviewed
Jun 9, 2026
antonis
reviewed
Jun 9, 2026
Comment on lines
+726
to
+732
| /** | ||
| * Attempts to consume the pending deep link and attach it to the given span. | ||
| * | ||
| * Returns `true` when a deep link was consumed (and the span was annotated), | ||
| * `false` otherwise. Callers may invoke this multiple times against the same | ||
| * span — once the pending value has been consumed it will not be re-applied. | ||
| */ |
Contributor
There was a problem hiding this comment.
q: is this comment referring to the applyPendingDeepLinkToSpan function below?
antonis
reviewed
Jun 9, 2026
| mockedNavigationContained.listeners['__unsafe_action__'](navigationAction); | ||
| }, | ||
| /** Dispatches a NAVIGATE action targeting "New Screen" without changing route state. */ | ||
| dispatchToNewScreen: () => { |
Contributor
There was a problem hiding this comment.
nit: though it's an oneliner we could use or call the identical emitNavigationWithoutStateChange function above
antonis
reviewed
Jun 9, 2026
antonis
left a comment
antonis
left a comment
Contributor
There was a problem hiding this comment.
Overall looks good. Left a few comments along with the agent feedback. We should also regenerate the api ref.
lucas-zimerman
left a comment
lucas-zimerman
left a comment
Collaborator
There was a problem hiding this comment.
No additional notes, worth checking the warden warnings that are not shown as comments and Antonis notes
…tion Second review round. Addresses two further Cursor Bugbot findings, one Warden bot finding, and three review nits from @antonis on #6264: * **Warm-open deep link can be consumed by a stale prior nav span** (Warden, medium). The previous design tagged `lastIdleNavSpan` (the most recently created idle nav span) unconditionally when a link arrived after `latestNavigationSpan` was cleared. For warm opens that meant tagging the *previous, unrelated* navigation span (still inside its idle window) with `navigation.trigger=deeplink`, while the actual link-triggered navigation that followed got nothing. Fix: `pendingDeepLink` now carries a `source: 'cold-start' | 'warm-open'` flag set by `deeplinkIntegration` based on which Linking API surfaced the URL (`getInitialURL()` vs `'url'` event). The retroactive attribution path in the navigation integration is gated on `source === 'cold-start'` and targets a single, frozen reference (`initialFinalizedNavSpan` — the first nav span that ever finished its state change) rather than a rolling "most recent" pointer. Warm opens always go through the pending slot and attach to the next dispatched navigation. * **Stale pending link not drained** (Cursor, medium). `applyPendingDeepLinkToSpan` previously early-returned without consuming the slot when the target span was already tagged (e.g. by the listener). An older link left in the slot would then attach to the next unrelated navigation. Fix: always consume the slot inside `applyPendingDeepLinkToSpan`. The function is now `void` and its only remaining responsibility is "drain pending, tag if appropriate". * **Late listener drops subsequent URLs** (Cursor, low). `handleLateDeepLink` returned `true` whenever a recording span was reachable, even if `tagSpanWithDeepLink` skipped tagging because the span was already annotated. The pendingDeepLink module then treated the URL as consumed and dropped it. Fix: `tagSpanWithDeepLink` now returns whether it actually tagged; the listener propagates that value so an already-tagged span falls through to the pending slot. Review nits: * Duplicate JSDoc above `sanitizeDeepLinkUrl` merged into a single block. * Orphaned JSDoc that had drifted above `taggedDeepLinkSpans` moved back onto `applyPendingDeepLinkToSpan` where it belongs. * Removed the redundant `dispatchToNewScreen` test helper — the existing `emitNavigationWithoutStateChange` does the same thing and is now paired with `completeStateChangeToNewScreen` in the cold-start ordering test. New tests: * `cold-start late arrival tags the initial finalized nav span while still recording` — covers the Expo Router auto-handled cold-start case. * `warm-open never tags the previous navigation span` — verifies Warden's bug is fixed. * `second link while a span is already tagged falls through to the pending slot` — verifies the listener no longer drops follow-up URLs. All 1558 SDK tests pass.
Addresses the final Cursor Bugbot finding on #6264: "Warm-open tags unrelated in-flight span". Scenario: the user dispatches an unrelated navigation (state change pending, `latestNavigationSpan = spanA`). While that span is mid-flight, a warm-open deep link arrives. The previous design's listener already skipped tagging `spanA` directly (cold-start gate added last round), but the slot consumer in `updateLatestNavigationSpanWithCurrentRoute` had no way to tell that the pending warm-open link did NOT cause `spanA`. The state change would then attribute the link to `spanA`, while the actual link-triggered navigation that followed got no attributes. The fix introduces causality tracking via a shared monotonic counter: * `pendingDeepLink` now stamps every recorded link with a `seq` drawn from `nextEventSeq()`. The same counter is incremented in the nav integration every time an idle nav span is created. * Each freshly-created span is stored in a `WeakMap<Span, number>` with its dispatch seq. * `applyPendingDeepLinkToSpan` peeks the slot before consuming. For warm-open links, it only attaches to a span whose dispatch seq is strictly greater than the link's seq (i.e. the span was dispatched *after* the link arrived, so it can plausibly be the link's target). Rejected warm-open links are left in the slot for the next eligible dispatch. * Cold-start links keep their unconditional retroactive attribution semantics (the whole point of that source — Expo Router auto-handles the link before our `getInitialURL()` chain resolves). `peekPendingDeepLink(maxAgeMs)` is a new export that mirrors `consumePendingDeepLink` without clearing the slot, so the consumer can inspect the seq before deciding whether to take the value. A counter beats wall-clock timestamps for this purpose because Jest's fake timers (and identical-tick events in real code) can produce identical `Date.now()` values for events that are causally ordered. The seq is monotonic across the whole module, so any pair of events has a deterministic order regardless of test timing. New test: `warm-open never tags an unrelated in-flight span (dispatched but not yet finalized)` — exercises the exact scenario Cursor flagged. All 1597 SDK tests pass.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2758a72. Configure here.
| // the link before our own `getInitialURL()` chain resolved). | ||
| if (initialFinalizedNavSpan && isSpanRecording(initialFinalizedNavSpan)) { | ||
| return tagSpanWithDeepLink(initialFinalizedNavSpan, link); | ||
| } |
There was a problem hiding this comment.
Late cold-start tags wrong span
Medium Severity
handleLateDeepLink checks latestNavigationSpan before initialFinalizedNavSpan. When getInitialURL() resolves after the initial route is finalized but a later navigation is already in flight, a cold-start launch URL is attributed to that unrelated in-flight span instead of the initial idle span, contradicting the stated rule that cold-start links must not tag later navigations.
Reviewed by Cursor Bugbot for commit 2758a72. Configure here.
| } | ||
| } | ||
| consumePendingDeepLink(maxAgeMs); | ||
| tagSpanWithDeepLink(span, pending); |
There was a problem hiding this comment.
Pending link cleared without tagging
Medium Severity
applyPendingDeepLinkToSpan calls consumePendingDeepLink before tagSpanWithDeepLink. If the span was already deeplink-tagged (for example a second cold-start URL while the first was applied via the listener), tagging is skipped but the pending entry is still cleared, so the URL is dropped and never attached to a later navigation.
Reviewed by Cursor Bugbot for commit 2758a72. Configure here.
|
|
||
| ### Features | ||
|
|
||
| - Correlate deep links with the navigation transaction they trigger. The next idle navigation span started within `routeChangeTimeoutMs` of a deep link arrival is tagged with `navigation.trigger: 'deeplink'`, `deeplink.url` (sanitized, respects `sendDefaultPii`), and `deeplink.dispatch_delay_ms` (ms gap between URL received and navigation dispatched). Covers both cold start (`Linking.getInitialURL()`) and warm open (`'url'` event) paths, including the late-arrival case where Expo Router auto-handles the link before our `getInitialURL()` chain resolves ([#6264](https://github.com/getsentry/sentry-react-native/pull/6264)) |
Collaborator
There was a problem hiding this comment.
What do you think of shorting the changelog? Users can see more details about it when opening the PR URL.
Suggested change
| - Correlate deep links with the navigation transaction they trigger. The next idle navigation span started within `routeChangeTimeoutMs` of a deep link arrival is tagged with `navigation.trigger: 'deeplink'`, `deeplink.url` (sanitized, respects `sendDefaultPii`), and `deeplink.dispatch_delay_ms` (ms gap between URL received and navigation dispatched). Covers both cold start (`Linking.getInitialURL()`) and warm open (`'url'` event) paths, including the late-arrival case where Expo Router auto-handles the link before our `getInitialURL()` chain resolves ([#6264](https://github.com/getsentry/sentry-react-native/pull/6264)) | |
| - Correlate deep links with the navigation they trigger, tagging the resulting transaction with `navigation.trigger: 'deeplink'`, the deep | |
| link URL (sanitized unless `sendDefaultPii` is enabled), and the time between link arrival and navigation. Works for both cold start and | |
| warm app launches ([#6264](https://github.com/getsentry/sentry-react-native/pull/6264)) |
Comment on lines
+114
to
+117
| pending = undefined; | ||
| if (!value) { | ||
| return undefined; | ||
| } |
Collaborator
There was a problem hiding this comment.
What about clearing pending only when it is defined?
Suggested change
| pending = undefined; | |
| if (!value) { | |
| return undefined; | |
| } | |
| if (!value) { | |
| return undefined; | |
| } | |
| pending = undefined; |
lucas-zimerman
approved these changes
Jun 10, 2026
lucas-zimerman
left a comment
lucas-zimerman
left a comment
Collaborator
There was a problem hiding this comment.
Looking good! I left a few nonblocking suggestions, thank you for the PR!
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
3 participants
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
Closes #6159 by linking deep-link arrival to the navigation transaction it triggers. Previously the deeplink breadcrumb and the React Navigation idle span lived on disconnected timelines — this PR bridges them so traces can show the full "URL received → navigation dispatched → screen mounted" story.
How it works
routeChangeTimeoutMs(default 1000ms) are dropped, and consumption is single-shot so a stale link can't leak onto a later, unrelated navigation.📝 Checklist
sendDefaultPIIis enabled🔮 Next steps