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

Make SvelteKit SDK use Vercel Edge runtime SDK #9107

Open
Tracked by #8087
AbhiPrasad opened this issue Sep 25, 2023 · 24 comments
Open
Tracked by #8087

Make SvelteKit SDK use Vercel Edge runtime SDK #9107

AbhiPrasad opened this issue Sep 25, 2023 · 24 comments
Labels
Package: sveltekit Issues related to the Sentry SvelteKit SDK Package: vercel-edge Issues related to the Sentry Vercel Edge SDK

Comments

@AbhiPrasad
Copy link
Member

AbhiPrasad commented Sep 25, 2023

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

@AbhiPrasad AbhiPrasad added Meta: Help Wanted Package: sveltekit Issues related to the Sentry SvelteKit SDK labels Sep 25, 2023
@isaacharrisholt
Copy link

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

@AbhiPrasad
Copy link
Member Author

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
Copy link
Member

lforst commented Sep 26, 2023

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
Copy link

knd775 commented Oct 12, 2023

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
Copy link
Member

lforst commented Oct 13, 2023

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
Copy link

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

@smart
Copy link

smart commented Nov 21, 2023

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

@isaacharrisholt
Copy link

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

@AbhiPrasad
Copy link
Member Author

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
Copy link

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
Copy link
Member

lforst commented Nov 22, 2023

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: vercel/vercel#10829 Feel free to 👍 it. I think it would help gauge interest in this.

@isaacharrisholt
Copy link

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: vercel/vercel#10829 Feel free to 👍 it. I think it would help gauge interest in this.

Done!

@lforst
Copy link
Member

lforst commented Nov 22, 2023

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
Copy link

Any update on this? vercel/vercel#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
Copy link
Member

lforst commented Jan 15, 2024

@EricAndresen No updates. You would see it here.

@AbhiPrasad AbhiPrasad added the Package: vercel-edge Issues related to the Sentry Vercel Edge SDK label Jun 13, 2024
@jstjoe
Copy link

jstjoe commented Jun 19, 2024

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

  1. staying on Node.js and sacrificing performance and user experience for my global users
  2. 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
Copy link
Member Author

@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: #6805

@knd775
Copy link

knd775 commented Jun 20, 2024

  1. 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
Copy link

jstjoe commented Jun 20, 2024 via email

@jstjoe
Copy link

jstjoe commented Jun 20, 2024

@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
Copy link
Member Author

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:

  1. call Sentry.init from @sentry/vercel-edge in hooks.server.js
  2. write your own handleError function in hooks.server.js to use the methods from @sentry/vercel-edge
  3. copy our request handler and use @sentry/vercel-edge exports -
    /**
    * A SvelteKit handle function that wraps the request for Sentry error and
    * performance monitoring.
    *
    * Usage:
    * ```
    * // src/hooks.server.ts
    * import { sentryHandle } from '@sentry/sveltekit';
    *
    * export const handle = sentryHandle();
    *
    * // Optionally use the `sequence` function to add additional handlers.
    * // export const handle = sequence(sentryHandle(), yourCustomHandler);
    * ```
    */
    export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle {

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:

  1. Stacktraces on the edge runtime suck
  2. 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
Copy link

JeroenvdV commented Jul 7, 2024

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: #6692 (comment)

@lforst
Copy link
Member

lforst commented Jul 9, 2024

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
Copy link

JeroenvdV commented Jul 9, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: sveltekit Issues related to the Sentry SvelteKit SDK Package: vercel-edge Issues related to the Sentry Vercel Edge SDK
Projects
Status: No status
Status: No status
Status: No status
Development

No branches or pull requests

10 participants