Skip to content

feat(server-utils): Add requiresParentSpan option to bindTracingChannelToSpan - #22113

Merged
logaretm merged 3 commits into
developfrom
awad/tracing-channel-requires-parent-span
Jul 8, 2026
Merged

feat(server-utils): Add requiresParentSpan option to bindTracingChannelToSpan#22113
logaretm merged 3 commits into
developfrom
awad/tracing-channel-requires-parent-span

Conversation

@logaretm

@logaretmlogaretm commented Jul 8, 2026

Copy link
Copy Markdown
Member

A bunch of the diagnostics-channel subscribers repeated the same guard at the top of their getSpan callback.

Usually the logic is to bail out with undefined when there's no active span, so we don't open a top-level db span for queries/connects that happen outside a request.

This pulls that into a requiresParentSpan option on bindTracingChannelToSpan and moves the check into the bind store, before getSpan runs. Behaviorally identical, just one place instead of five.

If you need to actually inspect the parent span before deciding, that still belongs in getSpan.

Feel free to push back if this over complicates the bindTracingChannelToSpan, I thought it was a nice opportunity to normalize this behavior but not a biggie.

…elToSpan
Hoists the repeated "skip when there's no active span" guard out of the
individual `getSpan` callbacks into a `requiresParentSpan` option on the
binding helper. Migrates mysql2, ioredis, postgres, and postgres-js onto it.
@github-actions

github-actionsBot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize% ChangeChange
@sentry/browser27.59 kB--
@sentry/browser - with treeshaking flags26.03 kB--
@sentry/browser (incl. Tracing)46.34 kB--
@sentry/browser (incl. Tracing + Span Streaming)48.13 kB--
@sentry/browser (incl. Tracing, Profiling)51.12 kB--
@sentry/browser (incl. Tracing, Replay)85.61 kB--
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags75.26 kB--
@sentry/browser (incl. Tracing, Replay with Canvas)90.32 kB--
@sentry/browser (incl. Tracing, Replay, Feedback)102.97 kB--
@sentry/browser (incl. Feedback)44.76 kB--
@sentry/browser (incl. sendFeedback)32.38 kB--
@sentry/browser (incl. FeedbackAsync)37.51 kB--
@sentry/browser (incl. Metrics)28.67 kB--
@sentry/browser (incl. Logs)28.91 kB--
@sentry/browser (incl. Metrics & Logs)29.59 kB--
@sentry/react29.38 kB--
@sentry/react (incl. Tracing)48.61 kB--
@sentry/vue33.03 kB--
@sentry/vue (incl. Tracing)48.24 kB--
@sentry/svelte27.61 kB--
CDN Bundle30 kB--
CDN Bundle (incl. Tracing)48.32 kB--
CDN Bundle (incl. Logs, Metrics)31.57 kB--
CDN Bundle (incl. Tracing, Logs, Metrics)49.64 kB--
CDN Bundle (incl. Replay, Logs, Metrics)70.81 kB--
CDN Bundle (incl. Tracing, Replay)85.84 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics)87.14 kB--
CDN Bundle (incl. Tracing, Replay, Feedback)91.64 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics)92.92 kB--
CDN Bundle - uncompressed89.35 kB--
CDN Bundle (incl. Tracing) - uncompressed146.1 kB--
CDN Bundle (incl. Logs, Metrics) - uncompressed94.05 kB--
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed150.07 kB--
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed218.75 kB--
CDN Bundle (incl. Tracing, Replay) - uncompressed265.27 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed269.23 kB--
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed278.97 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed282.92 kB--
@sentry/nextjs (client)51.16 kB--
@sentry/sveltekit (client)46.75 kB--
@sentry/core/server78.42 kB--
@sentry/core/browser64.74 kB--
@sentry/node-core62.72 kB+0.01%+2 B 🔺
@sentry/node125.36 kB+0.03%+32 B 🔺
@sentry/node (incl. diagnostics channel injection)135 kB+0.03%+35 B 🔺
@sentry/node/import (ESM hook with diagnostics-channel injection)69.95 kB--
@sentry/node/light50.72 kB--
@sentry/node - without tracing74.05 kB+0.01%+1 B 🔺
@sentry/aws-serverless85.5 kB+0.01%+1 B 🔺
@sentry/cloudflare (withSentry) - minified181.71 kB--
@sentry/cloudflare (withSentry)449.16 kB--

View base workflow run

@logaretm
logaretm marked this pull request as ready for review July 8, 2026 19:27
@logaretm
logaretm requested a review from a team as a code ownerJuly 8, 2026 19:27
@logaretm
logaretm requested review from JPeer264, andreiborza, isaacs and mydea and removed request for a teamJuly 8, 2026 19:27

@andreiborzaandreiborza 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.

I like the change, also matches the option we had in some integrations before. Good improvement.

Edit: Actually, the option we have in the integrations is called onlyIfParent, but I actually like your naming better.

@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 17b4fd9. Configure here.

Comment threadpackages/server-utils/src/tracing-channel.ts
@logaretm
logaretm merged commit 64bbb08 into developJul 8, 2026
886 of 890 checks passed
@logaretm
logaretm deleted the awad/tracing-channel-requires-parent-span branch July 8, 2026 23:45
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

@logaretm@andreiborza