feat(nextjs): Mark clientside prefetch request spans with http.request.prefetch: true attribute
#15980
Conversation
size-limit report 📦
|
| if (headers?.get('next-router-prefetch')) { | ||
| span?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.client.prefetch'); | ||
| } |
There was a problem hiding this comment.
Q: Have we checked if this change plays well with the product? Specifically I'm thinking of the "Network Requests" insights module
wondering if http.client.prefetch spans would still count towards the http.client request spans tracked in this module.
Also, might be worth adding the op to dev docs, wdyt?
There was a problem hiding this comment.
Very good point getsentry/sentry-docs#13239
There was a problem hiding this comment.
Based on an internal discussion we have agreed to instead add a http.request.prefetch: true attribute to the requests.
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
lforst
changed the title
http.client.prefetch ophttp.request.prefetch: true attribute
| @@ -12,6 +12,16 @@ export function browserTracingIntegration( | |||
| ...options, | |||
| instrumentNavigation: false, | |||
| instrumentPageLoad: false, | |||
| onRequestSpanStart(...args) { | |||
There was a problem hiding this comment.
l: do we care about cases where users register their own browserTracingIntegration()? As in should we merge a possible custom implementation with our logic? Not sure what our general approach to this is so feel free to declare undefined behaviour :D
There was a problem hiding this comment.
I think user's would be always importing this browserTracingIntegration (the Next.js one) so the behavior should already be merged (see line 23). Unless I am misunderstanding.
Adds a
http.request.prefetch: trueattribute to Next.js clientside requests if they're prefetching other pages.