feat(nextjs): Add option to automatically tunnel events #6425
Conversation
lforst
force-pushed
the
lforst-next-auto-tunnel
branch
from
acea201
to
7c9b129
Compare
lforst
changed the base branch from
master
to
lforst-generalize-prefix-loader
size-limit report 📦
|
packages/nextjs/src/index.client.ts
Outdated
| __DEBUG_BUILD__ && logger.info(`Tunneling events to "${tunnelPath}"`); | ||
| options.tunnel = tunnelPath; | ||
| } else { | ||
| __DEBUG_BUILD__ && logger.warn('Provided DSN is not a Sentry SaaS DSN. Will not tunnel events.'); |
There was a problem hiding this comment.
l: This should work with self-hosted as well right? Why indicate SaaS?
There was a problem hiding this comment.
Is there a reason to tunnel if you're already on self-hosted? Requests to self-hosted instances are probably not being blocked by ad blockers.
Another reason we limit this is because of next.js rewrite restrictions. For example, the protocol needs to be static.
There was a problem hiding this comment.
I was going to ask the same thing. It's for sure less likely for someone's self-hosted instance to land on a block-list, but not impossible.
There was a problem hiding this comment.
Is there a reason to tunnel if you're already on self-hosted
Good point, not sure actually, don't know enough to comment on this. @kamilogorek is tunneling a thing for self-hosted users?
There was a problem hiding this comment.
It's for sure less likely for someone's self-hosted instance to land on a block-list, but not impossible.
I believe if somebody's self-hosted instance is landing on a block-list, they're probably high-profile enough that their proxy route (via this option) will also land on a block list - rendering the option useless anyhow.
I personally would ship this without directly supporting self-hosted. And if we see demand for proxying to other hosts we can pretty easily add something like a tunnelTarget option that would allow for that. It should be perfectly compatible with this change.
I am not gonna go into detail here but there are a few things to consider when letting users choose the host. For example, we would need some special logic to tunnel requests to ingest.sentry.io because you cannot put that string into a tunnel request, since it would be blocked.
There was a problem hiding this comment.
They can also implement this logic themselves if they need to for the self-hosted case. Yeah that sounds reasonable enough for me.
There was a problem hiding this comment.
I believe if somebody's self-hosted instance is landing on a block-list, they're probably high-profile enough that their proxy route (via this option) will also land on a block list - rendering the option useless anyhow.
I personally would ship this without directly supporting self-hosted. And if we see demand for proxying to other hosts we can pretty easily add something like a
tunnelTargetoption that would allow for that. It should be perfectly compatible with this change.
Yup, that's fair. Let's leave it as is.
packages/nextjs/src/index.client.ts
Outdated
| __DEBUG_BUILD__ && logger.info(`Tunneling events to "${tunnelPath}"`); | ||
| options.tunnel = tunnelPath; | ||
| } else { | ||
| __DEBUG_BUILD__ && logger.warn('Provided DSN is not a Sentry SaaS DSN. Will not tunnel events.'); |
There was a problem hiding this comment.
I was going to ask the same thing. It's for sure less likely for someone's self-hosted instance to land on a block-list, but not impossible.
Co-authored-by: Katie Byers <katie.byers@sentry.io>
Adds the option
tunnelRouteto thewithSentryConfig()wrapper to automatically tunnel Sentry requests through the Next.js server via a configured route. This is a mechanism to prevent ad-blockers from blocking requests to Sentry.We are leveraging Next.js
rewritesoption for tunneling which is essentially just a proxy configuration. It has the upside that when deployed on Vercel it is quite fast.One thing we needed to take care of was that we're not creating spans/transactions for these proxied requests because they were extremely spammy.
Resolves: #5571