diff --git a/dev-packages/node-integration-tests/suites/tracing/redis-cache/docker-compose.yml b/dev-packages/node-integration-tests/suites/tracing/redis-cache/docker-compose.yml new file mode 100644 index 000000000000..164d5977e33d --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/redis-cache/docker-compose.yml @@ -0,0 +1,9 @@ +version: '3.9' + +services: + db: + image: redis:latest + restart: always + container_name: integration-tests-redis + ports: + - '6379:6379' diff --git a/dev-packages/node-integration-tests/suites/tracing/redis-cache/scenario-ioredis.js b/dev-packages/node-integration-tests/suites/tracing/redis-cache/scenario-ioredis.js new file mode 100644 index 000000000000..22385f81b064 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/redis-cache/scenario-ioredis.js @@ -0,0 +1,41 @@ +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: ['ioredis-cache:'] })], +}); + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +const Redis = require('ioredis'); + +const redis = new Redis({ port: 6379 }); + +async function run() { + await Sentry.startSpan( + { + name: 'Test Span', + op: 'test-span', + }, + async () => { + try { + await redis.set('test-key', 'test-value'); + await redis.set('ioredis-cache:test-key', 'test-value'); + + await redis.get('test-key'); + await redis.get('ioredis-cache:test-key'); + await redis.get('ioredis-cache:unavailable-data'); + } finally { + await redis.disconnect(); + } + }, + ); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts b/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts new file mode 100644 index 000000000000..0c2beaf7d4c8 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts @@ -0,0 +1,92 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +describe('redis auto instrumentation', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('should not add cache spans when key is not prefixed', done => { + const EXPECTED_TRANSACTION = { + transaction: 'Test Span', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: 'set test-key [1 other arguments]', + op: 'db', + data: expect.objectContaining({ + 'sentry.op': 'db', + 'db.system': 'redis', + 'net.peer.name': 'localhost', + 'net.peer.port': 6379, + 'db.statement': 'set test-key [1 other arguments]', + }), + }), + expect.objectContaining({ + description: 'get test-key', + op: 'db', + data: expect.objectContaining({ + 'sentry.op': 'db', + 'db.system': 'redis', + 'net.peer.name': 'localhost', + 'net.peer.port': 6379, + 'db.statement': 'get test-key', + }), + }), + ]), + }; + + createRunner(__dirname, 'scenario-ioredis.js') + .withDockerCompose({ workingDirectory: [__dirname], readyMatches: ['port=6379'] }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); + }); + + test('should create cache spans for prefixed keys', done => { + const EXPECTED_TRANSACTION = { + transaction: 'Test Span', + spans: expect.arrayContaining([ + // SET + expect.objectContaining({ + description: 'set ioredis-cache:test-key [1 other arguments]', + op: 'cache.put', + data: expect.objectContaining({ + 'db.statement': 'set ioredis-cache:test-key [1 other arguments]', + 'cache.key': 'ioredis-cache:test-key', + 'cache.item_size': 2, + 'network.peer.address': 'localhost', + 'network.peer.port': 6379, + }), + }), + // GET + expect.objectContaining({ + description: 'get ioredis-cache:test-key', + op: 'cache.get_item', // todo: will be changed to cache.get + data: expect.objectContaining({ + 'db.statement': 'get ioredis-cache:test-key', + 'cache.hit': true, + 'cache.key': 'ioredis-cache:test-key', + 'cache.item_size': 10, + 'network.peer.address': 'localhost', + 'network.peer.port': 6379, + }), + }), + // GET (unavailable) + expect.objectContaining({ + description: 'get ioredis-cache:unavailable-data', + op: 'cache.get_item', // todo: will be changed to cache.get + data: expect.objectContaining({ + 'db.statement': 'get ioredis-cache:unavailable-data', + 'cache.hit': false, + 'cache.key': 'ioredis-cache:unavailable-data', + 'network.peer.address': 'localhost', + 'network.peer.port': 6379, + }), + }), + ]), + }; + + createRunner(__dirname, 'scenario-ioredis.js') + .withDockerCompose({ workingDirectory: [__dirname], readyMatches: ['port=6379'] }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/redis/scenario-ioredis.js b/dev-packages/node-integration-tests/suites/tracing/redis/scenario-ioredis.js index 52df06b2a386..4c325c3a6c21 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis/scenario-ioredis.js +++ b/dev-packages/node-integration-tests/suites/tracing/redis/scenario-ioredis.js @@ -18,8 +18,8 @@ const redis = new Redis({ port: 6379 }); async function run() { await Sentry.startSpan( { - name: 'Test Transaction', - op: 'transaction', + name: 'Test Span', + op: 'test-span', }, async () => { try { diff --git a/dev-packages/node-integration-tests/suites/tracing/redis/test.ts b/dev-packages/node-integration-tests/suites/tracing/redis/test.ts index fd441201cebc..604b2751f05b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/redis/test.ts @@ -7,12 +7,13 @@ describe('redis auto instrumentation', () => { test('should auto-instrument `ioredis` package when using redis.set() and redis.get()', done => { const EXPECTED_TRANSACTION = { - transaction: 'Test Transaction', + transaction: 'Test Span', spans: expect.arrayContaining([ expect.objectContaining({ description: 'set test-key [1 other arguments]', op: 'db', data: expect.objectContaining({ + 'sentry.op': 'db', 'db.system': 'redis', 'net.peer.name': 'localhost', 'net.peer.port': 6379, @@ -23,6 +24,7 @@ describe('redis auto instrumentation', () => { description: 'get test-key', op: 'db', data: expect.objectContaining({ + 'sentry.op': 'db', 'db.system': 'redis', 'net.peer.name': 'localhost', 'net.peer.port': 6379, diff --git a/packages/core/src/semanticAttributes.ts b/packages/core/src/semanticAttributes.ts index 8f46de21a1ca..2c268110854c 100644 --- a/packages/core/src/semanticAttributes.ts +++ b/packages/core/src/semanticAttributes.ts @@ -35,3 +35,9 @@ export const SEMANTIC_ATTRIBUTE_SENTRY_MEASUREMENT_VALUE = 'sentry.measurement_v export const SEMANTIC_ATTRIBUTE_PROFILE_ID = 'sentry.profile_id'; export const SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME = 'sentry.exclusive_time'; + +export const SEMANTIC_ATTRIBUTE_CACHE_HIT = 'cache.hit'; + +export const SEMANTIC_ATTRIBUTE_CACHE_KEY = 'cache.key'; + +export const SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE = 'cache.item_size'; diff --git a/packages/node/src/integrations/tracing/redis.ts b/packages/node/src/integrations/tracing/redis.ts index 8e1eebb83b6a..f9309b2de33e 100644 --- a/packages/node/src/integrations/tracing/redis.ts +++ b/packages/node/src/integrations/tracing/redis.ts @@ -1,14 +1,89 @@ import { IORedisInstrumentation } from '@opentelemetry/instrumentation-ioredis'; -import { defineIntegration } from '@sentry/core'; +import { + SEMANTIC_ATTRIBUTE_CACHE_HIT, + SEMANTIC_ATTRIBUTE_CACHE_ITEM_SIZE, + SEMANTIC_ATTRIBUTE_CACHE_KEY, + SEMANTIC_ATTRIBUTE_SENTRY_OP, + defineIntegration, + spanToJSON, +} from '@sentry/core'; import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry'; import type { IntegrationFn } from '@sentry/types'; -const _redisIntegration = (() => { +function keyHasPrefix(key: string, prefixes: string[]): boolean { + return prefixes.some(prefix => key.startsWith(prefix)); +} + +/** Currently, caching only supports 'get' and 'set' commands. More commands will be added (setex, mget, del, expire) */ +function shouldConsiderForCache( + redisCommand: string, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + key: string | number | any[] | Buffer, + prefixes: string[], +): boolean { + return (redisCommand === 'get' || redisCommand === 'set') && typeof key === 'string' && keyHasPrefix(key, prefixes); +} + +function calculateCacheItemSize(response: unknown): number | undefined { + try { + if (Buffer.isBuffer(response)) return response.byteLength; + else if (typeof response === 'string') return response.length; + else if (typeof response === 'number') return response.toString().length; + else if (response === null || response === undefined) return 0; + return JSON.stringify(response).length; + } catch (e) { + return undefined; + } +} + +interface RedisOptions { + cachePrefixes?: string[]; +} + +const _redisIntegration = ((options?: RedisOptions) => { return { name: 'Redis', setupOnce() { addOpenTelemetryInstrumentation([ - new IORedisInstrumentation({}), + new IORedisInstrumentation({ + responseHook: (span, redisCommand, cmdArgs, response) => { + const key = cmdArgs[0]; + + if (!options?.cachePrefixes || !shouldConsiderForCache(redisCommand, key, options.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 (typeof key === 'string') { + switch (redisCommand) { + case 'get': + span.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'cache.get_item', // todo: will be changed to cache.get + [SEMANTIC_ATTRIBUTE_CACHE_KEY]: key, + }); + if (cacheItemSize !== undefined) span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, cacheItemSize > 0); + break; + case 'set': + span.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'cache.put', + [SEMANTIC_ATTRIBUTE_CACHE_KEY]: key, + }); + break; + } + } + }, + }), // todo: implement them gradually // new LegacyRedisInstrumentation({}), // new RedisInstrumentation({}),