diff --git a/MIGRATION.md b/MIGRATION.md index 5972f04f6ff8..ed436764d627 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -923,25 +923,26 @@ If you [opt out of span streaming](#opting-out-of-span-streaming), span names re The following span names were adjusted: -| Span op | Before | After | -| ------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `pageload` | The parameterized route, or the raw URL path if the SDK couldn't resolve one (`/users/123`) | The parameterized route, or `Pageload` if the SDK has none | -| `navigation` | The parameterized route, or the raw URL path if the SDK couldn't resolve one (`/users/123`) | The parameterized route, or `Navigation` if the SDK has none | -| `http.server` | The request method and route, or the raw URL path if the SDK couldn't resolve one (`GET /users/123`) | `GET /users/:id` when a route is known, otherwise just the request method (`GET`) | -| `http.client`, `http.client.stream` | The request method and sanitized URL (`GET https://api.example.com/users/123`) | The request method and the domain (`GET api.example.com`), or just the method if there is no domain (`GET`) | -| `router` | Framework-specific, sometimes containing the raw URL (`/users/123`, `SvelteKit Route Change`) | The span's `http.route`, or `Router` if the SDK has none | -| `handler` | Framework-specific, often carrying the request method (`GET /users/:id`, `route-handler`, `getUser`) | The span's `http.route`, or `Request handler` if the SDK has none | -| `graphql` | The graphql phase and, for operations, the operation name (`query GetUser`, `graphql.parse`, `graphql.resolve user.0.name`) | The operation type, or the processing type where there is none (`GraphQL query`, `GraphQL parse`, `GraphQL resolve`) | -| `gen_ai.chat`, `gen_ai.embeddings`, `gen_ai.generate_content` | `{operation} {model}`, or `{operation} unknown` if the model is missing (`chat unknown`) | `{operation} {model}`, or `{operation}` if the model is missing (`chat`) | -| `gen_ai.invoke_agent` | The LangChain chain name, prefixed with `chain` rather than the operation (`chain format_prompt`) | `{operation} {name}`, where the name is the span's `gen_ai.agent.name`, `gen_ai.pipeline.name` or `gen_ai.function_id`, in that order (`invoke_agent format_prompt`), or `{operation}` if the span carries none | -| `resource.*` | The resource URL, relative to the page origin for same-origin resources (`/assets/app.js`) | The resource domain (`cdn.example.com`), or `Resource` if the SDK has none | -| `mcp.server` | The method and its target, including the resource URI (`resources/read file:///docs/api.md`) | The method alone for resource methods (`resources/read`). Tool and prompt names are unchanged (`tools/call get-weather`) | -| `mcp.notification.client_to_server`, `mcp.notification.server_to_client` | The notification method name (`notifications/tools/list_changed`) | The notification method name, or `MCP notification` if the message carries none | -| `queue.publish` | Integration-specific (`publish my-exchange`, `send my-topic`) | The messaging operation type and the destination (`send my-exchange`), or just the operation type when the destination has no name (`send`) | -| `queue.process` | Integration-specific, sometimes containing per-message data (`my-queue process`, `order.created.12345 process`) | The messaging operation type and the destination (`process my-exchange`), or just the operation type when the destination has no name (`process`) | -| `queue.receive` | The kafkajs operation name (`poll my-topic`) | The messaging operation type and the destination (`receive my-topic`) | -| `cache.get`, `cache.put`, `cache.remove` | The cache key(s) (`user:123`), or for dataloader the operation and loader name (`dataloader.load usersLoader`) | The cache operation (`cache.get`, `cache.put`, `cache.remove`) | -| `db` (mongoose) | `mongoose..` (`mongoose.BlogPost.findOne`) | The operation and the collection (`findOne blogposts`), the database namespace when there is no collection, or `mongodb` when the SDK has neither | +| Span op | Before | After | +| ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `pageload` | The parameterized route, or the raw URL path if the SDK couldn't resolve one (`/users/123`) | The parameterized route, or `Pageload` if the SDK has none | +| `navigation` | The parameterized route, or the raw URL path if the SDK couldn't resolve one (`/users/123`) | The parameterized route, or `Navigation` if the SDK has none | +| `http.server` | The request method and route, or the raw URL path if the SDK couldn't resolve one (`GET /users/123`) | `GET /users/:id` when a route is known, otherwise just the request method (`GET`) | +| `http.client`, `http.client.stream` | The request method and sanitized URL (`GET https://api.example.com/users/123`) | The request method and the domain (`GET api.example.com`), or just the method if there is no domain (`GET`) | +| `router` | Framework-specific, sometimes containing the raw URL (`/users/123`, `SvelteKit Route Change`) | The span's `http.route`, or `Router` if the SDK has none | +| `handler` | Framework-specific, often carrying the request method (`GET /users/:id`, `route-handler`, `getUser`) | The span's `http.route`, or `Request handler` if the SDK has none | +| `graphql` | The graphql phase and, for operations, the operation name (`query GetUser`, `graphql.parse`, `graphql.resolve user.0.name`) | The operation type, or the processing type where there is none (`GraphQL query`, `GraphQL parse`, `GraphQL resolve`) | +| `gen_ai.chat`, `gen_ai.embeddings`, `gen_ai.generate_content` | `{operation} {model}`, or `{operation} unknown` if the model is missing (`chat unknown`) | `{operation} {model}`, or `{operation}` if the model is missing (`chat`) | +| `gen_ai.invoke_agent` | The LangChain chain name, prefixed with `chain` rather than the operation (`chain format_prompt`) | `{operation} {name}`, where the name is the span's `gen_ai.agent.name`, `gen_ai.pipeline.name` or `gen_ai.function_id`, in that order (`invoke_agent format_prompt`), or `{operation}` if the span carries none | +| `resource.*` | The resource URL, relative to the page origin for same-origin resources (`/assets/app.js`) | The resource domain (`cdn.example.com`), or `Resource` if the SDK has none | +| `mcp.server` | The method and its target, including the resource URI (`resources/read file:///docs/api.md`) | The method alone for resource methods (`resources/read`). Tool and prompt names are unchanged (`tools/call get-weather`) | +| `mcp.notification.client_to_server`, `mcp.notification.server_to_client` | The notification method name (`notifications/tools/list_changed`) | The notification method name, or `MCP notification` if the message carries none | +| `queue.publish` | Integration-specific (`publish my-exchange`, `send my-topic`) | The messaging operation type and the destination (`send my-exchange`), or just the operation type when the destination has no name (`send`) | +| `queue.process` | Integration-specific, sometimes containing per-message data (`my-queue process`, `order.created.12345 process`) | The messaging operation type and the destination (`process my-exchange`), or just the operation type when the destination has no name (`process`) | +| `queue.receive` | The kafkajs operation name (`poll my-topic`) | The messaging operation type and the destination (`receive my-topic`) | +| `cache.get`, `cache.put`, `cache.remove` | The cache key(s) (`user:123`), or for dataloader the operation and loader name (`dataloader.load usersLoader`) | The cache operation (`cache.get`, `cache.put`, `cache.remove`) | +| `db.query` (redis, ioredis) | The serialized command, with its arguments redacted (`set test-key [1 other arguments]`), or `redis-` on the diagnostics-channel path (`redis-SET`) | The operation and the connection (`SET localhost:6379`), the operation and the redis function for `FCALL`/`FCALL_RO` (`fcall my_func`), or `redis` when the SDK knows neither | +| `db` (mongoose) | `mongoose..` (`mongoose.BlogPost.findOne`) | The operation and the collection (`findOne blogposts`), the database namespace when there is no collection, or `mongodb` when the SDK has neither | `navigation.redirect` spans are started through the same code path as navigation spans, so they get the same names. @@ -981,6 +982,12 @@ Cache keys are unbounded, so they are no longer part of a cache span name. They A dataloader span no longer carries the loader's `name` either (`dataloader.load usersLoader` becomes `cache.get`), because the cache conventions have no slot for it in the name. It is reported on the `db.collection.name` attribute instead — a loader batches one entity type, so it is the closest thing dataloader has to a collection — and that attribute is set in both trace lifecycles. Unnamed loaders do not set it. +Redis has no SQL statement to summarize and no collection to pair a command with, so redis and ioredis `db.query` spans are named after the operation and the connection instead of the command that was sent. The command and its arguments remain available on `db.query.text`, redacted as before. `MULTI`/`PIPELINE` batch spans are unchanged — they were already named after their operation, which they now also report on `db.operation.name`. `db.namespace` is deliberately not used in the name: for redis it is the numeric database index, which says nothing about what the command did. + +`FCALL` and `FCALL_RO` call a redis function, the one redis construct the conventions model as a stored procedure, so those spans are named after the function and report it on a new `db.stored_procedure.name` attribute, set in both trace lifecycles. node-redis and ioredis redact arguments before publishing them on their diagnostics channels, so a redacted function name is left off both the name and the attribute. + +Relatedly, and in **both** trace lifecycles: node-redis clients now always report the connection they use. node-redis v4 wrote its `localhost:6379` defaults back into `client.options` and v5 does not, so an identically configured client used to report `server.address`/`server.port` on v4 and neither on v5. The SDK now fills in the same defaults node-redis itself publishes on its diagnostics channel, and a client connected over a unix socket reports its path as `server.address` and no port. + AWS SQS `SendMessage`, `SendMessageBatch` and `ReceiveMessage`, and SNS `Publish`, are messaging spans (e.g. `queue.publish`) rather than `rpc` ones now. Every other command on those clients, such as `DeleteMessage`, stays `rpc`. Their names follow the messaging conventions too, so the operation comes first (`my-queue receive` becomes `receive my-queue`, `my-topic send` becomes `send my-topic`). A streamed SNS `Publish` to a platform endpoint is named `send`, because the endpoint ARN it used to carry ends in a per-device id (`endpoint/GCM/myapp/ send`). The full ARN remains on `messaging.destination.name`. An amqplib span's destination is the exchange it uses, or the routing key when it uses the default exchange. RabbitMQ binds every queue to the default exchange under a key equal to the queue's own name, so `sendToQueue` spans are named after their queue (`send my-queue`) instead of dropping the destination. `messaging.destination.name` reports the same value, and the routing key remains on `messaging.rabbitmq.destination.routing_key` in full. diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/db.server.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/db.server.test.ts index 7ce9ef5bda26..92806fd517e7 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/db.server.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework-instrumentation/tests/performance/db.server.test.ts @@ -22,9 +22,12 @@ test.describe('server - orchestrion db instrumentation', () => { const childSpans = spans.filter(span => !span.is_segment); + // Under span streaming a redis span is named after the operation and the connection — the key + // it acts on is unbounded, so it stays on `db.query.text`. The route builds its client without + // a host or port, so ioredis' own `localhost:6379` defaults are what the name reports. expect(childSpans).toContainEqual( expect.objectContaining({ - name: 'set test-key [1 other arguments]', + name: 'set localhost:6379', status: 'ok', attributes: expect.objectContaining({ 'sentry.op': { value: 'db.query', type: 'string' }, @@ -32,12 +35,14 @@ test.describe('server - orchestrion db instrumentation', () => { 'db.system.name': { value: 'redis', type: 'string' }, 'db.operation.name': { value: 'set', type: 'string' }, 'db.query.text': { value: 'set test-key [1 other arguments]', type: 'string' }, + 'server.address': { value: 'localhost', type: 'string' }, + 'server.port': { value: 6379, type: 'integer' }, }), }), ); expect(childSpans).toContainEqual( expect.objectContaining({ - name: 'get test-key', + name: 'get localhost:6379', status: 'ok', attributes: expect.objectContaining({ 'sentry.op': { value: 'db.query', type: 'string' }, @@ -45,12 +50,16 @@ test.describe('server - orchestrion db instrumentation', () => { 'db.system.name': { value: 'redis', type: 'string' }, 'db.operation.name': { value: 'get', type: 'string' }, 'db.query.text': { value: 'get test-key', type: 'string' }, + 'server.address': { value: 'localhost', type: 'string' }, + 'server.port': { value: 6379, type: 'integer' }, }), }), ); // Each command maps to exactly one span (no offline-queue duplicate). - const setSpans = spans.filter(span => span.name === 'set test-key [1 other arguments]'); + const setSpans = spans.filter( + span => span.attributes['db.query.text']?.value === 'set test-key [1 other arguments]', + ); expect(setSpans).toHaveLength(1); // Every db span nests under the native instrumentation-API http.server segment. 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..3bdcaea303be 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,134 @@ describeWithDockerCompose( .completed(); }); }); + + // The same commands as above, asserted on the streamed span container. With span streaming the + // db spans are named `{db.operation.name} {server.address}:{server.port}` 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'; + 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 }; + + // A cache span is a db span the cache hook took over: it is renamed to its cache operation + // and reports the connection it inherited as peer attributes too. + const cacheSpan = ( + op: 'cache.get' | 'cache.put' | 'cache.remove', + attributes: Record, + ): unknown => streamedSpan(op, op, { ...PEER, 'cache.operation': op.slice('cache.'.length), ...attributes }); + + 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(`set ${HOST}:${PORT}`, 'db.query', { + 'db.operation.name': 'set', + 'db.query.text': 'set dc-test-key ?', + }), + cacheSpan('cache.put', { + 'db.operation.name': 'set', + 'db.query.text': 'set dc-cache:test-key ?', + 'cache.key': ['dc-cache:test-key'], + 'cache.item_size': 2, + }), + cacheSpan('cache.put', { + '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(`get ${HOST}:${PORT}`, 'db.query', { + 'db.operation.name': 'get', + 'db.query.text': 'get dc-test-key', + }), + cacheSpan('cache.get', { + '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, + }), + cacheSpan('cache.get', { + 'db.operation.name': 'get', + 'db.query.text': 'get dc-cache:unavailable-data', + 'cache.key': ['dc-cache:unavailable-data'], + 'cache.hit': false, + }), + streamedSpan(`mget ${HOST}:${PORT}`, 'db.query', { + 'db.operation.name': 'mget', + 'db.query.text': 'mget ? ? ?', + }), + ]); + }, + }) + .start() + .completed(); + }, + ); + }); + }); }, ); 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 813ddae859b1..87416e6893c4 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'; @@ -450,7 +452,8 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory test('should name cache spans after the cache operation when streamed (redis-4)', async () => { await createTestRunner() .withEnv({ STREAMED: 'true' }) - // The connect span is streamed in its own envelope, ahead of the command spans. + // The connect span opens its own segment. `unordered` keeps this expectation from + // depending on which segment the buffer happens to emit first. .unordered() .expect({ span: { items: EXPECTED_STREAMED_SPANS } }) .start() @@ -669,7 +672,8 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory test('should name cache spans after the cache operation when streamed (redis-5)', async () => { await createTestRunner() .withEnv({ STREAMED: 'true' }) - // The connect span is streamed in its own envelope, ahead of the command spans. + // The connect span opens its own segment. `unordered` keeps this expectation from + // depending on which segment the buffer happens to emit first. .unordered() .expect({ span: { items: EXPECTED_STREAMED_SPANS } }) .start() @@ -677,4 +681,358 @@ 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 named after + // their cache operation, 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 } }); + + // A cache span is a db span the cache hook took over: it is renamed to its cache operation + // and reports the connection it inherited as peer attributes too. + const cacheSpan = ( + op: 'cache.get' | 'cache.put' | 'cache.remove', + attributes: Record, + ): unknown => span(op, op, { ...peer, 'cache.operation': op.slice('cache.'.length), ...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]', + }), + cacheSpan('cache.put', { + '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, + }), + cacheSpan('cache.put', { + '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, + }), + cacheSpan('cache.put', { + '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', + }), + cacheSpan('cache.get', { + '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, + }), + cacheSpan('cache.get', { + 'db.operation.name': 'get', + 'db.query.text': 'get ioredis-cache:unavailable-data', + 'cache.key': ['ioredis-cache:unavailable-data'], + 'cache.hit': false, + }), + cacheSpan('cache.get', { + '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, + }), + cacheSpan('cache.remove', { + '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 } }); + + // A cache span is a db span the cache hook took over: it is renamed to its cache operation + // and reports the connection it inherited as peer attributes too. + const cacheSpan = ( + op: 'cache.get' | 'cache.put' | 'cache.remove', + attributes: Record, + ): unknown => span(op, op, { ...peer, 'cache.operation': op.slice('cache.'.length), ...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]', + }), + cacheSpan('cache.put', { + '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, + }), + cacheSpan('cache.put', { + '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, + }), + cacheSpan('cache.put', { + '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', + }), + cacheSpan('cache.get', { + '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, + }), + cacheSpan('cache.get', { + 'db.operation.name': 'GET', + 'db.query.text': 'GET redis-cache:unavailable-data', + 'cache.key': ['redis-cache:unavailable-data'], + 'cache.hit': false, + }), + cacheSpan('cache.get', { + '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, + }), + cacheSpan('cache.remove', { + '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.name': 'MULTI', '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, unlike v4. The + // integration fills in the library's own `localhost` default, so both report the same + // connection and get the same span name. + describe('redis-5', () => { + const segmentName = 'Test Span Redis 5'; + 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 } }); + + // A cache span is a db span the cache hook took over: it is renamed to its cache operation + // and reports the connection it inherited as peer attributes too. + const cacheSpan = ( + op: 'cache.get' | 'cache.put' | 'cache.remove', + attributes: Record, + ): unknown => span(op, op, { ...peer, 'cache.operation': op.slice('cache.'.length), ...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('SET localhost:6383', redisSpanOp, { + 'db.operation.name': 'SET', + 'db.query.text': 'SET redis-5-test-key [1 other arguments]', + }), + cacheSpan('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, + }), + cacheSpan('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, + }), + cacheSpan('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('GET localhost:6383', redisSpanOp, { + 'db.operation.name': 'GET', + 'db.query.text': 'GET redis-5-test-key', + }), + cacheSpan('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, + }), + cacheSpan('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, + }), + cacheSpan('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, + }), + cacheSpan('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.name': 'MULTI', 'db.operation.batch.size': 2 }), + span( + 'INCR localhost:6383', + 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..a02326b0a1cf 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,133 @@ describeWithDockerCompose( .completed(); }); }); + + // The same commands as above, asserted on the streamed span container. With span streaming the + // db spans are named `{db.operation.name} {server.address}:{server.port}` instead of + // `redis-{command}`. + 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 }; + + // A cache span is a db span the cache hook took over: it is renamed to its cache operation + // and reports the connection it inherited as peer attributes too. + const cacheSpan = ( + op: 'cache.get' | 'cache.put' | 'cache.remove', + attributes: Record, + ): unknown => streamedSpan(op, op, { ...PEER, 'cache.operation': op.slice('cache.'.length), ...attributes }); + + 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(`SET ${HOST}:${PORT}`, 'db.query', { + 'db.operation.name': 'SET', + 'db.query.text': 'SET dc-test-key ?', + }), + // cache SET: turned into a cache span, and renamed by the cache hook + cacheSpan('cache.put', { + '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 + cacheSpan('cache.put', { + '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(`GET ${HOST}:${PORT}`, 'db.query', { + 'db.operation.name': 'GET', + 'db.query.text': 'GET dc-test-key', + }), + // cache GET (hit) + cacheSpan('cache.get', { + '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) + cacheSpan('cache.get', { + '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(`MGET ${HOST}:${PORT}`, '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..9241f4936e80 100644 --- a/packages/server-utils/src/integrations/redis/index.ts +++ b/packages/server-utils/src/integrations/redis/index.ts @@ -22,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'; @@ -58,7 +59,7 @@ interface LegacyRedisClient { } interface NodeRedisClientOptions { - socket?: { host?: string; port?: number }; + socket?: { host?: string; port?: number; path?: string }; url?: string; } @@ -90,24 +91,38 @@ function stripCommandOptions(args: unknown[]): unknown[] { return args; } +// Resolves the connection the way node-redis >= 5.12 reports it on its own diagnostics channel: a +// unix socket reports its path and no port, a TCP client the `localhost:6379` it defaults to. +// Only v4 writes those defaults back into `client.options`, so reading the options as-is would +// report a connection for a v4 client and none for the identically configured v5 one. function nodeRedisAttributes(options: NodeRedisClientOptions | undefined): SpanAttributes { + const socket = options?.socket; + const host = socket?.path ?? socket?.host ?? 'localhost'; + const port = socket?.path != null ? undefined : (socket?.port ?? 6379); + return { [DB_SYSTEM_NAME]: DB_SYSTEM_VALUE_REDIS, - ...(options?.socket?.host != null ? { [SERVER_ADDRESS]: options.socket.host } : {}), - ...(options?.socket?.port != null ? { [SERVER_PORT]: options.socket.port } : {}), + [SERVER_ADDRESS]: host, + ...(port != null ? { [SERVER_PORT]: port } : {}), [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, }; } function startCommandSpan(commandName: string, commandArgs: Array, attributes: SpanAttributes): Span { const dbStatement = defaultDbStatementSerializer(commandName, commandArgs); + const { streamedName, attributes: namingAttributes } = getRedisQueryNaming(commandName, commandArgs, { + host: attributes[SERVER_ADDRESS], + port: attributes[SERVER_PORT], + }); + return startInactiveSpan({ - name: dbStatement || `redis-${commandName}`, + name: streamedName || dbStatement || `redis-${commandName}`, attributes: { [SENTRY_KIND]: 'client', ...attributes, [SENTRY_OP]: DB_QUERY, [DB_OPERATION_NAME]: commandName, + ...namingAttributes, [DB_QUERY_TEXT]: dbStatement, }, }); @@ -248,17 +263,16 @@ function bindNodeRedisBatchChannel(channelName: string, getOperation: (data: Com bindTracingChannelToSpan(channel, data => { const commands = data.arguments?.[0]; const size = Array.isArray(commands) ? commands.length : undefined; - const socket = (data.self as NodeRedisClient | undefined)?.options?.socket; + const options = (data.self as NodeRedisClient | undefined)?.options; + const operation = getOperation(data); return startInactiveSpan({ - name: getOperation(data), + name: operation, attributes: { [SENTRY_KIND]: 'client', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, + ...nodeRedisAttributes(options), [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 2e25ad7c5370..4f1b08889b75 100644 --- a/packages/server-utils/src/integrations/redis/ioredis-channel-subscriber.ts +++ b/packages/server-utils/src/integrations/redis/ioredis-channel-subscriber.ts @@ -15,6 +15,7 @@ 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,13 +77,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 { streamedName, attributes: namingAttributes } = getRedisQueryNaming(command.name, command.args ?? [], { + host, + port, + }); + return startInactiveSpan({ - name: statement, + name: streamedName || statement, attributes: { [SENTRY_KIND]: 'client', ...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..8ff655078f3c 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,18 @@ 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, { + host: data.serverAddress, + port: data.serverPort, + }); 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 +175,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..84af8ce48079 --- /dev/null +++ b/packages/server-utils/src/integrations/redis/redis-span-name.ts @@ -0,0 +1,65 @@ +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']; + +// The connection a command was sent over. Untyped values because call sites read them off a client, +// a diagnostics_channel payload or an already-built span attribute bag. +interface RedisConnection { + host?: unknown; + port?: unknown; +} + +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 have no address-only template, so both halves are required: a client +// configured with only a port has no target, and neither does a unix socket, whose +// `server.address` is a path with no port to pair it with. +function getServerTarget({ host, port }: RedisConnection): string | undefined { + const hasPort = typeof port === 'number' || (typeof port === 'string' && !!port); + return typeof host === 'string' && host && hasPort ? `${host}:${port}` : 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 + * no collection to pair the operation with, so the name is + * `{db.operation.name} {server.address}:{server.port}`, falling back to `{db.system.name}` when the + * client was configured without a host. `FCALL` names a redis function, which the conventions model + * as a stored procedure and rank ahead of the connection. + * + * `db.namespace` is deliberately not a target: for redis it is the numeric database index, which + * says nothing about what the command did. + */ +export function getRedisQueryNaming( + command: string, + args: ReadonlyArray, + connection: RedisConnection, +): { streamedName: string | undefined; attributes: SpanAttributes } { + const storedProcedure = getStoredProcedureName(command, args); + const target = storedProcedure || getServerTarget(connection); + const name = command && target ? `${command} ${target}` : target || DB_SYSTEM_VALUE_REDIS; + const client = getClient(); + + return { + streamedName: client && hasSpanStreamingEnabled(client) ? name : 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 b0e21487e95f..4ed2fb440ec4 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 @@ -5,8 +5,11 @@ import { startIORedisCommandSpan } from '../../../src/integrations/redis/ioredis const CONNECTION = { host: 'localhost', port: 6379 }; -function ctx(command: unknown): { arguments: unknown[]; self: { options: typeof CONNECTION } } { - return { arguments: [command], self: { options: CONNECTION } }; +function ctx( + command: unknown, + connection: { host?: string; port?: number } = CONNECTION, +): { arguments: unknown[]; self: { options: { host?: string; port?: number } } } { + return { arguments: [command], self: { options: connection } }; } describe('startIORedisCommandSpan', () => { @@ -39,6 +42,70 @@ 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('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}` — the conventions rank the stored + // procedure ahead of the connection, so it wins over `{server.address}:{server.port}` + 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 localhost:6379', + attributes: expect.not.objectContaining({ 'db.stored_procedure.name': expect.anything() }), + }), + ); + }); + + it('falls back to the db system name when the client has no host', () => { + vi.spyOn(SentryCore, 'getClient').mockReturnValue({ + getOptions: () => ({ traceLifecycle: 'stream' }), + } as unknown as ReturnType); + + startIORedisCommandSpan(ctx({ name: 'set', args: ['test-key', 'test-value'] }, { port: 6379 })); + + // `{db.system.name}` — the address/port template needs both halves + expect(startInactiveSpanSpy).toHaveBeenCalledWith(expect.objectContaining({ name: 'redis' })); + }); + 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'] };