feat(node): Add maxCacheKeyLength to Redis integration (remove truncation)

Author: s1gr1d

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

@@ -24,14 +24,32 @@ import {
 } from '../../utils/redisCache';
 
 interface RedisOptions {
+  /**
+   * Define cache prefixes for cache keys that should be captured as a cache span.
+   *
+   * Setting this to, for example, `['user:']` will capture cache keys that start with `user:`.
+   */
   cachePrefixes?: string[];
+  /**
+   * Maximum length of the cache key added to the span description. If the key exceeds this length, it will be truncated.
+   *
+   * By default, the full cache key is used.
+   */
+  maxCacheKeyLength?: number;
 }
 
 const INTEGRATION_NAME = 'Redis';
 
-let _redisOptions: RedisOptions = {};
+/* Only exported for testing purposes */
+export let _redisOptions: RedisOptions = {};
 
-const cacheResponseHook: RedisResponseCustomAttributeFunction = (span: Span, redisCommand, cmdArgs, response) => {
+/* Only exported for testing purposes */
+export const cacheResponseHook: RedisResponseCustomAttributeFunction = (
+  span: Span,
+  redisCommand,
+  cmdArgs,
+  response,
+) => {
   span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.redis');
 
   const safeKey = getCacheKeySafely(redisCommand, cmdArgs);
@@ -70,9 +88,12 @@ const cacheResponseHook: RedisResponseCustomAttributeFunction = (span: Span, red
     [SEMANTIC_ATTRIBUTE_CACHE_KEY]: safeKey,
   });
 
+  // todo: change to string[] once EAP supports it
   const spanDescription = safeKey.join(', ');
 
-  span.updateName(truncate(spanDescription, 1024));
+  span.updateName(
+    _redisOptions.maxCacheKeyLength ? truncate(spanDescription, _redisOptions.maxCacheKeyLength) : spanDescription,
+  );
 };
 
 const instrumentIORedis = generateInstrumentOnce(`${INTEGRATION_NAME}.IORedis`, () => {

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

@@ -1,4 +1,5 @@
-import { describe, expect, it } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { _redisOptions, cacheResponseHook } from '../../../src/integrations/tracing/redis';
 import {
   calculateCacheItemSize,
   GET_COMMANDS,
@@ -8,6 +9,82 @@ import {
 } from '../../../src/utils/redisCache';
 
 describe('Redis', () => {
+  describe('cacheResponseHook', () => {
+    let mockSpan: any;
+    let originalRedisOptions: any;
+
+    beforeEach(() => {
+      mockSpan = {
+        setAttribute: vi.fn(),
+        setAttributes: vi.fn(),
+        updateName: vi.fn(),
+        spanContext: () => ({ spanId: 'test-span-id', traceId: 'test-trace-id' }),
+      };
+
+      originalRedisOptions = { ..._redisOptions };
+    });
+
+    afterEach(() => {
+      vi.restoreAllMocks();
+      // Reset redis options by clearing all properties first, then restoring original ones
+      Object.keys(_redisOptions).forEach(key => delete (_redisOptions as any)[key]);
+      Object.assign(_redisOptions, originalRedisOptions);
+    });
+
+    describe('early returns', () => {
+      it.each([
+        { desc: 'no args', cmd: 'get', args: [], response: 'test' },
+        { desc: 'unsupported command', cmd: 'exists', args: ['key'], response: 'test' },
+        { desc: 'no cache prefixes', cmd: 'get', args: ['key'], response: 'test', options: {} },
+        { desc: 'non-matching prefix', cmd: 'get', args: ['key'], response: 'test', options: { cachePrefixes: ['c'] } },
+      ])('should always set sentry.origin but return early when $desc', ({ cmd, args, response, options = {} }) => {
+        vi.clearAllMocks();
+        Object.assign(_redisOptions, options);
+
+        cacheResponseHook(mockSpan, cmd, args, response);
+
+        expect(mockSpan.setAttribute).toHaveBeenCalledWith('sentry.origin', 'auto.db.otel.redis');
+        expect(mockSpan.setAttributes).not.toHaveBeenCalled();
+        expect(mockSpan.updateName).not.toHaveBeenCalled();
+      });
+    });
+
+    describe('span name truncation', () => {
+      beforeEach(() => {
+        Object.assign(_redisOptions, { cachePrefixes: ['cache:'] });
+      });
+
+      it('should not truncate span name when maxCacheKeyLength is not set', () => {
+        cacheResponseHook(
+          mockSpan,
+          'mget',
+          ['cache:very-long-key-name', 'cache:very-long-key-name-2', 'cache:very-long-key-name-3'],
+          'value',
+        );
+
+        expect(mockSpan.updateName).toHaveBeenCalledWith(
+          'cache:very-long-key-name, cache:very-long-key-name-2, cache:very-long-key-name-3',
+        );
+      });
+
+      it('should truncate span name when maxCacheKeyLength is set', () => {
+        Object.assign(_redisOptions, { maxCacheKeyLength: 10 });
+
+        cacheResponseHook(mockSpan, 'get', ['cache:very-long-key-name'], 'value');
+
+        expect(mockSpan.updateName).toHaveBeenCalledWith('cache:very...');
+      });
+
+      it('should truncate multiple keys joined with commas', () => {
+        Object.assign(_redisOptions, { maxCacheKeyLength: 20 });
+
+        cacheResponseHook(mockSpan, 'mget', ['cache:key1', 'cache:key2', 'cache:key3'], ['val1', 'val2', 'val3']);
+
+        expect(mockSpan.updateName).toHaveBeenCalledWith('cache:key1, cache:ke...');
+      });
+    });
+  });
+
   describe('getCacheKeySafely (single arg)', () => {
     it('should return an empty string if there are no command arguments', () => {
       const result = getCacheKeySafely('get', []);
@@ -26,7 +103,7 @@ describe('Redis', () => {
       expect(result).toStrictEqual(['key1']);
     });
 
-    it('should return only the key for multiple arguments', () => {
+    it('should return only the first key for commands that only accept a singe key (get)', () => {
       const cmdArgs = ['key1', 'the-value'];
       const result = getCacheKeySafely('get', cmdArgs);
       expect(result).toStrictEqual(['key1']);