feat(node): use diagnostics_channel for redis >= 5.12.0#20573
Merged
feat(node): use diagnostics_channel for redis >= 5.12.0#20573
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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.
| File | Description |
|---|---|
| packages/node/src/integrations/tracing/redis/vendored/redis-instrumentation.ts | Narrows the v5 IITM instrumentation version range to stop before 5.12.0. |
| packages/node/src/integrations/tracing/redis/redis-dc-subscriber.ts | New diagnostics_channel subscriber which creates/ends spans for command/batch/connect events and runs the responseHook. |
| packages/node/src/integrations/tracing/redis/index.ts | Wires the subscriber into Redis instrumentation (deferred to next tick). |
| packages/node/rollup.npm.config.mjs | Externalizes @sentry/opentelemetry/* imports so subpath entrypoints aren’t bundled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
size-limit report 📦
|
isaacs
force-pushed
the
isaacs/vendor-redis
branch
3 times, most recently
from
d150996 to
de731ed
Compare
isaacs
force-pushed
the
feat/redis-diagnostic-channels
branch
2 times, most recently
from
fdbf2b6 to
75744f4
Compare
isaacs
force-pushed
the
feat/redis-diagnostic-channels
branch
from
75744f4 to
22acdcd
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
isaacs
force-pushed
the
feat/redis-diagnostic-channels
branch
2 times, most recently
from
38623e1 to
f88ffb0
Compare
JPeer264
reviewed
May 5, 2026
logaretm
force-pushed
the
feat/redis-diagnostic-channels
branch
from
ec090bb to
340e44a
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ 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.
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).
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
force-pushed
the
feat/redis-diagnostic-channels
branch
from
340e44a to
3a8263a
Compare
isaacs
approved these changes
May 6, 2026
Member
isaacs
left a comment
isaacs
left a comment
There was a problem hiding this comment.
LGTM (pending CI not finding anything new)
isaacs
force-pushed
the
feat/redis-diagnostic-channels
branch
from
3a8263a to
78b2244
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #20510 and adds tracing channels subscribers via
node:diagnostics_channel.The module patchers are narrowed to
>=5.0.0 <5.12.0while 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.