Skip to content
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

Provide ability to instrument preload and load #10758

Open
pzuraq opened this issue Sep 20, 2023 · 0 comments
Open

Provide ability to instrument preload and load #10758

pzuraq opened this issue Sep 20, 2023 · 0 comments
Labels
needs-decision Not sure if we want to do this yet, also design work needed

Comments

@pzuraq
Copy link
Contributor

pzuraq commented Sep 20, 2023

Describe the problem

We are currently trying to setup up client-side instrumentation for our application using OpenTelemetry. For context, Otel generalizes a notion of a span (think like the callstack span you might see in the JS debugger) and allows you to create custom spans that can contain other spans across services. So in the browser, for instance, it is common to wrap fetch in a function that creates a span and passes it along to the server, so you can see all the different things that fetch request triggered and handled.

Instrumenting fetch on the client is it's own can of worms that's being discussed in #9530 and #10009, and those would help a lot with allowing apps to do this kind of instrumentation. However, there is another issue we've been running into, specifically around the load hooks of pages and the preloading feature.

Ideally, we would create a span to wrap each load hook that is called. This way we can see the collection of load requests that went into every single page's render, and also gather metrics such as duration. We started building a solution to this based on onNavigate (which is not ideal because it's tied to a component lifecycle rather than being global, but that's a separate issue), but we found that our spans created in onNavigate were never being associated with the fetch requests that the load function was making. After some head scratching, we realized this was due to the preloading functionality SvelteKit exposes.

When the user hovers over a link, SvelteKit starts preloading the data for the route it is going to by default. This is great behavior for optimizing UX out of the box, but currently there doesn't seem to be a way to instrument this behavior and group it together logically into a single notion of a "page load". This means we can't accurately track the impact it has on users, whether or not prefetching is erroring, and what the overall performance of these preload calls is.

We considered disabling preloading altogether, but there doesn't appear to be a way to do this globally in an app at the moment. Moreover, the issue is not with the functionality itself, but with our inability to observe it.

Describe the proposed solution

I think this could be handled a few different ways:

  1. Add handle to client side hooks, which would run for all client side load calls. This feels like it would be ideal from an API perspective if it would work, because then you are just conceptually mirroring the server functionality, but I could see this being tricky for a number of reasons on the client (different signatures, different notion of what an "event" is, etc).
  2. Add client-side handleLoad and/or handlePreload hooks, or alternatively handleNavigate (though, like onNavigate, that would imply it only runs on actual user navigation). This would be a more targeted version of adding hooks, which means the API is less generalized and not as much of a commitment.
  3. Add an onPreload function to the navigation module, which allows us to hook into the preload event much like a navigation event. This seems less ideal to me because preloading doesn't really seem to relate to a component as much, but it would solve the issue for us.

Alternatives considered

We've considered wrapping every single load hook with an instrumentLoad helper function, but this has a number of issues:

  1. It makes type annotations a pain to write overall, a lot more boilerplate per module
  2. Mistakes were very common, devs would forget to add the instrumentation all the time
  3. It runs on the server as well, where we don't have this issue because we can wrap all events in a request in a span using the handle hook

Importance

i cannot use SvelteKit without it

Additional Information

Long term we really need this functionality, I'm exploring ways I could have our build-system add instrumentLoad helpers to each page load via a Vite plugin but ideally this would be pluggable within the framework.

Also, happy to implement this if it is accepted.

@eltigerchino eltigerchino added the needs-decision Not sure if we want to do this yet, also design work needed label Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Not sure if we want to do this yet, also design work needed
Projects
None yet
Development

No branches or pull requests

2 participants