Skip to content

feat(server-utils): Migrate @opentelemetry/instrumentation-ioredis to orchestrion - #21849

Merged
chargome merged 12 commits into
developfrom
cg/redisio-orchestrion
Jul 3, 2026
Merged

feat(server-utils): Migrate @opentelemetry/instrumentation-ioredis to orchestrion#21849
chargome merged 12 commits into
developfrom
cg/redisio-orchestrion

Conversation

@chargome

@chargomechargome commented Jun 29, 2026

Copy link
Copy Markdown
Member

Migrates ioredis instrumentation from the otel to orchestrion diagnostics-channel injection.

  • Orchestrion covers ioredis <5.11.0; >=5.11.0 keeps using ioredis' own ioredis:* diagnostics_channel (unchanged).
  • New subscriber in @sentry/server-utils produces the same db spans as before with raw args redacted via the shared defaultDbStatementSerializer.
  • The OTel Redis integration stays in place (it still handles node-redis and ioredis ≥5.11) — only its ioredis monkey-patch is turned off when injection is on, except on Node <18.19, where orchestrion isn't available.

closes#20755

@chargomechargome self-assigned this Jun 30, 2026
chargomeand others added 2 commits June 30, 2026 10:11
Resolve conflict in experimentalUseDiagnosticsChannelInjection.ts: keep develop's
derive-from-names for the 1:1 channel replacements (mysql, lru-memoizer) and add
ioredisChannelIntegration separately, kept out of replacedOtelIntegrationNames
since it only supersedes the ioredis sub-part of the composite OTel 'Redis'.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rvives bundling
ioredis' connect() calls standard-as-callback's CJS default export; left
external it resolves to a non-function in the nitro/Rollup bundle. Inline it
alongside ioredis so the interop links consistently.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chargome

Copy link
Copy Markdown
MemberAuthor

bugbot run

Comment threadpackages/node/src/integrations/tracing/redis/index.ts
Comment threadpackages/server-utils/src/integrations/tracing-channel/ioredis.ts Outdated
- Keep the OTel ioredis monkey-patch on Node without tracingChannel (<18.19)
even when injection is opted in, since orchestrion can't run there — otherwise
ioredis <5.11.0 would lose tracing entirely.
- Defer the orchestrion ioredis bindTracingChannelToSpan calls via
waitForTracingChannelBinding, matching the native redis diagnostics-channel
subscriber, so the async-context binding registered by initOpenTelemetry() is
available before binding.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chargome

Copy link
Copy Markdown
MemberAuthor

bugbot run

Comment threadpackages/node/src/integrations/tracing/redis/index.ts
Comment threadpackages/node/src/sdk/experimentalUseDiagnosticsChannelInjection.ts Outdated
…nstrumentation
The orchestrion opt-in imports cacheResponseHook; importing it from redis/index
transitively pulled the vendored OTel IORedisInstrumentation/RedisInstrumentation
into the opt-in module graph. Move cacheResponseHook + _redisOptions into
redis/cache.ts (no OTel instrumentation imports) so apps using injection only for
mysql/lru-memoizer don't bundle the redis OTel instrumentation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chargome

Copy link
Copy Markdown
MemberAuthor

bugbot run

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit da258e2. Configure here.

@chargome
chargome marked this pull request as ready for review June 30, 2026 14:20
@chargome
chargome requested a review from a team as a code ownerJune 30, 2026 14:20
@chargome
chargome requested review from JPeer264, andreiborza, isaacs, mydea and nicohrubec and removed request for a team, JPeer264 and mydeaJune 30, 2026 14:20

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

Comment threadpackages/node/src/integrations/tracing/redis/index.ts
Comment threadpackages/server-utils/src/integrations/tracing-channel/ioredis.ts Outdated
chargomeand others added 2 commits July 2, 2026 14:56
develop now runs all node-integration tests a second time with INJECT_ORCHESTRION
(#21911). Under orchestrion, ioredis <5.11 is instrumented by the diagnostics-channel
subscriber, so its span origin is 'auto.db.orchestrion.redis' instead of the OTel
monkey-patch's 'auto.db.otel.redis'. Branch the expected ioredis origin on
isOrchestrionEnabled(); node-redis (redis-4/redis-5) is not ported and keeps the OTel
origin. All other span attributes are identical.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment threadpackages/node/src/integrations/tracing/redis/vendored/redis-common.ts Outdated
Comment threadpackages/node/src/sdk/experimentalUseDiagnosticsChannelInjection.ts Outdated
Comment threadpackages/server-utils/src/integrations/tracing-channel/ioredis.ts Outdated
Comment threadpackages/server-utils/src/integrations/tracing-channel/ioredis.ts Outdated
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chargome
chargomeforce-pushed the cg/redisio-orchestrion branch from 4509095 to b755ca1CompareJuly 2, 2026 14:08

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

…rion
# Conflicts:
#	packages/node/src/sdk/experimentalUseDiagnosticsChannelInjection.ts
#	packages/server-utils/src/orchestrion/channels.ts
#	packages/server-utils/src/orchestrion/config.ts
#	packages/server-utils/src/orchestrion/index.ts
…rion
# Conflicts:
#	packages/server-utils/src/orchestrion/config.ts
#	packages/server-utils/src/orchestrion/index.ts
…rion
# Conflicts:
#	packages/server-utils/src/orchestrion/index.ts
@chargome
chargome merged commit aa7a30b into developJul 3, 2026
387 of 389 checks passed
@chargome
chargome deleted the cg/redisio-orchestrion branch July 3, 2026 09:28
chargome added a commit that referenced this pull request Jul 8, 2026
… orchestrion (#21925)
Migrates `node-redis` from the OTel monkey-patch to orchestrion
diagnostics-channel injection. Builds on the ioredis migration (#21849).
- New `RedisChannel` subscriber in `@sentry/server-utils` produces the
same db spans as the OTel `RedisInstrumentation`: single commands,
`connect`, and multi/pipeline batches.
- Batch spans mirror the native `node-redis:batch` shape.
- node-redis ≥5.12 keeps using its own `node-redis:*`
diagnostics_channel (unchanged).
- When diagnostics-channel injection is opted in, the OTel
`RedisInstrumentation` monkey-patch is fully gated off; the composite
`Redis` integration stays for the native-channel subscriber/
Note: When we eventually delete the OTel Redis integration, we need to
move the native diagnostics-channel subscriber
(`subscribeRedisDiagnosticChannels`, for node-redis ≥5.12 / ioredis
≥5.11) into the orchestrion-owned side so it keeps running, then
collapse everything into one default `Redis` integration.
closes#20765
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

Rewrite @opentelemetry/instrumentation-ioredis to orchestrion

3 participants

@chargome@mydea@nicohrubec