Skip to content

Commit 4721438

Browse files
cursoragentszokeasaurusrex
authored andcommitted
Convert tag values to strings in Sentry SDK
Ensure tag values are always converted to strings using safe_str clean up Cursor's solution Convert tag values from int to str in scope and compatibility tests Checkpoint before follow-up message Fix request_data tag assertions in integration tests Convert boolean tags to strings in Redis and AWS Lambda integrations tests: Regenerate tox (#4457) (Yeah I'll automate this at some point) reformat tests fix `test_memory_usage` fix apidocs Only convert to string when serializing fix tests
1 parent ca42492 commit 4721438

File tree

11 files changed

+293
-21
lines changed

11 files changed

+293
-21
lines changed

sentry_sdk/opentelemetry/span_processor.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
try_profile_lifecycle_trace_start,
3838
)
3939
from sentry_sdk.profiler.transaction_profiler import Profile
40+
from sentry_sdk.utils import safe_str
4041
from sentry_sdk._types import TYPE_CHECKING
4142

4243
if TYPE_CHECKING:
@@ -311,7 +312,9 @@ def _common_span_transaction_attributes_as_json(self, span):
311312

312313
tags = extract_span_attributes(span, SentrySpanAttribute.TAG)
313314
if tags:
314-
common_json["tags"] = tags
315+
common_json["tags"] = {
316+
tag: safe_str(tag_value) for tag, tag_value in tags.items()
317+
}
315318

316319
return common_json
317320

sentry_sdk/scope.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
event_from_exception,
3939
exc_info_from_error,
4040
logger,
41+
safe_str,
4142
)
4243

4344
from typing import TYPE_CHECKING
@@ -1164,7 +1165,9 @@ def _apply_extra_to_event(self, event, hint, options):
11641165
def _apply_tags_to_event(self, event, hint, options):
11651166
# type: (Event, Hint, Optional[Dict[str, Any]]) -> None
11661167
if self._tags:
1167-
event.setdefault("tags", {}).update(self._tags)
1168+
event.setdefault("tags", {}).update(
1169+
{k: safe_str(v) for k, v in self._tags.items()}
1170+
)
11681171

11691172
def _apply_contexts_to_event(self, event, hint, options):
11701173
# type: (Event, Hint, Optional[Dict[str, Any]]) -> None

tests/integrations/aws_lambda/test_aws_lambda.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,10 +322,10 @@ def test_non_dict_event(
322322
assert transaction_event["request"] == request_data
323323

324324
if batch_size > 1:
325-
assert error_event["tags"]["batch_size"] == batch_size
326-
assert error_event["tags"]["batch_request"] is True
327-
assert transaction_event["tags"]["batch_size"] == batch_size
328-
assert transaction_event["tags"]["batch_request"] is True
325+
assert error_event["tags"]["batch_size"] == str(batch_size)
326+
assert error_event["tags"]["batch_request"] == "True"
327+
assert transaction_event["tags"]["batch_size"] == str(batch_size)
328+
assert transaction_event["tags"]["batch_request"] == "True"
329329

330330

331331
def test_request_data(lambda_client, test_environment):

tests/integrations/redis/asyncio/test_redis_asyncio.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ async def test_async_redis_pipeline(
7878
}
7979
)
8080
assert span["tags"] == {
81-
"redis.transaction": is_transaction,
82-
"redis.is_cluster": False,
81+
"redis.transaction": str(is_transaction),
82+
"redis.is_cluster": "False",
8383
}
8484

8585

tests/integrations/redis/cluster/test_redis_cluster.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ def test_rediscluster_basic(sentry_init, capture_events, send_default_pii, descr
9494
assert span["tags"] == {
9595
"db.operation": "SET",
9696
"redis.command": "SET",
97-
"redis.is_cluster": True,
97+
"redis.is_cluster": "True",
9898
"redis.key": "bar",
9999
}
100100

@@ -139,8 +139,8 @@ def test_rediscluster_pipeline(
139139
}
140140
)
141141
assert span["tags"] == {
142-
"redis.transaction": False, # For Cluster, this is always False
143-
"redis.is_cluster": True,
142+
"redis.transaction": "False", # For Cluster, this is always False
143+
"redis.is_cluster": "True",
144144
}
145145

146146

tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ async def test_async_basic(sentry_init, capture_events, send_default_pii, descri
9494
}
9595
)
9696
assert span["tags"] == {
97-
"redis.is_cluster": True,
97+
"redis.is_cluster": "True",
9898
"db.operation": "SET",
9999
"redis.command": "SET",
100100
"redis.key": "bar",
@@ -142,8 +142,8 @@ async def test_async_redis_pipeline(
142142
}
143143
)
144144
assert span["tags"] == {
145-
"redis.transaction": False,
146-
"redis.is_cluster": True,
145+
"redis.transaction": "False",
146+
"redis.is_cluster": "True",
147147
}
148148

149149

tests/integrations/redis/test_redis.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,8 @@ def test_redis_pipeline(
7575
assert span["data"]["redis.commands.count"] == 3
7676
assert span["data"]["redis.commands.first_ten"] == expected_first_ten
7777
assert span["tags"] == {
78-
"redis.transaction": is_transaction,
79-
"redis.is_cluster": False,
78+
"redis.transaction": str(is_transaction),
79+
"redis.is_cluster": "False",
8080
}
8181

8282

tests/integrations/redis_py_cluster_legacy/test_redis_py_cluster_legacy.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ def test_rediscluster_pipeline(
106106
}
107107
)
108108
assert span["tags"] == {
109-
"redis.transaction": False, # For Cluster, this is always False
110-
"redis.is_cluster": True,
109+
"redis.transaction": "False", # For Cluster, this is always False
110+
"redis.is_cluster": "True",
111111
}
112112

113113

tests/test_api.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
set_tags,
1515
get_global_scope,
1616
get_isolation_scope,
17+
set_tag,
1718
)
1819

1920
from sentry_sdk.client import Client, NonRecordingClient
@@ -158,3 +159,82 @@ def test_set_tags(sentry_init, capture_events):
158159
"tag2": "updated",
159160
"tag3": "new",
160161
}, "Updating tags with empty dict changed tags"
162+
163+
164+
@pytest.mark.parametrize(
165+
("key", "value", "expected"),
166+
[
167+
("int", 123, "123"),
168+
("float", 123.456, "123.456"),
169+
("bool", True, "True"),
170+
("none", None, "None"),
171+
("list", [1, 2, 3], "[1, 2, 3]"),
172+
],
173+
)
174+
def test_set_tag_converts_to_string(sentry_init, capture_events, key, value, expected):
175+
"""Test that the api.set_tag function converts values to strings."""
176+
sentry_init()
177+
events = capture_events()
178+
179+
set_tag(key, value)
180+
raise_and_capture()
181+
182+
(event,) = events
183+
tags = event.get("tags", {})
184+
185+
assert tags[key] == expected
186+
187+
188+
def test_set_tags_converts_to_string(sentry_init, capture_events):
189+
"""Test that the api.set_tags function converts values to strings."""
190+
sentry_init()
191+
events = capture_events()
192+
193+
set_tags(
194+
{
195+
"int": 456,
196+
"float": 789.012,
197+
"bool": False,
198+
"tuple": (1, 2, 3),
199+
"string": "already_string",
200+
}
201+
)
202+
203+
raise_and_capture()
204+
205+
(*_, event) = events
206+
tags = event.get("tags", {})
207+
208+
assert tags["int"] == "456"
209+
assert tags["float"] == "789.012"
210+
assert tags["bool"] == "False"
211+
assert tags["tuple"] == "(1, 2, 3)"
212+
assert tags["string"] == "already_string"
213+
214+
215+
def test_configure_scope_deprecation():
216+
with pytest.warns(DeprecationWarning):
217+
with configure_scope():
218+
...
219+
220+
221+
def test_push_scope_deprecation():
222+
with pytest.warns(DeprecationWarning):
223+
with push_scope():
224+
...
225+
226+
227+
def test_init_context_manager_deprecation():
228+
with pytest.warns(DeprecationWarning):
229+
with sentry_sdk.init():
230+
...
231+
232+
233+
def test_init_enter_deprecation():
234+
with pytest.warns(DeprecationWarning):
235+
sentry_sdk.init().__enter__()
236+
237+
238+
def test_init_exit_deprecation():
239+
with pytest.warns(DeprecationWarning):
240+
sentry_sdk.init().__exit__(None, None, None)

tests/test_scope.py

Lines changed: 111 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -823,9 +823,13 @@ def test_nested_scopes_with_tags(sentry_init, capture_envelopes):
823823
(envelope,) = envelopes
824824
transaction = envelope.items[0].get_transaction_event()
825825

826-
assert transaction["tags"] == {"isolation_scope1": 1, "current_scope2": 1, "trx": 1}
827-
assert transaction["spans"][0]["tags"] == ApproxDict({"a": 1})
828-
assert transaction["spans"][1]["tags"] == ApproxDict({"b": 1})
826+
assert transaction["tags"] == {
827+
"isolation_scope1": "1",
828+
"current_scope2": "1",
829+
"trx": "1",
830+
}
831+
assert transaction["spans"][0]["tags"] == ApproxDict({"a": "1"})
832+
assert transaction["spans"][1]["tags"] == ApproxDict({"b": "1"})
829833

830834

831835
def test_should_send_default_pii_true(sentry_init):
@@ -931,3 +935,107 @@ def test_root_span(sentry_init):
931935
assert sentry_sdk.get_current_scope().root_span == root_span
932936

933937
assert sentry_sdk.get_current_scope().root_span is None
938+
939+
940+
@pytest.mark.parametrize(
941+
("key", "value", "expected"),
942+
[
943+
("int", 123, "123"),
944+
("float", 123.456, "123.456"),
945+
("bool_true", True, "True"),
946+
("bool_false", False, "False"),
947+
("none", None, "None"),
948+
("list", [1, 2, 3], "[1, 2, 3]"),
949+
("dict", {"key": "value"}, "{'key': 'value'}"),
950+
("already_string", "test", "test"),
951+
],
952+
)
953+
def test_set_tag_converts_to_string(key, value, expected):
954+
"""Test that set_tag converts various types to strings."""
955+
scope = Scope()
956+
scope.set_tag(key, value)
957+
958+
event = scope.apply_to_event({}, {})
959+
tags = event.get("tags", {})
960+
961+
assert tags[key] == expected, f"Tag {key} was not converted properly"
962+
963+
964+
def test_set_tags_converts_to_string():
965+
"""Test that set_tags converts all values to strings."""
966+
scope = Scope()
967+
968+
scope.set_tags(
969+
{
970+
"int": 123,
971+
"float": 123.456,
972+
"bool": True,
973+
"none": None,
974+
"list": [1, 2, 3],
975+
"string": "test",
976+
}
977+
)
978+
979+
event = scope.apply_to_event({}, {})
980+
tags = event.get("tags", {})
981+
982+
assert tags["int"] == "123"
983+
assert tags["float"] == "123.456"
984+
assert tags["bool"] == "True"
985+
assert tags["none"] == "None"
986+
assert tags["list"] == "[1, 2, 3]"
987+
assert tags["string"] == "test"
988+
989+
990+
def test_set_tag_handles_conversion_failure():
991+
"""Test that set_tag handles objects that fail to convert to string."""
992+
scope = Scope()
993+
994+
# Create an object that raises an exception when str() is called
995+
class BadObject:
996+
def __str__(self):
997+
raise Exception("Cannot convert to string")
998+
999+
def __repr__(self):
1000+
return "BadObject()"
1001+
1002+
bad_obj = BadObject()
1003+
1004+
# This should not raise an exception
1005+
scope.set_tag("bad_object", bad_obj)
1006+
1007+
# The tag should be set with the repr value
1008+
event = scope.apply_to_event({}, {})
1009+
tags = event.get("tags", {})
1010+
1011+
assert tags["bad_object"] == "BadObject()", "Tag should be set with repr value"
1012+
1013+
1014+
def test_set_tags_handles_conversion_failure():
1015+
"""Test that set_tags handles objects that fail to convert to string."""
1016+
scope = Scope()
1017+
1018+
# Create an object that raises an exception when str() is called
1019+
class BadObject:
1020+
def __str__(self):
1021+
raise Exception("Cannot convert to string")
1022+
1023+
def __repr__(self):
1024+
return "BadObject()"
1025+
1026+
bad_obj = BadObject()
1027+
1028+
scope.set_tags(
1029+
{
1030+
"good_tag1": "value1",
1031+
"bad_tag": bad_obj,
1032+
"good_tag2": 123,
1033+
}
1034+
)
1035+
1036+
event = scope.apply_to_event({}, {})
1037+
tags = event.get("tags", {})
1038+
1039+
assert tags["good_tag1"] == "value1"
1040+
assert tags["bad_tag"] == "BadObject()", "Tag should be set with repr value"
1041+
assert tags["good_tag2"] == "123"

0 commit comments

Comments
 (0)