set op to cache.remove for redis delete commands

Author: msonnb

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

@@ -22,6 +22,8 @@ async function run() {
         await redis.get('ioredis-cache:unavailable-data');
 
         await redis.mget('test-key', 'ioredis-cache:test-key', 'ioredis-cache:unavailable-data');
+
+        await redis.del('ioredis-cache:test-key');
       } finally {
         await redis.disconnect();
       }

dev-packages/node-integration-tests/suites/tracing/redis-cache/scenario-redis-4.mjs

@@ -27,6 +27,8 @@ async function run() {
 
         await redisClient.mGet(['redis-test-key', 'redis-cache:test-key', 'redis-cache:unavailable-data']);
 
+        await redisClient.del('redis-cache:test-key');
+
         // MULTI/EXEC produces one span per queued command, all ended together on exec
         await redisClient.multi().set('redis-multi-key', 'multi-value').get('redis-multi-key').exec();
 

dev-packages/node-integration-tests/suites/tracing/redis-cache/scenario-redis-5.mjs

@@ -23,6 +23,8 @@ async function run() {
 
         await redisClient.mGet(['redis-5-test-key', 'redis-5-cache:test-key', 'redis-5-cache:unavailable-data']);
 
+        await redisClient.del('redis-5-cache:test-key');
+
         // MULTI/EXEC produces one span per queued command, all ended together on exec
         await redisClient.multi().set('redis-5-multi-key', 'multi-value').get('redis-5-multi-key').exec();
 

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

@@ -136,6 +136,19 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
             'network.peer.port': 6383,
           }),
         }),
+        // DEL
+        expect.objectContaining({
+          description: 'ioredis-cache:test-key',
+          op: 'cache.remove',
+          origin: redisOrigin,
+          data: expect.objectContaining({
+            'sentry.origin': redisOrigin,
+            'db.statement': 'del ioredis-cache:test-key',
+            'cache.key': ['ioredis-cache:test-key'],
+            'network.peer.address': 'localhost',
+            'network.peer.port': 6383,
+          }),
+        }),
       ]),
     };
 
@@ -258,6 +271,17 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
             'cache.key': ['redis-test-key', 'redis-cache:test-key', 'redis-cache:unavailable-data'],
           }),
         }),
+        // DEL
+        expect.objectContaining({
+          description: 'redis-cache:test-key',
+          op: 'cache.remove',
+          origin: redisOrigin,
+          data: expect.objectContaining({
+            'sentry.origin': redisOrigin,
+            'db.statement': 'DEL redis-cache:test-key',
+            'cache.key': ['redis-cache:test-key'],
+          }),
+        }),
         ...batchSpans,
         // a failing command produces a span with an error status
         expect.objectContaining({
@@ -400,6 +424,17 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
             'cache.key': ['redis-5-test-key', 'redis-5-cache:test-key', 'redis-5-cache:unavailable-data'],
           }),
         }),
+        // DEL
+        expect.objectContaining({
+          description: 'redis-5-cache:test-key',
+          op: 'cache.remove',
+          origin: redisOrigin,
+          data: expect.objectContaining({
+            'sentry.origin': redisOrigin,
+            'db.statement': 'DEL redis-5-cache:test-key',
+            'cache.key': ['redis-5-cache:test-key'],
+          }),
+        }),
         ...batchSpans,
         // a failing command produces a span with an error status
         expect.objectContaining({

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

@@ -14,6 +14,7 @@ import {
   getCacheKeySafely,
   getCacheOperation,
   isInCommands,
+  REMOVE_COMMANDS,
   shouldConsiderForCache,
 } from '../../../utils/redisCache';
 import type { IORedisResponseCustomAttributeFunction } from './vendored/types';
@@ -79,7 +80,8 @@ export const cacheResponseHook: IORedisResponseCustomAttributeFunction = (
     span.setAttributes({ 'network.peer.address': networkPeerAddress, 'network.peer.port': networkPeerPort });
   }
 
-  const cacheItemSize = calculateCacheItemSize(response);
+  // A remove response is a delete-count, not a cached value, so its size is meaningless.
+  const cacheItemSize = isInCommands(REMOVE_COMMANDS, redisCommand) ? undefined : calculateCacheItemSize(response);
 
   if (cacheItemSize) {
     span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE, cacheItemSize);

packages/node/src/utils/redisCache.ts

@@ -4,7 +4,8 @@ const SINGLE_ARG_COMMANDS = ['get', 'set', 'setex'];
 
 export const GET_COMMANDS = ['get', 'mget'];
 export const SET_COMMANDS = ['set', 'setex'];
-// todo: del, expire
+export const REMOVE_COMMANDS = ['del', 'unlink'];
+// todo: expire (no matching cache convention op yet)
 
 /** Checks if a given command is in the list of redis commands.
  *  Useful because commands can come in lowercase or uppercase (depending on the library). */
@@ -13,13 +14,13 @@ export function isInCommands(redisCommands: string[], command: string): boolean
 }
 
 /** Determine cache operation based on redis statement */
-export function getCacheOperation(
-  command: string,
-): 'cache.get' | 'cache.put' | 'cache.remove' | 'cache.flush' | undefined {
+export function getCacheOperation(command: string): 'cache.get' | 'cache.put' | 'cache.remove' | undefined {
   if (isInCommands(GET_COMMANDS, command)) {
     return 'cache.get';
   } else if (isInCommands(SET_COMMANDS, command)) {
     return 'cache.put';
+  } else if (isInCommands(REMOVE_COMMANDS, command)) {
+    return 'cache.remove';
   } else {
     return undefined;
   }

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

@@ -4,6 +4,7 @@ import {
   calculateCacheItemSize,
   GET_COMMANDS,
   getCacheKeySafely,
+  REMOVE_COMMANDS,
   SET_COMMANDS,
   shouldConsiderForCache,
 } from '../../../src/utils/redisCache';
@@ -256,7 +257,7 @@ describe('Redis', () => {
       expect(result).toBe(false);
     });
 
-    GET_COMMANDS.concat(SET_COMMANDS).forEach(command => {
+    GET_COMMANDS.concat(SET_COMMANDS, REMOVE_COMMANDS).forEach(command => {
       it(`should return true for ${command} command with matching prefix`, () => {
         const key = ['cache:test-key'];
         const result = shouldConsiderForCache(command, key, prefixes);