Skip to content

Commit aa7a30b

Browse files
chargomeclaude
andauthored
feat(server-utils): Migrate @opentelemetry/instrumentation-ioredis to orchestrion (#21849)
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 --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent efc96b1 commit aa7a30b

18 files changed

Lines changed: 687 additions & 134 deletions

File tree

dev-packages/e2e-tests/test-applications/nuxt-4-orchestrion/nuxt.config.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,16 @@ export default defineNuxtConfig({
3333
nitro: {
3434
// Nuxt's server is built by Nitro (Rollup), not Vite — so the orchestrion
3535
// code transform has to run as a Nitro Rollup plugin to reach `server/api/*`
36-
// routes. Force-bundle ONLY the instrumented deps (`mysql`) via
37-
// `externals.inline`; externalized deps are `require()`d from `node_modules`
38-
// at runtime and never pass through the transform.
36+
// routes. Force-bundle the instrumented deps via `externals.inline`;
37+
// externalized deps are `require()`d from `node_modules` at runtime and never
38+
// pass through the transform.
39+
//
40+
// `standard-as-callback` is ioredis' CJS `export default` helper used by
41+
// `connect()`. Left external, Rollup's interop resolves its `.default` to a
42+
// non-function in the bundle; inlining it alongside ioredis links the
43+
// interop consistently.
3944
externals: {
40-
inline: INSTRUMENTED_MODULE_NAMES,
45+
inline: [...INSTRUMENTED_MODULE_NAMES, 'standard-as-callback'],
4146
},
4247
rollupConfig: {
4348
plugins: [

dev-packages/e2e-tests/test-applications/nuxt-4-orchestrion/tests/db.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { expect, test } from '@playwright/test';
22
import { waitForTransaction } from '@sentry-internal/test-utils';
33

44
test('Instruments ioredis automatically', async ({ baseURL }) => {
5-
// This test works as well without orchestrion
65
const transactionEventPromise = waitForTransaction('nuxt-4-orchestrion', transactionEvent => {
76
return (
87
transactionEvent.contexts?.trace?.op === 'http.server' && transactionEvent.transaction === 'GET /api/db-ioredis'
@@ -21,7 +20,7 @@ test('Instruments ioredis automatically', async ({ baseURL }) => {
2120
expect(spans).toContainEqual(
2221
expect.objectContaining({
2322
op: 'db',
24-
origin: 'auto.db.otel.redis',
23+
origin: 'auto.db.orchestrion.redis',
2524
description: 'set test-key [1 other arguments]',
2625
status: 'ok',
2726
data: expect.objectContaining({
@@ -33,7 +32,7 @@ test('Instruments ioredis automatically', async ({ baseURL }) => {
3332
expect(spans).toContainEqual(
3433
expect.objectContaining({
3534
op: 'db',
36-
origin: 'auto.db.otel.redis',
35+
origin: 'auto.db.orchestrion.redis',
3736
description: 'get test-key',
3837
status: 'ok',
3938
data: expect.objectContaining({

dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,25 @@
11
import { afterAll, describe, expect } from 'vitest';
2+
import { isOrchestrionEnabled } from '../../../utils';
23
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';
34

45
describe('redis cache auto instrumentation', () => {
56
afterAll(() => {
67
cleanupChildProcesses();
78
});
89

10+
// Under orchestrion, ioredis <5.11 is instrumented by the diagnostics-channel
11+
// subscriber instead of the OTel monkey-patch, so ioredis span origins differ.
12+
// node-redis (redis-4/redis-5) is not ported, so those keep `auto.db.otel.redis`.
13+
const ioredisOrigin = isOrchestrionEnabled() ? 'auto.db.orchestrion.redis' : 'auto.db.otel.redis';
14+
915
describe('ioredis non-cache keys', () => {
1016
const EXPECTED_TRANSACTION = {
1117
transaction: 'Test Span',
1218
spans: expect.arrayContaining([
1319
expect.objectContaining({
1420
description: 'set test-key [1 other arguments]',
1521
op: 'db',
16-
origin: 'auto.db.otel.redis',
22+
origin: ioredisOrigin,
1723
data: expect.objectContaining({
1824
'sentry.op': 'db',
1925
'db.system': 'redis',
@@ -25,7 +31,7 @@ describe('redis cache auto instrumentation', () => {
2531
expect.objectContaining({
2632
description: 'get test-key',
2733
op: 'db',
28-
origin: 'auto.db.otel.redis',
34+
origin: ioredisOrigin,
2935
data: expect.objectContaining({
3036
'sentry.op': 'db',
3137
'db.system': 'redis',
@@ -56,9 +62,9 @@ describe('redis cache auto instrumentation', () => {
5662
expect.objectContaining({
5763
description: 'ioredis-cache:test-key',
5864
op: 'cache.put',
59-
origin: 'auto.db.otel.redis',
65+
origin: ioredisOrigin,
6066
data: expect.objectContaining({
61-
'sentry.origin': 'auto.db.otel.redis',
67+
'sentry.origin': ioredisOrigin,
6268
'db.statement': 'set ioredis-cache:test-key [1 other arguments]',
6369
'cache.key': ['ioredis-cache:test-key'],
6470
'cache.item_size': 2,
@@ -70,9 +76,9 @@ describe('redis cache auto instrumentation', () => {
7076
expect.objectContaining({
7177
description: 'ioredis-cache:test-key-set-EX',
7278
op: 'cache.put',
73-
origin: 'auto.db.otel.redis',
79+
origin: ioredisOrigin,
7480
data: expect.objectContaining({
75-
'sentry.origin': 'auto.db.otel.redis',
81+
'sentry.origin': ioredisOrigin,
7682
'db.statement': 'set ioredis-cache:test-key-set-EX [3 other arguments]',
7783
'cache.key': ['ioredis-cache:test-key-set-EX'],
7884
'cache.item_size': 2,
@@ -84,9 +90,9 @@ describe('redis cache auto instrumentation', () => {
8490
expect.objectContaining({
8591
description: 'ioredis-cache:test-key-setex',
8692
op: 'cache.put',
87-
origin: 'auto.db.otel.redis',
93+
origin: ioredisOrigin,
8894
data: expect.objectContaining({
89-
'sentry.origin': 'auto.db.otel.redis',
95+
'sentry.origin': ioredisOrigin,
9096
'db.statement': 'setex ioredis-cache:test-key-setex [2 other arguments]',
9197
'cache.key': ['ioredis-cache:test-key-setex'],
9298
'cache.item_size': 2,
@@ -98,9 +104,9 @@ describe('redis cache auto instrumentation', () => {
98104
expect.objectContaining({
99105
description: 'ioredis-cache:test-key',
100106
op: 'cache.get',
101-
origin: 'auto.db.otel.redis',
107+
origin: ioredisOrigin,
102108
data: expect.objectContaining({
103-
'sentry.origin': 'auto.db.otel.redis',
109+
'sentry.origin': ioredisOrigin,
104110
'db.statement': 'get ioredis-cache:test-key',
105111
'cache.hit': true,
106112
'cache.key': ['ioredis-cache:test-key'],
@@ -113,9 +119,9 @@ describe('redis cache auto instrumentation', () => {
113119
expect.objectContaining({
114120
description: 'ioredis-cache:unavailable-data',
115121
op: 'cache.get',
116-
origin: 'auto.db.otel.redis',
122+
origin: ioredisOrigin,
117123
data: expect.objectContaining({
118-
'sentry.origin': 'auto.db.otel.redis',
124+
'sentry.origin': ioredisOrigin,
119125
'db.statement': 'get ioredis-cache:unavailable-data',
120126
'cache.hit': false,
121127
'cache.key': ['ioredis-cache:unavailable-data'],
@@ -127,9 +133,9 @@ describe('redis cache auto instrumentation', () => {
127133
expect.objectContaining({
128134
description: 'test-key, ioredis-cache:test-key, ioredis-cache:unavailable-data',
129135
op: 'cache.get',
130-
origin: 'auto.db.otel.redis',
136+
origin: ioredisOrigin,
131137
data: expect.objectContaining({
132-
'sentry.origin': 'auto.db.otel.redis',
138+
'sentry.origin': ioredisOrigin,
133139
'db.statement': 'mget [3 other arguments]',
134140
'cache.hit': true,
135141
'cache.key': ['test-key', 'ioredis-cache:test-key', 'ioredis-cache:unavailable-data'],

dev-packages/node-integration-tests/suites/tracing/redis/test.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,27 @@
11
import { afterAll, describe, expect } from 'vitest';
2+
import { isOrchestrionEnabled } from '../../../utils';
23
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';
34

45
describe('redis auto instrumentation', () => {
56
afterAll(() => {
67
cleanupChildProcesses();
78
});
89

10+
// Under orchestrion, ioredis <5.11 is instrumented by the diagnostics-channel
11+
// subscriber instead of the OTel monkey-patch, so the span origin differs. All
12+
// other attributes are identical.
13+
const origin = isOrchestrionEnabled() ? 'auto.db.orchestrion.redis' : 'auto.db.otel.redis';
14+
915
const EXPECTED_TRANSACTION = {
1016
transaction: 'Test Span',
1117
spans: expect.arrayContaining([
1218
expect.objectContaining({
1319
description: 'set test-key [1 other arguments]',
1420
op: 'db',
15-
origin: 'auto.db.otel.redis',
21+
origin,
1622
data: expect.objectContaining({
1723
'sentry.op': 'db',
18-
'sentry.origin': 'auto.db.otel.redis',
24+
'sentry.origin': origin,
1925
'db.system': 'redis',
2026
'net.peer.name': 'localhost',
2127
'net.peer.port': 6380,
@@ -25,10 +31,10 @@ describe('redis auto instrumentation', () => {
2531
expect.objectContaining({
2632
description: 'get test-key',
2733
op: 'db',
28-
origin: 'auto.db.otel.redis',
34+
origin,
2935
data: expect.objectContaining({
3036
'sentry.op': 'db',
31-
'sentry.origin': 'auto.db.otel.redis',
37+
'sentry.origin': origin,
3238
'db.system': 'redis',
3339
'net.peer.name': 'localhost',
3440
'net.peer.port': 6380,
@@ -40,10 +46,10 @@ describe('redis auto instrumentation', () => {
4046
description: 'incr test-key',
4147
op: 'db',
4248
status: 'internal_error',
43-
origin: 'auto.db.otel.redis',
49+
origin,
4450
data: expect.objectContaining({
4551
'sentry.op': 'db',
46-
'sentry.origin': 'auto.db.otel.redis',
52+
'sentry.origin': origin,
4753
'db.system': 'redis',
4854
'net.peer.name': 'localhost',
4955
'net.peer.port': 6380,
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import type { Span } from '@sentry/core';
2+
import {
3+
SEMANTIC_ATTRIBUTE_CACHE_HIT,
4+
SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE,
5+
SEMANTIC_ATTRIBUTE_CACHE_KEY,
6+
SEMANTIC_ATTRIBUTE_SENTRY_OP,
7+
spanToJSON,
8+
truncate,
9+
} from '@sentry/core';
10+
import type { IORedisCommandArgs } from '../../../utils/redisCache';
11+
import {
12+
calculateCacheItemSize,
13+
GET_COMMANDS,
14+
getCacheKeySafely,
15+
getCacheOperation,
16+
isInCommands,
17+
shouldConsiderForCache,
18+
} from '../../../utils/redisCache';
19+
import type { IORedisResponseCustomAttributeFunction } from './vendored/types';
20+
21+
// This module deliberately does NOT import the vendored OTel `IORedisInstrumentation`/
22+
// `RedisInstrumentation`, so the orchestrion opt-in can pull `cacheResponseHook`
23+
// without dragging the OTel redis instrumentation into its module graph.
24+
25+
export interface RedisOptions {
26+
/**
27+
* Define cache prefixes for cache keys that should be captured as a cache span.
28+
*
29+
* Setting this to, for example, `['user:']` will capture cache keys that start with `user:`.
30+
*/
31+
cachePrefixes?: string[];
32+
/**
33+
* Maximum length of the cache key added to the span description. If the key exceeds this length, it will be truncated.
34+
*
35+
* Passing `0` will use the full cache key without truncation.
36+
*
37+
* By default, the full cache key is used.
38+
*/
39+
maxCacheKeyLength?: number;
40+
}
41+
42+
/* Only exported for testing purposes */
43+
export let _redisOptions: RedisOptions = {};
44+
45+
/** Set the options consumed by {@link cacheResponseHook}. */
46+
export function setRedisOptions(options: RedisOptions): void {
47+
_redisOptions = options;
48+
}
49+
50+
/* Only exported for testing purposes */
51+
export const cacheResponseHook: IORedisResponseCustomAttributeFunction = (
52+
span: Span,
53+
redisCommand: string,
54+
cmdArgs: IORedisCommandArgs,
55+
response: unknown,
56+
) => {
57+
const safeKey = getCacheKeySafely(redisCommand, cmdArgs);
58+
const cacheOperation = getCacheOperation(redisCommand);
59+
60+
if (
61+
!safeKey ||
62+
!cacheOperation ||
63+
!_redisOptions.cachePrefixes ||
64+
!shouldConsiderForCache(redisCommand, safeKey, _redisOptions.cachePrefixes)
65+
) {
66+
// not relevant for cache
67+
return;
68+
}
69+
70+
// otel/ioredis seems to be using the old standard, as there was a change to those params: https://github.com/open-telemetry/opentelemetry-specification/issues/3199
71+
// We are using params based on the docs: https://opentelemetry.io/docs/specs/semconv/attributes-registry/network/
72+
// Fall back to stable semconv attributes (server.address/server.port) when
73+
// old-semconv ones are absent, eg OTEL_SEMCONV_STABILITY_OPT_IN=database
74+
// set for node-redis v4/v5.
75+
const spanData = spanToJSON(span).data;
76+
const networkPeerAddress = spanData['net.peer.name'] ?? spanData['server.address'];
77+
const networkPeerPort = spanData['net.peer.port'] ?? spanData['server.port'];
78+
if (networkPeerPort && networkPeerAddress) {
79+
span.setAttributes({ 'network.peer.address': networkPeerAddress, 'network.peer.port': networkPeerPort });
80+
}
81+
82+
const cacheItemSize = calculateCacheItemSize(response);
83+
84+
if (cacheItemSize) {
85+
span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE, cacheItemSize);
86+
}
87+
88+
if (isInCommands(GET_COMMANDS, redisCommand) && cacheItemSize !== undefined) {
89+
span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, cacheItemSize > 0);
90+
}
91+
92+
span.setAttributes({
93+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: cacheOperation,
94+
[SEMANTIC_ATTRIBUTE_CACHE_KEY]: safeKey,
95+
});
96+
97+
// todo: change to string[] once EAP supports it
98+
const spanDescription = safeKey.join(', ');
99+
100+
span.updateName(
101+
_redisOptions.maxCacheKeyLength ? truncate(spanDescription, _redisOptions.maxCacheKeyLength) : spanDescription,
102+
);
103+
};

0 commit comments

Comments
 (0)