Make SvelteKit SDK use Vercel Edge runtime SDK

AbhiPrasad commented 12 months ago

Just like Next.js, make the SvelteKit SDK detect and use the Vercel Edge runtime SDK if need be.

isaacharrisholt commented 12 months ago

Hey again! Is there any way I can contribute to this?

AbhiPrasad commented 12 months ago

Hey @isaacharrisholt - for sure. Sorry we gave you a false ok for the last time, we had to figure stuff out technically on our side. Now we are in a better place with the @sentry/vercel-edge package, so we can just add it to the SvelteKit SDK.

This is a little tricky because we use a multiplexer approach via a webpack plugin in nextjs so that the SDK automatically switches between using the edge or node sdk. @lforst any tips on the SvelteKit SDK?

We can probably use a similar strategy to sveltekit adapter auto to decide between using @sentry/node and @sentry/vercel-edge https://github.com/sveltejs/kit/tree/master/packages/adapter-auto if the multiplexer is too complex, maybe a good first step.

lforst commented 12 months ago

Hacking this in is going to be hard. I suggest waiting for official support from our SDK. This is likely gonna happen near- to medium term.

knd775 commented 11 months ago

Hacking this in is going to be hard. I suggest waiting for official support from our SDK. This is likely gonna happen near- to medium term.

Don't mean to nag, but what do near and medium term mean to you? We're likely going to have to move away from sentry due to the lack of edge support, but I want to make sure we don't do that if this is right around the corner.

lforst commented 11 months ago

Don't mean to nag, but what do near and medium term mean to you? We're likely going to have to move away from sentry due to the lack of edge support, but I want to make sure we don't do that if this is right around the corner.

@knd775 1 month to 3 or so? Sveltekit is one of our smaller SDKs and the intersection of Sveltekit SDK users and Vercel Edge users is even smaller. It does not make sense for us to put this at a higher priority.

PRs are always welcome. The building blocks should all be there since we now have the @sentry/vercel-edge package which you could probably even use in its current form. Note that it is in alpha and doesn't contain any docs yet.

madeleineostoja commented 11 months ago

thanks for the transparency @lforst, not ideal news but appreciate knowing the rough roadmap

smart commented 10 months ago

Just adding a +1 to this as another customer who would like to use sveltekit and edge on vercel.

isaacharrisholt commented 10 months ago

Just out of curiosity, is there anything about the edge runtime that makes it incompatible with Node?

AbhiPrasad commented 10 months ago

We rely on Node APIs to gather metadata and instrument things node specific standard libraries like http for breadcrumbs and spans. This doesn't work in Vercel's edge runtime (or other non-node similar runtimes like cloudflare), so we need a separate SDK for this.

Sentry specifically, the vercel edge runtime also has way worse stacktraces. Due to sandboxing and bundling, you'll get an error that looks like below - the equivalent Node.js error will be a lot more detailed.

Error: Edge Stack Traces
    at (vc/edge/function:122:5661)
    at (vc/edge/function:125:10587)
    at (vc/edge/function:122:5622)
    at (vc/edge/function:147:11182)
    at (vc/edge/function:147:9157)
    at (vc/edge/function:147:11007)

isaacharrisholt commented 10 months ago

Sentry specifically, the vercel edge runtime also has way worse stacktraces. Due to sandboxing and bundling, you'll get an error that looks like below - the equivalent Node.js error will be a lot more detailed.

Hmm, interesting! I wonder if Vercel will add an option to generate sourcemaps...

lforst commented 10 months ago

Hmm, interesting! I wonder if Vercel will add an option to generate sourcemaps...

We are in talks with them but progress is rather slow. We have this issue open: https://github.com/vercel/vercel/issues/10829 Feel free to 👍 it. I think it would help gauge interest in this.

isaacharrisholt commented 10 months ago

We rely on Node APIs to gather metadata and instrument things node specific standard libraries like http for breadcrumbs and spans. This doesn't work in Vercel's edge runtime (or other non-node similar runtimes like cloudflare), so we need a separate SDK for this.

Having re-read this, I guess my question is this: is there anything in the new SDK that wouldn't be Node-compatible (assuming it's feature-complete)? Basically, what's stopping the Edge SDK being used by default?

We are in talks with them but progress is rather slow. We have this issue open: https://github.com/vercel/vercel/issues/10829 Feel free to 👍 it. I think it would help gauge interest in this.

Done!

lforst commented 10 months ago

Having re-read this, I guess my question is this: is there anything in the new SDK that wouldn't be Node-compatible (assuming it's feature-complete)? Basically, what's stopping the Edge SDK being used by default?

While it would be possible to use the Edge (WinterCG) SDK per default, we would miss out on a ton of instrumentation inside the Node.js Runtime because the Edge SDK isn't aware of all the Node APIs (e.g. http.get, http.request, node fetch). This is the main concern here.

EricAndresen commented 8 months ago

Any update on this? https://github.com/vercel/vercel/issues/10829 doesn't look like it's gotten much traction, so I'm assuming this is in the same state. Would love to see edge function support, it's enabling some really cool stuff for our app 💪 . Thanks! The fact that Sveltekit support is this good is amazing.

lforst commented 8 months ago

@EricAndresen No updates. You would see it here.

jstjoe commented 3 months ago

I've seen some mentions elsewhere that vercel edge support is coming? Right now as someone building on Vercel I face the choice between:

staying on Node.js and sacrificing performance and user experience for my global users
removing Sentry entirely (unless there's another middle ground?)

I'm currently leaning toward removing Sentry, as much as I love it, because it's not really a need-to-have with Vercel's built in logs and other tools. 😕 I really like Sentry but this is really slowing me down.

AbhiPrasad commented 3 months ago

@jstjoe I think Vercel themselves are stepping away from the edge runtime (see tweet by Lee Robinson) so this is less of a priority for us.

For now you can use @sentry/vercel-edge instead of @sentry/sveltekit on server-side, that'll make it work with the vercel edge runtime. Please note that the vercel edge runtime does not have amazing stacktraces, this is due to a limitation with how the edge runtime is built for Vercel: https://github.com/getsentry/sentry-javascript/issues/6805

knd775 commented 3 months ago

staying on Node.js and sacrificing performance and user experience for my global users

Unless you have DB replicas near the users as well, this is not actually how it plays out in real life.

jstjoe commented 3 months ago

👍 I do have DB near the users. And a lot of stuff that doesn’t require a DB roundtrip as well.

On Thu, Jun 20, 2024 at 11:06 Ben Ayles @.***> wrote:

staying on Node.js and sacrificing performance and user experience for my global users

Unless you have DB replicas near the users as well, this is not actually how it plays out in real life.

— Reply to this email directly, view it on GitHub https://github.com/getsentry/sentry-javascript/issues/9107#issuecomment-2181056861, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPMC5BQDPTSAPU5SMOK5P3ZIL4YNAVCNFSM6AAAAAA5GH4UKWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBRGA2TMOBWGE . You are receiving this because you were mentioned.Message ID: @.***>

jstjoe commented 3 months ago

@AbhiPrasad they aren't ending support for edge, that tweet was (very confusingly) about how Vercel uses Vercel for their own apps. See this clarification: https://x.com/leeerob/status/1780710814560960759?s=46

ETA: He's just saying it's not universally better, and that it's not the best choice for some apps including the ones that Vercel builds. They've always had a geocentric focus on the east coast userbase though.

Regardless if it's Vercel Edge or Cloudflare edge I'd like to use sentry in an edge environment. If Sentry doesn't plan to support that I understand but please clarify so we can stop waiting and move now.

AbhiPrasad commented 3 months ago

I apologize if I came across as dismissive, it was not my intention. The reason I brought up the tweet is simply just a matter of prioritization - considering it's the edge runtime is less emphasized at Vercel (only some use cases instead of every server use case) we can prioritize some other tasks like our ongoing work on Sentry SDKs for solid-js and nuxt.

Regardless if it's Vercel Edge or Cloudflare edge I'd like to use sentry in an edge environment. If Sentry doesn't plan to support that I understand but please clarify so we can stop waiting and move now.

To be clear, we support Vercel Edge: https://www.npmjs.com/package/@sentry/vercel-edge. This issue is basically about if we support automatically detecting and using the vercel edge sdk for SvelteKit apps with 0 config (like we do for nextjs).

The workaround for not supporting automatic detect is to add the Vercel Edge SDK yourself to the SvelteKit server entry points which basically is:

call Sentry.init from @sentry/vercel-edge in hooks.server.js
write your own handleError function in hooks.server.js to use the methods from @sentry/vercel-edge
copy our request handler and use @sentry/vercel-edge exports - https://github.com/getsentry/sentry-javascript/blob/44ef79a42558a0fff8801310202e0b9b797c4c36/packages/sveltekit/src/server/handle.ts#L138-L153

You shouldn't have to change anything with sourcemaps uploading or your client config - simply just the server-side stuff.

There are two notable limitations that you'll notice with Sentry data that is because of the runtime itself:

Stacktraces on the edge runtime suck
The edge runtime is built on top of cloudflare workers, which freezes timestamps unless I/O has happened. This means that relative timestamps for breadcrumbs or spans becomes much less useful unless you have a fetch call or similar happening.

Aside: the reason why we don't have a generic @sentry/wintercg or @sentry/edge yet is because the SDK relies on AsyncLocalStorage/AsyncContext pretty heavily (similar to OpenTelemetry and other instrumentation vendors). Every runtime has this implemented in slightly different ways, so we unfortunately can't have a fully isomorphic solution yet 😢.

JeroenvdV commented 2 months ago

I've been working on getting Sentry working in my Sveltekit app today that uses both the Vercel nodejs20.x and edge runtimes for different routes. I need to make far-reaching tweaks in order to be able to use Sentry in the entire project at all - I have functions that rely on the edge runtime, and having Sentry in the project breaks them entirely.

This seems to be tricky, because Sveltekit will first merge all the source with all the dependencies, including @sentry/sveltekit, and only then will the @sveltekit/adapter-vercel process run either generate_edge_function or generate_serverless_function on the result.

So far, to get it working I've had to alter the adapter-vercel code, using the esbuild call to replace away Sentry on the edge functions. Maybe I could get it to use @sentry/vercel-edge instead with more changes. However, is there any other known workaround for such a case already where the two libraries would have to be combined?

The error I initially see is this one, for reference: https://github.com/getsentry/sentry-javascript/issues/6692#issuecomment-1651258351

lforst commented 2 months ago

So far, to get it working I've had to alter the adapter-vercel code, using the esbuild call to replace away Sentry on the edge functions. Maybe I could get it to use @sentry/vercel-edge instead with more changes. However, is there any other known workaround for such a case already where the two libraries would have to be combined?

That is valid but I can see that this is very annoying. In theory you could strip Sentry out of your bundles on edge and you could use @sentry/vercel-edge in places where you are exclusively using the edge runtime.

Other than that I unfortunately don't have any news. @Lms24 may soon be able to update the SDK so that at least you're able to build when running on Edge.

JeroenvdV commented 2 months ago

I've worked on it some more and I have extended the workaround to use @sentry/vercel-edge instead of just noop mock code. However, it seems @sentry/vercel-edge is not a drop-in replacement for @sentry/sveltekit, which means I had to:

Create an esbuild plugin that replaces the imports of @sentry/sveltekit with custom code
Write the custom code that wraps @sentry/vercel-edge into a compatibility layer with for @sentry/sveltekit
Make esbuild actually resolve the custom code's import of @sentry/vercel-edge, which was involved

import * as SentryVercelEdge from '@sentry/vercel-edge';

export const init = SentryVercelEdge.init;
export const sentryHandle = () => (({ event, resolve }) => resolve(event)); // Default identity Handle
export const handleErrorWithSentry = () => {};
export const wrapServerLoadWithSentry = (fn) => fn;

If it were a drop-in replacement, I could have hopefully used esbuild's alias feature to simply replace one import with the other, and it would been much more elegant.

Note that to be able to do any of this, I currently have to fork @sveltekit/adapter-vercel because the official version doesn't expose any of the esbuild configuration it uses under the hood in generate_edge_function(). This is currently doable for me in this small project but that's not a fork that is maintainable in a 'real' project, as you probably agree. At least this has been educational.

In theory you could strip Sentry out of your bundles on edge [...]

I think that's effectively what I'm doing now, but if there's a better way to achieve it then that would probably be preferable. I feel like this is actually becoming more of a a sveltekit issue than a sentry issue though to be fair.

getsentry / sentry-javascript

Make SvelteKit SDK use Vercel Edge runtime SDK #9107