Skip to content

feat: Adopt bindTracingChannelToSpan across runtimes#21642

Merged
logaretm merged 6 commits into
awad/bind-tracing-channelfrom
awad/adopt-bind-tracing-channel
Jun 24, 2026
Merged

feat: Adopt bindTracingChannelToSpan across runtimes#21642
logaretm merged 6 commits into
awad/bind-tracing-channelfrom
awad/adopt-bind-tracing-channel

Conversation

@logaretm

@logaretm logaretm commented Jun 18, 2026

Copy link
Copy Markdown
Member

This PR adopts bindTracingChannelToSpan utility across our packages, and drops the tracingChannel helper we had.

The tracingChannel drop isn't a breaking change imo, its an internal export used by us exclusively.

@logaretm logaretm force-pushed the awad/bind-tracing-channel branch from 8c4ca47 to 61a560f Compare June 18, 2026 17:56
@logaretm logaretm force-pushed the awad/adopt-bind-tracing-channel branch from e189fda to 3f71785 Compare June 18, 2026 17:56
@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

Path Size % Change Change
@sentry/browser 27.47 kB - -
@sentry/browser - with treeshaking flags 25.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 flags 74.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/react 29.27 kB - -
@sentry/react (incl. Tracing) 48.28 kB - -
@sentry/vue 32.63 kB - -
@sentry/vue (incl. Tracing) 47.84 kB - -
@sentry/svelte 27.5 kB - -
CDN Bundle 29.89 kB +0.01% +2 B 🔺
CDN Bundle (incl. Tracing) 47.89 kB -0.02% -7 B 🔽
CDN Bundle (incl. Logs, Metrics) 31.44 kB -0.01% -2 B 🔽
CDN Bundle (incl. Tracing, Logs, Metrics) 49.24 kB -0.01% -4 B 🔽
CDN Bundle (incl. Replay, Logs, Metrics) 70.78 kB +0.02% +9 B 🔺
CDN Bundle (incl. Tracing, Replay) 85.4 kB -0.02% -15 B 🔽
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) 86.68 kB -0.02% -11 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) 91.19 kB -0.02% -15 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) 92.45 kB -0.01% -7 B 🔽
CDN Bundle - uncompressed 88.94 kB -0.01% -5 B 🔽
CDN Bundle (incl. Tracing) - uncompressed 145.03 kB +0.01% +3 B 🔺
CDN Bundle (incl. Logs, Metrics) - uncompressed 93.65 kB -0.01% -6 B 🔽
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed 149 kB +0.01% +2 B 🔺
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed 218.62 kB -0.01% -2 B 🔽
CDN Bundle (incl. Tracing, Replay) - uncompressed 264.05 kB +0.01% +8 B 🔺
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed 268 kB +0.01% +7 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 277.75 kB +0.01% +8 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed 281.69 kB +0.01% +7 B 🔺
@sentry/nextjs (client) 50.67 kB - -
@sentry/sveltekit (client) 46.37 kB - -
@sentry/core/server 76.41 kB +0.11% +83 B 🔺
@sentry/core/browser 63.56 kB +0.13% +81 B 🔺
@sentry/node-core 61.7 kB +0.13% +76 B 🔺
@sentry/node 122.9 kB -0.59% -717 B 🔽
@sentry/node/import (ESM hook with diagnostics-channel injection) 69.95 kB - -
@sentry/node/light 50.62 kB +0.13% +63 B 🔺
@sentry/node - without tracing 73.76 kB +0.09% +63 B 🔺
@sentry/aws-serverless 84.95 kB +0.07% +51 B 🔺
@sentry/cloudflare (withSentry) - minified 176.01 kB +0.18% +302 B 🔺
@sentry/cloudflare (withSentry) 437.76 kB +0.15% +646 B 🔺

View base workflow run

@logaretm logaretm force-pushed the awad/adopt-bind-tracing-channel branch 2 times, most recently from 8ec64f0 to 7a09a2e Compare June 19, 2026 01:18
expect.objectContaining({
handled: false,
type: 'auto.http.nitro.onTraceError',
type: 'auto.diagnostic_channels.bind_span',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is no good, this should be somewhat similar to what it was before 😬

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

okay I can add an option for error mechanism to preserve this

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done in d452bdb

@logaretm logaretm marked this pull request as ready for review June 19, 2026 13:24
@logaretm logaretm requested review from a team as code owners June 19, 2026 13:24
@logaretm logaretm requested review from JPeer264, chargome, mydea and s1gr1d and removed request for a team June 19, 2026 13:24
@logaretm logaretm force-pushed the awad/bind-tracing-channel branch from 2fbdda3 to 3cb7959 Compare June 19, 2026 13:24
@logaretm logaretm force-pushed the awad/adopt-bind-tracing-channel branch from 7a09a2e to a11202f Compare June 19, 2026 13:31
Comment thread packages/nitro/src/runtime/hooks/captureTracingEvents.ts
@logaretm logaretm force-pushed the awad/bind-tracing-channel branch from 3cb7959 to e34e28a Compare June 19, 2026 13:57
@logaretm logaretm force-pushed the awad/adopt-bind-tracing-channel branch from d452bdb to 2d8060e Compare June 19, 2026 13:57
Comment thread packages/nitro/src/runtime/hooks/captureTracingEvents.ts
Comment thread packages/nitro/src/runtime/hooks/captureTracingEvents.ts
Comment thread packages/nitro/src/runtime/hooks/captureStorageEvents.ts
Comment thread packages/nitro/src/runtime/hooks/captureStorageEvents.ts
Comment thread packages/nitro/src/runtime/hooks/captureTracingEvents.ts
@logaretm logaretm requested review from andreiborza and isaacs June 19, 2026 14:46
@logaretm logaretm force-pushed the awad/adopt-bind-tracing-channel branch from 766b254 to 05cfa5b Compare June 19, 2026 15:07
@logaretm logaretm force-pushed the awad/bind-tracing-channel branch from 08b9cbb to c02e8e1 Compare June 22, 2026 17:08
@logaretm logaretm force-pushed the awad/adopt-bind-tracing-channel branch from 05cfa5b to 5208656 Compare June 22, 2026 17:08

@JPeer264 JPeer264 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Just one nit.

Comment thread packages/server-utils/test/redis/redis-dc-subscriber.test.ts
Comment thread packages/nitro/src/runtime/hooks/captureTracingEvents.ts
@logaretm logaretm force-pushed the awad/bind-tracing-channel branch from c02e8e1 to 738418f Compare June 23, 2026 17:29
@logaretm logaretm force-pushed the awad/adopt-bind-tracing-channel branch 2 times, most recently from 68ab8c1 to b9a3ae4 Compare June 23, 2026 19:24
Comment thread packages/nitro/src/runtime/hooks/captureStorageEvents.ts
@logaretm logaretm force-pushed the awad/bind-tracing-channel branch from 0e7b31f to 3e93527 Compare June 23, 2026 20:11
@logaretm logaretm force-pushed the awad/adopt-bind-tracing-channel branch from b9a3ae4 to 757e9d1 Compare June 23, 2026 20:11
@logaretm logaretm force-pushed the awad/bind-tracing-channel branch from 3e93527 to 4a4adb2 Compare June 24, 2026 12:09
@logaretm logaretm force-pushed the awad/adopt-bind-tracing-channel branch from 757e9d1 to 103ebc0 Compare June 24, 2026 12:09
logaretm and others added 5 commits June 24, 2026 08:10
Migrate the Nitro HTTP/storage channels and the node/deno redis
diagnostics_channel subscribers off the `@sentry/opentelemetry`
`tracingChannel` helper onto `bindTracingChannelToSpan` from
`@sentry/server-utils`, using the `auto` lifecycle.

- Add `beforeSpanEnd(span, data)` to enrich the span on the canonical
  terminal event (sync `end` / async `asyncEnd`) right before it ends.
- Add `captureError` (default `true`) to gate exception capture; the span
  error status is always set. Redis opts out (`captureError: false`) since
  command failures are captured at the boundary that owns them.
- Unify the error mechanism to `auto.diagnostic_channels.bind_span`.
- Create channel spans with `startInactiveSpan`; the binding activates the
  span as the async context, so parenting is preserved.
- node and deno redis now pass the native `node:diagnostics_channel`
  `tracingChannel` directly; deno's bespoke factory is removed.

BREAKING: remove the `@sentry/opentelemetry/tracing-channel` subpath
export (shipped in 10.58.0); use `bindTracingChannelToSpan` instead.
Addresses review nit: tighten the bare toHaveBeenCalledWith assertions
to also verify the hook isn't invoked more than once.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follows the base-branch collapse of the binding API to a single
bindTracingChannelToSpan. Updates the redis and Nitro consumers to the new
name.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines 71 to +80
'db.collection.name': mountBase(data),
'db.system.name': data.driver?.name ?? 'unknown',
},
},
span => span,
);
});

channel.subscribe({
asyncEnd(data: TracingChannelContextWithSpan<TraceContext & { result?: unknown }>) {
if (data._sentrySpan && CACHE_HIT_OPERATIONS.has(operation)) {
const hit = operation === 'hasItem' ? Boolean(data.result) : isCacheHit(data.keys?.[0], data.result);
data._sentrySpan.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, hit);
}

data._sentrySpan?.setStatus({ code: SPAN_STATUS_OK });
data._sentrySpan?.end();

void flushIfServerless();
},
error(data: TracingChannelContextWithSpan<TraceContext & { error?: unknown }>) {
captureException(data.error, {
mechanism: { handled: false, type: ORIGIN },
});

data._sentrySpan?.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
data._sentrySpan?.end();

void flushIfServerless();
},
});
{
beforeSpanEnd(span, data) {
// Auto-capture + error status is handled by the binding; only enrich the success path.
if (!('error' in data)) {
const result = (data as { result?: unknown }).result;

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 refactoring to bindTracingChannelToSpan for storage events unintentionally changes the error mechanism type, losing the specific 'auto.cache.nitro' context.
Severity: LOW

Suggested Fix

Preserve the original error mechanism type for storage events by providing a custom captureError function when calling bindTracingChannelToSpan, similar to how HTTP events are handled. This function should return a mechanism object with the type set to 'auto.cache.nitro'. For example: captureError: () => ({ mechanism: { handled: false, type: 'auto.cache.nitro' } }).

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/nitro/src/runtime/hooks/captureStorageEvents.ts#L71-L80

Potential issue: During the migration to `bindTracingChannelToSpan`, the error capturing
for storage events was changed to use the default mechanism. This results in errors
being reported with a generic mechanism type of `'auto.diagnostic_channels.bind_span'`
instead of the previous, more specific `'auto.cache.nitro'`. This is inconsistent with
the handling of HTTP events in the same file, which explicitly preserve their custom
mechanism type. This change can negatively affect error grouping and filtering in
Sentry, making it harder to track and analyze storage-related failures.

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

An unset span status already serializes to 'ok' for streamed spans, so setting
it explicitly on the cache success path was redundant. Also correct the stale
comment: with captureError defaulting to false, the binding only sets error
status; the error is captured at the request boundary, not on cache ops.
@logaretm logaretm force-pushed the awad/adopt-bind-tracing-channel branch from 103ebc0 to bee3c83 Compare June 24, 2026 12:23

@cursor cursor Bot 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 bee3c83. Configure here.

error: onTraceError,
});
},
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Srvx trace errors not captured

Medium Severity

The srvx.request and srvx.middleware bindings no longer pass captureError to bindTracingChannelToSpan, so tracing-channel failures only update span status. Previously both channels used onTraceError, which called captureException with mechanism auto.http.nitro.onTraceError. The h3 channel still uses that pattern after review feedback; srvx does not, so srvx trace failures may no longer produce Sentry error events with the same mechanism as before.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit bee3c83. Configure here.

@logaretm logaretm merged commit 9edf695 into develop Jun 24, 2026
444 of 446 checks passed
@logaretm logaretm deleted the awad/adopt-bind-tracing-channel branch June 24, 2026 12:55
Sign up for free to 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.

4 participants