feat(redis): Add cache logic for redis-4 (#12429)

Follow-up as cache logic for `ioredis` was already added.

dev-packages/node-integration-tests/package.json

@@ -55,6 +55,7 @@
     "node-schedule": "^2.1.1",
     "pg": "^8.7.3",
     "proxy": "^2.1.1",
+    "redis-4": "npm:redis@^4.6.14",
     "reflect-metadata": "0.2.1",
     "rxjs": "^7.8.1",
     "yargs": "^16.2.0"

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

@@ -0,0 +1,46 @@
+const { loggingTransport } = require('@sentry-internal/node-integration-tests');
+const Sentry = require('@sentry/node');
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '1.0',
+  tracesSampleRate: 1.0,
+  transport: loggingTransport,
+  integrations: [Sentry.redisIntegration({ cachePrefixes: ['redis-cache:'] })],
+});
+
+// Stop the process from exiting before the transaction is sent
+setInterval(() => {}, 1000);
+
+const { createClient } = require('redis-4');
+
+async function run() {
+  const redisClient = await createClient().connect();
+
+  await Sentry.startSpan(
+    {
+      name: 'Test Span Redis 4',
+      op: 'test-span-redis-4',
+    },
+    async () => {
+      try {
+        await redisClient.set('redis-test-key', 'test-value');
+        await redisClient.set('redis-cache:test-key', 'test-value');
+
+        await redisClient.set('redis-cache:test-key-set-EX', 'test-value', { EX: 10 });
+        await redisClient.setEx('redis-cache:test-key-setex', 10, 'test-value');
+
+        await redisClient.get('redis-test-key');
+        await redisClient.get('redis-cache:test-key');
+        await redisClient.get('redis-cache:unavailable-data');
+
+        await redisClient.mGet(['redis-test-key', 'redis-cache:test-key', 'redis-cache:unavailable-data']);
+      } finally {
+        await redisClient.disconnect();
+      }
+    },
+  );
+}
+
+// eslint-disable-next-line @typescript-eslint/no-floating-promises
+run();

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

@@ -42,7 +42,7 @@ describe('redis cache auto instrumentation', () => {
       .start(done);
   });
 
-  test('should create cache spans for prefixed keys', done => {
+  test('should create cache spans for prefixed keys (ioredis)', done => {
     const EXPECTED_TRANSACTION = {
       transaction: 'Test Span',
       spans: expect.arrayContaining([
@@ -139,4 +139,95 @@ describe('redis cache auto instrumentation', () => {
       .expect({ transaction: EXPECTED_TRANSACTION })
       .start(done);
   });
+
+  test('should create cache spans for prefixed keys (redis-4)', done => {
+    const EXPECTED_REDIS_CONNECT = {
+      transaction: 'redis-connect',
+    };
+
+    const EXPECTED_TRANSACTION = {
+      transaction: 'Test Span Redis 4',
+      spans: expect.arrayContaining([
+        // SET
+        expect.objectContaining({
+          description: 'redis-cache:test-key',
+          op: 'cache.put',
+          origin: 'auto.db.otel.redis',
+          data: expect.objectContaining({
+            'sentry.origin': 'auto.db.otel.redis',
+            'db.statement': 'SET redis-cache:test-key [1 other arguments]',
+            'cache.key': ['redis-cache:test-key'],
+            'cache.item_size': 2,
+          }),
+        }),
+        // SET (with EX)
+        expect.objectContaining({
+          description: 'redis-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 redis-cache:test-key-set-EX [3 other arguments]',
+            'cache.key': ['redis-cache:test-key-set-EX'],
+            'cache.item_size': 2,
+          }),
+        }),
+        // SETEX
+        expect.objectContaining({
+          description: 'redis-cache:test-key-setex',
+          op: 'cache.put',
+          origin: 'auto.db.otel.redis',
+          data: expect.objectContaining({
+            'sentry.origin': 'auto.db.otel.redis',
+            'db.statement': 'SETEX redis-cache:test-key-setex [2 other arguments]',
+            'cache.key': ['redis-cache:test-key-setex'],
+            'cache.item_size': 2,
+          }),
+        }),
+        // GET
+        expect.objectContaining({
+          description: 'redis-cache:test-key',
+          op: 'cache.get',
+          origin: 'auto.db.otel.redis',
+          data: expect.objectContaining({
+            'sentry.origin': 'auto.db.otel.redis',
+            'db.statement': 'GET redis-cache:test-key',
+            'cache.hit': true,
+            'cache.key': ['redis-cache:test-key'],
+            'cache.item_size': 10,
+          }),
+        }),
+        // GET (unavailable - no cache hit)
+        expect.objectContaining({
+          description: 'redis-cache:unavailable-data',
+          op: 'cache.get',
+          origin: 'auto.db.otel.redis',
+          data: expect.objectContaining({
+            'sentry.origin': 'auto.db.otel.redis',
+            'db.statement': 'GET redis-cache:unavailable-data',
+            'cache.hit': false,
+            'cache.key': ['redis-cache:unavailable-data'],
+          }),
+        }),
+        // MGET
+        expect.objectContaining({
+          description: 'redis-test-key, redis-cache:test-key, redis-cache:unavailable-data',
+          op: 'cache.get',
+          origin: 'auto.db.otel.redis',
+          data: expect.objectContaining({
+            'sentry.origin': 'auto.db.otel.redis',
+            'db.statement': 'MGET [3 other arguments]',
+            'cache.hit': true,
+            'cache.key': ['redis-test-key', 'redis-cache:test-key', 'redis-cache:unavailable-data'],
+          }),
+        }),
+      ]),
+    };
+
+    createRunner(__dirname, 'scenario-redis-4.js')
+      .withDockerCompose({ workingDirectory: [__dirname], readyMatches: ['port=6379'] })
+      .expect({ transaction: EXPECTED_REDIS_CONNECT })
+      .expect({ transaction: EXPECTED_TRANSACTION })
+      .start(done);
+  });
 });

packages/node/package.json

@@ -88,6 +88,7 @@
     "@opentelemetry/instrumentation-mysql2": "0.39.0",
     "@opentelemetry/instrumentation-nestjs-core": "0.38.0",
     "@opentelemetry/instrumentation-pg": "0.42.0",
+    "@opentelemetry/instrumentation-redis-4": "0.40.0",
     "@opentelemetry/resources": "^1.25.0",
     "@opentelemetry/sdk-trace-base": "^1.25.0",
     "@opentelemetry/semantic-conventions": "^1.25.0",

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

@@ -13,7 +13,7 @@ import { instrumentMysql, mysqlIntegration } from './mysql';
 import { instrumentMysql2, mysql2Integration } from './mysql2';
 import { instrumentNest, nestIntegration } from './nest';
 import { instrumentPostgres, postgresIntegration } from './postgres';
-import { redisIntegration } from './redis';
+import { instrumentRedis, redisIntegration } from './redis';
 
 /**
  * With OTEL, all performance integrations will be added, as OTEL only initializes them when the patched package is actually required.
@@ -60,5 +60,6 @@ export function getOpenTelemetryInstrumentationToPreload(): (((options?: any) =>
     instrumentPostgres,
     instrumentHapi,
     instrumentGraphql,
+    instrumentRedis,
   ];
 }

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

@@ -1,4 +1,7 @@
+import type { Span } from '@opentelemetry/api';
+import type { RedisResponseCustomAttributeFunction } from '@opentelemetry/instrumentation-ioredis';
 import { IORedisInstrumentation } from '@opentelemetry/instrumentation-ioredis';
+import { RedisInstrumentation } from '@opentelemetry/instrumentation-redis-4';
 import {
   SEMANTIC_ATTRIBUTE_CACHE_HIT,
   SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE,
@@ -9,12 +12,14 @@ import {
   spanToJSON,
 } from '@sentry/core';
 import type { IntegrationFn } from '@sentry/types';
+import { truncate } from '@sentry/utils';
 import { generateInstrumentOnce } from '../../otel/instrument';
 import {
   GET_COMMANDS,
   calculateCacheItemSize,
   getCacheKeySafely,
   getCacheOperation,
+  isInCommands,
   shouldConsiderForCache,
 } from '../../utils/redisCache';
 
@@ -26,64 +31,80 @@ const INTEGRATION_NAME = 'Redis';
 
 let _redisOptions: RedisOptions = {};
 
-export const instrumentRedis = generateInstrumentOnce(INTEGRATION_NAME, () => {
+const cacheResponseHook: RedisResponseCustomAttributeFunction = (span: Span, redisCommand, cmdArgs, response) => {
+  span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.redis');
+
+  const safeKey = getCacheKeySafely(redisCommand, cmdArgs);
+  const cacheOperation = getCacheOperation(redisCommand);
+
+  if (
+    !safeKey ||
+    !cacheOperation ||
+    !_redisOptions?.cachePrefixes ||
+    !shouldConsiderForCache(redisCommand, safeKey, _redisOptions.cachePrefixes)
+  ) {
+    // not relevant for cache
+    return;
+  }
+
+  // otel/ioredis seems to be using the old standard, as there was a change to those params: https://github.com/open-telemetry/opentelemetry-specification/issues/3199
+  // We are using params based on the docs: https://opentelemetry.io/docs/specs/semconv/attributes-registry/network/
+  const networkPeerAddress = spanToJSON(span).data?.['net.peer.name'];
+  const networkPeerPort = spanToJSON(span).data?.['net.peer.port'];
+  if (networkPeerPort && networkPeerAddress) {
+    span.setAttributes({ 'network.peer.address': networkPeerAddress, 'network.peer.port': networkPeerPort });
+  }
+
+  const cacheItemSize = calculateCacheItemSize(response);
+
+  if (cacheItemSize) {
+    span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE, cacheItemSize);
+  }
+
+  if (isInCommands(GET_COMMANDS, redisCommand) && cacheItemSize !== undefined) {
+    span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, cacheItemSize > 0);
+  }
+
+  span.setAttributes({
+    [SEMANTIC_ATTRIBUTE_SENTRY_OP]: cacheOperation,
+    [SEMANTIC_ATTRIBUTE_CACHE_KEY]: safeKey,
+  });
+
+  const spanDescription = safeKey.join(', ');
+
+  span.updateName(truncate(spanDescription, 1024));
+};
+
+const instrumentIORedis = generateInstrumentOnce('IORedis', () => {
   return new IORedisInstrumentation({
-    responseHook: (span, redisCommand, cmdArgs, response) => {
-      span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.redis');
-
-      const safeKey = getCacheKeySafely(redisCommand, cmdArgs);
-      const cacheOperation = getCacheOperation(redisCommand);
-
-      if (
-        !safeKey ||
-        !cacheOperation ||
-        !_redisOptions?.cachePrefixes ||
-        !shouldConsiderForCache(redisCommand, safeKey, _redisOptions.cachePrefixes)
-      ) {
-        // not relevant for cache
-        return;
-      }
-
-      // otel/ioredis seems to be using the old standard, as there was a change to those params: https://github.com/open-telemetry/opentelemetry-specification/issues/3199
-      // We are using params based on the docs: https://opentelemetry.io/docs/specs/semconv/attributes-registry/network/
-      const networkPeerAddress = spanToJSON(span).data?.['net.peer.name'];
-      const networkPeerPort = spanToJSON(span).data?.['net.peer.port'];
-      if (networkPeerPort && networkPeerAddress) {
-        span.setAttributes({ 'network.peer.address': networkPeerAddress, 'network.peer.port': networkPeerPort });
-      }
-
-      const cacheItemSize = calculateCacheItemSize(response);
-
-      if (cacheItemSize) {
-        span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE, cacheItemSize);
-      }
-
-      if (GET_COMMANDS.includes(redisCommand) && cacheItemSize !== undefined) {
-        span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, cacheItemSize > 0);
-      }
-
-      span.setAttributes({
-        [SEMANTIC_ATTRIBUTE_SENTRY_OP]: cacheOperation,
-        [SEMANTIC_ATTRIBUTE_CACHE_KEY]: safeKey,
-      });
-
-      const spanDescription = safeKey.join(', ');
-
-      span.updateName(spanDescription.length > 1024 ? `${spanDescription.substring(0, 1024)}...` : spanDescription);
-    },
+    responseHook: cacheResponseHook,
   });
 });
 
+const instrumentRedis4 = generateInstrumentOnce('Redis-4', () => {
+  return new RedisInstrumentation({
+    responseHook: cacheResponseHook,
+  });
+});
+
+/** To be able to preload all Redis OTel instrumentations with just one ID ("Redis"), all the instrumentations are generated in this one function  */
+export const instrumentRedis = Object.assign(
+  (): void => {
+    instrumentIORedis();
+    instrumentRedis4();
+
+    // todo: implement them gradually
+    // new LegacyRedisInstrumentation({}),
+  },
+  { id: INTEGRATION_NAME },
+);
+
 const _redisIntegration = ((options: RedisOptions = {}) => {
   return {
     name: INTEGRATION_NAME,
     setupOnce() {
       _redisOptions = options;
       instrumentRedis();
-
-      // todo: implement them gradually
-      // new LegacyRedisInstrumentation({}),
-      // new RedisInstrumentation({}),
     },
   };
 }) satisfies IntegrationFn;

packages/node/src/utils/redisCache.ts

@@ -7,15 +7,19 @@ export const GET_COMMANDS = ['get', 'mget'];
 export const SET_COMMANDS = ['set', 'setex'];
 // todo: del, expire
 
+/** 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). */
+export function isInCommands(redisCommands: string[], command: string): boolean {
+  return redisCommands.includes(command.toLowerCase());
+}
+
 /** Determine cache operation based on redis statement */
 export function getCacheOperation(
   command: string,
 ): 'cache.get' | 'cache.put' | 'cache.remove' | 'cache.flush' | undefined {
-  const lowercaseStatement = command.toLowerCase();
-
-  if (GET_COMMANDS.includes(lowercaseStatement)) {
+  if (isInCommands(GET_COMMANDS, command)) {
     return 'cache.get';
-  } else if (SET_COMMANDS.includes(lowercaseStatement)) {
+  } else if (isInCommands(SET_COMMANDS, command)) {
     return 'cache.put';
   } else {
     return undefined;
@@ -44,7 +48,7 @@ export function getCacheKeySafely(redisCommand: string, cmdArgs: IORedisCommandA
       }
     };
 
-    if (SINGLE_ARG_COMMANDS.includes(redisCommand) && cmdArgs.length > 0) {
+    if (isInCommands(SINGLE_ARG_COMMANDS, redisCommand) && cmdArgs.length > 0) {
       return processArg(cmdArgs[0]);
     }
 

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

@@ -13,12 +13,18 @@ describe('Redis', () => {
       expect(result).toBe(undefined);
     });
 
-    it('should return a string representation of a single argument', () => {
+    it('should return a string array representation of a single argument', () => {
       const cmdArgs = ['key1'];
       const result = getCacheKeySafely('get', cmdArgs);
       expect(result).toStrictEqual(['key1']);
     });
 
+    it('should return a string array representation of a single argument (uppercase)', () => {
+      const cmdArgs = ['key1'];
+      const result = getCacheKeySafely('GET', cmdArgs);
+      expect(result).toStrictEqual(['key1']);
+    });
+
     it('should return only the key for multiple arguments', () => {
       const cmdArgs = ['key1', 'the-value'];
       const result = getCacheKeySafely('get', cmdArgs);