Skip to content

fix(tracing): Discard invalid navigation transactions via event processor#6051

Merged
alwx merged 4 commits intomainfrom
alwx/improvement/4431
Apr 28, 2026
Merged

fix(tracing): Discard invalid navigation transactions via event processor#6051
alwx merged 4 commits intomainfrom
alwx/improvement/4431

Conversation

@alwx
Copy link
Copy Markdown
Contributor

@alwx alwx commented Apr 27, 2026

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Fixes #4431

Our SDK was previously discarding the "invalid" root spans by changing _sampled to false which was a bit of a hack. This PR switches to proper handling instead with the following changes:

  • The new root span with the new sentry.rn.discard_reason attribute;
  • The reactNativeTracingIntegration event processor now filters out any transaction event with this attribute, returning null and calling recordDroppedEvent('event_processor', 'transaction').
  • The app start integration now checks for the discard marker instead of !spanIsSampled, so app start data still attaches to the next real transaction after a discarded one.

💚 How did you test it?

A few tests has been added.

📝 Checklist

  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

🔮 Next steps

alwx added 2 commits April 27, 2026 11:25
…ssor

Previously, the SDK discarded "invalid" navigation/interaction root spans
(empty back-navigations, route-change transactions that never received route
info, childless idle transactions, and the latest navigation span on RN
Navigation / React Navigation timeouts) by mutating the private `_sampled`
flag on the SentrySpan. This caused @sentry/core to treat the transactions
as sampled-out, producing misleading "dropped due to sampling" debug logs
and incorrect client-report reasons even when `tracesSampleRate=1.0`.

Switch to a proper event-processor-based discard:

- Mark the root span with a new `sentry.rn.discard_reason` attribute
  (`empty_back_navigation`, `no_route_info`, `no_child_spans`, or
  `discarded_latest_navigation`) and end it normally with its real sampling
  decision.
- The `reactNativeTracingIntegration` event processor now filters out any
  transaction event carrying that attribute, returning `null` and reporting
  `recordDroppedEvent('event_processor', 'transaction')`.
- The app start integration's lock-reset check looks for the discard marker
  instead of `!spanIsSampled`, so app start data still attaches to the next
  real transaction after a discarded one.

Tests cover the four discard sites, the event processor (drop / pass /
non-transaction passthrough), and update the existing `_sampled`-based
expectations to assert the new attribute and that the sampling flag is
preserved.

Refs: #4431
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 27, 2026

Fails
🚫 Pull request is not ready for merge, please add the "ready-to-merge" label to the pull request

Generated by 🚫 dangerJS against 2799050

@alwx alwx marked this pull request as ready for review April 27, 2026 09:44
Comment thread packages/core/src/js/tracing/reactnativetracing.ts
@alwx alwx self-assigned this Apr 27, 2026
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit da56af8. Configure here.

Comment thread packages/core/src/js/tracing/reactnativetracing.ts
Comment thread packages/core/src/js/tracing/reactnativetracing.ts Outdated
Comment thread packages/core/src/js/tracing/onSpanEndUtils.ts Outdated
Copy link
Copy Markdown
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your work on this! Overall looks good. I've left some comments along with the cursor findings.

Strengthen types (SentryDiscardReason, EventHint), remove double-counting recordDroppedEvent call (core already records event_processor drops automatically when processor returns null), and fix lost app-start data when the first transaction is discarded by skipping app-start attach when the discard marker is present.
@alwx alwx requested a review from antonis April 28, 2026 11:53
Copy link
Copy Markdown
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@alwx alwx merged commit 99a0692 into main Apr 28, 2026
56 of 62 checks passed
@alwx alwx deleted the alwx/improvement/4431 branch April 28, 2026 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ReactNativation] [ExpoRouter] Discard invalid navigation and interaction root spans using event processor instead sampled=false

2 participants