Uh oh!
There was an error while loading. Please reload this page.
ref(core): Move shouldPropagateTraceForUrl from opentelemetry to core - #19258
Conversation
Codecov Results 📊Generated by Codecov Action |
size-limit report 📦
|
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| const url = `${request.protocol}//${request.getHeader('host') || request.host}${request.path}`; | ||
| return ignoreOutgoingRequests(url, getRequestOptions(request)); | ||
| } |
There was a problem hiding this comment.
Missing suppress-tracing check causes transport request breadcrumbs
Medium Severity
The OTel-based SentryHttpInstrumentation._shouldIgnoreOutgoingRequest checks isTracingSuppressed(context.active()) to skip the SDK's own transport requests (which are wrapped in suppressTracing()). The light mode's onOutgoingRequestCreated and onOutgoingRequestFinish have no equivalent check for __SENTRY_SUPPRESS_TRACING__ in scope metadata, even though the light mode ACS's suppressTracing sets exactly that. This means every event sent to Sentry's ingest endpoint will generate a spurious HTTP breadcrumb and get trace propagation headers injected.
Additional Locations (1)
ae6b3de to
89c90a1CompareAdd outgoing HTTP request trace propagation to the light mode httpIntegration (renamed from httpServerIntegration). This uses node:diagnostics_channel to inject sentry-trace, traceparent, and baggage headers into outgoing http/https requests. Also extracts shared HTTP request utils (header injection, breadcrumbs, URL construction) into outgoingHttpRequest.ts to reduce duplication between light mode and full (OpenTelemetry) mode.
89c90a1 to
81bb9c3Compare| import type { LightNodeClient } from '../client'; | ||
| const INTEGRATION_NAME = 'Http.Server'; | ||
| const INTEGRATION_NAME = 'Http'; |
There was a problem hiding this comment.
m: Isn't this basically a breaking change? If one filters by Read the "not released" part too lateHttp.Server it wouldn't work anymore as it got renamed.
Uh oh!
There was an error while loading. Please reload this page.
This PR adds outgoing trace propagation for http requests to node-core/light.
For that we ended up renaming the integration to
httpIntegration(fromhttpServerIntegration). We haven't released node-core/light yet so it's not a breaking change.We also extract shared HTTP request utils (header injection, breadcrumbs, URL construction) into a helper to reduce duplication between light mode and node-core.
Closes#19259 (added automatically)