From a1e98003da067f00265c908bea79a98f889d00a9 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 28 Aug 2026 16:34:29 +0200 Subject: [PATCH 1/2] feat(server-utils)!: Emit low cardinality redis span names Redis reports no SQL statement, so there is no query summary to name its spans after. With span streaming they use the next conventions template that can be filled instead: the command paired with `{server.address}:{server.port}`, since redis has no collection or namespace to pair with. It falls back to `{db.system.name}` when the client was configured without a host. This keeps the serialized command, which carries the key and its arguments, out of the span name. It stays on `db.query.text`. `traceLifecycle: 'static'` keeps the existing names. Refs #23523 Co-Authored-By: Claude Opus 5 (1M context) --- .../suites/tracing/ioredis-dc/instrument.mjs | 2 +- .../suites/tracing/ioredis-dc/test.ts | 130 ++++++- .../redis-cache/instrument-ioredis.mjs | 2 +- .../redis-cache/instrument-redis-4.mjs | 2 +- .../redis-cache/instrument-redis-5.mjs | 2 +- .../suites/tracing/redis-cache/test.ts | 348 ++++++++++++++++++ .../suites/tracing/redis-dc/instrument.mjs | 2 +- .../suites/tracing/redis-dc/test.ts | 129 ++++++- .../suites/tracing/redis/instrument.mjs | 2 +- .../suites/tracing/redis/test.ts | 91 ++++- .../src/integrations/redis/index.ts | 15 +- .../redis/ioredis-channel-subscriber.ts | 15 +- .../redis/ioredis-channel-subscriber.test.ts | 17 + 13 files changed, 745 insertions(+), 12 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/ioredis-dc/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/ioredis-dc/instrument.mjs index fef89b43c532..c0a1998369a5 100644 --- a/dev-packages/node-integration-tests/suites/tracing/ioredis-dc/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/ioredis-dc/instrument.mjs @@ -2,7 +2,7 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', + traceLifecycle: process.env.STREAMED === 'true' ? 'stream' : 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', tracesSampleRate: 1.0, diff --git a/dev-packages/node-integration-tests/suites/tracing/ioredis-dc/test.ts b/dev-packages/node-integration-tests/suites/tracing/ioredis-dc/test.ts index 39969e658748..3ec0e4a78840 100644 --- a/dev-packages/node-integration-tests/suites/tracing/ioredis-dc/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/ioredis-dc/test.ts @@ -1,4 +1,6 @@ -import { afterAll, expect } from 'vitest'; +import { SENTRY_TRACE_LIFECYCLE } from '@sentry/conventions/attributes'; +import type { SerializedStreamedSpanContainer } from '@sentry/core'; +import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests, describeWithDockerCompose } from '../../../utils/runner'; describeWithDockerCompose( @@ -103,5 +105,131 @@ describeWithDockerCompose( .completed(); }); }); + + // The same commands as above, asserted on the streamed span container. The native + // diagnostics_channel subscriber already names db spans `redis-{command}`, which is low + // cardinality, so span streaming does not change them. + describe('streamed', () => { + const ORIGIN = 'auto.db.redis.diagnostic_channel'; + const SEGMENT_NAME = 'Test Span IORedis 5.11 DC'; + const HOST = '127.0.0.1'; + const PORT = 6382; + + const streamAttribute = (value: unknown): { type: string; value: unknown } => ({ + type: Array.isArray(value) ? 'array' : Number.isInteger(value) ? 'integer' : typeof value, + value, + }); + + // Streamed spans carry `{ type, value }` attribute pairs; the expectations below are written + // as plain values and wrapped here. + const streamAttributes = (values: Record): Record => + Object.fromEntries(Object.entries(values).map(([key, value]) => [key, streamAttribute(value)])); + + function streamedSpan(name: string, op: string, attributes: Record): unknown { + return { + name, + attributes: { + ...streamAttributes({ + 'db.system.name': 'redis', + 'sentry.environment': 'production', + 'sentry.op': op, + 'sentry.origin': ORIGIN, + 'sentry.release': '1.0', + 'sentry.sdk.name': 'sentry.javascript.node', + 'sentry.segment.name': SEGMENT_NAME, + 'server.address': HOST, + 'server.port': PORT, + [SENTRY_TRACE_LIFECYCLE]: 'stream', + ...attributes, + }), + 'sentry.sdk.version': { type: 'string', value: expect.any(String) }, + 'sentry.segment.id': { type: 'string', value: expect.stringMatching(/^[\da-f]{16}$/) }, + }, + end_timestamp: expect.any(Number), + is_segment: false, + parent_span_id: expect.stringMatching(/^[\da-f]{16}$/), + span_id: expect.stringMatching(/^[\da-f]{16}$/), + start_timestamp: expect.any(Number), + status: 'ok', + trace_id: expect.stringMatching(/^[\da-f]{32}$/), + }; + } + + const PEER = { 'network.peer.address': HOST, 'network.peer.port': PORT }; + + createEsmAndCjsTests(__dirname, 'scenario-ioredis-5-11.mjs', 'instrument.mjs', (createTestRunner, test) => { + test( + 'creates streamed spans for ioredis v5.11 commands via diagnostics_channel', + { timeout: 75_000 }, + async () => { + await createTestRunner() + .withEnv({ STREAMED: 'true' }) + .expect({ + span: (container: SerializedStreamedSpanContainer) => { + // The connect span opens its own segment but shares the trace with the test span, + // so both segments arrive in the same container. + expect(container.items.filter(item => item.is_segment).map(item => item.name)).toEqual([ + 'redis-connect', + SEGMENT_NAME, + ]); + + // ioredis' own handshake commands (`client SETINFO`, `info`) are emitted on the + // channel too, but belong to the connect segment — the test span's children are the + // commands the scenario issues. + const spans = container.items.filter( + item => !item.is_segment && item.attributes['sentry.segment.name']?.value === SEGMENT_NAME, + ); + + expect(spans).toEqual([ + streamedSpan('redis-set', 'db.query', { + 'db.operation.name': 'set', + 'db.query.text': 'set dc-test-key ?', + }), + streamedSpan('dc-cache:test-key', 'cache.put', { + ...PEER, + 'db.operation.name': 'set', + 'db.query.text': 'set dc-cache:test-key ?', + 'cache.key': ['dc-cache:test-key'], + 'cache.item_size': 2, + }), + streamedSpan('dc-cache:test-key-ex', 'cache.put', { + ...PEER, + 'db.operation.name': 'set', + 'db.query.text': 'set dc-cache:test-key-ex ? ? ?', + 'cache.key': ['dc-cache:test-key-ex'], + 'cache.item_size': 2, + }), + streamedSpan('redis-get', 'db.query', { + 'db.operation.name': 'get', + 'db.query.text': 'get dc-test-key', + }), + streamedSpan('dc-cache:test-key', 'cache.get', { + ...PEER, + 'db.operation.name': 'get', + 'db.query.text': 'get dc-cache:test-key', + 'cache.key': ['dc-cache:test-key'], + 'cache.hit': true, + 'cache.item_size': 10, + }), + streamedSpan('dc-cache:unavailable-data', 'cache.get', { + ...PEER, + 'db.operation.name': 'get', + 'db.query.text': 'get dc-cache:unavailable-data', + 'cache.key': ['dc-cache:unavailable-data'], + 'cache.hit': false, + }), + streamedSpan('redis-mget', 'db.query', { + 'db.operation.name': 'mget', + 'db.query.text': 'mget ? ? ?', + }), + ]); + }, + }) + .start() + .completed(); + }, + ); + }); + }); }, ); diff --git a/dev-packages/node-integration-tests/suites/tracing/redis-cache/instrument-ioredis.mjs b/dev-packages/node-integration-tests/suites/tracing/redis-cache/instrument-ioredis.mjs index 9732fa703714..8b1385e92375 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis-cache/instrument-ioredis.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/redis-cache/instrument-ioredis.mjs @@ -2,7 +2,7 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', + traceLifecycle: process.env.STREAMED === 'true' ? 'stream' : 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', tracesSampleRate: 1.0, diff --git a/dev-packages/node-integration-tests/suites/tracing/redis-cache/instrument-redis-4.mjs b/dev-packages/node-integration-tests/suites/tracing/redis-cache/instrument-redis-4.mjs index 02e29733ce03..9b24191fd0a1 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis-cache/instrument-redis-4.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/redis-cache/instrument-redis-4.mjs @@ -2,7 +2,7 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', + traceLifecycle: process.env.STREAMED === 'true' ? 'stream' : 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', tracesSampleRate: 1.0, diff --git a/dev-packages/node-integration-tests/suites/tracing/redis-cache/instrument-redis-5.mjs b/dev-packages/node-integration-tests/suites/tracing/redis-cache/instrument-redis-5.mjs index 50b4748a1659..58f4a0875acd 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis-cache/instrument-redis-5.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/redis-cache/instrument-redis-5.mjs @@ -2,7 +2,7 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', + traceLifecycle: process.env.STREAMED === 'true' ? 'stream' : 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', tracesSampleRate: 1.0, 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 index 712d7b3852e3..05ed83f06c3a 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts @@ -1,3 +1,5 @@ +import { SENTRY_TRACE_LIFECYCLE } from '@sentry/conventions/attributes'; +import type { SerializedStreamedSpanContainer } from '@sentry/core'; import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests, describeWithDockerCompose } from '../../../utils/runner'; @@ -425,4 +427,350 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory }); }); }); + + describe('streamed', () => { + // The blocks above assert the same commands as transactions. With span streaming, span names + // have to be low cardinality, so `db.query` spans drop the serialized statement from their + // name — it stays on `db.query.text` — and are named + // `{db.operation.name} {server.address}:{server.port}` instead. Cache spans are still named + // after the cache key by the cache hook, and batch spans keep their `MULTI`/`PIPELINE` name. + const streamAttribute = (value: unknown): { type: string; value: unknown } => ({ + type: Array.isArray(value) ? 'array' : Number.isInteger(value) ? 'integer' : typeof value, + value, + }); + + // Streamed spans carry `{ type, value }` attribute pairs; the expectations below are written + // as plain values and wrapped here. + const streamAttributes = (values: Record): Record => + Object.fromEntries(Object.entries(values).map(([key, value]) => [key, streamAttribute(value)])); + + const commonAttributes = (segmentName: string): Record => ({ + ...streamAttributes({ + 'db.system.name': 'redis', + 'sentry.environment': 'production', + 'sentry.kind': 'client', + 'sentry.origin': redisOrigin, + 'sentry.release': '1.0', + 'sentry.sdk.name': 'sentry.javascript.node', + 'sentry.segment.name': segmentName, + [SENTRY_TRACE_LIFECYCLE]: 'stream', + }), + 'sentry.sdk.version': { type: 'string', value: expect.any(String) }, + 'sentry.segment.id': { type: 'string', value: expect.stringMatching(/^[\da-f]{16}$/) }, + }); + + function streamedSpan({ + name, + op, + segmentName, + status = 'ok', + attributes, + }: { + name: string; + op: string; + segmentName: string; + status?: string; + attributes: Record; + }): unknown { + return { + name, + attributes: { + ...commonAttributes(segmentName), + ...streamAttributes({ 'sentry.op': op, ...attributes }), + }, + end_timestamp: expect.any(Number), + is_segment: false, + parent_span_id: expect.stringMatching(/^[\da-f]{16}$/), + span_id: expect.stringMatching(/^[\da-f]{16}$/), + start_timestamp: expect.any(Number), + status, + trace_id: expect.stringMatching(/^[\da-f]{32}$/), + }; + } + + const childSpans = (container: SerializedStreamedSpanContainer): SerializedStreamedSpanContainer['items'] => + container.items.filter(item => !item.is_segment); + + describe('ioredis', () => { + const segmentName = 'Test Span'; + const connection = { 'server.address': 'localhost', 'server.port': 6383 }; + const peer = { 'network.peer.address': 'localhost', 'network.peer.port': 6383 }; + + const span = (name: string, op: string, attributes: Record, status?: string): unknown => + streamedSpan({ name, op, segmentName, status, attributes: { ...connection, ...attributes } }); + + createEsmAndCjsTests(__dirname, 'scenario-ioredis.mjs', 'instrument-ioredis.mjs', (createTestRunner, test) => { + test('creates streamed db and cache spans (ioredis)', { timeout: 60_000 }, async () => { + await createTestRunner() + .withEnv({ STREAMED: 'true' }) + .expect({ + span: (container: SerializedStreamedSpanContainer) => { + expect(container.items.find(item => item.is_segment)?.name).toBe(segmentName); + + expect(childSpans(container)).toEqual([ + span('set localhost:6383', redisSpanOp, { + 'db.operation.name': 'set', + 'db.query.text': 'set test-key [1 other arguments]', + }), + span('ioredis-cache:test-key', 'cache.put', { + ...peer, + 'db.operation.name': 'set', + 'db.query.text': 'set ioredis-cache:test-key [1 other arguments]', + 'cache.key': ['ioredis-cache:test-key'], + 'cache.item_size': 2, + }), + span('ioredis-cache:test-key-set-EX', 'cache.put', { + ...peer, + 'db.operation.name': 'set', + 'db.query.text': 'set ioredis-cache:test-key-set-EX [3 other arguments]', + 'cache.key': ['ioredis-cache:test-key-set-EX'], + 'cache.item_size': 2, + }), + span('ioredis-cache:test-key-setex', 'cache.put', { + ...peer, + 'db.operation.name': 'setex', + 'db.query.text': 'setex ioredis-cache:test-key-setex [2 other arguments]', + 'cache.key': ['ioredis-cache:test-key-setex'], + 'cache.item_size': 2, + }), + span('get localhost:6383', redisSpanOp, { + 'db.operation.name': 'get', + 'db.query.text': 'get test-key', + }), + span('ioredis-cache:test-key', 'cache.get', { + ...peer, + 'db.operation.name': 'get', + 'db.query.text': 'get ioredis-cache:test-key', + 'cache.key': ['ioredis-cache:test-key'], + 'cache.hit': true, + 'cache.item_size': 10, + }), + span('ioredis-cache:unavailable-data', 'cache.get', { + ...peer, + 'db.operation.name': 'get', + 'db.query.text': 'get ioredis-cache:unavailable-data', + 'cache.key': ['ioredis-cache:unavailable-data'], + 'cache.hit': false, + }), + span('test-key, ioredis-cache:test-key, ioredis-cache:unavailable-data', 'cache.get', { + ...peer, + 'db.operation.name': 'mget', + 'db.query.text': 'mget [3 other arguments]', + 'cache.key': ['test-key', 'ioredis-cache:test-key', 'ioredis-cache:unavailable-data'], + 'cache.hit': true, + 'cache.item_size': 20, + }), + span('ioredis-cache:test-key', 'cache.remove', { + ...peer, + 'db.operation.name': 'del', + 'db.query.text': 'del ioredis-cache:test-key', + 'cache.key': ['ioredis-cache:test-key'], + }), + ]); + }, + }) + .start() + .completed(); + }); + }); + }); + + // node-redis v4 fills in `socket.host`, so its `db.query` spans get the + // `{db.operation.name} {server.address}:{server.port}` name. + describe('redis-4', () => { + const segmentName = 'Test Span Redis 4'; + const connection = { 'server.address': 'localhost', 'server.port': 6383 }; + const peer = { 'network.peer.address': 'localhost', 'network.peer.port': 6383 }; + + const span = (name: string, op: string, attributes: Record, status?: string): unknown => + streamedSpan({ name, op, segmentName, status, attributes: { ...connection, ...attributes } }); + + createEsmAndCjsTests(__dirname, 'scenario-redis-4.mjs', 'instrument-redis-4.mjs', (createTestRunner, test) => { + test('creates streamed db and cache spans (redis-4)', { timeout: 60_000 }, async () => { + await createTestRunner() + .withEnv({ STREAMED: 'true' }) + .expect({ + span: (container: SerializedStreamedSpanContainer) => { + // The connect span opens its own segment, but shares the trace with the test span, + // so both segments arrive in the same container. + expect(container.items.filter(item => item.is_segment).map(item => item.name)).toEqual([ + 'redis-connect', + segmentName, + ]); + + expect(childSpans(container)).toEqual([ + span('SET localhost:6383', redisSpanOp, { + 'db.operation.name': 'SET', + 'db.query.text': 'SET redis-test-key [1 other arguments]', + }), + span('redis-cache:test-key', 'cache.put', { + ...peer, + 'db.operation.name': 'SET', + 'db.query.text': 'SET redis-cache:test-key [1 other arguments]', + 'cache.key': ['redis-cache:test-key'], + 'cache.item_size': 2, + }), + span('redis-cache:test-key-set-EX', 'cache.put', { + ...peer, + 'db.operation.name': 'SET', + 'db.query.text': 'SET redis-cache:test-key-set-EX [3 other arguments]', + 'cache.key': ['redis-cache:test-key-set-EX'], + 'cache.item_size': 2, + }), + span('redis-cache:test-key-setex', 'cache.put', { + ...peer, + 'db.operation.name': 'SETEX', + 'db.query.text': 'SETEX redis-cache:test-key-setex [2 other arguments]', + 'cache.key': ['redis-cache:test-key-setex'], + 'cache.item_size': 2, + }), + span('GET localhost:6383', redisSpanOp, { + 'db.operation.name': 'GET', + 'db.query.text': 'GET redis-test-key', + }), + span('redis-cache:test-key', 'cache.get', { + ...peer, + 'db.operation.name': 'GET', + 'db.query.text': 'GET redis-cache:test-key', + 'cache.key': ['redis-cache:test-key'], + 'cache.hit': true, + 'cache.item_size': 10, + }), + span('redis-cache:unavailable-data', 'cache.get', { + ...peer, + 'db.operation.name': 'GET', + 'db.query.text': 'GET redis-cache:unavailable-data', + 'cache.key': ['redis-cache:unavailable-data'], + 'cache.hit': false, + }), + span('redis-test-key, redis-cache:test-key, redis-cache:unavailable-data', 'cache.get', { + ...peer, + 'db.operation.name': 'MGET', + 'db.query.text': 'MGET [3 other arguments]', + 'cache.key': ['redis-test-key', 'redis-cache:test-key', 'redis-cache:unavailable-data'], + 'cache.hit': true, + 'cache.item_size': 20, + }), + span('redis-cache:test-key', 'cache.remove', { + ...peer, + 'db.operation.name': 'DEL', + 'db.query.text': 'DEL redis-cache:test-key', + 'cache.key': ['redis-cache:test-key'], + }), + // Batch spans are named after the batch operation, which is already low cardinality. + span('MULTI', redisSpanOp, { 'db.operation.batch.size': 2 }), + span( + 'INCR localhost:6383', + redisSpanOp, + { + 'db.operation.name': 'INCR', + 'db.query.text': 'INCR redis-test-key', + 'error.type': 'Error', + 'sentry.status.message': 'ERR value is not an integer or out of range', + }, + 'error', + ), + ]); + }, + }) + .start() + .completed(); + }); + }); + }); + + // node-redis v5 leaves `socket.host` unset when only a port is passed, so there is no + // `server.address` to pair the operation with and the span name falls back to + // `{db.system.name}`. + describe('redis-5', () => { + const segmentName = 'Test Span Redis 5'; + const connection = { 'server.port': 6383 }; + + const span = (name: string, op: string, attributes: Record, status?: string): unknown => + streamedSpan({ name, op, segmentName, status, attributes: { ...connection, ...attributes } }); + + createEsmAndCjsTests(__dirname, 'scenario-redis-5.mjs', 'instrument-redis-5.mjs', (createTestRunner, test) => { + test('creates streamed db and cache spans (redis-5)', { timeout: 60_000 }, async () => { + await createTestRunner() + .withEnv({ STREAMED: 'true' }) + .expect({ + span: (container: SerializedStreamedSpanContainer) => { + expect(container.items.filter(item => item.is_segment).map(item => item.name)).toEqual([ + 'redis-connect', + segmentName, + ]); + + expect(childSpans(container)).toEqual([ + span('redis', redisSpanOp, { + 'db.operation.name': 'SET', + 'db.query.text': 'SET redis-5-test-key [1 other arguments]', + }), + span('redis-5-cache:test-key', 'cache.put', { + 'db.operation.name': 'SET', + 'db.query.text': 'SET redis-5-cache:test-key [1 other arguments]', + 'cache.key': ['redis-5-cache:test-key'], + 'cache.item_size': 2, + }), + span('redis-5-cache:test-key-set-EX', 'cache.put', { + 'db.operation.name': 'SET', + 'db.query.text': 'SET redis-5-cache:test-key-set-EX [3 other arguments]', + 'cache.key': ['redis-5-cache:test-key-set-EX'], + 'cache.item_size': 2, + }), + span('redis-5-cache:test-key-setex', 'cache.put', { + 'db.operation.name': 'SETEX', + 'db.query.text': 'SETEX redis-5-cache:test-key-setex [2 other arguments]', + 'cache.key': ['redis-5-cache:test-key-setex'], + 'cache.item_size': 2, + }), + span('redis', redisSpanOp, { + 'db.operation.name': 'GET', + 'db.query.text': 'GET redis-5-test-key', + }), + span('redis-5-cache:test-key', 'cache.get', { + 'db.operation.name': 'GET', + 'db.query.text': 'GET redis-5-cache:test-key', + 'cache.key': ['redis-5-cache:test-key'], + 'cache.hit': true, + 'cache.item_size': 10, + }), + span('redis-5-cache:unavailable-data', 'cache.get', { + 'db.operation.name': 'GET', + 'db.query.text': 'GET redis-5-cache:unavailable-data', + 'cache.key': ['redis-5-cache:unavailable-data'], + 'cache.hit': false, + }), + span('redis-5-test-key, redis-5-cache:test-key, redis-5-cache:unavailable-data', 'cache.get', { + 'db.operation.name': 'MGET', + 'db.query.text': 'MGET [3 other arguments]', + 'cache.key': ['redis-5-test-key', 'redis-5-cache:test-key', 'redis-5-cache:unavailable-data'], + 'cache.hit': true, + 'cache.item_size': 20, + }), + span('redis-5-cache:test-key', 'cache.remove', { + 'db.operation.name': 'DEL', + 'db.query.text': 'DEL redis-5-cache:test-key', + 'cache.key': ['redis-5-cache:test-key'], + }), + span('MULTI', redisSpanOp, { 'db.operation.batch.size': 2 }), + span( + 'redis', + redisSpanOp, + { + 'db.operation.name': 'INCR', + 'db.query.text': 'INCR redis-5-test-key', + 'error.type': 'Error', + 'sentry.status.message': 'ERR value is not an integer or out of range', + }, + 'error', + ), + ]); + }, + }) + .start() + .completed(); + }); + }); + }); + }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/redis-dc/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/redis-dc/instrument.mjs index fef89b43c532..c0a1998369a5 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis-dc/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/redis-dc/instrument.mjs @@ -2,7 +2,7 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', + traceLifecycle: process.env.STREAMED === 'true' ? 'stream' : 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', tracesSampleRate: 1.0, diff --git a/dev-packages/node-integration-tests/suites/tracing/redis-dc/test.ts b/dev-packages/node-integration-tests/suites/tracing/redis-dc/test.ts index d97db9ae92fe..8c576abc69c8 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis-dc/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/redis-dc/test.ts @@ -1,4 +1,6 @@ -import { afterAll, expect } from 'vitest'; +import { SENTRY_TRACE_LIFECYCLE } from '@sentry/conventions/attributes'; +import type { SerializedStreamedSpanContainer } from '@sentry/core'; +import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests, describeWithDockerCompose } from '../../../utils/runner'; describeWithDockerCompose( @@ -111,5 +113,130 @@ describeWithDockerCompose( .completed(); }); }); + + // The same commands as above, asserted on the streamed span container. The native + // diagnostics_channel subscriber already names db spans `redis-{command}`, which is low + // cardinality, so span streaming does not change them. + describe('streamed', () => { + const ORIGIN = 'auto.db.redis.diagnostic_channel'; + const SEGMENT_NAME = 'Test Span Redis 5 DC'; + const HOST = '127.0.0.1'; + const PORT = 6381; + + const streamAttribute = (value: unknown): { type: string; value: unknown } => ({ + type: Array.isArray(value) ? 'array' : Number.isInteger(value) ? 'integer' : typeof value, + value, + }); + + // Streamed spans carry `{ type, value }` attribute pairs; the expectations below are written + // as plain values and wrapped here. + const streamAttributes = (values: Record): Record => + Object.fromEntries(Object.entries(values).map(([key, value]) => [key, streamAttribute(value)])); + + function streamedSpan(name: string, op: string, attributes: Record): unknown { + return { + name, + attributes: { + ...streamAttributes({ + 'db.system.name': 'redis', + 'sentry.environment': 'production', + 'sentry.op': op, + 'sentry.origin': ORIGIN, + 'sentry.release': '1.0', + 'sentry.sdk.name': 'sentry.javascript.node', + 'sentry.segment.name': SEGMENT_NAME, + 'server.address': HOST, + 'server.port': PORT, + [SENTRY_TRACE_LIFECYCLE]: 'stream', + ...attributes, + }), + 'sentry.sdk.version': { type: 'string', value: expect.any(String) }, + 'sentry.segment.id': { type: 'string', value: expect.stringMatching(/^[\da-f]{16}$/) }, + }, + end_timestamp: expect.any(Number), + is_segment: false, + parent_span_id: expect.stringMatching(/^[\da-f]{16}$/), + span_id: expect.stringMatching(/^[\da-f]{16}$/), + start_timestamp: expect.any(Number), + status: 'ok', + trace_id: expect.stringMatching(/^[\da-f]{32}$/), + }; + } + + const PEER = { 'network.peer.address': HOST, 'network.peer.port': PORT }; + + createEsmAndCjsTests(__dirname, 'scenario-redis-5-tracing.mjs', 'instrument.mjs', (createTestRunner, test) => { + test('creates streamed spans for redis v5 commands via diagnostics_channel', { timeout: 60_000 }, async () => { + await createTestRunner() + .withEnv({ STREAMED: 'true' }) + .expect({ + span: (container: SerializedStreamedSpanContainer) => { + // The connect span opens its own segment but shares the trace with the test span, + // so both segments arrive in the same container. + expect(container.items.filter(item => item.is_segment).map(item => item.name)).toEqual([ + 'redis-connect', + SEGMENT_NAME, + ]); + + const spans = container.items.filter( + item => !item.is_segment && item.attributes['sentry.segment.name']?.value === SEGMENT_NAME, + ); + + expect(spans).toEqual([ + streamedSpan('redis-SET', 'db.query', { + 'db.operation.name': 'SET', + 'db.query.text': 'SET dc-test-key ?', + }), + // cache SET: span name updated to the key by the cache hook + streamedSpan('dc-cache:test-key', 'cache.put', { + ...PEER, + 'db.operation.name': 'SET', + 'db.query.text': 'SET dc-cache:test-key ?', + 'cache.key': ['dc-cache:test-key'], + 'cache.item_size': 2, + }), + // cache SET with EX option: redis v5 sends SET key value EX 10 as the command + streamedSpan('dc-cache:test-key-ex', 'cache.put', { + ...PEER, + 'db.operation.name': 'SET', + 'db.query.text': 'SET dc-cache:test-key-ex ? ? ?', + 'cache.key': ['dc-cache:test-key-ex'], + 'cache.item_size': 2, + }), + streamedSpan('redis-GET', 'db.query', { + 'db.operation.name': 'GET', + 'db.query.text': 'GET dc-test-key', + }), + // cache GET (hit) + streamedSpan('dc-cache:test-key', 'cache.get', { + ...PEER, + 'db.operation.name': 'GET', + 'db.query.text': 'GET dc-cache:test-key', + 'cache.key': ['dc-cache:test-key'], + 'cache.hit': true, + 'cache.item_size': 10, + }), + // cache GET (miss) + streamedSpan('dc-cache:unavailable-data', 'cache.get', { + ...PEER, + 'db.operation.name': 'GET', + 'db.query.text': 'GET dc-cache:unavailable-data', + 'cache.key': ['dc-cache:unavailable-data'], + 'cache.hit': false, + }), + // MGET: node-redis sanitizes args for diagnostics_channel (keys become '?'), + // so cache detection cannot match prefixes — remains a plain db.query span. + streamedSpan('redis-MGET', 'db.query', { + 'db.operation.name': 'MGET', + 'db.query.text': 'MGET ? ? ?', + }), + ]); + }, + }) + .start() + .completed(); + }); + }); + }); }, ); diff --git a/dev-packages/node-integration-tests/suites/tracing/redis/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/redis/instrument.mjs index 170ad6f6a702..22bf57f14364 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/redis/instrument.mjs @@ -2,7 +2,7 @@ import * as Sentry from '@sentry/node'; import { loggingTransport } from '@sentry-internal/node-integration-tests'; Sentry.init({ - traceLifecycle: 'static', + traceLifecycle: process.env.STREAMED === 'true' ? 'stream' : 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', tracesSampleRate: 1.0, 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 90a625eb8adf..ee0d3cd6c9c3 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/redis/test.ts @@ -1,4 +1,6 @@ -import { afterAll, expect } from 'vitest'; +import { SENTRY_TRACE_LIFECYCLE } from '@sentry/conventions/attributes'; +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, type SerializedStreamedSpanContainer } from '@sentry/core'; +import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests, describeWithDockerCompose } from '../../../utils/runner'; describeWithDockerCompose('redis auto instrumentation', { workingDirectory: [__dirname] }, () => { @@ -67,4 +69,91 @@ describeWithDockerCompose('redis auto instrumentation', { workingDirectory: [__d }, ); }); + + describe('streamed', () => { + // The same three commands as above, asserted on the streamed span container. Only the span + // name differs: with span streaming names have to be low cardinality, so the serialized + // statement is reported through `db.query.text` alone and the name becomes + // `{db.operation.name} {server.address}:{server.port}`. + const COMMON_ATTRIBUTES = { + 'db.system.name': { type: 'string', value: 'redis' }, + 'server.address': { type: 'string', value: 'localhost' }, + 'server.port': { type: 'integer', value: 6380 }, + 'sentry.kind': { type: 'string', value: 'client' }, + 'sentry.environment': { type: 'string', value: 'production' }, + 'sentry.op': { type: 'string', value: redisSpanOp }, + 'sentry.origin': { type: 'string', value: origin }, + 'sentry.release': { type: 'string', value: '1.0' }, + 'sentry.sdk.name': { type: 'string', value: 'sentry.javascript.node' }, + 'sentry.sdk.version': { type: 'string', value: expect.any(String) }, + 'sentry.segment.id': { type: 'string', value: expect.stringMatching(/^[\da-f]{16}$/) }, + 'sentry.segment.name': { type: 'string', value: 'Test Span' }, + [SENTRY_TRACE_LIFECYCLE]: { type: 'string', value: 'stream' }, + }; + + function expectedDbSpan({ + operation, + statement, + status = 'ok', + errorMessage, + }: { + operation: string; + statement: string; + status?: string; + errorMessage?: string; + }): unknown { + return { + attributes: { + ...COMMON_ATTRIBUTES, + 'db.operation.name': { type: 'string', value: operation }, + 'db.query.text': { type: 'string', value: statement }, + ...(errorMessage + ? { + 'error.type': { type: 'string', value: 'ReplyError' }, + 'sentry.status.message': { type: 'string', value: errorMessage }, + } + : {}), + }, + name: `${operation} localhost:6380`, + end_timestamp: expect.any(Number), + is_segment: false, + parent_span_id: expect.stringMatching(/^[\da-f]{16}$/), + span_id: expect.stringMatching(/^[\da-f]{16}$/), + start_timestamp: expect.any(Number), + status, + trace_id: expect.stringMatching(/^[\da-f]{32}$/), + }; + } + + createEsmAndCjsTests(__dirname, 'scenario-ioredis.mjs', 'instrument.mjs', (createTestRunner, test) => { + test('should auto-instrument `ioredis` package with span streaming enabled', { timeout: 75_000 }, async () => { + await createTestRunner() + .withEnv({ STREAMED: 'true' }) + .expect({ + span: (container: SerializedStreamedSpanContainer) => { + const segmentSpan = container.items.find(item => item.is_segment); + expect(segmentSpan?.name).toBe('Test Span'); + + const dbSpans = container.items.filter( + item => item.attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP]?.value === redisSpanOp, + ); + + expect(dbSpans).toEqual([ + expectedDbSpan({ operation: 'set', statement: 'set test-key [1 other arguments]' }), + expectedDbSpan({ operation: 'get', statement: 'get test-key' }), + // a failing command produces a span with an error status + expectedDbSpan({ + operation: 'incr', + statement: 'incr test-key', + status: 'error', + errorMessage: 'ERR value is not an integer or out of range', + }), + ]); + }, + }) + .start() + .completed(); + }); + }); + }); }); diff --git a/packages/server-utils/src/integrations/redis/index.ts b/packages/server-utils/src/integrations/redis/index.ts index f7b5678318a5..e237a47d2574 100644 --- a/packages/server-utils/src/integrations/redis/index.ts +++ b/packages/server-utils/src/integrations/redis/index.ts @@ -15,6 +15,8 @@ import { isObjectLike, defineIntegration, getActiveSpan, + getClient, + hasSpanStreamingEnabled, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, startInactiveSpan, @@ -101,8 +103,19 @@ function nodeRedisAttributes(options: NodeRedisClientOptions | undefined): SpanA function startCommandSpan(commandName: string, commandArgs: Array, attributes: SpanAttributes): Span { const dbStatement = defaultDbStatementSerializer(commandName, commandArgs); + const host = attributes[SERVER_ADDRESS]; + const port = attributes[SERVER_PORT]; + + const client = getClient(); + const name = + client && hasSpanStreamingEnabled(client) + ? host && port != null + ? `${commandName} ${host}:${port}` + : DB_SYSTEM_VALUE_REDIS + : dbStatement || `redis-${commandName}`; + return startInactiveSpan({ - name: dbStatement || `redis-${commandName}`, + name, attributes: { [SENTRY_KIND]: 'client', ...attributes, diff --git a/packages/server-utils/src/integrations/redis/ioredis-channel-subscriber.ts b/packages/server-utils/src/integrations/redis/ioredis-channel-subscriber.ts index 2e25ad7c5370..9866514e352a 100644 --- a/packages/server-utils/src/integrations/redis/ioredis-channel-subscriber.ts +++ b/packages/server-utils/src/integrations/redis/ioredis-channel-subscriber.ts @@ -10,7 +10,7 @@ import { } from '@sentry/conventions/attributes'; import { DB_QUERY, DB } from '@sentry/conventions/op'; import type { Span } from '@sentry/core'; -import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/core'; +import { getClient, hasSpanStreamingEnabled, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/core'; import { CHANNELS } from '../../orchestrion/channels'; import { bindTracingChannelToSpan } from '../../tracing-channel'; import type { RedisCacheOptions } from './redis-cache'; @@ -76,8 +76,19 @@ export function startIORedisCommandSpan(data: IORedisCommandContext): Span | und tracedCommands.add(command); const { host, port } = getConnectionOptions(data.self); const statement = defaultDbStatementSerializer(command.name, command.args ?? []); + const client = getClient(); + // The serialized statement carries command arguments, so with span streaming — where span names have + // to be low cardinality — `{db.operation.name} {server.address}:{server.port}` is used instead. + // Redis has no collection or namespace to pair with, so `{db.system.name}` is next. + const streamedName = + client && hasSpanStreamingEnabled(client) + ? host && port != null + ? `${command.name} ${host}:${port}` + : 'redis' + : undefined; + return startInactiveSpan({ - name: statement, + name: streamedName || statement, attributes: { [SENTRY_KIND]: 'client', ...connectionAttributes(host, port), diff --git a/packages/server-utils/test/integrations/redis/ioredis-channel-subscriber.test.ts b/packages/server-utils/test/integrations/redis/ioredis-channel-subscriber.test.ts index b0e21487e95f..e96434afe1a9 100644 --- a/packages/server-utils/test/integrations/redis/ioredis-channel-subscriber.test.ts +++ b/packages/server-utils/test/integrations/redis/ioredis-channel-subscriber.test.ts @@ -39,6 +39,23 @@ describe('startIORedisCommandSpan', () => { ); }); + it('names the span from the conventions with span streaming enabled', () => { + vi.spyOn(SentryCore, 'getClient').mockReturnValue({ + getOptions: () => ({ traceLifecycle: 'stream' }), + } as unknown as ReturnType); + + startIORedisCommandSpan(ctx({ name: 'set', args: ['test-key', 'test-value'] })); + + expect(startInactiveSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ + // `{db.operation.name} {server.address}:{server.port}` — redis has no collection or namespace + name: 'set localhost:6379', + // the serialized statement, which carries the key, is still reported as an attribute + attributes: expect.objectContaining({ 'db.query.text': 'set test-key [1 other arguments]' }), + }), + ); + }); + it('emits a single span when the same command is re-sent from the offline queue', () => { const command = { name: 'set', args: ['test-key', 'test-value'] }; From c74b12a03ee896599fa574ec1ff467e4acb7ab86 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 28 Aug 2026 17:50:57 +0200 Subject: [PATCH 2/2] feat(server-utils): Name streamed redis spans after the command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pairing the command with `{server.address}:{server.port}` put a host and port in every redis span name, which says nothing about what ran. Redis has nothing low cardinality to pair the operation with, so the name is now the bare command, matching the span name OTel prescribes for redis. The key and its arguments stay on `db.query.text`. `db.namespace` is deliberately left out of the name: for redis it is the numeric database index, which OTel excludes from span names for that reason. `FCALL`/`FCALL_RO` are the exception, since they name a redis function — the one redis construct the conventions model as a stored procedure. Those spans report `db.stored_procedure.name` and pair it with the operation, unless the publishing library redacted the function name. Batch spans now report the `MULTI`/`PIPELINE` operation they were already named after, so their name follows from their attributes too. Refs #23523 Co-Authored-By: Claude Opus 5 (1M context) --- .../suites/tracing/ioredis-dc/test.ts | 12 +++--- .../suites/tracing/redis-cache/test.ts | 31 ++++++------- .../suites/tracing/redis-dc/test.ts | 11 +++-- .../suites/tracing/redis/test.ts | 4 +- .../src/integrations/redis/index.ts | 21 +++------ .../redis/ioredis-channel-subscriber.ts | 15 ++----- .../integrations/redis/redis-dc-subscriber.ts | 9 +++- .../src/integrations/redis/redis-span-name.ts | 43 +++++++++++++++++++ .../redis/ioredis-channel-subscriber.test.ts | 42 ++++++++++++++++-- 9 files changed, 126 insertions(+), 62 deletions(-) create mode 100644 packages/server-utils/src/integrations/redis/redis-span-name.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/ioredis-dc/test.ts b/dev-packages/node-integration-tests/suites/tracing/ioredis-dc/test.ts index 3ec0e4a78840..798dd9b555d0 100644 --- a/dev-packages/node-integration-tests/suites/tracing/ioredis-dc/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/ioredis-dc/test.ts @@ -106,9 +106,9 @@ describeWithDockerCompose( }); }); - // The same commands as above, asserted on the streamed span container. The native - // diagnostics_channel subscriber already names db spans `redis-{command}`, which is low - // cardinality, so span streaming does not change them. + // The same commands as above, asserted on the streamed span container. With span streaming the + // db spans are named after `db.operation.name` (the bare command) instead of `redis-{command}`. + // ioredis reports its commands lowercase, so the name follows suit. describe('streamed', () => { const ORIGIN = 'auto.db.redis.diagnostic_channel'; const SEGMENT_NAME = 'Test Span IORedis 5.11 DC'; @@ -181,7 +181,7 @@ describeWithDockerCompose( ); expect(spans).toEqual([ - streamedSpan('redis-set', 'db.query', { + streamedSpan('set', 'db.query', { 'db.operation.name': 'set', 'db.query.text': 'set dc-test-key ?', }), @@ -199,7 +199,7 @@ describeWithDockerCompose( 'cache.key': ['dc-cache:test-key-ex'], 'cache.item_size': 2, }), - streamedSpan('redis-get', 'db.query', { + streamedSpan('get', 'db.query', { 'db.operation.name': 'get', 'db.query.text': 'get dc-test-key', }), @@ -218,7 +218,7 @@ describeWithDockerCompose( 'cache.key': ['dc-cache:unavailable-data'], 'cache.hit': false, }), - streamedSpan('redis-mget', 'db.query', { + streamedSpan('mget', 'db.query', { 'db.operation.name': 'mget', 'db.query.text': 'mget ? ? ?', }), 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 index 05ed83f06c3a..346758d988a5 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts @@ -431,9 +431,9 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory describe('streamed', () => { // The blocks above assert the same commands as transactions. With span streaming, span names // have to be low cardinality, so `db.query` spans drop the serialized statement from their - // name — it stays on `db.query.text` — and are named - // `{db.operation.name} {server.address}:{server.port}` instead. Cache spans are still named - // after the cache key by the cache hook, and batch spans keep their `MULTI`/`PIPELINE` name. + // name — it stays on `db.query.text` — and are named after `db.operation.name` instead, the + // bare command. Cache spans are still named after the cache key by the cache hook, and batch + // spans keep their `MULTI`/`PIPELINE` name. const streamAttribute = (value: unknown): { type: string; value: unknown } => ({ type: Array.isArray(value) ? 'array' : Number.isInteger(value) ? 'integer' : typeof value, value, @@ -508,7 +508,7 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory expect(container.items.find(item => item.is_segment)?.name).toBe(segmentName); expect(childSpans(container)).toEqual([ - span('set localhost:6383', redisSpanOp, { + span('set', redisSpanOp, { 'db.operation.name': 'set', 'db.query.text': 'set test-key [1 other arguments]', }), @@ -533,7 +533,7 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory 'cache.key': ['ioredis-cache:test-key-setex'], 'cache.item_size': 2, }), - span('get localhost:6383', redisSpanOp, { + span('get', redisSpanOp, { 'db.operation.name': 'get', 'db.query.text': 'get test-key', }), @@ -575,8 +575,6 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory }); }); - // node-redis v4 fills in `socket.host`, so its `db.query` spans get the - // `{db.operation.name} {server.address}:{server.port}` name. describe('redis-4', () => { const segmentName = 'Test Span Redis 4'; const connection = { 'server.address': 'localhost', 'server.port': 6383 }; @@ -599,7 +597,7 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory ]); expect(childSpans(container)).toEqual([ - span('SET localhost:6383', redisSpanOp, { + span('SET', redisSpanOp, { 'db.operation.name': 'SET', 'db.query.text': 'SET redis-test-key [1 other arguments]', }), @@ -624,7 +622,7 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory 'cache.key': ['redis-cache:test-key-setex'], 'cache.item_size': 2, }), - span('GET localhost:6383', redisSpanOp, { + span('GET', redisSpanOp, { 'db.operation.name': 'GET', 'db.query.text': 'GET redis-test-key', }), @@ -658,9 +656,9 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory 'cache.key': ['redis-cache:test-key'], }), // Batch spans are named after the batch operation, which is already low cardinality. - span('MULTI', redisSpanOp, { 'db.operation.batch.size': 2 }), + span('MULTI', redisSpanOp, { 'db.operation.name': 'MULTI', 'db.operation.batch.size': 2 }), span( - 'INCR localhost:6383', + 'INCR', redisSpanOp, { 'db.operation.name': 'INCR', @@ -679,9 +677,6 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory }); }); - // node-redis v5 leaves `socket.host` unset when only a port is passed, so there is no - // `server.address` to pair the operation with and the span name falls back to - // `{db.system.name}`. describe('redis-5', () => { const segmentName = 'Test Span Redis 5'; const connection = { 'server.port': 6383 }; @@ -701,7 +696,7 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory ]); expect(childSpans(container)).toEqual([ - span('redis', redisSpanOp, { + span('SET', redisSpanOp, { 'db.operation.name': 'SET', 'db.query.text': 'SET redis-5-test-key [1 other arguments]', }), @@ -723,7 +718,7 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory 'cache.key': ['redis-5-cache:test-key-setex'], 'cache.item_size': 2, }), - span('redis', redisSpanOp, { + span('GET', redisSpanOp, { 'db.operation.name': 'GET', 'db.query.text': 'GET redis-5-test-key', }), @@ -752,9 +747,9 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory 'db.query.text': 'DEL redis-5-cache:test-key', 'cache.key': ['redis-5-cache:test-key'], }), - span('MULTI', redisSpanOp, { 'db.operation.batch.size': 2 }), + span('MULTI', redisSpanOp, { 'db.operation.name': 'MULTI', 'db.operation.batch.size': 2 }), span( - 'redis', + 'INCR', redisSpanOp, { 'db.operation.name': 'INCR', diff --git a/dev-packages/node-integration-tests/suites/tracing/redis-dc/test.ts b/dev-packages/node-integration-tests/suites/tracing/redis-dc/test.ts index 8c576abc69c8..1e8ef9d87140 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis-dc/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/redis-dc/test.ts @@ -114,9 +114,8 @@ describeWithDockerCompose( }); }); - // The same commands as above, asserted on the streamed span container. The native - // diagnostics_channel subscriber already names db spans `redis-{command}`, which is low - // cardinality, so span streaming does not change them. + // The same commands as above, asserted on the streamed span container. With span streaming the + // db spans are named after `db.operation.name` (the bare command) instead of `redis-{command}`. describe('streamed', () => { const ORIGIN = 'auto.db.redis.diagnostic_channel'; const SEGMENT_NAME = 'Test Span Redis 5 DC'; @@ -183,7 +182,7 @@ describeWithDockerCompose( ); expect(spans).toEqual([ - streamedSpan('redis-SET', 'db.query', { + streamedSpan('SET', 'db.query', { 'db.operation.name': 'SET', 'db.query.text': 'SET dc-test-key ?', }), @@ -203,7 +202,7 @@ describeWithDockerCompose( 'cache.key': ['dc-cache:test-key-ex'], 'cache.item_size': 2, }), - streamedSpan('redis-GET', 'db.query', { + streamedSpan('GET', 'db.query', { 'db.operation.name': 'GET', 'db.query.text': 'GET dc-test-key', }), @@ -226,7 +225,7 @@ describeWithDockerCompose( }), // MGET: node-redis sanitizes args for diagnostics_channel (keys become '?'), // so cache detection cannot match prefixes — remains a plain db.query span. - streamedSpan('redis-MGET', 'db.query', { + streamedSpan('MGET', 'db.query', { 'db.operation.name': 'MGET', 'db.query.text': 'MGET ? ? ?', }), 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 ee0d3cd6c9c3..62082a525726 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/redis/test.ts @@ -74,7 +74,7 @@ describeWithDockerCompose('redis auto instrumentation', { workingDirectory: [__d // The same three commands as above, asserted on the streamed span container. Only the span // name differs: with span streaming names have to be low cardinality, so the serialized // statement is reported through `db.query.text` alone and the name becomes - // `{db.operation.name} {server.address}:{server.port}`. + // `{db.operation.name}` — for redis, the bare command. const COMMON_ATTRIBUTES = { 'db.system.name': { type: 'string', value: 'redis' }, 'server.address': { type: 'string', value: 'localhost' }, @@ -114,7 +114,7 @@ describeWithDockerCompose('redis auto instrumentation', { workingDirectory: [__d } : {}), }, - name: `${operation} localhost:6380`, + name: operation, end_timestamp: expect.any(Number), is_segment: false, parent_span_id: expect.stringMatching(/^[\da-f]{16}$/), diff --git a/packages/server-utils/src/integrations/redis/index.ts b/packages/server-utils/src/integrations/redis/index.ts index e237a47d2574..6ec5731d2dd4 100644 --- a/packages/server-utils/src/integrations/redis/index.ts +++ b/packages/server-utils/src/integrations/redis/index.ts @@ -15,8 +15,6 @@ import { isObjectLike, defineIntegration, getActiveSpan, - getClient, - hasSpanStreamingEnabled, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, startInactiveSpan, @@ -24,6 +22,7 @@ import { waitForTracingChannelBinding, } from '@sentry/core'; import { CHANNELS } from '../../orchestrion/channels'; +import { getRedisQueryNaming } from './redis-span-name'; import { defaultDbStatementSerializer } from './redis-statement-serializer'; import type { RedisCacheOptions } from './redis-cache'; import { applyRedisCacheAttributes } from './redis-cache'; @@ -103,24 +102,16 @@ function nodeRedisAttributes(options: NodeRedisClientOptions | undefined): SpanA function startCommandSpan(commandName: string, commandArgs: Array, attributes: SpanAttributes): Span { const dbStatement = defaultDbStatementSerializer(commandName, commandArgs); - const host = attributes[SERVER_ADDRESS]; - const port = attributes[SERVER_PORT]; - - const client = getClient(); - const name = - client && hasSpanStreamingEnabled(client) - ? host && port != null - ? `${commandName} ${host}:${port}` - : DB_SYSTEM_VALUE_REDIS - : dbStatement || `redis-${commandName}`; + const { streamedName, attributes: namingAttributes } = getRedisQueryNaming(commandName, commandArgs); return startInactiveSpan({ - name, + name: streamedName || dbStatement || `redis-${commandName}`, attributes: { [SENTRY_KIND]: 'client', ...attributes, [SENTRY_OP]: DB_QUERY, [DB_OPERATION_NAME]: commandName, + ...namingAttributes, [DB_QUERY_TEXT]: dbStatement, }, }); @@ -262,13 +253,15 @@ function bindNodeRedisBatchChannel(channelName: string, getOperation: (data: Com const commands = data.arguments?.[0]; const size = Array.isArray(commands) ? commands.length : undefined; const socket = (data.self as NodeRedisClient | undefined)?.options?.socket; + const operation = getOperation(data); return startInactiveSpan({ - name: getOperation(data), + name: operation, attributes: { [SENTRY_KIND]: 'client', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, [SENTRY_OP]: DB_QUERY, [DB_SYSTEM_NAME]: DB_SYSTEM_VALUE_REDIS, + [DB_OPERATION_NAME]: operation, ...(size && size > 1 ? { [DB_OPERATION_BATCH_SIZE]: size } : {}), ...(socket?.host != null ? { [SERVER_ADDRESS]: socket.host } : {}), ...(socket?.port != null ? { [SERVER_PORT]: socket.port } : {}), diff --git a/packages/server-utils/src/integrations/redis/ioredis-channel-subscriber.ts b/packages/server-utils/src/integrations/redis/ioredis-channel-subscriber.ts index 9866514e352a..778c5142fd8e 100644 --- a/packages/server-utils/src/integrations/redis/ioredis-channel-subscriber.ts +++ b/packages/server-utils/src/integrations/redis/ioredis-channel-subscriber.ts @@ -10,11 +10,12 @@ import { } from '@sentry/conventions/attributes'; import { DB_QUERY, DB } from '@sentry/conventions/op'; import type { Span } from '@sentry/core'; -import { getClient, hasSpanStreamingEnabled, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/core'; import { CHANNELS } from '../../orchestrion/channels'; import { bindTracingChannelToSpan } from '../../tracing-channel'; import type { RedisCacheOptions } from './redis-cache'; import { applyRedisCacheAttributes } from './redis-cache'; +import { getRedisQueryNaming } from './redis-span-name'; import { defaultDbStatementSerializer } from './redis-statement-serializer'; const ORIGIN = 'auto.db.redis'; @@ -76,16 +77,7 @@ export function startIORedisCommandSpan(data: IORedisCommandContext): Span | und tracedCommands.add(command); const { host, port } = getConnectionOptions(data.self); const statement = defaultDbStatementSerializer(command.name, command.args ?? []); - const client = getClient(); - // The serialized statement carries command arguments, so with span streaming — where span names have - // to be low cardinality — `{db.operation.name} {server.address}:{server.port}` is used instead. - // Redis has no collection or namespace to pair with, so `{db.system.name}` is next. - const streamedName = - client && hasSpanStreamingEnabled(client) - ? host && port != null - ? `${command.name} ${host}:${port}` - : 'redis' - : undefined; + const { streamedName, attributes: namingAttributes } = getRedisQueryNaming(command.name, command.args ?? []); return startInactiveSpan({ name: streamedName || statement, @@ -94,6 +86,7 @@ export function startIORedisCommandSpan(data: IORedisCommandContext): Span | und ...connectionAttributes(host, port), [SENTRY_OP]: DB_QUERY, [DB_OPERATION_NAME]: command.name, + ...namingAttributes, [DB_QUERY_TEXT]: statement, }, }); diff --git a/packages/server-utils/src/integrations/redis/redis-dc-subscriber.ts b/packages/server-utils/src/integrations/redis/redis-dc-subscriber.ts index 830b95aa5e81..e182c61dcbc0 100644 --- a/packages/server-utils/src/integrations/redis/redis-dc-subscriber.ts +++ b/packages/server-utils/src/integrations/redis/redis-dc-subscriber.ts @@ -13,6 +13,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/cor import { bindTracingChannelToSpan } from '../../tracing-channel'; import type { RedisCacheOptions } from './redis-cache'; import { applyRedisCacheAttributes } from './redis-cache'; +import { getRedisQueryNaming } from './redis-span-name'; // Channel names published by node-redis >= 5.12.0 and ioredis >= 5.11.0. // Hardcoded so the subscriber does not have to import either library — the @@ -141,13 +142,15 @@ function setupCommandChannel( // spaces to mirror the format the libraries themselves intend. const args = getCommandArgs(data); const statement = args.length ? `${data.command} ${args.join(' ')}` : data.command; + const { streamedName, attributes: namingAttributes } = getRedisQueryNaming(data.command, args); return startInactiveSpan({ - name: `redis-${data.command}`, + name: streamedName || `redis-${data.command}`, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, [SENTRY_OP]: DB_QUERY, [DB_SYSTEM_NAME]: DB_SYSTEM_NAME_VALUE_REDIS, [DB_OPERATION_NAME]: data.command, + ...namingAttributes, [DB_QUERY_TEXT]: statement, ...(data.serverAddress != null ? { [SERVER_ADDRESS]: data.serverAddress } : {}), ...(data.serverPort != null ? { [SERVER_PORT]: data.serverPort } : {}), @@ -169,12 +172,14 @@ function setupBatchChannel( getOperationName: (data: RedisBatchData) => string, ): void { bindTracingChannelToSpan(tracingChannel(channelName), data => { + const operation = getOperationName(data); return startInactiveSpan({ - name: getOperationName(data), + name: operation, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, [SENTRY_OP]: DB_QUERY, [DB_SYSTEM_NAME]: DB_SYSTEM_NAME_VALUE_REDIS, + [DB_OPERATION_NAME]: operation, // should only include batch size greater than 1, // or else it isn't properly considered a "batch" ...(Number(data.batchSize) > 1 ? { [DB_OPERATION_BATCH_SIZE]: data.batchSize } : {}), diff --git a/packages/server-utils/src/integrations/redis/redis-span-name.ts b/packages/server-utils/src/integrations/redis/redis-span-name.ts new file mode 100644 index 000000000000..6f886079a6b3 --- /dev/null +++ b/packages/server-utils/src/integrations/redis/redis-span-name.ts @@ -0,0 +1,43 @@ +import { DB_STORED_PROCEDURE_NAME } from '@sentry/conventions/attributes'; +import type { SpanAttributes } from '@sentry/core'; +import { getClient, hasSpanStreamingEnabled } from '@sentry/core'; + +const DB_SYSTEM_VALUE_REDIS = 'redis'; + +// `FCALL`/`FCALL_RO` invoke a redis function by name as their first argument. Redis functions are +// the one construct the conventions' db naming templates can model for redis — everything else a +// command touches is a key, which is exactly the high cardinality the streamed name has to avoid. +const STORED_PROCEDURE_COMMANDS = ['fcall', 'fcall_ro']; + +function getStoredProcedureName(command: string, args: ReadonlyArray): string | undefined { + if (!STORED_PROCEDURE_COMMANDS.includes(command.toLowerCase())) { + return undefined; + } + const raw = args[0]; + const name = typeof raw === 'string' ? raw : Buffer.isBuffer(raw) ? raw.toString() : undefined; + // node-redis and ioredis redact arguments before publishing them on their channels. A redacted + // function name says nothing, so leave the attribute unset rather than emit `FCALL ?`. + return name && name !== '?' ? name : undefined; +} + +/** + * The conventions attributes that name a redis command span, and the name itself when span + * streaming is enabled (`undefined` otherwise, leaving the caller's existing name in place). + * + * `db.query.text` carries the key and its arguments, so it cannot name a streamed span. Redis has + * nothing to pair the operation with, so the name is the bare `{db.operation.name}` — matching the + * span name OTel prescribes for redis. `FCALL` additionally names the function it calls. + */ +export function getRedisQueryNaming( + command: string, + args: ReadonlyArray, +): { streamedName: string | undefined; attributes: SpanAttributes } { + const storedProcedure = getStoredProcedureName(command, args); + const name = storedProcedure ? `${command} ${storedProcedure}` : command; + const client = getClient(); + + return { + streamedName: client && hasSpanStreamingEnabled(client) ? name || DB_SYSTEM_VALUE_REDIS : undefined, + attributes: storedProcedure ? { [DB_STORED_PROCEDURE_NAME]: storedProcedure } : {}, + }; +} diff --git a/packages/server-utils/test/integrations/redis/ioredis-channel-subscriber.test.ts b/packages/server-utils/test/integrations/redis/ioredis-channel-subscriber.test.ts index e96434afe1a9..f2fc3a40d7ce 100644 --- a/packages/server-utils/test/integrations/redis/ioredis-channel-subscriber.test.ts +++ b/packages/server-utils/test/integrations/redis/ioredis-channel-subscriber.test.ts @@ -48,10 +48,46 @@ describe('startIORedisCommandSpan', () => { expect(startInactiveSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ - // `{db.operation.name} {server.address}:{server.port}` — redis has no collection or namespace - name: 'set localhost:6379', + // `{db.operation.name}` — redis has nothing low cardinality to pair the operation with + name: 'set', // the serialized statement, which carries the key, is still reported as an attribute - attributes: expect.objectContaining({ 'db.query.text': 'set test-key [1 other arguments]' }), + attributes: expect.objectContaining({ + 'db.query.text': 'set test-key [1 other arguments]', + }), + }), + ); + }); + + it('names the span after the redis function it calls with span streaming enabled', () => { + vi.spyOn(SentryCore, 'getClient').mockReturnValue({ + getOptions: () => ({ traceLifecycle: 'stream' }), + } as unknown as ReturnType); + + startIORedisCommandSpan(ctx({ name: 'fcall', args: ['my_func', '1', 'test-key'] })); + + expect(startInactiveSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ + // `{db.operation.name} {db.stored_procedure.name}` — a redis function is named, so unlike + // an ordinary command it has a low cardinality second token to pair with + name: 'fcall my_func', + attributes: expect.objectContaining({ + 'db.stored_procedure.name': 'my_func', + }), + }), + ); + }); + + it('leaves the stored procedure unset when the function name was redacted', () => { + vi.spyOn(SentryCore, 'getClient').mockReturnValue({ + getOptions: () => ({ traceLifecycle: 'stream' }), + } as unknown as ReturnType); + + startIORedisCommandSpan(ctx({ name: 'fcall', args: ['?', '1', 'test-key'] })); + + expect(startInactiveSpanSpy).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'fcall', + attributes: expect.not.objectContaining({ 'db.stored_procedure.name': expect.anything() }), }), ); });