Merge pull request #12380 from getsentry/sig/ioredis-setex

feat(redis): Support `setex` command in caching functionality

Author: s1gr1d

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

@@ -27,6 +27,9 @@ async function run() {
         await redis.set('test-key', 'test-value');
         await redis.set('ioredis-cache:test-key', 'test-value');
 
+        await redis.set('ioredis-cache:test-key-set-EX', 'test-value', 'EX', 10);
+        await redis.setex('ioredis-cache:test-key-setex', 10, 'test-value');
+
         await redis.get('test-key');
         await redis.get('ioredis-cache:test-key');
         await redis.get('ioredis-cache:unavailable-data');

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

@@ -60,6 +60,34 @@ describe('redis cache auto instrumentation', () => {
             'network.peer.port': 6379,
           }),
         }),
+        // SET (with EX)
+        expect.objectContaining({
+          description: 'ioredis-cache:test-key-set-EX',
+          op: 'cache.put',
+          origin: 'auto.db.otel.redis',
+          data: expect.objectContaining({
+            'sentry.origin': 'auto.db.otel.redis',
+            'db.statement': 'set ioredis-cache:test-key-set-EX [3 other arguments]',
+            'cache.key': ['ioredis-cache:test-key-set-EX'],
+            'cache.item_size': 2,
+            'network.peer.address': 'localhost',
+            'network.peer.port': 6379,
+          }),
+        }),
+        // SETEX
+        expect.objectContaining({
+          description: 'ioredis-cache:test-key-setex',
+          op: 'cache.put',
+          origin: 'auto.db.otel.redis',
+          data: expect.objectContaining({
+            'sentry.origin': 'auto.db.otel.redis',
+            'db.statement': 'setex ioredis-cache:test-key-setex [2 other arguments]',
+            'cache.key': ['ioredis-cache:test-key-setex'],
+            'cache.item_size': 2,
+            'network.peer.address': 'localhost',
+            'network.peer.port': 6379,
+          }),
+        }),
         // GET
         expect.objectContaining({
           description: 'ioredis-cache:test-key',

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

@@ -29,10 +29,9 @@ let _redisOptions: RedisOptions = {};
 export const instrumentRedis = generateInstrumentOnce(INTEGRATION_NAME, () => {
   return new IORedisInstrumentation({
     responseHook: (span, redisCommand, cmdArgs, response) => {
-      const safeKey = getCacheKeySafely(redisCommand, cmdArgs);
-
       span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.redis');
 
+      const safeKey = getCacheKeySafely(redisCommand, cmdArgs);
       const cacheOperation = getCacheOperation(redisCommand);
 
       if (

packages/node/src/utils/redisCache.ts

@@ -4,7 +4,7 @@ import { flatten } from '@sentry/utils';
 const SINGLE_ARG_COMMANDS = ['get', 'set', 'setex'];
 
 export const GET_COMMANDS = ['get', 'mget'];
-export const SET_COMMANDS = ['set' /* todo: 'setex' */];
+export const SET_COMMANDS = ['set', 'setex'];
 // todo: del, expire
 
 /** Determine cache operation based on redis statement */
@@ -56,14 +56,17 @@ export function getCacheKeySafely(redisCommand: string, cmdArgs: IORedisCommandA
 
 /** Determines whether a redis operation should be considered as "cache operation" by checking if a key is prefixed.
  *  We only support certain commands (such as 'set', 'get', 'mget'). */
-export function shouldConsiderForCache(redisCommand: string, key: string[], prefixes: string[]): boolean {
+export function shouldConsiderForCache(redisCommand: string, keys: string[], prefixes: string[]): boolean {
   if (!getCacheOperation(redisCommand)) {
     return false;
   }
 
-  return key.reduce((prev, key) => {
-    return prev || keyHasPrefix(key, prefixes);
-  }, false);
+  for (const key of keys) {
+    if (keyHasPrefix(key, prefixes)) {
+      return true;
+    }
+  }
+  return false;
 }
 
 /** Calculates size based on the cache response value */

packages/opentelemetry/src/utils/parseSpanDescription.ts

@@ -51,7 +51,7 @@ export function parseSpanDescription(span: AbstractSpan): SpanDescription {
 
   // If db.type exists then this is a database call span
   // If the Redis DB is used as a cache, the span description should not be changed
-  if (dbSystem && (!attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !opIsCache)) {
+  if (dbSystem && !opIsCache) {
     return descriptionForDbSystem({ attributes, name });
   }