Skip to content

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

Open
chargome wants to merge 5 commits into
developfrom
cg/redisio-orchestrion
Open

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

Conversation

@chargome

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

@chargome chargome self-assigned this Jun 30, 2026
chargome and 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
Member Author

bugbot run

Comment thread packages/node/src/integrations/tracing/redis/index.ts
Comment thread packages/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
Member Author

bugbot run

Comment thread packages/node/src/integrations/tracing/redis/index.ts
Comment thread packages/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
Member Author

bugbot run

@cursor cursor Bot 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 owner June 30, 2026 14:20
@chargome chargome requested review from JPeer264, andreiborza, isaacs, mydea and nicohrubec and removed request for a team, JPeer264 and mydea June 30, 2026 14:20

@cursor cursor Bot 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 thread packages/node/src/integrations/tracing/redis/index.ts
const INTEGRATION_NAME = 'IORedis' as const;

const ORIGIN = 'auto.db.orchestrion.redis';
const ATTR_DB_SYSTEM = 'db.system';

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.

can we just import these (as much as possible) from @sentry/conventions instead?

Sign up for free to 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