Skip to content

feat(node): use diagnostics_channel for redis >= 5.12.0 - #20573

Merged
isaacs merged 10 commits into
developfrom
feat/redis-diagnostic-channels
May 6, 2026
Merged

feat(node): use diagnostics_channel for redis >= 5.12.0#20573
isaacs merged 10 commits into
developfrom
feat/redis-diagnostic-channels

Conversation

@logaretm

@logaretmlogaretm commented Apr 28, 2026

Copy link
Copy Markdown
Member

Builds on #20510 and adds tracing channels subscribers via node:diagnostics_channel.

The module patchers are narrowed to >=5.0.0 <5.12.0 while the subscriber path runs unconditionally. it will be inert in older releases anyways and will activate when version 5.12 publishes the events.

Verified that it works on Node and Bun equally as well, while IITM fails on Bun.

CopilotAI review requested due to automatic review settings April 28, 2026 20:36
@logaretm
logaretm marked this pull request as draft April 28, 2026 20:36
Comment threadpackages/node/src/integrations/tracing/redis/redis-dc-subscriber.ts Outdated

CopilotAI left a comment

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.

Pull request overview

Adds diagnostics_channel-based Redis tracing for upcoming node-redis (>= 5.12.0) while narrowing the existing IITM patching to avoid double instrumentation once diagnostics events are available.

Changes:

  • Restrict vendored node-redis IITM patcher to >=5.0.0 <5.12.0.
  • Add a diagnostics_channel subscriber which creates Sentry spans from node-redis:* tracing channels.
  • Ensure @sentry/opentelemetry/* subpath imports stay external in the Node rollup build.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

FileDescription
packages/node/src/integrations/tracing/redis/vendored/redis-instrumentation.tsNarrows the v5 IITM instrumentation version range to stop before 5.12.0.
packages/node/src/integrations/tracing/redis/redis-dc-subscriber.tsNew diagnostics_channel subscriber which creates/ends spans for command/batch/connect events and runs the responseHook.
packages/node/src/integrations/tracing/redis/index.tsWires the subscriber into Redis instrumentation (deferred to next tick).
packages/node/rollup.npm.config.mjsExternalizes @sentry/opentelemetry/* imports so subpath entrypoints aren’t bundled.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadpackages/node/src/integrations/tracing/redis/index.ts Outdated
Comment threadpackages/node/src/integrations/tracing/redis/index.ts Outdated
@github-actions

github-actionsBot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize% ChangeChange
@sentry/browser26.3 kB--
@sentry/browser - with treeshaking flags24.78 kB--
@sentry/browser (incl. Tracing)44.17 kB--
@sentry/browser (incl. Tracing + Span Streaming)46.39 kB--
@sentry/browser (incl. Tracing, Profiling)49.14 kB--
@sentry/browser (incl. Tracing, Replay)83.63 kB--
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags73.08 kB--
@sentry/browser (incl. Tracing, Replay with Canvas)88.32 kB--
@sentry/browser (incl. Tracing, Replay, Feedback)100.93 kB--
@sentry/browser (incl. Feedback)43.44 kB--
@sentry/browser (incl. sendFeedback)31.11 kB--
@sentry/browser (incl. FeedbackAsync)36.19 kB--
@sentry/browser (incl. Metrics)27.6 kB--
@sentry/browser (incl. Logs)27.73 kB--
@sentry/browser (incl. Metrics & Logs)28.43 kB--
@sentry/react28.04 kB--
@sentry/react (incl. Tracing)46.4 kB--
@sentry/vue31.18 kB--
@sentry/vue (incl. Tracing)46.02 kB--
@sentry/svelte26.32 kB--
CDN Bundle28.91 kB--
CDN Bundle (incl. Tracing)46.94 kB--
CDN Bundle (incl. Logs, Metrics)30.34 kB--
CDN Bundle (incl. Tracing, Logs, Metrics)48.04 kB--
CDN Bundle (incl. Replay, Logs, Metrics)69.46 kB--
CDN Bundle (incl. Tracing, Replay)84.13 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics)85.2 kB--
CDN Bundle (incl. Tracing, Replay, Feedback)89.94 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics)91.02 kB--
CDN Bundle - uncompressed84.88 kB--
CDN Bundle (incl. Tracing) - uncompressed140.44 kB--
CDN Bundle (incl. Logs, Metrics) - uncompressed89.08 kB--
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed143.9 kB--
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed213.29 kB--
CDN Bundle (incl. Tracing, Replay) - uncompressed258.54 kB--
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed261.99 kB--
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed272.24 kB--
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed275.68 kB--
@sentry/nextjs (client)48.9 kB--
@sentry/sveltekit (client)44.64 kB--
@sentry/node-core60.08 kB+0.01%+5 B 🔺
@sentry/node165.12 kB+0.82%+1.33 kB 🔺
@sentry/node - without tracing73.08 kB+0.02%+9 B 🔺
@sentry/aws-serverless107.24 kB+0.01%+8 B 🔺
@sentry/cloudflare (withSentry) - minified169.35 kB--
@sentry/cloudflare (withSentry)427.5 kB--

View base workflow run

@isaacs
isaacsforce-pushed the isaacs/vendor-redis branch 3 times, most recently from d150996 to de731edCompareMay 4, 2026 19:17
Base automatically changed from isaacs/vendor-redis to developMay 4, 2026 19:58
@isaacs
isaacsforce-pushed the feat/redis-diagnostic-channels branch 2 times, most recently from fdbf2b6 to 75744f4CompareMay 4, 2026 20:35
@logaretm
logaretm requested a review from isaacsMay 4, 2026 20:40
@logaretm
logaretm marked this pull request as ready for review May 4, 2026 20:40
@logaretm
logaretm requested a review from a team as a code ownerMay 4, 2026 20:40
@isaacs
isaacsforce-pushed the feat/redis-diagnostic-channels branch from 75744f4 to 22acdcdCompareMay 4, 2026 22:01
Comment threadpackages/node/src/integrations/tracing/redis/redis-dc-subscriber.ts Outdated
@isaacs
isaacsforce-pushed the feat/redis-diagnostic-channels branch 2 times, most recently from 38623e1 to f88ffb0CompareMay 4, 2026 23:07
Comment threaddev-packages/node-integration-tests/package.json
Comment threadpackages/node/src/integrations/tracing/redis/index.ts
Comment threadpackages/node/src/integrations/tracing/redis/index.ts
@logaretm
logaretmforce-pushed the feat/redis-diagnostic-channels branch from ec090bb to 340e44aCompareMay 5, 2026 20:40

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

There are 3 total unresolved issues (including 2 from previous reviews).

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 340e44a. Configure here.

Comment threadpackages/node/src/integrations/tracing/redis/index.ts
logaretmand others added 3 commits May 6, 2026 13:39
node-redis 5.12.0 publishes command, batch, and connect events via
node:diagnostics_channel. Subscribe to those channels via
@sentry/opentelemetry/tracing-channel to produce spans without IITM-
based monkey-patching, which lets the redis integration work on
runtimes that don't support IITM (Bun, Deno, Cloudflare Workers).
The existing OTel patcher is narrowed to '<5.12.0' so it does not
double-instrument when both paths are present. The DC subscription is
deferred to the next microtask so it runs after initOpenTelemetry()
sets up the Sentry context manager (required for bindStore).
logaretmand others added 6 commits May 6, 2026 13:39
node-redis sanitizeArgs replaces MGET key arguments with '?' in
the diagnostics_channel payload, so cache prefix detection cannot
match — MGET remains a plain db.redis span.
The hook unconditionally set origin to 'auto.db.otel.redis', clobbering
the diagnostic_channel origin on all DC-sourced spans. Remove the
override so each instrumentation path keeps its own origin.
…instrumentations
Move origin attribute from the shared cacheResponseHook into the
vendored OTel instrumentations so each path sets origin at span
creation time, consistent with the DC subscriber.
…nnect
node-redis eagerly creates native TracingChannels on require(), and
the DC subscriber is deferred via Promise.resolve().then(). Moving
the require inside run() after a microtick ensures the subscriber
is registered before the connect event fires.
@isaacs
isaacsforce-pushed the feat/redis-diagnostic-channels branch from 340e44a to 3a8263aCompareMay 6, 2026 20:40

@isaacsisaacs 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 (pending CI not finding anything new)

@isaacs
isaacsforce-pushed the feat/redis-diagnostic-channels branch from 3a8263a to 78b2244CompareMay 6, 2026 21:02
@isaacs
isaacs merged commit 7e49571 into developMay 6, 2026
172 checks passed
@isaacs
isaacs deleted the feat/redis-diagnostic-channels branch May 6, 2026 21:30
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@isaacs@JPeer264