8000 fix(sveltekit): Ensure navigations and redirects always create a new transaction by Lms24 · Pull Request #10656 · getsentry/sentry-javascript · GitHub
[go: up one dir, main page]
Skip to content

fix(sveltekit): Ensure navigations and redirects always create a new transaction #10656

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Feb 15, 2024
Merged

fix(sveltekit): Ensure navigations and redirects always create a new transaction #10656

merged 6 commits into from
Feb 15, 2024

Conversation

Lms24
Copy link
Member
@Lms24 Lms24 commented Feb 14, 2024 •
edited
Loading

Similarly to #10646, this PR adjusts the root span/transaction creation behaviour of the SvelteKit router instrumentation:

  • We no longer check for an active span to create a new navigation span
    • This is possible without any pageloadOngoing flag because, luckily, pageload and navigation instrumentation is completely decoupled
  • Consequently, navigations happening directly after the first pageload will now create their own transaction instead of being attached to the pageload transaction
  • Consequently, redirects within a (user perceived) navigation are now treated as separate transactions, where the "current" navigation finishes the transaction of the previous redirect.

Additional changes:

  • Removed tag for routing instrumentation (we now have sentry.origin for this anyway)
  • Added more navigation info to the spans:
    • sentry.sveltekit.navigation.type: indicates the source of the navigation (link click, back/forward button press, goto/redirect call, etc)
    • sentry.sveltekit.navigation.from: parameterized navigation start
    • sentry.sveltekit.navigation.to: parameterized navigation end

Added e2e tests to cover new scenarios and additional data.

closes #10634

Lms24
Lms24 commented Feb 14, 2024
View reviewed changes
packages/sveltekit/src/client/browserTracingIntegration.ts
Comment on lines +80 to +82
if (!pageloadSpan) {
return;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

we can just early return here if we don't create a pageload span. No need to subscribe to the page store to update the name then.

@Lms24 Lms24 marked this pull request as ready for review February 14, 2024 15:50
@Lms24 Lms24 mentioned this pull request Feb 14, 2024
Closed
2 tasks
@Lms24 @mydea
Update dev-packages/e2e-tests/test-applications/sveltekit-2/src/route…
f1a85bd 
…s/+layout.svelte

Co-authored-by: Francesco Novy <francesco.novy@sentry.io>
@github-actions GitHub Actions
Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 77.82 KB (0%)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 69.06 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas) - Webpack (gzipped) 72.98 KB (0%)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 62.67 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 33.25 KB (0%)
@sentry/browser (incl. browserTracingIntegration) - Webpack (gzipped) 33.15 KB (0%)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 31.16 KB (0%)
@sentry/browser (incl. sendFeedback) - Webpack (gzipped) 31.16 KB (0%)
@sentry/browser - Webpack (gzipped) 22.43 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 76.11 KB (+0.02% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 67.64 KB (+0.06% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 33.6 KB (+0.05% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped) 24.71 KB (+0.06% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 212.88 KB (+0.04% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 101.5 KB (+0.07% 🔺)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 74.01 KB (+0.09% 🔺)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 36.83 KB (+0.04% 🔺)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 69.43 KB (0%)
@sentry/react - Webpack (gzipped) 22.45 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 87.19 KB (+0.11% 🔺)
@sentry/nextjs Client - Webpack (gzipped) 50.36 KB (+0.01% 🔺)
@sentry-internal/feedback - Webpack (gzipped) 17.12 KB (0%)

@Lms24 Lms24 merged commit 05bb609 into develop Feb 15, 2024
@Lms24 Lms24 deleted the lms/fix-sveltekit-navgigation-transactions branch February 15, 2024 10:14
@RChutchev RChutchev mentioned this pull request Oct 8, 2024
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mydea mydea mydea approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ensure Angular & Sveltekit browser tracing does not check getActiveSpan()
2 participants
@Lms24 @mydea
0