Skip to content

Commit 2ec4056

Browse files
authored
Merge pull request #12380 from getsentry/sig/ioredis-setex
feat(redis): Support `setex` command in caching functionality
2 parents bc5d2e1 + c460245 commit 2ec4056

File tree

5 files changed

+41
-8
lines changed

5 files changed

+41
-8
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ async function run() {
2727
await redis.set('test-key', 'test-value');
2828
await redis.set('ioredis-cache:test-key', 'test-value');
2929

30+
await redis.set('ioredis-cache:test-key-set-EX', 'test-value', 'EX', 10);
31+
await redis.setex('ioredis-cache:test-key-setex', 10, 'test-value');
32+
3033
await redis.get('test-key');
3134
await redis.get('ioredis-cache:test-key');
3235
await redis.get('ioredis-cache:unavailable-data');

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,34 @@ describe('redis cache auto instrumentation', () => {
6060
'network.peer.port': 6379,
6161
}),
6262
}),
63+
// SET (with EX)
64+
expect.objectContaining({
65+
description: 'ioredis-cache:test-key-set-EX',
66+
op: 'cache.put',
67+
origin: 'auto.db.otel.redis',
68+
data: expect.objectContaining({
69+
'sentry.origin': 'auto.db.otel.redis',
70+
'db.statement': 'set ioredis-cache:test-key-set-EX [3 other arguments]',
71+
'cache.key': ['ioredis-cache:test-key-set-EX'],
72+
'cache.item_size': 2,
73+
'network.peer.address': 'localhost',
74+
'network.peer.port': 6379,
75+
}),
76+
}),
77+
// SETEX
78+
expect.objectContaining({
79+
description: 'ioredis-cache:test-key-setex',
80+
op: 'cache.put',
81+
origin: 'auto.db.otel.redis',
82+
data: expect.objectContaining({
83+
'sentry.origin': 'auto.db.otel.redis',
84+
'db.statement': 'setex ioredis-cache:test-key-setex [2 other arguments]',
85+
'cache.key': ['ioredis-cache:test-key-setex'],
86+
'cache.item_size': 2,
87+
'network.peer.address': 'localhost',
88+
'network.peer.port': 6379,
89+
}),
90+
}),
6391
// GET
6492
expect.objectContaining({
6593
description: 'ioredis-cache:test-key',

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,9 @@ let _redisOptions: RedisOptions = {};
2929
export const instrumentRedis = generateInstrumentOnce(INTEGRATION_NAME, () => {
3030
return new IORedisInstrumentation({
3131
responseHook: (span, redisCommand, cmdArgs, response) => {
32-
const safeKey = getCacheKeySafely(redisCommand, cmdArgs);
33-
3432
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.redis');
3533

34+
const safeKey = getCacheKeySafely(redisCommand, cmdArgs);
3635
const cacheOperation = getCacheOperation(redisCommand);
3736

3837
if (

packages/node/src/utils/redisCache.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { flatten } from '@sentry/utils';
44
const SINGLE_ARG_COMMANDS = ['get', 'set', 'setex'];
55

66
export const GET_COMMANDS = ['get', 'mget'];
7-
export const SET_COMMANDS = ['set' /* todo: 'setex' */];
7+
export const SET_COMMANDS = ['set', 'setex'];
88
// todo: del, expire
99

1010
/** Determine cache operation based on redis statement */
@@ -56,14 +56,17 @@ export function getCacheKeySafely(redisCommand: string, cmdArgs: IORedisCommandA
5656

5757
/** Determines whether a redis operation should be considered as "cache operation" by checking if a key is prefixed.
5858
* We only support certain commands (such as 'set', 'get', 'mget'). */
59-
export function shouldConsiderForCache(redisCommand: string, key: string[], prefixes: string[]): boolean {
59+
export function shouldConsiderForCache(redisCommand: string, keys: string[], prefixes: string[]): boolean {
6060
if (!getCacheOperation(redisCommand)) {
6161
return false;
6262
}
6363

64-
return key.reduce((prev, key) => {
65-
return prev || keyHasPrefix(key, prefixes);
66-
}, false);
64+
for (const key of keys) {
65+
if (keyHasPrefix(key, prefixes)) {
66+
return true;
67+
}
68+
}
69+
return false;
6770
}
6871

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

packages/opentelemetry/src/utils/parseSpanDescription.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export function parseSpanDescription(span: AbstractSpan): SpanDescription {
5151

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
54-
if (dbSystem && (!attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !opIsCache)) {
54+
if (dbSystem && !opIsCache) {
5555
return descriptionForDbSystem({ attributes, name });
5656
}
5757

0 commit comments

Comments
 (0)