Skip to content

feat(redis): Set db.query.text and cache.key span attributes#6639

Open
ericapisani wants to merge 1 commit into
masterfrom
py-2549-set-db-attr-redis
Open

feat(redis): Set db.query.text and cache.key span attributes#6639
ericapisani wants to merge 1 commit into
masterfrom
py-2549-set-db-attr-redis

Conversation

@ericapisani

@ericapisani ericapisani commented Jun 23, 2026

Copy link
Copy Markdown
Member

When using the span streaming path, db.query.text was not being set on db.redis spans and cache.key was not being set on cache spans. Both values were already computed and used as the span description/name, but were missing from the attributes dict passed to the span constructor.

This adds SPANDATA.DB_QUERY_TEXT to db spans and SPANDATA.CACHE_KEY to cache spans in both the sync and async Redis common modules, and extends existing tests to assert these attributes are present.

Fixes PY-2549
Fixes #6638

When using span streaming, set SPANDATA.DB_QUERY_TEXT on db.redis spans
and SPANDATA.CACHE_KEY on cache spans. These were already set as the span
description/name but were missing from the attributes dict in the streaming
path.

Refs PY-2549
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Jun 23, 2026

Copy link
Copy Markdown

PY-2549

@ericapisani ericapisani marked this pull request as ready for review June 23, 2026 18:59
@ericapisani ericapisani requested a review from a team as a code owner June 23, 2026 18:59
@@ -116,6 +116,7 @@ def sentry_patched_execute_command(
attributes={
"sentry.op": cache_properties["op"],
"sentry.origin": SPAN_ORIGIN,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The cache span is initialized with SPANDATA.CACHE_KEY set to the description string, not the key tuple, causing an incorrect attribute type in exception paths.
Severity: LOW

Suggested Fix

Modify the span creation to use the correct property from the start. Instead of setting SPANDATA.CACHE_KEY to cache_properties["description"], set it to cache_properties["key"]. This ensures the attribute has the correct type (a tuple of strings) from the moment of initialization, even in exception paths.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: sentry_sdk/integrations/redis/_sync_common.py#L118

Potential issue: The code in `_sync_common.py` and `_async_common.py` incorrectly
initializes the `SPANDATA.CACHE_KEY` attribute on the cache span with
`cache_properties["description"]`, which is a string. The correct value should be
`cache_properties["key"]`, a tuple of strings. While a subsequent call to
`_set_cache_data` overwrites this with the correct value in the normal execution flow,
this correction does not happen if an exception is raised during the Redis command
execution. In such an exception scenario, the span would be captured with the wrong
attribute type (a string instead of a list of strings), leading to inconsistent
telemetry data.

Also affects:

  • sentry_sdk/integrations/redis/_async_common.py:119~119

Did we get this right? 👍 / 👎 to inform future reviews.

@github-actions

Copy link
Copy Markdown
Contributor

Codecov Results 📊

91401 passed | ⏭️ 6136 skipped | Total: 97537 | Pass Rate: 93.71% | Execution Time: 332m 56s

📊 Comparison with Base Branch

Metric Change
Total Tests
Passed Tests
Failed Tests
Skipped Tests

✨ No test changes detected

All tests are passing successfully.

✅ Patch coverage is 100.00%. Project has 2403 uncovered lines.
❌ Project coverage is 89.89%. Comparing base (base) to head (head).

Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
- Coverage    89.90%    89.89%    -0.01%
==========================================
  Files          192       192         —
  Lines        23763     23763         —
  Branches      8206      8206         —
==========================================
+ Hits         21362     21360        -2
- Misses        2401      2403        +2
- Partials      1344      1346        +2

Generated by Codecov Action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Set db.query.text attribute on redis streamed spans

1 participant