feat(server-utils)!: Emit low cardinality redis span names - #23741
Draft
Lms24 wants to merge 2 commits into
Draft
Conversation
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) <noreply@anthropic.com>
Contributor
size-limit report 📦
|
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
注册 for free
to join this conversation on GitHub.
Already have an account?
登录 to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Redis has no SQL statement to summarize, and nothing low cardinality to pair the command with. So with span streaming enabled,
db.queryspans are named afterdb.operation.name— the bare command — and the key and its arguments stay ondb.query.text. This is the span name OTel prescribes for redis.db.namespaceis deliberately kept out of the name: for redis it is the numeric database index, which OTel excludes from span names for exactly that reason.set test-key [1 other arguments]setSET test-key [1 other arguments]SETredis-SETSETFCALLfcall my_funcMULTI/PIPELINE{db.operation.name}rule yet — I'm adding one to sentry-conventions to cover thisFCALL/FCALL_ROname a redis function, the one redis construct the conventions model as a stored procedure, so those spans reportdb.stored_procedure.nameand pair it with the operation. Skipped when the publishing library redacted the function nameMULTI/PIPELINEoperation they were already named after, so their name follows from their attributes tootraceLifecycle: 'static'keeps the existing namesStreamed integration tests mirror the transaction-based ones across the
redis,redis-cache,redis-dcandioredis-dcsuites, switched by aSTREAMEDenv var on the existing instrument files. Split out of #23603, which keeps the mongodb half.Refs #23523