Skip to content

fix(sveltekit): Handle nested server calls in sentryHandle - #7598

Merged
Lms24 merged 3 commits into
developfrom
lms/sveltekit-fix-sentryHandle-domains
Mar 29, 2023
Merged

fix(sveltekit): Handle nested server calls in sentryHandle#7598
Lms24 merged 3 commits into
developfrom
lms/sveltekit-fix-sentryHandle-domains

Conversation

@Lms24

@Lms24Lms24 commented Mar 23, 2023

Copy link
Copy Markdown
Member

Previously, we always created a new domain when entering the sentryHandle function. This caused "nested" SvelteKit Server calls to create new transaction whenever a new domain was created in the sentryHandle function because the active span is bound to the domain. This PR introduces a check if there is an active transaction, in which case we just execute the handler and don't create a new domain, as we are already in this active domain.

As a result, nested SK server calls become a span of the parent server call instead of a new transaction:

image

ref #7526

@Lms24
Lms24 requested a review from AbhiPrasadMarch 23, 2023 16:09
@Lms24Lms24 self-assigned this Mar 23, 2023
@Lms24
Lms24force-pushed the lms/sveltekit-fix-sentryHandle-domains branch 2 times, most recently from 7ff774a to 8c0762eCompareMarch 29, 2023 08:55
@Lms24
Lms24 marked this pull request as ready for review March 29, 2023 08:57
@Lms24
Lms24force-pushed the lms/sveltekit-fix-sentryHandle-domains branch from 8c0762e to 6bc071eCompareMarch 29, 2023 08:57
@Lms24Lms24 changed the title fix(sveltekit): Check for active domains in sentryHandlefix(sveltekit): Handle nested server calls in sentryHandleMar 29, 2023
@github-actions

github-actionsBot commented Mar 29, 2023

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
@sentry/browser - ES5 CDN Bundle (gzipped + minified)20.62 KB (+0.06% 🔺)
@sentry/browser - ES5 CDN Bundle (minified)64.4 KB (+0.02% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped + minified)19.16 KB (-0.02% 🔽)
@sentry/browser - ES6 CDN Bundle (minified)56.78 KB (+0.02% 🔺)
@sentry/browser - Webpack (gzipped + minified)21.53 KB (-0.39% 🔽)
@sentry/browser - Webpack (minified)72 KB (-0.04% 🔽)
@sentry/react - Webpack (gzipped + minified)21.55 KB (-0.39% 🔽)
@sentry/nextjs Client - Webpack (gzipped + minified)51.98 KB (-0.01% 🔽)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified)28.16 KB (+0.06% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified)26.36 KB (+0.06% 🔺)
@sentry/replay ES6 CDN Bundle (gzipped + minified)44.74 KB (+0.62% 🔺)
@sentry/replay - Webpack (gzipped + minified)38.86 KB (+0.83% 🔺)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified)63.41 KB (+0.46% 🔺)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified)56.49 KB (+0.5% 🔺)

@Lms24
Lms24 merged commit 773f180 into developMar 29, 2023
@Lms24
Lms24 deleted the lms/sveltekit-fix-sentryHandle-domains branch March 29, 2023 10:35
Sign up for freeto 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.

2 participants

@Lms24@AbhiPrasad