-
-
Notifications
You must be signed in to change notification settings - Fork 76
feat(sveltekit): Add support for SDK setup with instrumentation.server.ts
#1077
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
Conversation
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1077 +/- ##
==========================================
+ Coverage 31.53% 32.06% +0.52%
==========================================
Files 129 133 +4
Lines 15169 15638 +469
Branches 1032 1093 +61
==========================================
+ Hits 4784 5014 +230
- Misses 10368 10607 +239
Partials 17 17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Hmm I really dislike that our e2e tests don't contribute to our coverage calculation. Maybe we can find some way to run them within the same process as the test. Might be a nice addition for vitest and child process coverage doesn't seem to go anywhere (vitest-dev/vitest#2911) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to rename the sveltekit
test app to sveltekit-hooks
to avoid ambiguity with sveltekit-tracing
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decided to split this file up since there's quite a lot going on in it. Hence the various new files in the sdk-setup
dir
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice!
Co-authored-by: Andrei <168741329+andreiborza@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
This PR adds support for setting up the SDK with SvelteKit's new
instrumentation.server.ts
instrumentation file, as well as by leveraging the new spans emitted from kit. This requires a SvelteKit version of at least 2.31.0. Of course we want to stay backwards compatible, so this PR adds the following flow:We check for version from
package.json
(I'm, this is flawed and could mean we identify a version <2.31.0 when in fact it is already used. But then again, if not explicitly set in package.json, we can't rely on >= 2.31.0 being used). If we identify <2.31.0, we recommend updating to the newest version.From this point, there are 3 options for the user:
If we detect a version >=2.31.0, we don't ask anything but directly continue with the new setup.
For the old setup, nothing changes.
For the new setup, we now apply the following changes:
svelte.config.js
Sentry.init
intoinstrumentation.server.mjs
hooks.server.ts
hooks.client.ts
(So to clear up any confusion: We still make use of hooks files but the server-side SDK init got extracted to the instrumentaiton file)
Additionally, added some tests and a new e2e test.
closes #1075