Skip to content

Commit ad67968

Browse files
ericapisaniericapisani
authored
feat(redis): Set db.query.text span attributes (#6639)
When using the span streaming path, `db.query.text` was not being set on `db.redis` spans This adds `SPANDATA.DB_QUERY_TEXT` to 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
1 parent 72a7248 commit ad67968

5 files changed

Lines changed: 82 additions & 2 deletions

File tree

‎sentry_sdk/integrations/redis/_async_common.py‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
from typing import TYPE_CHECKING
22

33
import sentry_sdk
4-
from sentry_sdk.consts import OP
4+
from sentry_sdk.consts import OP, SPANDATA
55
from sentry_sdk.integrations.redis.consts import SPAN_ORIGIN
66
from sentry_sdk.integrations.redis.modules.caches import (
77
_compile_cache_span_properties,
88
_set_cache_data,
99
)
1010
from sentry_sdk.integrations.redis.modules.queries import _compile_db_span_properties
1111
from sentry_sdk.integrations.redis.utils import (
12+
_get_safe_command,
1213
_set_client_data,
1314
_set_pipeline_data,
1415
)
@@ -109,6 +110,12 @@ async def _sentry_execute_command(
109110
integration,
110111
)
111112

113+
additional_cache_span_attributes = {}
114+
with capture_internal_exceptions():
115+
additional_cache_span_attributes[SPANDATA.DB_QUERY_TEXT] = (
116+
_get_safe_command(name, args)
117+
)
118+
112119
cache_span: "Optional[Union[Span, StreamedSpan]]" = None
113120
if cache_properties["is_cache_key"] and cache_properties["op"] is not None:
114121
if span_streaming:
@@ -117,6 +124,7 @@ async def _sentry_execute_command(
117124
attributes={
118125
"sentry.op": cache_properties["op"],
119126
"sentry.origin": SPAN_ORIGIN,
127+
**additional_cache_span_attributes,
120128
},
121129
)
122130
else:
@@ -129,13 +137,20 @@ async def _sentry_execute_command(
129137

130138
db_properties = _compile_db_span_properties(integration, name, args)
131139

140+
additional_db_span_attributes = {}
141+
with capture_internal_exceptions():
142+
additional_db_span_attributes[SPANDATA.DB_QUERY_TEXT] = _get_safe_command(
143+
name, args
144+
)
145+
132146
db_span: "Union[Span, StreamedSpan]"
133147
if span_streaming:
134148
db_span = sentry_sdk.traces.start_span(
135149
name=db_properties["description"],
136150
attributes={
137151
"sentry.op": db_properties["op"],
138152
"sentry.origin": SPAN_ORIGIN,
153+
**additional_db_span_attributes,
139154
},
140155
)
141156
else:

‎sentry_sdk/integrations/redis/_sync_common.py‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
from typing import TYPE_CHECKING
22

33
import sentry_sdk
4-
from sentry_sdk.consts import OP
4+
from sentry_sdk.consts import OP, SPANDATA
55
from sentry_sdk.integrations.redis.consts import SPAN_ORIGIN
66
from sentry_sdk.integrations.redis.modules.caches import (
77
_compile_cache_span_properties,
88
_set_cache_data,
99
)
1010
from sentry_sdk.integrations.redis.modules.queries import _compile_db_span_properties
1111
from sentry_sdk.integrations.redis.utils import (
12+
_get_safe_command,
1213
_set_client_data,
1314
_set_pipeline_data,
1415
)
@@ -108,6 +109,12 @@ def sentry_patched_execute_command(
108109
integration,
109110
)
110111

112+
additional_cache_span_attributes = {}
113+
with capture_internal_exceptions():
114+
additional_cache_span_attributes[SPANDATA.DB_QUERY_TEXT] = (
115+
_get_safe_command(name, args)
116+
)
117+
111118
cache_span: "Optional[Union[Span, StreamedSpan]]" = None
112119
if cache_properties["is_cache_key"] and cache_properties["op"] is not None:
113120
if span_streaming:
@@ -116,6 +123,7 @@ def sentry_patched_execute_command(
116123
attributes={
117124
"sentry.op": cache_properties["op"],
118125
"sentry.origin": SPAN_ORIGIN,
126+
**additional_cache_span_attributes,
119127
},
120128
)
121129
else:
@@ -128,13 +136,20 @@ def sentry_patched_execute_command(
128136

129137
db_properties = _compile_db_span_properties(integration, name, args)
130138

139+
additional_db_span_attributes = {}
140+
with capture_internal_exceptions():
141+
additional_db_span_attributes[SPANDATA.DB_QUERY_TEXT] = _get_safe_command(
142+
name, args
143+
)
144+
131145
db_span: "Union[Span, StreamedSpan]"
132146
if span_streaming:
133147
db_span = sentry_sdk.traces.start_span(
134148
name=db_properties["description"],
135149
attributes={
136150
"sentry.op": db_properties["op"],
137151
"sentry.origin": SPAN_ORIGIN,
152+
**additional_db_span_attributes,
138153
},
139154
)
140155
else:

‎tests/integrations/redis/test_redis.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ def test_sensitive_data(sentry_init, capture_events, capture_items, span_streami
139139

140140
assert parent_span["name"] == "custom parent"
141141
assert redis_span["name"] == "GET [Filtered]"
142+
assert redis_span["attributes"][SPANDATA.DB_QUERY_TEXT] == "GET [Filtered]"
142143
assert redis_span["attributes"]["sentry.op"] == "db.redis"
143144
else:
144145
events = capture_events()
@@ -177,8 +178,10 @@ def test_pii_data_redacted(sentry_init, capture_events, capture_items, span_stre
177178

178179
assert parent["name"] == "custom parent"
179180
assert set1["name"] == "SET 'somekey1' [Filtered]"
181+
assert set1["attributes"][SPANDATA.DB_QUERY_TEXT] == "SET 'somekey1' [Filtered]"
180182
assert set1["attributes"]["sentry.op"] == "db.redis"
181183
assert set2["name"] == "SET 'somekey2' [Filtered]"
184+
assert set2["attributes"][SPANDATA.DB_QUERY_TEXT] == "SET 'somekey2' [Filtered]"
182185
assert get["name"] == "GET 'somekey2'"
183186
assert delete["name"] == "DEL 'somekey1' [Filtered]"
184187
else:
@@ -223,8 +226,16 @@ def test_pii_data_sent(sentry_init, capture_events, capture_items, span_streamin
223226

224227
assert parent["name"] == "custom parent"
225228
assert set1["name"] == "SET 'somekey1' 'my secret string1'"
229+
assert (
230+
set1["attributes"][SPANDATA.DB_QUERY_TEXT]
231+
== "SET 'somekey1' 'my secret string1'"
232+
)
226233
assert set1["attributes"]["sentry.op"] == "db.redis"
227234
assert set2["name"] == "SET 'somekey2' 'my secret string2'"
235+
assert (
236+
set2["attributes"][SPANDATA.DB_QUERY_TEXT]
237+
== "SET 'somekey2' 'my secret string2'"
238+
)
228239
assert get["name"] == "GET 'somekey2'"
229240
assert delete["name"] == "DEL 'somekey1' 'somekey2'"
230241
else:
@@ -271,8 +282,16 @@ def test_no_data_truncation_by_default(
271282

272283
assert parent["name"] == "custom parent"
273284
assert set1["name"] == f"SET 'somekey1' '{long_string}'"
285+
assert (
286+
set1["attributes"][SPANDATA.DB_QUERY_TEXT]
287+
== f"SET 'somekey1' '{long_string}'"
288+
)
274289
assert set1["attributes"]["sentry.op"] == "db.redis"
275290
assert set2["name"] == f"SET 'somekey2' '{short_string}'"
291+
assert (
292+
set2["attributes"][SPANDATA.DB_QUERY_TEXT]
293+
== f"SET 'somekey2' '{short_string}'"
294+
)
276295
else:
277296
events = capture_events()
278297
with start_transaction():
@@ -317,8 +336,16 @@ def test_data_truncation_custom(
317336

318337
assert parent["name"] == "custom parent"
319338
assert set1["name"] == expected_long
339+
assert (
340+
set1["attributes"][SPANDATA.DB_QUERY_TEXT]
341+
== f"SET 'somekey1' '{long_string}'"
342+
)
320343
assert set1["attributes"]["sentry.op"] == "db.redis"
321344
assert set2["name"] == expected_short
345+
assert (
346+
set2["attributes"][SPANDATA.DB_QUERY_TEXT]
347+
== f"SET 'somekey2' '{short_string}'"
348+
)
322349
else:
323350
events = capture_events()
324351
with start_transaction():
@@ -401,6 +428,7 @@ def test_db_connection_attributes_client(
401428
assert redis_span["name"] == "GET 'foobar'"
402429
attrs = redis_span["attributes"]
403430
assert attrs["sentry.op"] == "db.redis"
431+
assert attrs[SPANDATA.DB_QUERY_TEXT] == "GET 'foobar'"
404432
assert attrs[SPANDATA.DB_SYSTEM_NAME] == "redis"
405433
assert attrs[SPANDATA.DB_DRIVER_NAME] == "redis-py"
406434
assert attrs[SPANDATA.DB_NAMESPACE] == "1"
@@ -508,6 +536,9 @@ def test_span_origin(sentry_init, capture_events, capture_items, span_streaming)
508536
assert parent_span["name"] == "custom parent"
509537
assert parent_span["attributes"]["sentry.origin"] == "manual"
510538
assert set_span["attributes"]["sentry.origin"] == "auto.db.redis"
539+
assert (
540+
set_span["attributes"][SPANDATA.DB_QUERY_TEXT] == "SET 'somekey' [Filtered]"
541+
)
511542
assert pipeline_span["attributes"]["sentry.origin"] == "auto.db.redis"
512543
else:
513544
events = capture_events()

‎tests/integrations/redis/test_redis_cache_module.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,21 +83,34 @@ def test_cache_basic(sentry_init, capture_events, capture_items, span_streaming)
8383
assert payloads[1]["attributes"]["sentry.op"] == "db.redis"
8484
assert payloads[1]["attributes"][SPANDATA.DB_OPERATION_NAME] == "GET"
8585
assert payloads[2]["attributes"]["sentry.op"] == "cache.get"
86+
assert payloads[2]["attributes"][SPANDATA.DB_QUERY_TEXT] == "GET 'mycachekey'"
8687

8788
# set: db then cache.put
8889
assert payloads[3]["attributes"]["sentry.op"] == "db.redis"
8990
assert payloads[3]["attributes"][SPANDATA.DB_OPERATION_NAME] == "SET"
9091
assert payloads[4]["attributes"]["sentry.op"] == "cache.put"
92+
assert (
93+
payloads[4]["attributes"][SPANDATA.DB_QUERY_TEXT]
94+
== "SET 'mycachekey1' [Filtered]"
95+
)
9196

9297
# setex: db then cache.put
9398
assert payloads[5]["attributes"]["sentry.op"] == "db.redis"
9499
assert payloads[5]["attributes"][SPANDATA.DB_OPERATION_NAME] == "SETEX"
95100
assert payloads[6]["attributes"]["sentry.op"] == "cache.put"
101+
assert (
102+
payloads[6]["attributes"][SPANDATA.DB_QUERY_TEXT]
103+
== "SETEX 'mycachekey2' [Filtered] [Filtered]"
104+
)
96105

97106
# mget: db then cache.get
98107
assert payloads[7]["attributes"]["sentry.op"] == "db.redis"
99108
assert payloads[7]["attributes"][SPANDATA.DB_OPERATION_NAME] == "MGET"
100109
assert payloads[8]["attributes"]["sentry.op"] == "cache.get"
110+
assert (
111+
payloads[8]["attributes"][SPANDATA.DB_QUERY_TEXT]
112+
== "MGET 'mycachekey1' [Filtered]"
113+
)
101114

102115
assert payloads[9]["name"] == "custom parent"
103116
else:
@@ -169,12 +182,14 @@ def test_cache_keys(sentry_init, capture_events, capture_items, span_streaming):
169182
assert payloads[1]["name"] == "GET 'blub'"
170183
assert payloads[2]["attributes"]["sentry.op"] == "cache.get"
171184
assert payloads[2]["name"] == "blub"
185+
assert payloads[2]["attributes"][SPANDATA.DB_QUERY_TEXT] == "GET 'blub'"
172186

173187
# blubkeything: db then cache.get
174188
assert payloads[3]["attributes"]["sentry.op"] == "db.redis"
175189
assert payloads[3]["name"] == "GET 'blubkeything'"
176190
assert payloads[4]["attributes"]["sentry.op"] == "cache.get"
177191
assert payloads[4]["name"] == "blubkeything"
192+
assert payloads[4]["attributes"][SPANDATA.DB_QUERY_TEXT] == "GET 'blubkeything'"
178193

179194
# bl: db only (no prefix match)
180195
assert payloads[5]["attributes"]["sentry.op"] == "db.redis"

‎tests/integrations/redis/test_redis_cache_module_async.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
)
1616

1717
import sentry_sdk
18+
from sentry_sdk.consts import SPANDATA
1819
from sentry_sdk.integrations.redis import RedisIntegration
1920
from sentry_sdk.utils import parse_version
2021

@@ -83,6 +84,7 @@ async def test_cache_basic(sentry_init, capture_events, capture_items, span_stre
8384
assert parent_span["name"] == "custom parent"
8485
assert db_span["attributes"]["sentry.op"] == "db.redis"
8586
assert cache_span["attributes"]["sentry.op"] == "cache.get"
87+
assert cache_span["attributes"][SPANDATA.CACHE_KEY] == ["myasynccachekey"]
8688
else:
8789
events = capture_events()
8890
with sentry_sdk.start_transaction():
@@ -132,12 +134,14 @@ async def test_cache_keys(sentry_init, capture_events, capture_items, span_strea
132134
assert payloads[1]["name"] == "GET 'ablub'"
133135
assert payloads[2]["attributes"]["sentry.op"] == "cache.get"
134136
assert payloads[2]["name"] == "ablub"
137+
assert payloads[2]["attributes"][SPANDATA.CACHE_KEY] == ["ablub"]
135138

136139
# ablubkeything: db then cache.get
137140
assert payloads[3]["attributes"]["sentry.op"] == "db.redis"
138141
assert payloads[3]["name"] == "GET 'ablubkeything'"
139142
assert payloads[4]["attributes"]["sentry.op"] == "cache.get"
140143
assert payloads[4]["name"] == "ablubkeything"
144+
assert payloads[4]["attributes"][SPANDATA.CACHE_KEY] == ["ablubkeything"]
141145

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

0 commit comments

Comments
 (0)