Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ericapisani wants to merge 1 commit into
base: master
Choose a base branch
from
+44 −2
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion sentry_sdk/integrations/redis/_async_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from typing import TYPE_CHECKING

import sentry_sdk
from sentry_sdk.consts import OP
from sentry_sdk.consts import OP, SPANDATA
from sentry_sdk.integrations.redis.consts import SPAN_ORIGIN
from sentry_sdk.integrations.redis.modules.caches import (
_compile_cache_span_properties,
Expand Down Expand Up @@ -117,6 +117,7 @@ async def _sentry_execute_command(
attributes={
"sentry.op": cache_properties["op"],
"sentry.origin": SPAN_ORIGIN,
SPANDATA.CACHE_KEY: cache_properties["description"],
},
)
else:
Expand All @@ -136,6 +137,7 @@ async def _sentry_execute_command(
attributes={
"sentry.op": db_properties["op"],
"sentry.origin": SPAN_ORIGIN,
SPANDATA.DB_QUERY_TEXT: db_properties["description"],
},
)
else:
Expand Down
4 changes: 3 additions & 1 deletion sentry_sdk/integrations/redis/_sync_common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from typing import TYPE_CHECKING

import sentry_sdk
from sentry_sdk.consts import OP
from sentry_sdk.consts import OP, SPANDATA
from sentry_sdk.integrations.redis.consts import SPAN_ORIGIN
from sentry_sdk.integrations.redis.modules.caches import (
_compile_cache_span_properties,
Expand Down Expand Up @@ -116,6 +116,7 @@ def sentry_patched_execute_command(
attributes={
"sentry.op": cache_properties["op"],
"sentry.origin": SPAN_ORIGIN,

@sentry sentry Bot Jun 23, 2026

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.

SPANDATA.CACHE_KEY: cache_properties["description"],
},
)
else:
Expand All @@ -135,6 +136,7 @@ def sentry_patched_execute_command(
attributes={
"sentry.op": db_properties["op"],
"sentry.origin": SPAN_ORIGIN,
SPANDATA.DB_QUERY_TEXT: db_properties["description"],
},
)
else:
Expand Down
25 changes: 25 additions & 0 deletions tests/integrations/redis/test_redis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ def test_sensitive_data(sentry_init, capture_events, capture_items, span_streami

assert parent_span["name"] == "custom parent"
assert redis_span["name"] == "GET [Filtered]"
assert redis_span["attributes"][SPANDATA.DB_QUERY_TEXT] == "GET [Filtered]"
assert redis_span["attributes"]["sentry.op"] == "db.redis"
else:
events = capture_events()
Expand Down Expand Up @@ -177,8 +178,10 @@ def test_pii_data_redacted(sentry_init, capture_events, capture_items, span_stre

assert parent["name"] == "custom parent"
assert set1["name"] == "SET 'somekey1' [Filtered]"
assert set1["attributes"][SPANDATA.DB_QUERY_TEXT] == "SET 'somekey1' [Filtered]"
assert set1["attributes"]["sentry.op"] == "db.redis"
assert set2["name"] == "SET 'somekey2' [Filtered]"
assert set2["attributes"][SPANDATA.DB_QUERY_TEXT] == "SET 'somekey2' [Filtered]"
assert get["name"] == "GET 'somekey2'"
assert delete["name"] == "DEL 'somekey1' [Filtered]"
else:
Expand Down Expand Up @@ -223,8 +226,16 @@ def test_pii_data_sent(sentry_init, capture_events, capture_items, span_streamin

assert parent["name"] == "custom parent"
assert set1["name"] == "SET 'somekey1' 'my secret string1'"
assert (
set1["attributes"][SPANDATA.DB_QUERY_TEXT]
== "SET 'somekey1' 'my secret string1'"
)
assert set1["attributes"]["sentry.op"] == "db.redis"
assert set2["name"] == "SET 'somekey2' 'my secret string2'"
assert (
set2["attributes"][SPANDATA.DB_QUERY_TEXT]
== "SET 'somekey2' 'my secret string2'"
)
assert get["name"] == "GET 'somekey2'"
assert delete["name"] == "DEL 'somekey1' 'somekey2'"
else:
Expand Down Expand Up @@ -271,8 +282,16 @@ def test_no_data_truncation_by_default(

assert parent["name"] == "custom parent"
assert set1["name"] == f"SET 'somekey1' '{long_string}'"
assert (
set1["attributes"][SPANDATA.DB_QUERY_TEXT]
== f"SET 'somekey1' '{long_string}'"
)
assert set1["attributes"]["sentry.op"] == "db.redis"
assert set2["name"] == f"SET 'somekey2' '{short_string}'"
assert (
set2["attributes"][SPANDATA.DB_QUERY_TEXT]
== f"SET 'somekey2' '{short_string}'"
)
else:
events = capture_events()
with start_transaction():
Expand Down Expand Up @@ -317,8 +336,10 @@ def test_data_truncation_custom(

assert parent["name"] == "custom parent"
assert set1["name"] == expected_long
assert set1["attributes"][SPANDATA.DB_QUERY_TEXT] == expected_long
assert set1["attributes"]["sentry.op"] == "db.redis"
assert set2["name"] == expected_short
assert set2["attributes"][SPANDATA.DB_QUERY_TEXT] == expected_short
else:
events = capture_events()
with start_transaction():
Expand Down Expand Up @@ -401,6 +422,7 @@ def test_db_connection_attributes_client(
assert redis_span["name"] == "GET 'foobar'"
attrs = redis_span["attributes"]
assert attrs["sentry.op"] == "db.redis"
assert attrs[SPANDATA.DB_QUERY_TEXT] == "GET 'foobar'"
assert attrs[SPANDATA.DB_SYSTEM_NAME] == "redis"
assert attrs[SPANDATA.DB_DRIVER_NAME] == "redis-py"
assert attrs[SPANDATA.DB_NAMESPACE] == "1"
Expand Down Expand Up @@ -508,6 +530,9 @@ def test_span_origin(sentry_init, capture_events, capture_items, span_streaming)
assert parent_span["name"] == "custom parent"
assert parent_span["attributes"]["sentry.origin"] == "manual"
assert set_span["attributes"]["sentry.origin"] == "auto.db.redis"
assert (
set_span["attributes"][SPANDATA.DB_QUERY_TEXT] == "SET 'somekey' [Filtered]"
)
assert pipeline_span["attributes"]["sentry.origin"] == "auto.db.redis"
else:
events = capture_events()
Expand Down
9 changes: 9 additions & 0 deletions tests/integrations/redis/test_redis_cache_module.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,28 @@ def test_cache_basic(sentry_init, capture_events, capture_items, span_streaming)
assert payloads[1]["attributes"]["sentry.op"] == "db.redis"
assert payloads[1]["attributes"][SPANDATA.DB_OPERATION_NAME] == "GET"
assert payloads[2]["attributes"]["sentry.op"] == "cache.get"
assert payloads[2]["attributes"][SPANDATA.CACHE_KEY] == ["mycachekey"]

# set: db then cache.put
assert payloads[3]["attributes"]["sentry.op"] == "db.redis"
assert payloads[3]["attributes"][SPANDATA.DB_OPERATION_NAME] == "SET"
assert payloads[4]["attributes"]["sentry.op"] == "cache.put"
assert payloads[4]["attributes"][SPANDATA.CACHE_KEY] == ["mycachekey1"]

# setex: db then cache.put
assert payloads[5]["attributes"]["sentry.op"] == "db.redis"
assert payloads[5]["attributes"][SPANDATA.DB_OPERATION_NAME] == "SETEX"
assert payloads[6]["attributes"]["sentry.op"] == "cache.put"
assert payloads[6]["attributes"][SPANDATA.CACHE_KEY] == ["mycachekey2"]

# mget: db then cache.get
assert payloads[7]["attributes"]["sentry.op"] == "db.redis"
assert payloads[7]["attributes"][SPANDATA.DB_OPERATION_NAME] == "MGET"
assert payloads[8]["attributes"]["sentry.op"] == "cache.get"
assert payloads[8]["attributes"][SPANDATA.CACHE_KEY] == [
"mycachekey1",
"mycachekey2",
]

assert payloads[9]["name"] == "custom parent"
else:
Expand Down Expand Up @@ -169,12 +176,14 @@ def test_cache_keys(sentry_init, capture_events, capture_items, span_streaming):
assert payloads[1]["name"] == "GET 'blub'"
assert payloads[2]["attributes"]["sentry.op"] == "cache.get"
assert payloads[2]["name"] == "blub"
assert payloads[2]["attributes"][SPANDATA.CACHE_KEY] == ["blub"]

# blubkeything: db then cache.get
assert payloads[3]["attributes"]["sentry.op"] == "db.redis"
assert payloads[3]["name"] == "GET 'blubkeything'"
assert payloads[4]["attributes"]["sentry.op"] == "cache.get"
assert payloads[4]["name"] == "blubkeything"
assert payloads[4]["attributes"][SPANDATA.CACHE_KEY] == ["blubkeything"]

# bl: db only (no prefix match)
assert payloads[5]["attributes"]["sentry.op"] == "db.redis"
Expand Down
4 changes: 4 additions & 0 deletions tests/integrations/redis/test_redis_cache_module_async.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
)

import sentry_sdk
from sentry_sdk.consts import SPANDATA
from sentry_sdk.integrations.redis import RedisIntegration
from sentry_sdk.utils import parse_version

Expand Down Expand Up @@ -83,6 +84,7 @@ async def test_cache_basic(sentry_init, capture_events, capture_items, span_stre
assert parent_span["name"] == "custom parent"
assert db_span["attributes"]["sentry.op"] == "db.redis"
assert cache_span["attributes"]["sentry.op"] == "cache.get"
assert cache_span["attributes"][SPANDATA.CACHE_KEY] == ["myasynccachekey"]
else:
events = capture_events()
with sentry_sdk.start_transaction():
Expand Down Expand Up @@ -132,12 +134,14 @@ async def test_cache_keys(sentry_init, capture_events, capture_items, span_strea
assert payloads[1]["name"] == "GET 'ablub'"
assert payloads[2]["attributes"]["sentry.op"] == "cache.get"
assert payloads[2]["name"] == "ablub"
assert payloads[2]["attributes"][SPANDATA.CACHE_KEY] == ["ablub"]

# ablubkeything: db then cache.get
assert payloads[3]["attributes"]["sentry.op"] == "db.redis"
assert payloads[3]["name"] == "GET 'ablubkeything'"
assert payloads[4]["attributes"]["sentry.op"] == "cache.get"
assert payloads[4]["name"] == "ablubkeything"
assert payloads[4]["attributes"][SPANDATA.CACHE_KEY] == ["ablubkeything"]

# abl: db only (no prefix match)
assert payloads[5]["attributes"]["sentry.op"] == "db.redis"
Expand Down
Loading