ref(node): Streamline undici (node-fetch) instrumentation#21650
ref(node): Streamline undici (node-fetch) instrumentation#21650logaretm wants to merge 1 commit into
Conversation
Refactor the vendored `UndiciInstrumentation` (the span-emitting half of the
NodeFetch integration) to use Sentry's span APIs instead of OpenTelemetry's
tracing/propagation APIs.
- Span creation: `tracer.startSpan` -> `startInactiveSpan({ kind: CLIENT })`
(the span is created on `undici:request:create` and ended later in
`onDone`/`onError`, so it must be inactive).
- Status: `SpanStatusCode` -> `SPAN_STATUS_ERROR`; drop the no-op
`recordException`.
- Trace propagation: replace `propagation.inject(trace.setSpan(...))` with
`withActiveSpan(span, () => getTraceData(...))` gated by
`shouldPropagateTraceForUrl`. This keeps the propagated `sentry-trace`
referencing the http.client span (not the parent), matching prior output.
Passing `{ span }` to `getTraceData` alone is insufficient: for an inactive
span it resolves to the span's captured scope, whose active span is the
parent.
- Drop the OTel metrics (no MeterProvider is wired up) and the dead
`requireParentforSpans` code path (the SDK always passes `false`).
- Replace the diag logger with `debug` + `DEBUG_BUILD`, swap OTel type imports
for `@sentry/core`, inline the semantic-convention constants into a local
`semconv.ts`, and drop the blanket `eslint-disable`s.
The only remaining `@opentelemetry/api` usage is `SpanKind`; the
`@opentelemetry/instrumentation` patching base is kept.
Adds integration coverage for the error path (`internal_error` span) and for
`headersToSpanAttributes` (request + response header capture).
size-limit report 📦
|
logaretm
requested review from
JPeer264,
andreiborza,
isaacs,
mydea and
nicohrubec
and removed request for
a team and
mydea
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2b3dbd0. Configure here.
| name: requestMethod === '_OTHER' ? 'HTTP' : requestMethod, | ||
| kind: SpanKind.CLIENT, | ||
| attributes, | ||
| }); |
There was a problem hiding this comment.
Missing http.client sentry.op
High Severity
startInactiveSpan is called without sentry.op (http.client) on the span or in merged attributes. The integration tests and prior OTel export path expect outgoing fetch spans to use op http.client, but native SentrySpan JSON only reads op from attributes and does not infer it from kind or http.request.method.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 2b3dbd0. Configure here.
| name: requestMethod === '_OTHER' ? 'HTTP' : requestMethod, | ||
| kind: SpanKind.CLIENT, | ||
| attributes, | ||
| }); |
There was a problem hiding this comment.
Fetch span description missing URL
Medium Severity
Outgoing fetch spans are started with name set to the HTTP method only (e.g. GET), while integration tests expect descriptions like GET http://host/path. The OTel exporter used to derive that description from URL attributes and CLIENT kind; that inference no longer runs for these native spans.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2b3dbd0. Configure here.
| span.recordException(error); | ||
| span.setStatus({ | ||
| code: SpanStatusCode.ERROR, | ||
| code: SPAN_STATUS_ERROR, |
There was a problem hiding this comment.
Error status not internal_error
Medium Severity
On undici:request:error, the span status message is set to the raw error.message, so serialized status is typically the system error string rather than internal_error. The new fetch-error integration test expects status: 'internal_error', consistent with @sentry/core fetch instrumentation.
Reviewed by Cursor Bugbot for commit 2b3dbd0. Configure here.
1 participant
Streamlines the vendored
UndiciInstrumentation(the span-emitting half of the NodeFetch integration) onto Sentry's span APIs.closes #20743