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
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
Nice! I think this is a great improvement and something we can implement in other meta frameworks as well in the future
| ...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.
2 participants
Adds a
http.request.prefetch: trueattribute to Next.js clientside requests if they're prefetching other pages.