Skip to content

Commit 0d9856e

Browse files
s1gr1ds1gr1d
committed
feat(redis): Support mget command in caching functionality
1 parent 7ee6852 commit 0d9856e

File tree

6 files changed

+286
-54
lines changed

6 files changed

+286
-54
lines changed

‎dev-packages/node-integration-tests/suites/tracing/redis-cache/scenario-ioredis.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ async function run() {
3030
await redis.get('test-key');
3131
await redis.get('ioredis-cache:test-key');
3232
await redis.get('ioredis-cache:unavailable-data');
33+
34+
await redis.mget('test-key', 'ioredis-cache:test-key', 'ioredis-cache:unavailable-data');
3335
} finally {
3436
await redis.disconnect();
3537
}

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

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,12 @@ describe('redis cache auto instrumentation', () => {
4848
spans: expect.arrayContaining([
4949
// SET
5050
expect.objectContaining({
51-
description: 'set ioredis-cache:test-key [1 other arguments]',
51+
description: 'ioredis-cache:test-key',
5252
op: 'cache.put',
5353
origin: 'auto.db.otel.redis',
5454
data: expect.objectContaining({
55-
'db.statement': 'set ioredis-cache:test-key [1 other arguments]',
55+
'sentry.origin': 'auto.db.otel.redis',
56+
'db.statement': 'ioredis-cache:test-key [1 other arguments]',
5657
'cache.key': 'ioredis-cache:test-key',
5758
'cache.item_size': 2,
5859
'network.peer.address': 'localhost',
@@ -61,10 +62,11 @@ describe('redis cache auto instrumentation', () => {
6162
}),
6263
// GET
6364
expect.objectContaining({
64-
description: 'get ioredis-cache:test-key',
65-
op: 'cache.get_item', // todo: will be changed to cache.get
65+
description: 'ioredis-cache:test-key',
66+
op: 'cache.get',
6667
origin: 'auto.db.otel.redis',
6768
data: expect.objectContaining({
69+
'sentry.origin': 'auto.db.otel.redis',
6870
'db.statement': 'get ioredis-cache:test-key',
6971
'cache.hit': true,
7072
'cache.key': 'ioredis-cache:test-key',
@@ -73,19 +75,34 @@ describe('redis cache auto instrumentation', () => {
7375
'network.peer.port': 6379,
7476
}),
7577
}),
76-
// GET (unavailable)
78+
// GET (unavailable - no cache hit)
7779
expect.objectContaining({
78-
description: 'get ioredis-cache:unavailable-data',
79-
op: 'cache.get_item', // todo: will be changed to cache.get
80+
description: 'ioredis-cache:unavailable-data',
81+
op: 'cache.get',
8082
origin: 'auto.db.otel.redis',
8183
data: expect.objectContaining({
84+
'sentry.origin': 'auto.db.otel.redis',
8285
'db.statement': 'get ioredis-cache:unavailable-data',
8386
'cache.hit': false,
8487
'cache.key': 'ioredis-cache:unavailable-data',
8588
'network.peer.address': 'localhost',
8689
'network.peer.port': 6379,
8790
}),
8891
}),
92+
// MGET
93+
expect.objectContaining({
94+
description: 'test-key,ioredis-cache:test-key,ioredis-cache:unavailable-data',
95+
op: 'cache.get',
96+
origin: 'auto.db.otel.redis',
97+
data: expect.objectContaining({
98+
'sentry.origin': 'auto.db.otel.redis',
99+
'db.statement': 'mget [3 other arguments]',
100+
'cache.hit': true,
101+
'cache.key': 'test-key,ioredis-cache:test-key,ioredis-cache:unavailable-data',
102+
'network.peer.address': 'localhost',
103+
'network.peer.port': 6379,
104+
}),
105+
}),
89106
]),
90107
};
91108

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

Lines changed: 24 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -10,32 +10,12 @@ import {
1010
} from '@sentry/core';
1111
import type { IntegrationFn } from '@sentry/types';
1212
import { generateInstrumentOnce } from '../../otel/instrument';
13-
14-
function keyHasPrefix(key: string, prefixes: string[]): boolean {
15-
return prefixes.some(prefix => key.startsWith(prefix));
16-
}
17-
18-
/** Currently, caching only supports 'get' and 'set' commands. More commands will be added (setex, mget, del, expire) */
19-
function shouldConsiderForCache(
20-
redisCommand: string,
21-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
22-
key: string | number | any[] | Buffer,
23-
prefixes: string[],
24-
): boolean {
25-
return (redisCommand === 'get' || redisCommand === 'set') && typeof key === 'string' && keyHasPrefix(key, prefixes);
26-
}
27-
28-
function calculateCacheItemSize(response: unknown): number | undefined {
29-
try {
30-
if (Buffer.isBuffer(response)) return response.byteLength;
31-
else if (typeof response === 'string') return response.length;
32-
else if (typeof response === 'number') return response.toString().length;
33-
else if (response === null || response === undefined) return 0;
34-
return JSON.stringify(response).length;
35-
} catch (e) {
36-
return undefined;
37-
}
38-
}
13+
import {
14+
calculateCacheItemSize,
15+
getCacheKeySafely,
16+
getCacheOperation,
17+
shouldConsiderForCache,
18+
} from '../../utils/redisCache';
3919

4020
interface RedisOptions {
4121
cachePrefixes?: string[];
@@ -48,11 +28,14 @@ let _redisOptions: RedisOptions = {};
4828
export const instrumentRedis = generateInstrumentOnce(INTEGRATION_NAME, () => {
4929
return new IORedisInstrumentation({
5030
responseHook: (span, redisCommand, cmdArgs, response) => {
51-
const key = cmdArgs[0];
31+
const safeKey = getCacheKeySafely(cmdArgs);
5232

5333
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.redis');
5434

55-
if (!_redisOptions?.cachePrefixes || !shouldConsiderForCache(redisCommand, key, _redisOptions.cachePrefixes)) {
35+
if (
36+
!_redisOptions?.cachePrefixes ||
37+
!shouldConsiderForCache(redisCommand, safeKey, _redisOptions.cachePrefixes)
38+
) {
5639
// not relevant for cache
5740
return;
5841
}
@@ -65,26 +48,22 @@ export const instrumentRedis = generateInstrumentOnce(INTEGRATION_NAME, () => {
6548
span.setAttributes({ 'network.peer.address': networkPeerAddress, 'network.peer.port': networkPeerPort });
6649
}
6750

51+
const cacheOperation = getCacheOperation(redisCommand);
52+
53+
if (!cacheOperation) return; // redis command unsupported as cache operation
54+
6855
const cacheItemSize = calculateCacheItemSize(response);
6956
if (cacheItemSize) span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE, cacheItemSize);
7057

71-
if (typeof key === 'string') {
72-
switch (redisCommand) {
73-
case 'get':
74-
span.setAttributes({
75-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'cache.get_item', // todo: will be changed to cache.get
76-
[SEMANTIC_ATTRIBUTE_CACHE_KEY]: key,
77-
});
78-
if (cacheItemSize !== undefined) span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, cacheItemSize > 0);
79-
break;
80-
case 'set':
81-
span.setAttributes({
82-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'cache.put',
83-
[SEMANTIC_ATTRIBUTE_CACHE_KEY]: key,
84-
});
85-
break;
86-
}
87-
}
58+
if (cacheOperation === 'cache.get' && cacheItemSize !== undefined)
59+
span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, cacheItemSize > 0);
60+
61+
span.setAttributes({
62+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: cacheOperation,
63+
[SEMANTIC_ATTRIBUTE_CACHE_KEY]: safeKey,
64+
});
65+
66+
span.updateName(`${safeKey.substring(0, 1024)}...`);
8867
},
8968
});
9069
});

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
import type { CommandArgs as IORedisCommandArgs } from '@opentelemetry/instrumentation-ioredis';
2+
3+
export const GET_COMMANDS = ['get', 'mget'];
4+
export const SET_COMMANDS = ['set' /* todo: 'setex' */];
5+
// todo: del, expire
6+
7+
/** Determine cache operation based on redis statement */
8+
export function getCacheOperation(
9+
statement: string,
10+
): 'cache.get' | 'cache.put' | 'cache.remove' | 'cache.flush' | undefined {
11+
const lowercaseStatement = statement.toLowerCase();
12+
13+
if (GET_COMMANDS.includes(lowercaseStatement)) {
14+
return 'cache.get';
15+
} else if (SET_COMMANDS.includes(lowercaseStatement)) {
16+
return 'cache.put';
17+
} else {
18+
return undefined;
19+
}
20+
}
21+
22+
function keyHasPrefix(key: string, prefixes: string[]): boolean {
23+
return prefixes.some(prefix => key.startsWith(prefix));
24+
}
25+
26+
/** Safely converts a redis key to a string (comma-separated if there are multiple keys) */
27+
export function getCacheKeySafely(cmdArgs: IORedisCommandArgs): string {
28+
try {
29+
if (cmdArgs.length === 0) {
30+
return '';
31+
}
32+
33+
const keys: string[] = [];
34+
35+
cmdArgs.forEach(arg => {
36+
if (typeof arg === 'string' || typeof arg === 'number') {
37+
keys.push(arg.toString());
38+
} else if (Buffer.isBuffer(arg)) {
39+
keys.push(arg.toString());
40+
} else if (Array.isArray(arg)) {
41+
arg.forEach(subArg => {
42+
if (typeof subArg === 'string' || typeof subArg === 'number') {
43+
keys.push(subArg.toString());
44+
} else if (Buffer.isBuffer(subArg)) {
45+
keys.push(subArg.toString());
46+
}
47+
});
48+
}
49+
});
50+
51+
return keys.join(',');
52+
} catch (e) {
53+
return '';
54+
}
55+
}
56+
57+
/** Determines whether a redis operation should be considered as "cache operation" by checking if a key is prefixed.
58+
* We only support certain commands (such as 'set', 'get', 'mget'). */
59+
export function shouldConsiderForCache(redisCommand: string, key: string, prefixes: string[]): boolean {
60+
const lowercaseCommand = redisCommand.toLowerCase();
61+
if (!SET_COMMANDS.includes(lowercaseCommand) && !GET_COMMANDS.includes(lowercaseCommand)) return false;
62+
63+
return key.split(',').reduce((prev, key) => prev || keyHasPrefix(key, prefixes), false);
64+
}
65+
66+
/** Calculates size based on the cache response value */
67+
export function calculateCacheItemSize(response: unknown): number | undefined {
68+
try {
69+
if (Buffer.isBuffer(response)) return response.byteLength;
70+
else if (typeof response === 'string') return response.length;
71+
else if (typeof response === 'number') return response.toString().length;
72+
else if (response === null || response === undefined) return 0;
73+
return JSON.stringify(response).length;
74+
} catch (e) {
75+
return undefined;
76+
}
77+
}

0 commit comments

Comments
 (0)