Skip to content

Commit 7e49571

Browse files
logaretmisaacs
logaretm
and
isaacs
authored
feat(node): use diagnostics_channel for redis >= 5.12.0 (#20573)
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. --------- Co-authored-by: isaacs <i@izs.me>
1 parent a8ab715 commit 7e49571

11 files changed

Lines changed: 637 additions & 8 deletions

File tree

‎dev-packages/node-integration-tests/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
"prisma": "6.15.0",
8181
"proxy": "^2.1.1",
8282
"redis-4": "npm:redis@^4.6.14",
83+
"redis-5": "npm:redis@^5.12.0",
8384
"reflect-metadata": "0.2.1",
8485
"rxjs": "^7.8.2",
8586
"tedious": "^19.2.1",

‎dev-packages/node-integration-tests/suites/tracing/redis-dc/docker-compose.yml‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
version: '3.9'
2+
3+
services:
4+
db:
5+
image: redis:latest
6+
restart: always
7+
container_name: integration-tests-redis-dc
8+
ports:
9+
- '6379:6379'
10+
healthcheck:
11+
test: ['CMD-SHELL', 'redis-cli ping | grep -q PONG']
12+
interval: 2s
13+
timeout: 3s
14+
retries: 30
15+
start_period: 5s

‎dev-packages/node-integration-tests/suites/tracing/redis-dc/scenario-redis-5.js‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
const { loggingTransport } = require('@sentry-internal/node-integration-tests');
2+
const Sentry = require('@sentry/node');
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
transport: loggingTransport,
9+
integrations: [Sentry.redisIntegration({ cachePrefixes: ['dc-cache:'] })],
10+
});
11+
12+
// Stop the process from exiting before the transaction is sent
13+
setInterval(() => {}, 1000);
14+
15+
async function run() {
16+
// Yield a microtick so the DC subscriber (deferred via Promise.resolve().then)
17+
// is registered before node-redis eagerly creates its native TracingChannels on require().
18+
await Promise.resolve();
19+
20+
const { createClient } = require('redis-5');
21+
const redisClient = await createClient({ socket: { host: '127.0.0.1', port: 6379 } }).connect();
22+
23+
await Sentry.startSpan(
24+
{
25+
name: 'Test Span Redis 5 DC',
26+
op: 'test-span-redis-5-dc',
27+
},
28+
async () => {
29+
try {
30+
await redisClient.set('dc-test-key', 'test-value');
31+
await redisClient.set('dc-cache:test-key', 'test-value');
32+
33+
await redisClient.set('dc-cache:test-key-ex', 'test-value', { EX: 10 });
34+
35+
await redisClient.get('dc-test-key');
36+
await redisClient.get('dc-cache:test-key');
37+
await redisClient.get('dc-cache:unavailable-data');
38+
39+
await redisClient.mGet(['dc-test-key', 'dc-cache:test-key', 'dc-cache:unavailable-data']);
40+
} finally {
41+
await redisClient.disconnect();
42+
}
43+
},
44+
);
45+
}
46+
47+
run();

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

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
import { afterAll, describe, expect, test } from 'vitest';
2+
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
3+
4+
describe('redis v5 diagnostics_channel auto instrumentation', () => {
5+
afterAll(() => {
6+
cleanupChildProcesses();
7+
});
8+
9+
test('should create spans for redis v5 commands via diagnostics_channel', { timeout: 60_000 }, async () => {
10+
const EXPECTED_TRANSACTION = {
11+
transaction: 'Test Span Redis 5 DC',
12+
spans: expect.arrayContaining([
13+
expect.objectContaining({
14+
op: 'db.redis',
15+
origin: 'auto.db.redis.diagnostic_channel',
16+
data: expect.objectContaining({
17+
'sentry.op': 'db.redis',
18+
'sentry.origin': 'auto.db.redis.diagnostic_channel',
19+
'db.system': 'redis',
20+
'db.statement': 'SET dc-test-key [1 other arguments]',
21+
}),
22+
}),
23+
// cache SET: span name updated to key by cacheResponseHook
24+
expect.objectContaining({
25+
description: 'dc-cache:test-key',
26+
op: 'cache.put',
27+
origin: 'auto.db.redis.diagnostic_channel',
28+
data: expect.objectContaining({
29+
'sentry.origin': 'auto.db.redis.diagnostic_channel',
30+
'db.statement': 'SET dc-cache:test-key [1 other arguments]',
31+
'cache.key': ['dc-cache:test-key'],
32+
'cache.item_size': 2,
33+
}),
34+
}),
35+
// cache SET with EX option: redis v5 sends SET key value EX 10 as the command
36+
expect.objectContaining({
37+
description: 'dc-cache:test-key-ex',
38+
op: 'cache.put',
39+
origin: 'auto.db.redis.diagnostic_channel',
40+
data: expect.objectContaining({
41+
'sentry.origin': 'auto.db.redis.diagnostic_channel',
42+
'db.statement': 'SET dc-cache:test-key-ex [3 other arguments]',
43+
'cache.key': ['dc-cache:test-key-ex'],
44+
'cache.item_size': 2,
45+
}),
46+
}),
47+
expect.objectContaining({
48+
op: 'db.redis',
49+
origin: 'auto.db.redis.diagnostic_channel',
50+
data: expect.objectContaining({
51+
'sentry.op': 'db.redis',
52+
'sentry.origin': 'auto.db.redis.diagnostic_channel',
53+
'db.system': 'redis',
54+
'db.statement': 'GET dc-test-key',
55+
}),
56+
}),
57+
// cache GET (hit)
58+
expect.objectContaining({
59+
description: 'dc-cache:test-key',
60+
op: 'cache.get',
61+
origin: 'auto.db.redis.diagnostic_channel',
62+
data: expect.objectContaining({
63+
'sentry.origin': 'auto.db.redis.diagnostic_channel',
64+
'db.statement': 'GET dc-cache:test-key',
65+
'cache.hit': true,
66+
'cache.key': ['dc-cache:test-key'],
67+
'cache.item_size': 10,
68+
}),
69+
}),
70+
// cache GET (miss)
71+
expect.objectContaining({
72+
description: 'dc-cache:unavailable-data',
73+
op: 'cache.get',
74+
origin: 'auto.db.redis.diagnostic_channel',
75+
data: expect.objectContaining({
76+
'sentry.origin': 'auto.db.redis.diagnostic_channel',
77+
'db.statement': 'GET dc-cache:unavailable-data',
78+
'cache.hit': false,
79+
'cache.key': ['dc-cache:unavailable-data'],
80+
}),
81+
}),
82+
// MGET: node-redis sanitizes args for diagnostics_channel (keys become '?'),
83+
// so cache detection cannot match prefixes — remains a plain db.redis span.
84+
expect.objectContaining({
85+
op: 'db.redis',
86+
origin: 'auto.db.redis.diagnostic_channel',
87+
data: expect.objectContaining({
88+
'sentry.op': 'db.redis',
89+
'sentry.origin': 'auto.db.redis.diagnostic_channel',
90+
'db.system': 'redis',
91+
'db.statement': 'MGET [3 other arguments]',
92+
}),
93+
}),
94+
]),
95+
};
96+
97+
// node-redis emits a node-redis:connect DC event for the initial connection.
98+
// That fires before startSpan so it arrives as the first envelope.
99+
const EXPECTED_CONNECT = {
100+
transaction: 'redis-connect',
101+
};
102+
103+
await createRunner(__dirname, 'scenario-redis-5.js')
104+
.withDockerCompose({ workingDirectory: [__dirname] })
105+
.expect({ transaction: EXPECTED_CONNECT })
106+
.expect({ transaction: EXPECTED_TRANSACTION })
107+
.start()
108+
.completed();
109+
});
110+
});

‎packages/node/rollup.npm.config.mjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export default [
66
makeBaseNPMConfig({
77
entrypoints: ['src/index.ts', 'src/init.ts', 'src/preload.ts'],
88
packageSpecificConfig: {
9+
external: [/^@sentry\/opentelemetry/],
910
output: {
1011
// set exports to 'named' or 'auto' so that rollup doesn't warn
1112
exports: 'named',

‎packages/node/src/integrations/tracing/redis/index.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import {
66
SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE,
77
SEMANTIC_ATTRIBUTE_CACHE_KEY,
88
SEMANTIC_ATTRIBUTE_SENTRY_OP,
9-
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
109
spanToJSON,
1110
truncate,
1211
} from '@sentry/core';
@@ -23,6 +22,7 @@ import {
2322
import type { IORedisResponseCustomAttributeFunction } from './vendored/types';
2423
import { IORedisInstrumentation } from './vendored/ioredis-instrumentation';
2524
import { RedisInstrumentation } from './vendored/redis-instrumentation';
25+
import { subscribeRedisDiagnosticChannels } from './redis-dc-subscriber';
2626

2727
interface RedisOptions {
2828
/**
@@ -53,8 +53,6 @@ export const cacheResponseHook: IORedisResponseCustomAttributeFunction = (
5353
cmdArgs: IORedisCommandArgs,
5454
response: unknown,
5555
) => {
56-
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.redis');
57-
5856
const safeKey = getCacheKeySafely(redisCommand, cmdArgs);
5957
const cacheOperation = getCacheOperation(redisCommand);
6058

@@ -120,6 +118,11 @@ export const instrumentRedis = Object.assign(
120118
(): void => {
121119
instrumentIORedis();
122120
instrumentRedisModule();
121+
// node-redis >= 5.12.0 publishes via diagnostics_channel. The subscriber uses
122+
// `@sentry/opentelemetry/tracing-channel`, which needs the Sentry OTel context manager
123+
// to be registered before it can `bindStore`. `initOpenTelemetry()` runs after integration
124+
// `setupOnce`, so defer to the next tick.
125+
void Promise.resolve().then(() => subscribeRedisDiagnosticChannels(cacheResponseHook));
123126

124127
// todo: implement them gradually
125128
// new LegacyRedisInstrumentation({}),

0 commit comments

Comments
 (0)