From 7f030e86f8b6638ece051e5e7d006e62d8531f1e Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 28 Oct 2025 17:37:45 +0100 Subject: [PATCH 1/3] feat(node): Add `maxCacheKeyLength` to Redis integration (remove truncation) --- .../node/src/integrations/tracing/redis.ts | 27 ++++++- .../test/integrations/tracing/redis.test.ts | 81 ++++++++++++++++++- 2 files changed, 103 insertions(+), 5 deletions(-) diff --git a/packages/node/src/integrations/tracing/redis.ts b/packages/node/src/integrations/tracing/redis.ts index 8376c99c1998..4556e942f8df 100644 --- a/packages/node/src/integrations/tracing/redis.ts +++ b/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`, () => { diff --git a/packages/node/test/integrations/tracing/redis.test.ts b/packages/node/test/integrations/tracing/redis.test.ts index 38a5b80eb759..43e8b6298085 100644 --- a/packages/node/test/integrations/tracing/redis.test.ts +++ b/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']); From 7b8787005a697800aca526cac4e8414fea8d39cb Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Tue, 4 Nov 2025 13:39:24 +0100 Subject: [PATCH 2/3] fix lint error --- packages/node/test/integrations/tracing/redis.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/node/test/integrations/tracing/redis.test.ts b/packages/node/test/integrations/tracing/redis.test.ts index 43e8b6298085..fbd1e4d54965 100644 --- a/packages/node/test/integrations/tracing/redis.test.ts +++ b/packages/node/test/integrations/tracing/redis.test.ts @@ -27,6 +27,7 @@ describe('Redis', () => { afterEach(() => { vi.restoreAllMocks(); // Reset redis options by clearing all properties first, then restoring original ones + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete Object.keys(_redisOptions).forEach(key => delete (_redisOptions as any)[key]); Object.assign(_redisOptions, originalRedisOptions); }); From f32141b150b224dc9a249859ff24d923b6d1cc7f Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Thu, 6 Nov 2025 10:36:49 +0100 Subject: [PATCH 3/3] review comment --- packages/node/test/integrations/tracing/redis.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/node/test/integrations/tracing/redis.test.ts b/packages/node/test/integrations/tracing/redis.test.ts index fbd1e4d54965..ae5b879c0b8e 100644 --- a/packages/node/test/integrations/tracing/redis.test.ts +++ b/packages/node/test/integrations/tracing/redis.test.ts @@ -39,7 +39,6 @@ describe('Redis', () => { { 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);