Skip to content

Commit 918534b

Browse files
s1gr1dbillyvg
s1gr1d
authored and
billyvg
committed
create array key
1 parent 378d6cb commit 918534b

File tree

5 files changed

+41
-44
lines changed

5 files changed

+41
-44
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ describe('redis cache auto instrumentation', () => {
5454
data: expect.objectContaining({
5555
'sentry.origin': 'auto.db.otel.redis',
5656
'db.statement': 'set ioredis-cache:test-key [1 other arguments]',
57-
'cache.key': 'ioredis-cache:test-key',
57+
'cache.key': ['ioredis-cache:test-key'],
5858
'cache.item_size': 2,
5959
'network.peer.address': 'localhost',
6060
'network.peer.port': 6379,
@@ -69,7 +69,7 @@ describe('redis cache auto instrumentation', () => {
6969
'sentry.origin': 'auto.db.otel.redis',
7070
'db.statement': 'get ioredis-cache:test-key',
7171
'cache.hit': true,
72-
'cache.key': 'ioredis-cache:test-key',
72+
'cache.key': ['ioredis-cache:test-key'],
7373
'cache.item_size': 10,
7474
'network.peer.address': 'localhost',
7575
'network.peer.port': 6379,
@@ -84,7 +84,7 @@ describe('redis cache auto instrumentation', () => {
8484
'sentry.origin': 'auto.db.otel.redis',
8585
'db.statement': 'get ioredis-cache:unavailable-data',
8686
'cache.hit': false,
87-
'cache.key': 'ioredis-cache:unavailable-data',
87+
'cache.key': ['ioredis-cache:unavailable-data'],
8888
'network.peer.address': 'localhost',
8989
'network.peer.port': 6379,
9090
}),
@@ -98,7 +98,7 @@ describe('redis cache auto instrumentation', () => {
9898
'sentry.origin': 'auto.db.otel.redis',
9999
'db.statement': 'mget [3 other arguments]',
100100
'cache.hit': true,
101-
'cache.key': 'test-key, ioredis-cache:test-key, ioredis-cache:unavailable-data',
101+
'cache.key': ['test-key', 'ioredis-cache:test-key', 'ioredis-cache:unavailable-data'],
102102
'network.peer.address': 'localhost',
103103
'network.peer.port': 6379,
104104
}),

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export const instrumentRedis = generateInstrumentOnce(INTEGRATION_NAME, () => {
3636
const cacheOperation = getCacheOperation(redisCommand);
3737

3838
if (
39+
!safeKey ||
3940
!cacheOperation ||
4041
!_redisOptions?.cachePrefixes ||
4142
!shouldConsiderForCache(redisCommand, safeKey, _redisOptions.cachePrefixes)
@@ -52,11 +53,8 @@ export const instrumentRedis = generateInstrumentOnce(INTEGRATION_NAME, () => {
5253
span.setAttributes({ 'network.peer.address': networkPeerAddress, 'network.peer.port': networkPeerPort });
5354
}
5455

55-
// eslint-disable-next-line no-console
56-
console.log('response', response);
5756
const cacheItemSize = calculateCacheItemSize(response);
58-
// eslint-disable-next-line no-console
59-
console.log('cacheItemSize', cacheItemSize);
57+
6058
if (cacheItemSize) {
6159
span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE, cacheItemSize);
6260
}
@@ -70,9 +68,9 @@ export const instrumentRedis = generateInstrumentOnce(INTEGRATION_NAME, () => {
7068
[SEMANTIC_ATTRIBUTE_CACHE_KEY]: safeKey,
7169
});
7270

73-
span.updateName(safeKey.length > 1024 ? `${safeKey.substring(0, 1024)}...` : safeKey);
74-
// eslint-disable-next-line no-console
75-
console.log('span',span);
71+
const spanDescription = safeKey.join(', ');
72+
73+
span.updateName(spanDescription.length > 1024 ? `${spanDescription.substring(0, 1024)}...` : spanDescription);
7674
},
7775
});
7876
});

‎packages/node/src/utils/redisCache.ts

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { CommandArgs as IORedisCommandArgs } from '@opentelemetry/instrumentation-ioredis';
2+
import { flatten } from '@sentry/utils';
23

34
const SINGLE_ARG_COMMANDS = ['get', 'set', 'setex'];
45

@@ -26,45 +27,43 @@ function keyHasPrefix(key: string, prefixes: string[]): boolean {
2627
}
2728

2829
/** Safely converts a redis key to a string (comma-separated if there are multiple keys) */
29-
export function getCacheKeySafely(redisCommand: string, cmdArgs: IORedisCommandArgs): string {
30+
export function getCacheKeySafely(redisCommand: string, cmdArgs: IORedisCommandArgs): string[] | undefined {
3031
try {
3132
if (cmdArgs.length === 0) {
32-
return '';
33+
return undefined;
3334
}
3435

3536
// 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)}`;
38-
39-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
40-
const processArg = (arg: string | Buffer | number | any[]): string => {
37+
const processArg = (arg: string | Buffer | number | any[]): string[] => {
4138
if (typeof arg === 'string' || typeof arg === 'number' || Buffer.isBuffer(arg)) {
42-
return arg.toString();
39+
return [arg.toString()];
4340
} else if (Array.isArray(arg)) {
44-
return arg.reduce(joinArgsWithComma, '');
41+
return flatten(arg.map(arg => processArg(arg)));
4542
} else {
46-
return '<unknown>';
43+
return ['<unknown>'];
4744
}
4845
};
4946

5047
if (SINGLE_ARG_COMMANDS.includes(redisCommand) && cmdArgs.length > 0) {
5148
return processArg(cmdArgs[0]);
5249
}
5350

54-
return cmdArgs.reduce(joinArgsWithComma, '');
51+
return flatten(cmdArgs.map(arg => processArg(arg)));
5552
} catch (e) {
56-
return '';
53+
return undefined;
5754
}
5855
}
5956

6057
/** Determines whether a redis operation should be considered as "cache operation" by checking if a key is prefixed.
6158
* We only support certain commands (such as 'set', 'get', 'mget'). */
62-
export function shouldConsiderForCache(redisCommand: string, key: string, prefixes: string[]): boolean {
59+
export function shouldConsiderForCache(redisCommand: string, key: string[], prefixes: string[]): boolean {
6360
if (!getCacheOperation(redisCommand)) {
6461
return false;
6562
}
6663

67-
return key.split(',').reduce((prev, key) => prev || keyHasPrefix(key, prefixes), false);
64+
return key.reduce((prev, key) => {
65+
return prev || keyHasPrefix(key, prefixes);
66+
}, false);
6867
}
6968

7069
/** Calculates size based on the cache response value */

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

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,53 +10,53 @@ describe('Redis', () => {
1010
describe('getCacheKeySafely (single arg)', () => {
1111
it('should return an empty string if there are no command arguments', () => {
1212
const result = getCacheKeySafely('get', []);
13-
expect(result).toBe('');
13+
expect(result).toBe(undefined);
1414
});
1515

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

2222
it('should return only the key for multiple arguments', () => {
2323
const cmdArgs = ['key1', 'the-value'];
2424
const result = getCacheKeySafely('get', cmdArgs);
25-
expect(result).toBe('key1');
25+
expect(result).toStrictEqual(['key1']);
2626
});
2727

2828
it('should handle number arguments', () => {
2929
const cmdArgs = [1, 'the-value'];
3030
const result = getCacheKeySafely('get', cmdArgs);
31-
expect(result).toBe('1');
31+
expect(result).toStrictEqual(['1']);
3232
});
3333

3434
it('should handle Buffer arguments', () => {
3535
const cmdArgs = [Buffer.from('key1'), Buffer.from('key2')];
3636
const result = getCacheKeySafely('get', cmdArgs);
37-
expect(result).toBe('key1');
37+
expect(result).toStrictEqual(['key1']);
3838
});
3939

4040
it('should return <unknown> if the arg type is not supported', () => {
4141
const cmdArgs = [Symbol('key1')];
4242
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
4343
// @ts-ignore
4444
const result = getCacheKeySafely('get', cmdArgs);
45-
expect(result).toBe('<unknown>');
45+
expect(result).toStrictEqual(['<unknown>']);
4646
});
4747
});
4848

4949
describe('getCacheKeySafely (multiple args)', () => {
5050
it('should return a comma-separated string for multiple arguments with mget command', () => {
5151
const cmdArgs = ['key1', 'key2', 'key3'];
5252
const result = getCacheKeySafely('mget', cmdArgs);
53-
expect(result).toBe('key1, key2, key3');
53+
expect(result).toStrictEqual(['key1', 'key2', 'key3']);
5454
});
5555

5656
it('should handle Buffer arguments', () => {
5757
const cmdArgs = [Buffer.from('key1'), Buffer.from('key2')];
5858
const result = getCacheKeySafely('mget', cmdArgs);
59-
expect(result).toBe('key1, key2');
59+
expect(result).toStrictEqual(['key1', 'key2']);
6060
});
6161

6262
it('should handle array arguments', () => {
@@ -65,13 +65,13 @@ describe('Redis', () => {
6565
['key3', 'key4'],
6666
];
6767
const result = getCacheKeySafely('mget', cmdArgs);
68-
expect(result).toBe('key1, key2, key3, key4');
68+
expect(result).toStrictEqual(['key1', 'key2', 'key3', 'key4']);
6969
});
7070

7171
it('should handle mixed type arguments', () => {
7272
const cmdArgs = [Buffer.from('key1'), ['key2', 'key3'], [Buffer.from('key4'), 'key5', 'key6', 7, ['key8']]];
7373
const result = getCacheKeySafely('mget', cmdArgs);
74-
expect(result).toBe('key1, key2, key3, key4, key5, key6, 7, key8');
74+
expect(result).toStrictEqual(['key1', 'key2', 'key3', 'key4', 'key5', 'key6', '7', 'key8']);
7575
});
7676

7777
it('should handle nested arrays with mixed types in arguments', () => {
@@ -80,15 +80,15 @@ describe('Redis', () => {
8080
['key3', 'key4', [Buffer.from('key5'), ['key6']]],
8181
];
8282
const result = getCacheKeySafely('mget', cmdArgs);
83-
expect(result).toBe('key1, key2, key3, key4, key5, key6');
83+
expect(result).toStrictEqual(['key1', 'key2', 'key3', 'key4', 'key5', 'key6']);
8484
});
8585

8686
it('should return <unknown> if the arg type is not supported', () => {
8787
const cmdArgs = [Symbol('key1')];
8888
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
8989
// @ts-ignore
9090
const result = getCacheKeySafely('mget', cmdArgs);
91-
expect(result).toBe('<unknown>');
91+
expect(result).toStrictEqual(['<unknown>']);
9292
});
9393
});
9494

@@ -137,7 +137,7 @@ describe('Redis', () => {
137137
it('should return false for non-cache commands', () => {
138138
const command = 'EXISTS';
139139
const commandLowercase = 'exists';
140-
const key = 'cache:test-key';
140+
const key = ['cache:test-key'];
141141
const result1 = shouldConsiderForCache(command, key, prefixes);
142142
const result2 = shouldConsiderForCache(commandLowercase, key, prefixes);
143143
expect(result1).toBe(false);
@@ -146,35 +146,35 @@ describe('Redis', () => {
146146

147147
it('should return true for cache commands with matching prefix', () => {
148148
const command = 'get';
149-
const key = 'cache:test-key';
149+
const key = ['cache:test-key'];
150150
const result = shouldConsiderForCache(command, key, prefixes);
151151
expect(result).toBe(true);
152152
});
153153

154154
it('should return false for cache commands without matching prefix', () => {
155155
const command = 'get';
156-
const key = 'test-key';
156+
const key = ['test-key'];
157157
const result = shouldConsiderForCache(command, key, prefixes);
158158
expect(result).toBe(false);
159159
});
160160

161161
it('should return true for multiple keys with at least one matching prefix', () => {
162162
const command = 'mget';
163-
const key = 'test-key,cache:test-key';
163+
const key = ['test-key', 'cache:test-key'];
164164
const result = shouldConsiderForCache(command, key, prefixes);
165165
expect(result).toBe(true);
166166
});
167167

168168
it('should return false for multiple keys without any matching prefix', () => {
169169
const command = 'mget';
170-
const key = 'test-key,test-key2';
170+
const key = ['test-key', 'test-key2'];
171171
const result = shouldConsiderForCache(command, key, prefixes);
172172
expect(result).toBe(false);
173173
});
174174

175175
GET_COMMANDS.concat(SET_COMMANDS).forEach(command => {
176176
it(`should return true for ${command} command with matching prefix`, () => {
177-
const key = 'cache:test-key';
177+
const key = ['cache:test-key'];
178178
const result = shouldConsiderForCache(command, key, prefixes);
179179
expect(result).toBe(true);
180180
});

‎packages/opentelemetry/src/utils/parseSpanDescription.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export function parseSpanDescription(span: AbstractSpan): SpanDescription {
5252
// If db.type exists then this is a database call span
5353
// If the Redis DB is used as a cache, the span description should not be changed
5454
if (dbSystem && (!attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !opIsCache)) {
55-
// return descriptionForDbSystem({ attributes, name });
55+
return descriptionForDbSystem({ attributes, name });
5656
}
5757

5858
// If rpc.service exists then this is a rpc call span.

0 commit comments

Comments
 (0)