Skip to content

Commit b25ba8d

Browse files
committed
fix(instrumentation-redis): add spans for cluster multi/transaction commands
When using createCluster, commands run inside multi().exec() were not producing spans. This fix patches cluster/index.js to store options on the multi command object, and cluster/multi-command.js to wrap addCommand and exec, mirroring the existing standalone client patching. Fixes #3369
1 parent 5e30713 commit b25ba8d

2 files changed

Lines changed: 229 additions & 2 deletions

File tree

packages/instrumentation-redis/src/v4-v5/instrumentation.ts

Lines changed: 106 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,14 +242,88 @@ export class RedisInstrumentationV4_V5 extends InstrumentationBase<RedisInstrume
242242
}
243243
);
244244

245+
const clusterIndexModule = new InstrumentationNodeModuleFile(
246+
`${basePackageName}/dist/lib/cluster/index.js`,
247+
['^1.0.0', '^5.0.0'],
248+
(moduleExports: any) => {
249+
const redisClusterPrototype = moduleExports?.default?.prototype;
250+
251+
// Patch MULTI to store cluster options on the multi command object
252+
// so that _traceClientCommand can read connection attributes later
253+
if (redisClusterPrototype?.MULTI) {
254+
if (isWrapped(redisClusterPrototype?.MULTI)) {
255+
this._unwrap(redisClusterPrototype, 'MULTI');
256+
}
257+
this._wrap(
258+
redisClusterPrototype,
259+
'MULTI',
260+
this._getPatchRedisClusterMulti()
261+
);
262+
}
263+
264+
return moduleExports;
265+
},
266+
(moduleExports: any) => {
267+
const redisClusterPrototype = moduleExports?.default?.prototype;
268+
if (isWrapped(redisClusterPrototype?.MULTI)) {
269+
this._unwrap(redisClusterPrototype, 'MULTI');
270+
}
271+
}
272+
);
273+
274+
const clusterMultiCommanderModule = new InstrumentationNodeModuleFile(
275+
`${basePackageName}/dist/lib/cluster/multi-command.js`,
276+
['^1.0.0', '^5.0.0'],
277+
(moduleExports: any) => {
278+
const redisClusterMultiCommandPrototype =
279+
moduleExports?.default?.prototype;
280+
281+
if (isWrapped(redisClusterMultiCommandPrototype?.exec)) {
282+
this._unwrap(redisClusterMultiCommandPrototype, 'exec');
283+
}
284+
this._wrap(
285+
redisClusterMultiCommandPrototype,
286+
'exec',
287+
this._getPatchMultiCommandsExec(false)
288+
);
289+
290+
if (isWrapped(redisClusterMultiCommandPrototype?.addCommand)) {
291+
this._unwrap(redisClusterMultiCommandPrototype, 'addCommand');
292+
}
293+
this._wrap(
294+
redisClusterMultiCommandPrototype,
295+
'addCommand',
296+
this._getPatchClusterMultiCommandsAddCommand()
297+
);
298+
299+
return moduleExports;
300+
},
301+
(moduleExports: any) => {
302+
const redisClusterMultiCommandPrototype =
303+
moduleExports?.default?.prototype;
304+
if (isWrapped(redisClusterMultiCommandPrototype?.exec)) {
305+
this._unwrap(redisClusterMultiCommandPrototype, 'exec');
306+
}
307+
if (isWrapped(redisClusterMultiCommandPrototype?.addCommand)) {
308+
this._unwrap(redisClusterMultiCommandPrototype, 'addCommand');
309+
}
310+
}
311+
);
312+
245313
return new InstrumentationNodeModuleDefinition(
246314
basePackageName,
247315
['^1.0.0', '^5.0.0'],
248316
(moduleExports: any) => {
249317
return moduleExports;
250318
},
251319
() => {},
252-
[commanderModuleFile, multiCommanderModule, clientIndexModule]
320+
[
321+
commanderModuleFile,
322+
multiCommanderModule,
323+
clientIndexModule,
324+
clusterIndexModule,
325+
clusterMultiCommanderModule,
326+
]
253327
);
254328
}
255329

@@ -331,6 +405,36 @@ export class RedisInstrumentationV4_V5 extends InstrumentationBase<RedisInstrume
331405
};
332406
}
333407

408+
private _getPatchClusterMultiCommandsAddCommand() {
409+
const plugin = this;
410+
return function addCommandWrapper(original: Function) {
411+
return function addCommandPatch(
412+
this: any,
413+
firstKeyOrArgs: any,
414+
isReadonly: any,
415+
args: Array<string | Buffer>
416+
) {
417+
// Cluster addCommand is called in two ways:
418+
// 1. Internally by named commands: (firstKey, isReadonly, args, transformReply)
419+
// 2. Directly by user via .addCommand([...]): (args) - single array argument
420+
const redisArgs = Array.isArray(firstKeyOrArgs)
421+
? firstKeyOrArgs
422+
: args;
423+
return plugin._traceClientCommand(original, this, arguments, redisArgs);
424+
};
425+
};
426+
}
427+
private _getPatchRedisClusterMulti() {
428+
return function multiPatchWrapper(original: Function) {
429+
return function multiPatch(this: any) {
430+
const multiRes = original.apply(this, arguments);
431+
// Store cluster options so _traceClientCommand can read connection attributes
432+
multiRes[MULTI_COMMAND_OPTIONS] = this._options;
433+
return multiRes;
434+
};
435+
};
436+
}
437+
334438
private _getPatchRedisClientMulti() {
335439
return function multiPatchWrapper(original: Function) {
336440
return function multiPatch(this: any) {
@@ -541,4 +645,4 @@ export class RedisInstrumentationV4_V5 extends InstrumentationBase<RedisInstrume
541645
}
542646
span.end();
543647
}
544-
}
648+
}
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
import {
17+
getTestSpans,
18+
registerInstrumentationTesting,
19+
} from '@opentelemetry/contrib-test-utils';
20+
import { RedisInstrumentation } from '../../src/index';
21+
import * as assert from 'assert';
22+
import { context } from '@opentelemetry/api';
23+
import { suppressTracing } from '@opentelemetry/core';
24+
import { SpanStatusCode } from '@opentelemetry/api';
25+
import { ATTR_DB_STATEMENT } from '../../src/semconv';
26+
import {
27+
ATTR_DB_QUERY_TEXT,
28+
ATTR_DB_OPERATION_NAME,
29+
} from '@opentelemetry/semantic-conventions';
30+
process.env.OTEL_SEMCONV_STABILITY_OPT_IN = 'database/dup';
31+
registerInstrumentationTesting(new RedisInstrumentation());
32+
import { createCluster } from 'redis';
33+
import type { RedisClusterType } from 'redis';
34+
const shouldTest = process.env.RUN_REDIS_CLUSTER_TESTS;
35+
const clusterRootNodes = [
36+
{ url: `redis://${process.env.OPENTELEMETRY_REDIS_CLUSTER_HOST || 'localhost'}:${process.env.OPENTELEMETRY_REDIS_CLUSTER_PORT || 6379}` },
37+
];
38+
describe('redis v4-v5 cluster', () => {
39+
before(function () {
40+
if (!shouldTest) {
41+
this.test!.parent!.pending = true;
42+
this.skip();
43+
}
44+
});
45+
let client: RedisClusterType;
46+
beforeEach(async () => {
47+
client = createCluster({ rootNodes: clusterRootNodes });
48+
await context.with(suppressTracing(context.active()), async () => {
49+
await client.connect();
50+
});
51+
});
52+
afterEach(async () => {
53+
await client?.disconnect();
54+
});
55+
describe('cluster multi (transaction) commands', () => {
56+
it('should produce spans for commands run inside multi().exec() on a cluster', async () => {
57+
const key = 'test-cluster-multi-key';
58+
const [zremResult, zcardResult] = await client
59+
.multi()
60+
.ZREMRANGEBYSCORE(key, '-inf', Date.now())
61+
.ZCARD(key)
62+
.execTyped();
63+
assert.strictEqual(typeof zremResult, 'number');
64+
assert.strictEqual(typeof zcardResult, 'number');
65+
const spans = getTestSpans();
66+
const spanNames = spans.map(s => s.name);
67+
const zremSpan = spans.find(s => s.name === 'redis-ZREMRANGEBYSCORE');
68+
const zcardSpan = spans.find(s => s.name === 'redis-ZCARD');
69+
assert.ok(zremSpan, `Expected redis-ZREMRANGEBYSCORE span, got: ${spanNames.join(', ')}`);
70+
assert.ok(zcardSpan, `Expected redis-ZCARD span, got: ${spanNames.join(', ')}`);
71+
assert.strictEqual(zremSpan.attributes['db.system'], 'redis');
72+
assert.ok((zremSpan.attributes[ATTR_DB_STATEMENT] as string)?.startsWith('ZREMRANGEBYSCORE'));
73+
assert.ok((zremSpan.attributes[ATTR_DB_QUERY_TEXT] as string)?.startsWith('ZREMRANGEBYSCORE'));
74+
assert.strictEqual(zremSpan.attributes[ATTR_DB_OPERATION_NAME], 'MULTI');
75+
assert.strictEqual(zcardSpan.attributes['db.system'], 'redis');
76+
assert.ok((zcardSpan.attributes[ATTR_DB_STATEMENT] as string)?.startsWith('ZCARD'));
77+
assert.ok((zcardSpan.attributes[ATTR_DB_QUERY_TEXT] as string)?.startsWith('ZCARD'));
78+
assert.strictEqual(zcardSpan.attributes[ATTR_DB_OPERATION_NAME], 'MULTI');
79+
});
80+
it('should produce spans for commands run inside multi().exec() with generic addCommand on a cluster', async () => {
81+
const key = 'test-cluster-addcommand-key';
82+
const [setReply] = await client
83+
.multi()
84+
.addCommand(key, false, ['SET', key, 'value'])
85+
.exec();
86+
assert.strictEqual(setReply, 'OK');
87+
const spans = getTestSpans();
88+
const setSpan = spans.find(s => s.name === 'redis-SET');
89+
assert.ok(setSpan, 'Expected redis-SET span');
90+
assert.strictEqual(setSpan.attributes['db.system'], 'redis');
91+
assert.ok((setSpan.attributes[ATTR_DB_STATEMENT] as string)?.startsWith('SET'));
92+
});
93+
it('should handle errors in cluster multi commands', async () => {
94+
const key = 'test-cluster-error-key';
95+
await client.set(key, 'string-value');
96+
97+
try {
98+
await client.multi().set(key, 'value').incr(key).exec();
99+
} catch (err: any) {
100+
101+
}
102+
const spans = getTestSpans();
103+
const incrSpan = spans.find(s => s.name === 'redis-INCR');
104+
assert.ok(incrSpan, 'Expected redis-INCR span');
105+
assert.strictEqual(incrSpan.status.code, SpanStatusCode.ERROR);
106+
});
107+
});
108+
describe('cluster regular commands', () => {
109+
it('should produce spans for regular cluster commands', async () => {
110+
const key = 'test-cluster-regular-key';
111+
await client.set(key, 'value');
112+
const value = await client.get(key);
113+
assert.strictEqual(value, 'value');
114+
const spans = getTestSpans();
115+
const setSpan = spans.find(s => s.name === 'redis-SET');
116+
const getSpan = spans.find(s => s.name === 'redis-GET');
117+
assert.ok(setSpan, 'Expected redis-SET span');
118+
assert.ok(getSpan, 'Expected redis-GET span');
119+
assert.strictEqual(setSpan.attributes['db.system'], 'redis');
120+
assert.strictEqual(getSpan.attributes['db.system'], 'redis');
121+
});
122+
});
123+
});

0 commit comments

Comments
 (0)