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

@logaretmlogaretm 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
logaretmforce-pushed the awad/adopt-bind-tracing-channel branch from e189fda to 3f71785CompareJune 18, 2026 17:56
@github-actions

github-actionsBot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize% ChangeChange
@sentry/browser27.47 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+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 - uncompressed88.94 kB-0.01%-5 B 🔽
CDN Bundle (incl. Tracing) - uncompressed145.03 kB+0.01%+3 B 🔺
CDN Bundle (incl. Logs, Metrics) - uncompressed93.65 kB-0.01%-6 B 🔽
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed149 kB+0.01%+2 B 🔺
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed218.62 kB-0.01%-2 B 🔽
CDN Bundle (incl. Tracing, Replay) - uncompressed264.05 kB+0.01%+8 B 🔺
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed268 kB+0.01%+7 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed277.75 kB+0.01%+8 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed281.69 kB+0.01%+7 B 🔺
@sentry/nextjs (client)50.67 kB--
@sentry/sveltekit (client)46.37 kB--
@sentry/core/server76.41 kB+0.11%+83 B 🔺
@sentry/core/browser63.56 kB+0.13%+81 B 🔺
@sentry/node-core61.7 kB+0.13%+76 B 🔺
@sentry/node122.9 kB-0.59%-717 B 🔽
@sentry/node/import (ESM hook with diagnostics-channel injection)69.95 kB--
@sentry/node/light50.62 kB+0.13%+63 B 🔺
@sentry/node - without tracing73.76 kB+0.09%+63 B 🔺
@sentry/aws-serverless84.95 kB+0.07%+51 B 🔺
@sentry/cloudflare (withSentry) - minified176.01 kB+0.18%+302 B 🔺
@sentry/cloudflare (withSentry)437.76 kB+0.15%+646 B 🔺

View base workflow run

@logaretm
logaretmforce-pushed the awad/adopt-bind-tracing-channel branch 2 times, most recently from 8ec64f0 to 7a09a2eCompareJune 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
MemberAuthor

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
MemberAuthor

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 ownersJune 19, 2026 13:24
@logaretm
logaretm requested review from JPeer264, chargome, mydea and s1gr1d and removed request for a teamJune 19, 2026 13:24
@logaretm
logaretmforce-pushed the awad/adopt-bind-tracing-channel branch from 7a09a2e to a11202fCompareJune 19, 2026 13:31
Comment threadpackages/nitro/src/runtime/hooks/captureTracingEvents.ts
@logaretm
logaretmforce-pushed the awad/adopt-bind-tracing-channel branch from d452bdb to 2d8060eCompareJune 19, 2026 13:57
Comment threadpackages/nitro/src/runtime/hooks/captureTracingEvents.ts
Comment threadpackages/nitro/src/runtime/hooks/captureTracingEvents.ts
Comment threadpackages/nitro/src/runtime/hooks/captureStorageEvents.ts
Comment threadpackages/nitro/src/runtime/hooks/captureStorageEvents.ts
Comment threadpackages/nitro/src/runtime/hooks/captureTracingEvents.ts
@logaretm
logaretmforce-pushed the awad/adopt-bind-tracing-channel branch from 766b254 to 05cfa5bCompareJune 19, 2026 15:07
@logaretm
logaretmforce-pushed the awad/adopt-bind-tracing-channel branch from 05cfa5b to 5208656CompareJune 22, 2026 17:08

@JPeer264JPeer264 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 threadpackages/server-utils/test/redis/redis-dc-subscriber.test.ts
Comment threadpackages/nitro/src/runtime/hooks/captureTracingEvents.ts
@logaretm
logaretmforce-pushed the awad/adopt-bind-tracing-channel branch 2 times, most recently from 68ab8c1 to b9a3ae4CompareJune 23, 2026 19:24
Comment threadpackages/nitro/src/runtime/hooks/captureStorageEvents.ts
@logaretm
logaretmforce-pushed the awad/adopt-bind-tracing-channel branch from b9a3ae4 to 757e9d1CompareJune 23, 2026 20:11
@logaretm
logaretmforce-pushed the awad/adopt-bind-tracing-channel branch from 757e9d1 to 103ebc0CompareJune 24, 2026 12:09
logaretmand 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
logaretmforce-pushed the awad/adopt-bind-tracing-channel branch from 103ebc0 to bee3c83CompareJune 24, 2026 12:23

@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 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 CursorFix in Web

Reviewed by Cursor Bugbot for commit bee3c83. Configure here.

@logaretm
logaretm merged commit 9edf695 into developJun 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 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.

4 participants

@logaretm@mydea@JPeer264@s1gr1d