Skip to content

ref(node): Move node-fetch instrumentation away from InstrumentBase - #21778

Merged
mydea merged 3 commits into
developfrom
fn/undici-no-otel
Jun 29, 2026
Merged

ref(node): Move node-fetch instrumentation away from InstrumentBase#21778
mydea merged 3 commits into
developfrom
fn/undici-no-otel

Conversation

@mydea

Copy link
Copy Markdown
Member

This streamlines and de-otelifies the undici instrumentation in node package.

Note that there is still a OTEL instrumentation (channel based, but still) in node-core which can be updated in a separate follow up step.

This also updates to use sentry/conventions and drop any config we hard coded so far, streamlining this. Also, unsubscribe code was removed and other things we do not need were removed.

@mydeamydea self-assigned this Jun 25, 2026

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit cc1a55a. Configure here.

@github-actions

github-actionsBot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize% ChangeChange
@sentry/browser27.48 kB--
@sentry/browser - with treeshaking flags25.91 kB--
@sentry/browser (incl. Tracing)45.97 kB--
@sentry/browser (incl. Tracing + Span Streaming)47.72 kB--
@sentry/browser (incl. Tracing, Profiling)50.76 kB--
@sentry/browser (incl. Tracing, Replay)85.22 kB--
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags74.81 kB--
@sentry/browser (incl. Tracing, Replay with Canvas)89.91 kB--
@sentry/browser (incl. Tracing, Replay, Feedback)102.57 kB--
@sentry/browser (incl. Feedback)44.66 kB--
@sentry/browser (incl. sendFeedback)32.26 kB--
@sentry/browser (incl. FeedbackAsync)37.4 kB--
@sentry/browser (incl. Metrics)28.54 kB--
@sentry/browser (incl. Logs)28.78 kB--
@sentry/browser (incl. Metrics & Logs)29.47 kB--
@sentry/react29.27 kB--
@sentry/react (incl. Tracing)48.28 kB--
@sentry/vue32.63 kB--
@sentry/vue (incl. Tracing)47.84 kB--
@sentry/svelte27.5 kB--
CDN Bundle29.89 kB--
CDN Bundle (incl. Tracing)47.89 kB--
CDN Bundle (incl. Logs, Metrics)31.44 kB--
CDN Bundle (incl. Tracing, Logs, Metrics)49.24 kB--
CDN Bundle (incl. Replay, Logs, Metrics)70.78 kB--
CDN Bundle (incl. Tracing, Replay)85.4 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics)86.68 kB--
CDN Bundle (incl. Tracing, Replay, Feedback)91.19 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics)92.45 kB--
CDN Bundle - uncompressed88.94 kB--
CDN Bundle (incl. Tracing) - uncompressed145.03 kB--
CDN Bundle (incl. Logs, Metrics) - uncompressed93.65 kB--
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed149 kB--
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed218.62 kB--
CDN Bundle (incl. Tracing, Replay) - uncompressed264.05 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed268 kB--
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed277.75 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed281.69 kB--
@sentry/nextjs (client)50.67 kB--
@sentry/sveltekit (client)46.37 kB--
@sentry/core/server77.54 kB--
@sentry/core/browser63.88 kB--
@sentry/node-core61.51 kB-0.01%-1 B 🔽
@sentry/node123.18 kB-0.26%-311 B 🔽
@sentry/node/import (ESM hook with diagnostics-channel injection)69.95 kB--
@sentry/node/light50.4 kB-0.01%-1 B 🔽
@sentry/node - without tracing73.28 kB-0.45%-329 B 🔽
@sentry/aws-serverless84.07 kB-0.39%-327 B 🔽
@sentry/cloudflare (withSentry) - minified180.31 kB--
@sentry/cloudflare (withSentry)446.24 kB--

View base workflow run

@mydea
mydeaforce-pushed the fn/undici-no-otel branch from cc1a55a to 1adc6ceCompareJune 25, 2026 11:40
@mydea
mydea marked this pull request as ready for review June 25, 2026 12:06
@mydea
mydea requested a review from a team as a code ownerJune 25, 2026 12:06
@mydea
mydea requested review from JPeer264 and andreiborza and removed request for a teamJune 25, 2026 12:06
Comment on lines -27 to -32
breadcrumbs?: boolean;

/**
* If set to false, do not emit any spans.
* This will ensure that the default UndiciInstrumentation from OpenTelemetry is not setup,
* only the Sentry-specific instrumentation for breadcrumbs & trace propagation is applied.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The tracePropagation: false option is ignored by the new instrumentUndici function, causing trace headers to be injected unconditionally for span instrumentation.
Severity: MEDIUM

Suggested Fix

Update the UndiciInstrumentationConfig type to include the tracePropagation option. Pass this option from the nativeNodeFetchIntegration options into instrumentUndici(). Finally, modify instrumentUndici() and its helper injectTracePropagationHeaders() to respect the tracePropagation flag and only inject headers when it is not false.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/node/src/integrations/node-fetch/index.ts#L27-L32
Potential issue: The new undici span instrumentation does not honor the
`tracePropagation: false` option. The `instrumentUndici()` function, which handles span
instrumentation, is called without the `tracePropagation` option and has no mechanism to
receive it. As a result, its internal `injectTracePropagationHeaders()` function is
always called, leading to trace headers being unconditionally injected for outgoing
requests when spans are enabled. This is a regression from previous behavior where the
option was respected, and it contradicts the user's explicit configuration.

Also affects:

  • packages/node/src/integrations/node-fetch/undici-instrumentation.ts:385~411

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clarified the comment for this

@mydea
mydeaforce-pushed the fn/undici-no-otel branch from 1adc6ce to eea6bf0CompareJune 26, 2026 08:11
@mydea
mydeaforce-pushed the fn/undici-no-otel branch from eea6bf0 to 805ba1bCompareJune 26, 2026 09:42
@mydea
mydeaforce-pushed the fn/undici-no-otel branch from 805ba1b to 0d8e590CompareJune 29, 2026 08:05
@mydea
mydea merged commit 2f9894d into developJun 29, 2026
355 of 358 checks passed
@mydea
mydea deleted the fn/undici-no-otel branch June 29, 2026 09:33
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@mydea@andreiborza