-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(svelte): Add Component Tracking #5612
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
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.
Preprocessor functions should additionally return a map object alongside code and dependencies, where map is a sourcemap representing the transformation.
If this is the case, we might need to use something a library to do this processing, and pass down the emitted sourcemap. For example: https://github.com/sveltejs/svelte-preprocess/blob/main/src/transformers/babel.ts
packages/svelte/src/performance.ts
Outdated
options.trackMount && recordMountSpan(transaction, componentName); | ||
options.trackUpdates && recordUpdateSpans(componentName); |
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.
nit: let's make these explicit if statements.
Is there a way we can make the update spans children of the mount spans?
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.
Is there a way we can make the update spans children of the mount spans?
You mean for the times when update is called directly after onMount?
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.
Oh good point, thanks for catching that! Makes a lot of sense. I think I'll try out MagicString since we use it already in the Unplugin |
apply code review suggestions fix linter errors
size-limit report 📦
|
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.
Looking good!
packages/svelte/src/preprocessors.ts
Outdated
} | ||
return true; | ||
} | ||
function getComponentName(filename: string): string { |
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.
There is a node api function that does exactly what this function is doing: path.basename(filename, '.svelte')
. No need to change this though.
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.
Well well well, the more you know 😅
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.
So I tried replacing my function with path
but my Svelte app won't build because I'm using a Node API. There's probably a way to get around this but for now, I'll leave it as is. We can revisit this, if we ever get problems with that function
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.
IMO we can just rename getComponentName
to reflect what it's doing (getting the basename) and not worry about using the path API.
packages/svelte/src/performance.ts
Outdated
// If we are mounting the component when the update span is started, we start it as child | ||
// of the mount span. Else, we start it as a child of the transaction. |
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.
Could you elaborate a bit why the update span is a child of the component's mount span? Without having thought about it too much it doesn't click for me but maybe there's a reason.
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.
So, tbh this is totally up for discussion but the reason why I think it makes sense to make the update a child span of the mount span is because in the component lifecycle of Svelte, the beforeUpdate
hook will be called before the mounting of the component is finished. Meaning, the first update of a component is part of its initial initialization. So overall, I think it makes sense to let this update (and only this one) be a child of the mount span.
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.
Ah ok. The way you explained this makes a lot of sense. I'd keep it too then!
packages/svelte/src/constants.ts
Outdated
// TODO: we might want to call this ui.svelte.init instead because | ||
// it doesn't only track mounting time (there's no before-/afterMount) | ||
// but component init to mount time. | ||
export const UI_SVELTE_MOUNT = 'ui.svelte.mount'; |
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 am always in favor of calling things the way users are familiar with. If "initializing" is a term that is used in the svelte community for mounting components, I vote we call it ui.svelte.init
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.
My train of thought for maybe renaming this to init instead of mount is that this span does not only track mounting time. Because the onMount
hook is the only lifecycle hook we have for mounting, we only know when mounting is completed but not when it is started. What we do know, however, is when we start initializing the component (i.e. when the <script> block of a component is executed). So this span tracks exactly this duration: From the beginning of script execution until the component is completely mounted in the DOM. Which technically makes this more an initialization span than a mounting span.
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 renamed it to init. Makes more sense IMO. Let's go with that
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.
IMO nothing major blocking the merging of this. Looking good!
Reminder - we’ll need to add a docs for this feature upon release! |
Currently on it ;) |
Summary
This PR introduces component tracking to the Sentry Svelte SDK. I'm currently trying to implement an approach for users that is simple and only requires minimal configuration on their end. The idea is to let them specify once if and which components they actually want to track (like we do it in Vue for example).
Because Svelte components are compiled into plain JS at build time, we cannot reliably hook into a Svelte runtime or do similar approaches monkey patching approaches. What we can however do, is to hook into the component compilation by using an official Svelte compiler feature: preprocessors.
With this PR, we add a Svelte preprocessor that injects a function call (+ the necessary import) into the top of the
<script>
part of Svelte components. The called function takes care creating the lifecycle spans and putting them onto a transaction.Usage
In your
svelte.config.js
, import and add the SentrycomponentTrackingPreprocessor
:Under the Hood
In the compiled JS (after the Svelte compiler is done), this function call translates to the following code for
Component1.svelte
:Alternative Usage
If you don't want to use our Preprocessor to automatically track components, you can call our tracking function in every component you would like to see tracked yourself:
Result
This is the result of the PR:

Compatibility with SvelteKit
If users are using the Svelte SDK in a SvelteKit app, they can use this preprocessor (or make the manual function call). I tested component tracking with the Realworld SvelteKit app and it seems to work normally, as long as the SDK is only initialized on the browser end (which is how it is supposed to be used, if at all). There might be weird edge cases but generally, the lifecycle hooks are called on the client-side and the spans are recorded correctly. We can revisit this, if we get bug reports.
To Do