-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Description
Describe the problem
TL;DR: We (Sentry) want to instrument client-side fetch requests and we currently cannot instrument some of them. We propose a handleFetch hook on the client side as a solution.
Hi, I'm an SDK engineer at Sentry and we're currently working on a dedicated SvelteKit SDK. Things are progressing nicely but we just discovered a potential problem:
Based on my findings, SvelteKit stores window.fetch in the native_fetch variable, monkey patches it and only uses this one for client-side fetch requests:
| export const native_fetch = window.fetch; |
This is problematic for our use case because it means that our SDK, which we'll most probably instruct users to initialize in the client and server hooks files comes in too late to apply our fetch instrumentation to window.fetch.
We already wrap load functions to create spans for our performance monitoring product. So we have a way to instrument the fetch function that is passed to the client-side universal load function in the LoadEvent.
However, for a page that has a server-only load function, Kit makes a route/path/__data.json fetch request to invoke this server-only load function. Up to this point, we haven't found a way to instrument this fetch call. We would really like to instrument the call though, so that we can attach our Http headers for distributed tracing (see Additinal Information below). Without this, we cannot connect the client side with the server side and our users don't get the full picture what happened during a client-side navigation.
Please find our proposed solution(s) below. We're more than happy to open a PR or to work with you in any way to help finding a good solution here. Also, if it happens that we missed something, please let us know :)
Describe the proposed solution
To solve this, we propose to add support for a handleFetch hook in the hooks.client.js file. This hook would be invoked every time a fetch call is made on the client side, be it by a user calling fetch inside a load function as well as by the framework, when it makes a fetch call to invoke a server-only load function.
We believe this would be the cleanest approach to open up Kit's fetch to SDKs like ours. It would also make it easier for Kit users to e.g. attach custom Http headers or other data to outgoing fetch requests.
Since I've already spent some time reading Kit source code and I already played around with hooks, I'm more than happy to open a PR for this change :)
Alternatives considered
We have considered a few alternatives that we believe would also work but are potentially less clean or might require more SvelteKit-internal changes:
- Make
native_fetch(or betterinitial_fetchandsubsequent_fetch) patchable for SDKs in another way than via a client-sidehandleFetchhook. We're definitely open to other suggestions but right now, everything we hook into in SvelteKit would reside in the hooks files which means great DX for our users as hooks seem to be the most SvelteKit-native way. - SvelteKit could monkey-patch
window.fetchdirectly without storing it away in variables. This way, our instrumentation can wrap the correctfetchwithout any additional code on our end. Given how Kit currently handles fetch, I believe this is unlikely to happen but it would nevertheless be an option. - If there's no way how we can instrument all
fetchcalls during runtime, we'll need to inject some code at build-time, probably via a Vite plugin to be able to accessnative_fetchto instrument it. We've done this for other framework SDKs before but would like to avoid it if at all possible. It's a rather brittle solution and has caused a lot of bugs in said framework SDKs in the past. That being said, we'll go down this route if there's no other way.
Importance
would make my life easier; very important for SvelteKit Sentry users
Additional Information
A little more background on why we need to instrument fetch:
- Sentry SDKs add distributed tracing headers (our proprietary
sentry-traceas well as the W3C standardizedbaggageheaders) to outgoing fetch (and XHR) requests. With the help of these headers we can connect transactions and spans in our performance monitoring product, giving our users the full picture of what happened on the client and on the server (+ additional backend services). - Sentry SDKs collect "breadcrumbs" for
fetchrequests to give our users clues what happened e.g. before an error was reported to Sentry.
Thank you for considering this issue!