Skip to content

Commit 7814781

Browse files
committed
fix single arg commands
1 parent 7fcfa91 commit 7814781

File tree

3 files changed

+54
-26
lines changed

3 files changed

+54
-26
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ let _redisOptions: RedisOptions = {};
2828
export const instrumentRedis = generateInstrumentOnce(INTEGRATION_NAME, () => {
2929
return new IORedisInstrumentation({
3030
responseHook: (span, redisCommand, cmdArgs, response) => {
31-
const safeKey = getCacheKeySafely(cmdArgs);
31+
const safeKey = getCacheKeySafely(redisCommand, cmdArgs);
3232

3333
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.redis');
3434

packages/node/src/utils/redisCache.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import type { CommandArgs as IORedisCommandArgs } from '@opentelemetry/instrumentation-ioredis';
22

3+
const SINGLE_ARG_COMMANDS = ['get', 'set', 'setex'];
4+
35
export const GET_COMMANDS = ['get', 'mget'];
46
export const SET_COMMANDS = ['set' /* todo: 'setex' */];
57
// todo: del, expire
@@ -24,28 +26,32 @@ function keyHasPrefix(key: string, prefixes: string[]): boolean {
2426
}
2527

2628
/** Safely converts a redis key to a string (comma-separated if there are multiple keys) */
27-
export function getCacheKeySafely(cmdArgs: IORedisCommandArgs): string {
29+
export function getCacheKeySafely(redisCommand: string, cmdArgs: IORedisCommandArgs): string {
2830
try {
2931
if (cmdArgs.length === 0) {
3032
return '';
3133
}
3234

33-
const keys: string[] = [];
35+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
36+
const joinArgsWithComma = (acc: string, currArg: string | Buffer | number | any[]): string =>
37+
acc.length === 0 ? processArg(currArg) : `${acc}, ${processArg(currArg)}`;
3438

3539
// eslint-disable-next-line @typescript-eslint/no-explicit-any
36-
const processArg = (arg: string | Buffer | number | any[]): void => {
40+
const processArg = (arg: string | Buffer | number | any[]): string => {
3741
if (typeof arg === 'string' || typeof arg === 'number' || Buffer.isBuffer(arg)) {
38-
keys.push(arg.toString());
42+
return arg.toString();
3943
} else if (Array.isArray(arg)) {
40-
arg.forEach(processArg);
44+
return arg.reduce(joinArgsWithComma, '');
4145
} else {
42-
keys.push('<unknown>');
46+
return '<unknown>';
4347
}
4448
};
4549

46-
cmdArgs.forEach(processArg);
50+
if (SINGLE_ARG_COMMANDS.includes(redisCommand) && cmdArgs.length > 0) {
51+
return processArg(cmdArgs[0]);
52+
}
4753

48-
return keys.join(', ');
54+
return cmdArgs.reduce(joinArgsWithComma, '');
4955
} catch (e) {
5056
return '';
5157
}

packages/node/test/integrations/tracing/redis.test.ts

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,33 +7,55 @@ import {
77
} from '../../../src/utils/redisCache';
88

99
describe('Redis', () => {
10-
describe('getCacheKeySafely', () => {
10+
describe('getCacheKeySafely (single arg)', () => {
1111
it('should return an empty string if there are no command arguments', () => {
12-
const result = getCacheKeySafely([]);
12+
const result = getCacheKeySafely('get', []);
1313
expect(result).toBe('');
1414
});
1515

1616
it('should return a string representation of a single argument', () => {
1717
const cmdArgs = ['key1'];
18-
const result = getCacheKeySafely(cmdArgs);
18+
const result = getCacheKeySafely('get', cmdArgs);
1919
expect(result).toBe('key1');
2020
});
2121

22-
it('should return a comma-separated string for multiple arguments', () => {
23-
const cmdArgs = ['key1', 'key2', 'key3'];
24-
const result = getCacheKeySafely(cmdArgs);
25-
expect(result).toBe('key1, key2, key3');
22+
it('should return only the key for multiple arguments', () => {
23+
const cmdArgs = ['key1', 'the-value'];
24+
const result = getCacheKeySafely('get', cmdArgs);
25+
expect(result).toBe('key1');
2626
});
2727

2828
it('should handle number arguments', () => {
29-
const cmdArgs = [1, 2, 3];
30-
const result = getCacheKeySafely(cmdArgs);
31-
expect(result).toBe('1, 2, 3');
29+
const cmdArgs = [1, 'the-value'];
30+
const result = getCacheKeySafely('get', cmdArgs);
31+
expect(result).toBe('1');
32+
});
33+
34+
it('should handle Buffer arguments', () => {
35+
const cmdArgs = [Buffer.from('key1'), Buffer.from('key2')];
36+
const result = getCacheKeySafely('get', cmdArgs);
37+
expect(result).toBe('key1');
38+
});
39+
40+
it('should return <unknown> if the arg type is not supported', () => {
41+
const cmdArgs = [Symbol('key1')];
42+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
43+
// @ts-ignore
44+
const result = getCacheKeySafely('get', cmdArgs);
45+
expect(result).toBe('<unknown>');
46+
});
47+
});
48+
49+
describe('getCacheKeySafely (multiple args)', () => {
50+
it('should return a comma-separated string for multiple arguments with mget command', () => {
51+
const cmdArgs = ['key1', 'key2', 'key3'];
52+
const result = getCacheKeySafely('mget', cmdArgs);
53+
expect(result).toBe('key1, key2, key3');
3254
});
3355

3456
it('should handle Buffer arguments', () => {
3557
const cmdArgs = [Buffer.from('key1'), Buffer.from('key2')];
36-
const result = getCacheKeySafely(cmdArgs);
58+
const result = getCacheKeySafely('mget', cmdArgs);
3759
expect(result).toBe('key1, key2');
3860
});
3961

@@ -42,30 +64,30 @@ describe('Redis', () => {
4264
['key1', 'key2'],
4365
['key3', 'key4'],
4466
];
45-
const result = getCacheKeySafely(cmdArgs);
67+
const result = getCacheKeySafely('mget', cmdArgs);
4668
expect(result).toBe('key1, key2, key3, key4');
4769
});
4870

4971
it('should handle mixed type arguments', () => {
5072
const cmdArgs = [Buffer.from('key1'), ['key2', 'key3'], [Buffer.from('key4'), 'key5', 'key6', 7, ['key8']]];
51-
const result = getCacheKeySafely(cmdArgs);
73+
const result = getCacheKeySafely('mget', cmdArgs);
5274
expect(result).toBe('key1, key2, key3, key4, key5, key6, 7, key8');
5375
});
5476

55-
it('should handle nested arrays in arguments', () => {
77+
it('should handle nested arrays with mixed types in arguments', () => {
5678
const cmdArgs = [
5779
['key1', 'key2'],
58-
['key3', 'key4', ['key5', ['key6']]],
80+
['key3', 'key4', [Buffer.from('key5'), ['key6']]],
5981
];
60-
const result = getCacheKeySafely(cmdArgs);
82+
const result = getCacheKeySafely('mget', cmdArgs);
6183
expect(result).toBe('key1, key2, key3, key4, key5, key6');
6284
});
6385

6486
it('should return <unknown> if the arg type is not supported', () => {
6587
const cmdArgs = [Symbol('key1')];
6688
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
6789
// @ts-ignore
68-
const result = getCacheKeySafely(cmdArgs);
90+
const result = getCacheKeySafely('mget', cmdArgs);
6991
expect(result).toBe('<unknown>');
7092
});
7193
});

0 commit comments

Comments
 (0)