-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix(node): Ensure httpIntegration
propagates traces
#15233
Conversation
size-limit report 📦
|
packages/node/src/integrations/http/SentryHttpInstrumentation.ts
Outdated
Show resolved
Hide resolved
// If the core UndiciInstrumentation is registered, it will already have set the headers | ||
// We do not want to add any then | ||
if (!headers[k]) { | ||
headers[k] = v; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m: This works for sentry-trace
but since baggage
is standardized, other libraries or users might add their own entries. Which means we need to check for sentry-
values in existing headers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated it to merge baggages together!
We used to rely on the existence of another `httpIntegration`
Co-authored-by: Lukas Stracke <[email protected]>
3a5fb3a
to
2c8d0d6
Compare
dff0b91
to
81090de
Compare
Related to #15231, I noticed that we today would not propagate traces in outgoing http requests if:
httpIntegration({ spans: false })
HttpInstrumentation
Admittedly and edge case. More importantly, though, by actually adding distributed tracing information here, we are unblocked from potentially stopping to ship the http-instrumentation to users that do not need spans (and/or have a custom otel setup).