Skip to content

Commit c1a1976

Browse files
Lms24claude
Lms24
and
claude
committed
feat(server-utils): Name streamed redis spans after the command
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) <[email protected]>
1 parent b38834e commit c1a1976

9 files changed

Lines changed: 126 additions & 62 deletions

File tree

‎dev-packages/node-integration-tests/suites/tracing/ioredis-dc/test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,9 @@ describeWithDockerCompose(
106106
});
107107
});
108108

109-
// The same commands as above, asserted on the streamed span container. The native
110-
// diagnostics_channel subscriber already names db spans `redis-{command}`, which is low
111-
// cardinality, so span streaming does not change them.
109+
// The same commands as above, asserted on the streamed span container. With span streaming the
110+
// db spans are named after `db.operation.name` (the bare command) instead of `redis-{command}`.
111+
// ioredis reports its commands lowercase, so the name follows suit.
112112
describe('streamed', () => {
113113
const ORIGIN = 'auto.db.redis.diagnostic_channel';
114114
const SEGMENT_NAME = 'Test Span IORedis 5.11 DC';
@@ -181,7 +181,7 @@ describeWithDockerCompose(
181181
);
182182

183183
expect(spans).toEqual([
184-
streamedSpan('redis-set', 'db.query', {
184+
streamedSpan('set', 'db.query', {
185185
'db.operation.name': 'set',
186186
'db.query.text': 'set dc-test-key ?',
187187
}),
@@ -199,7 +199,7 @@ describeWithDockerCompose(
199199
'cache.key': ['dc-cache:test-key-ex'],
200200
'cache.item_size': 2,
201201
}),
202-
streamedSpan('redis-get', 'db.query', {
202+
streamedSpan('get', 'db.query', {
203203
'db.operation.name': 'get',
204204
'db.query.text': 'get dc-test-key',
205205
}),
@@ -218,7 +218,7 @@ describeWithDockerCompose(
218218
'cache.key': ['dc-cache:unavailable-data'],
219219
'cache.hit': false,
220220
}),
221-
streamedSpan('redis-mget', 'db.query', {
221+
streamedSpan('mget', 'db.query', {
222222
'db.operation.name': 'mget',
223223
'db.query.text': 'mget ? ? ?',
224224
}),

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

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -683,9 +683,9 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
683683
describe('streamed', () => {
684684
// The blocks above assert the same commands as transactions. With span streaming, span names
685685
// have to be low cardinality, so `db.query` spans drop the serialized statement from their
686-
// name — it stays on `db.query.text` — and are named
687-
// `{db.operation.name} {server.address}:{server.port}` instead. Cache spans are still named
688-
// after the cache key by the cache hook, and batch spans keep their `MULTI`/`PIPELINE` name.
686+
// name — it stays on `db.query.text` — and are named after `db.operation.name` instead, the
687+
// bare command. Cache spans are still named after the cache key by the cache hook, and batch
688+
// spans keep their `MULTI`/`PIPELINE` name.
689689
const streamAttribute = (value: unknown): { type: string; value: unknown } => ({
690690
type: Array.isArray(value) ? 'array' : Number.isInteger(value) ? 'integer' : typeof value,
691691
value,
@@ -760,7 +760,7 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
760760
expect(container.items.find(item => item.is_segment)?.name).toBe(segmentName);
761761

762762
expect(childSpans(container)).toEqual([
763-
span('set localhost:6383', redisSpanOp, {
763+
span('set', redisSpanOp, {
764764
'db.operation.name': 'set',
765765
'db.query.text': 'set test-key [1 other arguments]',
766766
}),
@@ -785,7 +785,7 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
785785
'cache.key': ['ioredis-cache:test-key-setex'],
786786
'cache.item_size': 2,
787787
}),
788-
span('get localhost:6383', redisSpanOp, {
788+
span('get', redisSpanOp, {
789789
'db.operation.name': 'get',
790790
'db.query.text': 'get test-key',
791791
}),
@@ -827,8 +827,6 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
827827
});
828828
});
829829

830-
// node-redis v4 fills in `socket.host`, so its `db.query` spans get the
831-
// `{db.operation.name} {server.address}:{server.port}` name.
832830
describe('redis-4', () => {
833831
const segmentName = 'Test Span Redis 4';
834832
const connection = { 'server.address': 'localhost', 'server.port': 6383 };
@@ -851,7 +849,7 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
851849
]);
852850

853851
expect(childSpans(container)).toEqual([
854-
span('SET localhost:6383', redisSpanOp, {
852+
span('SET', redisSpanOp, {
855853
'db.operation.name': 'SET',
856854
'db.query.text': 'SET redis-test-key [1 other arguments]',
857855
}),
@@ -876,7 +874,7 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
876874
'cache.key': ['redis-cache:test-key-setex'],
877875
'cache.item_size': 2,
878876
}),
879-
span('GET localhost:6383', redisSpanOp, {
877+
span('GET', redisSpanOp, {
880878
'db.operation.name': 'GET',
881879
'db.query.text': 'GET redis-test-key',
882880
}),
@@ -910,9 +908,9 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
910908
'cache.key': ['redis-cache:test-key'],
911909
}),
912910
// Batch spans are named after the batch operation, which is already low cardinality.
913-
span('MULTI', redisSpanOp, { 'db.operation.batch.size': 2 }),
911+
span('MULTI', redisSpanOp, { 'db.operation.name': 'MULTI', 'db.operation.batch.size': 2 }),
914912
span(
915-
'INCR localhost:6383',
913+
'INCR',
916914
redisSpanOp,
917915
{
918916
'db.operation.name': 'INCR',
@@ -931,9 +929,6 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
931929
});
932930
});
933931

934-
// node-redis v5 leaves `socket.host` unset when only a port is passed, so there is no
935-
// `server.address` to pair the operation with and the span name falls back to
936-
// `{db.system.name}`.
937932
describe('redis-5', () => {
938933
const segmentName = 'Test Span Redis 5';
939934
const connection = { 'server.port': 6383 };
@@ -953,7 +948,7 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
953948
]);
954949

955950
expect(childSpans(container)).toEqual([
956-
span('redis', redisSpanOp, {
951+
span('SET', redisSpanOp, {
957952
'db.operation.name': 'SET',
958953
'db.query.text': 'SET redis-5-test-key [1 other arguments]',
959954
}),
@@ -975,7 +970,7 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
975970
'cache.key': ['redis-5-cache:test-key-setex'],
976971
'cache.item_size': 2,
977972
}),
978-
span('redis', redisSpanOp, {
973+
span('GET', redisSpanOp, {
979974
'db.operation.name': 'GET',
980975
'db.query.text': 'GET redis-5-test-key',
981976
}),
@@ -1004,9 +999,9 @@ describeWithDockerCompose('redis cache auto instrumentation', { workingDirectory
1004999
'db.query.text': 'DEL redis-5-cache:test-key',
10051000
'cache.key': ['redis-5-cache:test-key'],
10061001
}),
1007-
span('MULTI', redisSpanOp, { 'db.operation.batch.size': 2 }),
1002+
span('MULTI', redisSpanOp, { 'db.operation.name': 'MULTI', 'db.operation.batch.size': 2 }),
10081003
span(
1009-
'redis',
1004+
'INCR',
10101005
redisSpanOp,
10111006
{
10121007
'db.operation.name': 'INCR',

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,8 @@ describeWithDockerCompose(
114114
});
115115
});
116116

117-
// The same commands as above, asserted on the streamed span container. The native
118-
// diagnostics_channel subscriber already names db spans `redis-{command}`, which is low
119-
// cardinality, so span streaming does not change them.
117+
// The same commands as above, asserted on the streamed span container. With span streaming the
118+
// db spans are named after `db.operation.name` (the bare command) instead of `redis-{command}`.
120119
describe('streamed', () => {
121120
const ORIGIN = 'auto.db.redis.diagnostic_channel';
122121
const SEGMENT_NAME = 'Test Span Redis 5 DC';
@@ -183,7 +182,7 @@ describeWithDockerCompose(
183182
);
184183

185184
expect(spans).toEqual([
186-
streamedSpan('redis-SET', 'db.query', {
185+
streamedSpan('SET', 'db.query', {
187186
'db.operation.name': 'SET',
188187
'db.query.text': 'SET dc-test-key ?',
189188
}),
@@ -203,7 +202,7 @@ describeWithDockerCompose(
203202
'cache.key': ['dc-cache:test-key-ex'],
204203
'cache.item_size': 2,
205204
}),
206-
streamedSpan('redis-GET', 'db.query', {
205+
streamedSpan('GET', 'db.query', {
207206
'db.operation.name': 'GET',
208207
'db.query.text': 'GET dc-test-key',
209208
}),
@@ -226,7 +225,7 @@ describeWithDockerCompose(
226225
}),
227226
// MGET: node-redis sanitizes args for diagnostics_channel (keys become '?'),
228227
// so cache detection cannot match prefixes — remains a plain db.query span.
229-
streamedSpan('redis-MGET', 'db.query', {
228+
streamedSpan('MGET', 'db.query', {
230229
'db.operation.name': 'MGET',
231230
'db.query.text': 'MGET ? ? ?',
232231
}),

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ describeWithDockerCompose('redis auto instrumentation', { workingDirectory: [__d
7474
// The same three commands as above, asserted on the streamed span container. Only the span
7575
// name differs: with span streaming names have to be low cardinality, so the serialized
7676
// statement is reported through `db.query.text` alone and the name becomes
77-
// `{db.operation.name} {server.address}:{server.port}`.
77+
// `{db.operation.name}` — for redis, the bare command.
7878
const COMMON_ATTRIBUTES = {
7979
'db.system.name': { type: 'string', value: 'redis' },
8080
'server.address': { type: 'string', value: 'localhost' },
@@ -114,7 +114,7 @@ describeWithDockerCompose('redis auto instrumentation', { workingDirectory: [__d
114114
}
115115
: {}),
116116
},
117-
name: `${operation} localhost:6380`,
117+
name: operation,
118118
end_timestamp: expect.any(Number),
119119
is_segment: false,
120120
parent_span_id: expect.stringMatching(/^[\da-f]{16}$/),

‎packages/server-utils/src/integrations/redis/index.ts‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,14 @@ import {
1515
isObjectLike,
1616
defineIntegration,
1717
getActiveSpan,
18-
getClient,
19-
hasSpanStreamingEnabled,
2018
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
2119
SPAN_STATUS_ERROR,
2220
startInactiveSpan,
2321
withActiveSpan,
2422
waitForTracingChannelBinding,
2523
} from '@sentry/core';
2624
import { CHANNELS } from '../../orchestrion/channels';
25+
import { getRedisQueryNaming } from './redis-span-name';
2726
import { defaultDbStatementSerializer } from './redis-statement-serializer';
2827
import type { RedisCacheOptions } from './redis-cache';
2928
import { applyRedisCacheAttributes } from './redis-cache';
@@ -103,24 +102,16 @@ function nodeRedisAttributes(options: NodeRedisClientOptions | undefined): SpanA
103102

104103
function startCommandSpan(commandName: string, commandArgs: Array<string | Buffer>, attributes: SpanAttributes): Span {
105104
const dbStatement = defaultDbStatementSerializer(commandName, commandArgs);
106-
const host = attributes[SERVER_ADDRESS];
107-
const port = attributes[SERVER_PORT];
108-
109-
const client = getClient();
110-
const name =
111-
client && hasSpanStreamingEnabled(client)
112-
? host && port != null
113-
? `${commandName} ${host}:${port}`
114-
: DB_SYSTEM_VALUE_REDIS
115-
: dbStatement || `redis-${commandName}`;
105+
const { streamedName, attributes: namingAttributes } = getRedisQueryNaming(commandName, commandArgs);
116106

117107
return startInactiveSpan({
118-
name,
108+
name: streamedName || dbStatement || `redis-${commandName}`,
119109
attributes: {
120110
[SENTRY_KIND]: 'client',
121111
...attributes,
122112
[SENTRY_OP]: DB_QUERY,
123113
[DB_OPERATION_NAME]: commandName,
114+
...namingAttributes,
124115
[DB_QUERY_TEXT]: dbStatement,
125116
},
126117
});
@@ -262,13 +253,15 @@ function bindNodeRedisBatchChannel(channelName: string, getOperation: (data: Com
262253
const commands = data.arguments?.[0];
263254
const size = Array.isArray(commands) ? commands.length : undefined;
264255
const socket = (data.self as NodeRedisClient | undefined)?.options?.socket;
256+
const operation = getOperation(data);
265257
return startInactiveSpan({
266-
name: getOperation(data),
258+
name: operation,
267259
attributes: {
268260
[SENTRY_KIND]: 'client',
269261
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN,
270262
[SENTRY_OP]: DB_QUERY,
271263
[DB_SYSTEM_NAME]: DB_SYSTEM_VALUE_REDIS,
264+
[DB_OPERATION_NAME]: operation,
272265
...(size && size > 1 ? { [DB_OPERATION_BATCH_SIZE]: size } : {}),
273266
...(socket?.host != null ? { [SERVER_ADDRESS]: socket.host } : {}),
274267
...(socket?.port != null ? { [SERVER_PORT]: socket.port } : {}),

‎packages/server-utils/src/integrations/redis/ioredis-channel-subscriber.ts‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@ import {
1010
} from '@sentry/conventions/attributes';
1111
import { DB_QUERY, DB } from '@sentry/conventions/op';
1212
import type { Span } from '@sentry/core';
13-
import { getClient, hasSpanStreamingEnabled, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/core';
13+
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/core';
1414
import { CHANNELS } from '../../orchestrion/channels';
1515
import { bindTracingChannelToSpan } from '../../tracing-channel';
1616
import type { RedisCacheOptions } from './redis-cache';
1717
import { applyRedisCacheAttributes } from './redis-cache';
18+
import { getRedisQueryNaming } from './redis-span-name';
1819
import { defaultDbStatementSerializer } from './redis-statement-serializer';
1920

2021
const ORIGIN = 'auto.db.redis';
@@ -76,16 +77,7 @@ export function startIORedisCommandSpan(data: IORedisCommandContext): Span | und
7677
tracedCommands.add(command);
7778
const { host, port } = getConnectionOptions(data.self);
7879
const statement = defaultDbStatementSerializer(command.name, command.args ?? []);
79-
const client = getClient();
80-
// The serialized statement carries command arguments, so with span streaming — where span names have
81-
// to be low cardinality — `{db.operation.name} {server.address}:{server.port}` is used instead.
82-
// Redis has no collection or namespace to pair with, so `{db.system.name}` is next.
83-
const streamedName =
84-
client && hasSpanStreamingEnabled(client)
85-
? host && port != null
86-
? `${command.name} ${host}:${port}`
87-
: 'redis'
88-
: undefined;
80+
const { streamedName, attributes: namingAttributes } = getRedisQueryNaming(command.name, command.args ?? []);
8981

9082
return startInactiveSpan({
9183
name: streamedName || statement,
@@ -94,6 +86,7 @@ export function startIORedisCommandSpan(data: IORedisCommandContext): Span | und
9486
...connectionAttributes(host, port),
9587
[SENTRY_OP]: DB_QUERY,
9688
[DB_OPERATION_NAME]: command.name,
89+
...namingAttributes,
9790
[DB_QUERY_TEXT]: statement,
9891
},
9992
});

‎packages/server-utils/src/integrations/redis/redis-dc-subscriber.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/cor
1313
import { bindTracingChannelToSpan } from '../../tracing-channel';
1414
import type { RedisCacheOptions } from './redis-cache';
1515
import { applyRedisCacheAttributes } from './redis-cache';
16+
import { getRedisQueryNaming } from './redis-span-name';
1617

1718
// Channel names published by node-redis >= 5.12.0 and ioredis >= 5.11.0.
1819
// Hardcoded so the subscriber does not have to import either library — the
@@ -141,13 +142,15 @@ function setupCommandChannel<T extends RedisCommandData | IORedisCommandData>(
141142
// spaces to mirror the format the libraries themselves intend.
142143
const args = getCommandArgs(data);
143144
const statement = args.length ? `${data.command} ${args.join(' ')}` : data.command;
145+
const { streamedName, attributes: namingAttributes } = getRedisQueryNaming(data.command, args);
144146
return startInactiveSpan({
145-
name: `redis-${data.command}`,
147+
name: streamedName || `redis-${data.command}`,
146148
attributes: {
147149
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN,
148150
[SENTRY_OP]: DB_QUERY,
149151
[DB_SYSTEM_NAME]: DB_SYSTEM_NAME_VALUE_REDIS,
150152
[DB_OPERATION_NAME]: data.command,
153+
...namingAttributes,
151154
[DB_QUERY_TEXT]: statement,
152155
...(data.serverAddress != null ? { [SERVER_ADDRESS]: data.serverAddress } : {}),
153156
...(data.serverPort != null ? { [SERVER_PORT]: data.serverPort } : {}),
@@ -169,12 +172,14 @@ function setupBatchChannel(
169172
getOperationName: (data: RedisBatchData) => string,
170173
): void {
171174
bindTracingChannelToSpan(tracingChannel<RedisBatchData>(channelName), data => {
175+
const operation = getOperationName(data);
172176
return startInactiveSpan({
173-
name: getOperationName(data),
177+
name: operation,
174178
attributes: {
175179
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN,
176180
[SENTRY_OP]: DB_QUERY,
177181
[DB_SYSTEM_NAME]: DB_SYSTEM_NAME_VALUE_REDIS,
182+
[DB_OPERATION_NAME]: operation,
178183
// should only include batch size greater than 1,
179184
// or else it isn't properly considered a "batch"
180185
...(Number(data.batchSize) > 1 ? { [DB_OPERATION_BATCH_SIZE]: data.batchSize } : {}),

‎packages/server-utils/src/integrations/redis/redis-span-name.ts‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { DB_STORED_PROCEDURE_NAME } from '@sentry/conventions/attributes';
2+
import type { SpanAttributes } from '@sentry/core';
3+
import { getClient, hasSpanStreamingEnabled } from '@sentry/core';
4+
5+
const DB_SYSTEM_VALUE_REDIS = 'redis';
6+
7+
// `FCALL`/`FCALL_RO` invoke a redis function by name as their first argument. Redis functions are
8+
// the one construct the conventions' db naming templates can model for redis — everything else a
9+
// command touches is a key, which is exactly the high cardinality the streamed name has to avoid.
10+
const STORED_PROCEDURE_COMMANDS = ['fcall', 'fcall_ro'];
11+
12+
function getStoredProcedureName(command: string, args: ReadonlyArray<unknown>): string | undefined {
13+
if (!STORED_PROCEDURE_COMMANDS.includes(command.toLowerCase())) {
14+
return undefined;
15+
}
16+
const raw = args[0];
17+
const name = typeof raw === 'string' ? raw : Buffer.isBuffer(raw) ? raw.toString() : undefined;
18+
// node-redis and ioredis redact arguments before publishing them on their channels. A redacted
19+
// function name says nothing, so leave the attribute unset rather than emit `FCALL ?`.
20+
return name && name !== '?' ? name : undefined;
21+
}
22+
23+
/**
24+
* The conventions attributes that name a redis command span, and the name itself when span
25+
* streaming is enabled (`undefined` otherwise, leaving the caller's existing name in place).
26+
*
27+
* `db.query.text` carries the key and its arguments, so it cannot name a streamed span. Redis has
28+
* nothing to pair the operation with, so the name is the bare `{db.operation.name}` — matching the
29+
* span name OTel prescribes for redis. `FCALL` additionally names the function it calls.
30+
*/
31+
export function getRedisQueryNaming(
32+
command: string,
33+
args: ReadonlyArray<unknown>,
34+
): { streamedName: string | undefined; attributes: SpanAttributes } {
35+
const storedProcedure = getStoredProcedureName(command, args);
36+
const name = storedProcedure ? `${command} ${storedProcedure}` : command;
37+
const client = getClient();
38+
39+
return {
40+
streamedName: client && hasSpanStreamingEnabled(client) ? name || DB_SYSTEM_VALUE_REDIS : undefined,
41+
attributes: storedProcedure ? { [DB_STORED_PROCEDURE_NAME]: storedProcedure } : {},
42+
};
43+
}

0 commit comments

Comments
 (0)