Skip to content

feat(browser): Add beforeStartNavigationSpan lifecycle hook #16863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 10, 2025

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Jul 9, 2025

Also affects: @sentry/core

This PR adds a new beforeStartNavigationSpan lifecycle hook that gets triggered directly before the startNavigationSpan hook by the startBrowserTracingNavigationSpan helper function. The reason we need this is because we have logic for standalone CLS and LCP spans that needs to run before we start preparing any work that's executed at the startNavigationSpan hook:

  • beforeStartNavigationSpan: send standalone span with the pageload trace id (i.e. the currently still active trace id on the propagation context
  • startNavigationSpan: first recycles the propagation context, then starts the new span

In a follow-up PR, I'll switch LCP and CLS standalone span sending over to this event rather than startNavigationSpan which previously caused these spans to be added to the new navigation trace id rather than to the pageload trace id.

@Lms24 Lms24 self-assigned this Jul 9, 2025
@Lms24 Lms24 requested review from mydea and s1gr1d July 9, 2025 15:00
Copy link
Contributor

github-actions bot commented Jul 9, 2025

size-limit report 📦

Path Size % Change Change
@sentry/browser 23.81 kB - -
@sentry/browser - with treeshaking flags 22.53 kB - -
@sentry/browser (incl. Tracing) 39.66 kB +0.05% +17 B 🔺
@sentry/browser (incl. Tracing, Replay) 77.79 kB +0.03% +23 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 67.73 kB +0.04% +21 B 🔺
@sentry/browser (incl. Tracing, Replay with Canvas) 82.49 kB +0.03% +23 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback) 94.61 kB +0.03% +22 B 🔺
@sentry/browser (incl. Feedback) 40.48 kB - -
@sentry/browser (incl. sendFeedback) 28.49 kB - -
@sentry/browser (incl. FeedbackAsync) 33.38 kB - -
@sentry/react 25.58 kB - -
@sentry/react (incl. Tracing) 41.64 kB +0.05% +18 B 🔺
@sentry/vue 28.26 kB - -
@sentry/vue (incl. Tracing) 41.46 kB +0.07% +26 B 🔺
@sentry/svelte 23.83 kB - -
CDN Bundle 25.36 kB - -
CDN Bundle (incl. Tracing) 39.63 kB +0.04% +13 B 🔺
CDN Bundle (incl. Tracing, Replay) 75.57 kB +0.03% +17 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) 81.02 kB +0.03% +18 B 🔺
CDN Bundle - uncompressed 74.08 kB - -
CDN Bundle (incl. Tracing) - uncompressed 117.69 kB +0.08% +93 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed 231.61 kB +0.05% +93 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 244.42 kB +0.04% +93 B 🔺
@sentry/nextjs (client) 43.28 kB +0.05% +20 B 🔺
@sentry/sveltekit (client) 40.11 kB +0.05% +18 B 🔺
@sentry/node 167.85 kB -0.01% -1 B 🔽
@sentry/node - without tracing 100.59 kB - -
@sentry/aws-serverless 128.69 kB - -

View base workflow run

@mydea
Copy link
Member

mydea commented Jul 10, 2025

no strong feelings, but imho this can also be feat(browser) as this is what's really affect by this :)

@Lms24 Lms24 changed the title feat(core): Add beforeStartNavigationSpan lifecycle hook feat(browser): Add beforeStartNavigationSpan lifecycle hook Jul 10, 2025
@Lms24 Lms24 force-pushed the lms/feat-core-beforeStartNavigationSpan branch from 768d7ce to ed3c762 Compare July 10, 2025 08:32
cursor[bot]

This comment was marked as outdated.

@Lms24 Lms24 enabled auto-merge (squash) July 10, 2025 11:05
@Lms24 Lms24 merged commit 1864b22 into develop Jul 10, 2025
168 checks passed
@Lms24 Lms24 deleted the lms/feat-core-beforeStartNavigationSpan branch July 10, 2025 11:06
Lms24 added a commit that referenced this pull request Jul 10, 2025
…eload span (#16864)

Fix a bug in standalone web vital spans where, if they were
sent because of a navigation, they would incorrectly be added to the
`navigation` span's trace instead of the initial `pageload` span's
trace. This surfaced while dogfooding these spans on sentry and
inspecting them in the EAP traceview.

To fix this, I added a lifecycle hook in #16863 that triggers right
before we start preparing the navigation span and hence, right before we
recycle the propagation context to the new traceId. This patch now makes
use of the new hook for CLS and LCP spans and it also makes the
integration tests stricter to check for the correct trace ids.
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.

3 participants