diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index f346569255..1a6a13d151 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -42,6 +42,7 @@ event_from_exception, exc_info_from_error, logger, + safe_str, ) import typing @@ -1421,7 +1422,9 @@ def _apply_extra_to_event(self, event, hint, options): def _apply_tags_to_event(self, event, hint, options): # type: (Event, Hint, Optional[Dict[str, Any]]) -> None if self._tags: - event.setdefault("tags", {}).update(self._tags) + event.setdefault("tags", {}).update( + {k: safe_str(v) for k, v in self._tags.items()} + ) def _apply_contexts_to_event(self, event, hint, options): # type: (Event, Hint, Optional[Dict[str, Any]]) -> None diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index fc40221b9f..fb899dbca5 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -13,6 +13,7 @@ logger, nanosecond_time, should_be_treated_as_error, + safe_str, ) from typing import TYPE_CHECKING @@ -313,7 +314,7 @@ def __init__( self.scope = scope self.origin = origin self._measurements = {} # type: Dict[str, MeasurementValue] - self._tags = {} # type: MutableMapping[str, str] + self._tags = {} # type: MutableMapping[str, object] self._data = {} # type: Dict[str, Any] self._containing_transaction = containing_transaction self._flags = {} # type: Dict[str, bool] @@ -718,7 +719,7 @@ def to_json(self): tags = self._tags if tags: - rv["tags"] = tags + rv["tags"] = {k: safe_str(v) for k, v in tags.items()} data = {} data.update(self._flags) @@ -1045,7 +1046,7 @@ def finish( "transaction": self.name, "transaction_info": {"source": self.source}, "contexts": contexts, - "tags": self._tags, + "tags": {k: safe_str(v) for k, v in self._tags.items()}, "timestamp": self.timestamp, "start_timestamp": self.start_timestamp, "spans": finished_spans, diff --git a/tests/integrations/aws_lambda/test_aws_lambda.py b/tests/integrations/aws_lambda/test_aws_lambda.py index 85da7e0b14..91f93f70dc 100644 --- a/tests/integrations/aws_lambda/test_aws_lambda.py +++ b/tests/integrations/aws_lambda/test_aws_lambda.py @@ -325,10 +325,10 @@ def test_non_dict_event( assert transaction_event["request"] == request_data if batch_size > 1: - assert error_event["tags"]["batch_size"] == batch_size - assert error_event["tags"]["batch_request"] is True - assert transaction_event["tags"]["batch_size"] == batch_size - assert transaction_event["tags"]["batch_request"] is True + assert error_event["tags"]["batch_size"] == str(batch_size) + assert error_event["tags"]["batch_request"] == "True" + assert transaction_event["tags"]["batch_size"] == str(batch_size) + assert transaction_event["tags"]["batch_request"] == "True" def test_request_data(lambda_client, test_environment): diff --git a/tests/integrations/redis/asyncio/test_redis_asyncio.py b/tests/integrations/redis/asyncio/test_redis_asyncio.py index 17130b337b..5cf3f12636 100644 --- a/tests/integrations/redis/asyncio/test_redis_asyncio.py +++ b/tests/integrations/redis/asyncio/test_redis_asyncio.py @@ -80,8 +80,8 @@ async def test_async_redis_pipeline( } ) assert span["tags"] == { - "redis.transaction": is_transaction, - "redis.is_cluster": False, + "redis.transaction": str(is_transaction), + "redis.is_cluster": "False", } diff --git a/tests/integrations/redis/cluster/test_redis_cluster.py b/tests/integrations/redis/cluster/test_redis_cluster.py index 83d1b45cc9..b09d8e08bb 100644 --- a/tests/integrations/redis/cluster/test_redis_cluster.py +++ b/tests/integrations/redis/cluster/test_redis_cluster.py @@ -94,7 +94,7 @@ def test_rediscluster_basic(sentry_init, capture_events, send_default_pii, descr assert span["tags"] == { "db.operation": "SET", "redis.command": "SET", - "redis.is_cluster": True, + "redis.is_cluster": "True", "redis.key": "bar", } @@ -141,8 +141,8 @@ def test_rediscluster_pipeline( } ) assert span["tags"] == { - "redis.transaction": False, # For Cluster, this is always False - "redis.is_cluster": True, + "redis.transaction": "False", # For Cluster, this is always False + "redis.is_cluster": "True", } diff --git a/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py b/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py index 993a2962ca..5a6fc61ee0 100644 --- a/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py +++ b/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py @@ -94,7 +94,7 @@ async def test_async_basic(sentry_init, capture_events, send_default_pii, descri } ) assert span["tags"] == { - "redis.is_cluster": True, + "redis.is_cluster": "True", "db.operation": "SET", "redis.command": "SET", "redis.key": "bar", @@ -144,8 +144,8 @@ async def test_async_redis_pipeline( } ) assert span["tags"] == { - "redis.transaction": False, - "redis.is_cluster": True, + "redis.transaction": "False", + "redis.is_cluster": "True", } diff --git a/tests/integrations/redis/test_redis.py b/tests/integrations/redis/test_redis.py index 5173885f33..108ab7ebc2 100644 --- a/tests/integrations/redis/test_redis.py +++ b/tests/integrations/redis/test_redis.py @@ -77,8 +77,8 @@ def test_redis_pipeline( "first_ten": expected_first_ten, } assert span["tags"] == { - "redis.transaction": is_transaction, - "redis.is_cluster": False, + "redis.transaction": str(is_transaction), + "redis.is_cluster": "False", } diff --git a/tests/integrations/redis_py_cluster_legacy/test_redis_py_cluster_legacy.py b/tests/integrations/redis_py_cluster_legacy/test_redis_py_cluster_legacy.py index 36a27d569d..30a3be9b0d 100644 --- a/tests/integrations/redis_py_cluster_legacy/test_redis_py_cluster_legacy.py +++ b/tests/integrations/redis_py_cluster_legacy/test_redis_py_cluster_legacy.py @@ -108,8 +108,8 @@ def test_rediscluster_pipeline( } ) assert span["tags"] == { - "redis.transaction": False, # For Cluster, this is always False - "redis.is_cluster": True, + "redis.transaction": "False", # For Cluster, this is always False + "redis.is_cluster": "True", } diff --git a/tests/new_scopes_compat/test_new_scopes_compat.py b/tests/new_scopes_compat/test_new_scopes_compat.py index 21e2ac27d3..04c3d77553 100644 --- a/tests/new_scopes_compat/test_new_scopes_compat.py +++ b/tests/new_scopes_compat/test_new_scopes_compat.py @@ -35,9 +35,9 @@ def test_configure_scope_sdk1(sentry_init, capture_events): (event_a, event_b, event_z) = events # Check against the results the same code returned in SDK 1.x - assert event_a["tags"] == {"A": 1} - assert event_b["tags"] == {"A": 1, "B1": 1, "B2": 1} - assert event_z["tags"] == {"A": 1, "B1": 1, "B2": 1, "Z": 1} + assert event_a["tags"] == {"A": "1"} + assert event_b["tags"] == {"A": "1", "B1": "1", "B2": "1"} + assert event_z["tags"] == {"A": "1", "B1": "1", "B2": "1", "Z": "1"} def test_push_scope_sdk1(sentry_init, capture_events): @@ -64,9 +64,9 @@ def test_push_scope_sdk1(sentry_init, capture_events): (event_a, event_b, event_z) = events # Check against the results the same code returned in SDK 1.x - assert event_a["tags"] == {"A": 1} - assert event_b["tags"] == {"A": 1, "B1": 1, "B2": 1} - assert event_z["tags"] == {"A": 1, "Z": 1} + assert event_a["tags"] == {"A": "1"} + assert event_b["tags"] == {"A": "1", "B1": "1", "B2": "1"} + assert event_z["tags"] == {"A": "1", "Z": "1"} def test_with_hub_sdk1(sentry_init, capture_events): @@ -93,9 +93,9 @@ def test_with_hub_sdk1(sentry_init, capture_events): (event_a, event_b, event_z) = events # Check against the results the same code returned in SDK 1.x - assert event_a["tags"] == {"A": 1} - assert event_b["tags"] == {"A": 1, "B1": 1, "B2": 1} - assert event_z["tags"] == {"A": 1, "B1": 1, "B2": 1, "Z": 1} + assert event_a["tags"] == {"A": "1"} + assert event_b["tags"] == {"A": "1", "B1": "1", "B2": "1"} + assert event_z["tags"] == {"A": "1", "B1": "1", "B2": "1", "Z": "1"} def test_with_hub_configure_scope_sdk1(sentry_init, capture_events): @@ -127,17 +127,24 @@ def test_with_hub_configure_scope_sdk1(sentry_init, capture_events): (event_a, event_b, event_c, event_z) = events # Check against the results the same code returned in SDK 1.x - assert event_a["tags"] == {"A": 1} - assert event_b["tags"] == {"A": 1, "B1": 1, "B2": 1, "B3": 1, "B4": 1} - assert event_c["tags"] == {"A": 1, "B1": 1, "B2": 1, "B3": 1, "B4": 1, "B5": 1} + assert event_a["tags"] == {"A": "1"} + assert event_b["tags"] == {"A": "1", "B1": "1", "B2": "1", "B3": "1", "B4": "1"} + assert event_c["tags"] == { + "A": "1", + "B1": "1", + "B2": "1", + "B3": "1", + "B4": "1", + "B5": "1", + } assert event_z["tags"] == { - "A": 1, - "B1": 1, - "B2": 1, - "B3": 1, - "B4": 1, - "B5": 1, - "Z": 1, + "A": "1", + "B1": "1", + "B2": "1", + "B3": "1", + "B4": "1", + "B5": "1", + "Z": "1", } @@ -170,10 +177,10 @@ def test_with_hub_push_scope_sdk1(sentry_init, capture_events): (event_a, event_b, event_c, event_z) = events # Check against the results the same code returned in SDK 1.x - assert event_a["tags"] == {"A": 1} - assert event_b["tags"] == {"A": 1, "B1": 1, "B2": 1, "B3": 1, "B4": 1} - assert event_c["tags"] == {"A": 1, "B1": 1, "B5": 1} - assert event_z["tags"] == {"A": 1, "B1": 1, "B5": 1, "Z": 1} + assert event_a["tags"] == {"A": "1"} + assert event_b["tags"] == {"A": "1", "B1": "1", "B2": "1", "B3": "1", "B4": "1"} + assert event_c["tags"] == {"A": "1", "B1": "1", "B5": "1"} + assert event_z["tags"] == {"A": "1", "B1": "1", "B5": "1", "Z": "1"} def test_with_cloned_hub_sdk1(sentry_init, capture_events): @@ -200,9 +207,9 @@ def test_with_cloned_hub_sdk1(sentry_init, capture_events): (event_a, event_b, event_z) = events # Check against the results the same code returned in SDK 1.x - assert event_a["tags"] == {"A": 1} - assert event_b["tags"] == {"A": 1, "B1": 1, "B2": 1} - assert event_z["tags"] == {"A": 1, "Z": 1} + assert event_a["tags"] == {"A": "1"} + assert event_b["tags"] == {"A": "1", "B1": "1", "B2": "1"} + assert event_z["tags"] == {"A": "1", "Z": "1"} def test_with_cloned_hub_configure_scope_sdk1(sentry_init, capture_events): @@ -234,10 +241,17 @@ def test_with_cloned_hub_configure_scope_sdk1(sentry_init, capture_events): (event_a, event_b, event_c, event_z) = events # Check against the results the same code returned in SDK 1.x - assert event_a["tags"] == {"A": 1} - assert event_b["tags"] == {"A": 1, "B1": 1, "B2": 1, "B3": 1, "B4": 1} - assert event_c["tags"] == {"A": 1, "B1": 1, "B2": 1, "B3": 1, "B4": 1, "B5": 1} - assert event_z["tags"] == {"A": 1, "Z": 1} + assert event_a["tags"] == {"A": "1"} + assert event_b["tags"] == {"A": "1", "B1": "1", "B2": "1", "B3": "1", "B4": "1"} + assert event_c["tags"] == { + "A": "1", + "B1": "1", + "B2": "1", + "B3": "1", + "B4": "1", + "B5": "1", + } + assert event_z["tags"] == {"A": "1", "Z": "1"} def test_with_cloned_hub_push_scope_sdk1(sentry_init, capture_events): @@ -269,7 +283,7 @@ def test_with_cloned_hub_push_scope_sdk1(sentry_init, capture_events): (event_a, event_b, event_c, event_z) = events # Check against the results the same code returned in SDK 1.x - assert event_a["tags"] == {"A": 1} - assert event_b["tags"] == {"A": 1, "B1": 1, "B2": 1, "B3": 1, "B4": 1} - assert event_c["tags"] == {"A": 1, "B1": 1, "B5": 1} - assert event_z["tags"] == {"A": 1, "Z": 1} + assert event_a["tags"] == {"A": "1"} + assert event_b["tags"] == {"A": "1", "B1": "1", "B2": "1", "B3": "1", "B4": "1"} + assert event_c["tags"] == {"A": "1", "B1": "1", "B5": "1"} + assert event_z["tags"] == {"A": "1", "Z": "1"} diff --git a/tests/test_api.py b/tests/test_api.py index 08c295a5c4..710b98264a 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -19,6 +19,7 @@ get_global_scope, get_current_scope, get_isolation_scope, + set_tag, ) from sentry_sdk.client import Client, NonRecordingClient @@ -189,6 +190,57 @@ def test_set_tags(sentry_init, capture_events): }, "Updating tags with empty dict changed tags" +@pytest.mark.parametrize( + ("key", "value", "expected"), + [ + ("int", 123, "123"), + ("float", 123.456, "123.456"), + ("bool", True, "True"), + ("none", None, "None"), + ("list", [1, 2, 3], "[1, 2, 3]"), + ], +) +def test_set_tag_converts_to_string(sentry_init, capture_events, key, value, expected): + """Test that the api.set_tag function converts values to strings.""" + sentry_init() + events = capture_events() + + set_tag(key, value) + raise_and_capture() + + (event,) = events + tags = event.get("tags", {}) + + assert tags[key] == expected + + +def test_set_tags_converts_to_string(sentry_init, capture_events): + """Test that the api.set_tags function converts values to strings.""" + sentry_init() + events = capture_events() + + set_tags( + { + "int": 456, + "float": 789.012, + "bool": False, + "tuple": (1, 2, 3), + "string": "already_string", + } + ) + + raise_and_capture() + + (*_, event) = events + tags = event.get("tags", {}) + + assert tags["int"] == "456" + assert tags["float"] == "789.012" + assert tags["bool"] == "False" + assert tags["tuple"] == "(1, 2, 3)" + assert tags["string"] == "already_string" + + def test_configure_scope_deprecation(): with pytest.warns(DeprecationWarning): with configure_scope(): diff --git a/tests/test_scope.py b/tests/test_scope.py index 9b16dc4344..1ecc68bf9d 100644 --- a/tests/test_scope.py +++ b/tests/test_scope.py @@ -812,9 +812,13 @@ def test_nested_scopes_with_tags(sentry_init, capture_envelopes): (envelope,) = envelopes transaction = envelope.items[0].get_transaction_event() - assert transaction["tags"] == {"isolation_scope1": 1, "current_scope2": 1, "trx": 1} - assert transaction["spans"][0]["tags"] == {"a": 1} - assert transaction["spans"][1]["tags"] == {"b": 1} + assert transaction["tags"] == { + "isolation_scope1": "1", + "current_scope2": "1", + "trx": "1", + } + assert transaction["spans"][0]["tags"] == {"a": "1"} + assert transaction["spans"][1]["tags"] == {"b": "1"} def test_should_send_default_pii_true(sentry_init): @@ -905,3 +909,107 @@ def test_last_event_id_cleared(sentry_init): Scope.get_isolation_scope().clear() assert Scope.last_event_id() is None, "last_event_id should be cleared" + + +@pytest.mark.parametrize( + ("key", "value", "expected"), + [ + ("int", 123, "123"), + ("float", 123.456, "123.456"), + ("bool_true", True, "True"), + ("bool_false", False, "False"), + ("none", None, "None"), + ("list", [1, 2, 3], "[1, 2, 3]"), + ("dict", {"key": "value"}, "{'key': 'value'}"), + ("already_string", "test", "test"), + ], +) +def test_set_tag_converts_to_string(key, value, expected): + """Test that set_tag converts various types to strings.""" + scope = Scope() + scope.set_tag(key, value) + + event = scope.apply_to_event({}, {}) + tags = event.get("tags", {}) + + assert tags[key] == expected, f"Tag {key} was not converted properly" + + +def test_set_tags_converts_to_string(): + """Test that set_tags converts all values to strings.""" + scope = Scope() + + scope.set_tags( + { + "int": 123, + "float": 123.456, + "bool": True, + "none": None, + "list": [1, 2, 3], + "string": "test", + } + ) + + event = scope.apply_to_event({}, {}) + tags = event.get("tags", {}) + + assert tags["int"] == "123" + assert tags["float"] == "123.456" + assert tags["bool"] == "True" + assert tags["none"] == "None" + assert tags["list"] == "[1, 2, 3]" + assert tags["string"] == "test" + + +def test_set_tag_handles_conversion_failure(): + """Test that set_tag handles objects that fail to convert to string.""" + scope = Scope() + + # Create an object that raises an exception when str() is called + class BadObject: + def __str__(self): + raise Exception("Cannot convert to string") + + def __repr__(self): + return "BadObject()" + + bad_obj = BadObject() + + # This should not raise an exception + scope.set_tag("bad_object", bad_obj) + + # The tag should be set with the repr value + event = scope.apply_to_event({}, {}) + tags = event.get("tags", {}) + + assert tags["bad_object"] == "BadObject()", "Tag should be set with repr value" + + +def test_set_tags_handles_conversion_failure(): + """Test that set_tags handles objects that fail to convert to string.""" + scope = Scope() + + # Create an object that raises an exception when str() is called + class BadObject: + def __str__(self): + raise Exception("Cannot convert to string") + + def __repr__(self): + return "BadObject()" + + bad_obj = BadObject() + + scope.set_tags( + { + "good_tag1": "value1", + "bad_tag": bad_obj, + "good_tag2": 123, + } + ) + + event = scope.apply_to_event({}, {}) + tags = event.get("tags", {}) + + assert tags["good_tag1"] == "value1" + assert tags["bad_tag"] == "BadObject()", "Tag should be set with repr value" + assert tags["good_tag2"] == "123" diff --git a/tests/tracing/test_span_tags.py b/tests/tracing/test_span_tags.py new file mode 100644 index 0000000000..9ceb70335d --- /dev/null +++ b/tests/tracing/test_span_tags.py @@ -0,0 +1,113 @@ +import pytest +import sentry_sdk +from sentry_sdk.tracing import Span + + +@pytest.mark.parametrize( + ("key", "value", "expected"), + [ + ("int", 123, "123"), + ("float", 123.456, "123.456"), + ("bool_true", True, "True"), + ("bool_false", False, "False"), + ("none", None, "None"), + ("list", [1, 2, 3], "[1, 2, 3]"), + ("dict", {"key": "value"}, "{'key': 'value'}"), + ("already_string", "test", "test"), + ], +) +def test_span_set_tag_converts_to_string(key, value, expected): + """Test that Span.set_tag converts various types to strings.""" + span = Span() + span.set_tag(key, value) + + # Check the tags in the span's JSON representation + span_json = span.to_json() + tags = span_json.get("tags", {}) + + assert tags[key] == expected, f"Tag {key} was not converted properly" + + +def test_span_set_tag_handles_conversion_failure(): + """Test that Span.set_tag handles objects that fail to convert to string, + but have a valid __repr__.""" + span = Span() + + # Create an object that raises an exception when str() is called + class BadObject: + def __str__(self): + raise NotImplementedError("Cannot convert to string") + + def __repr__(self): + return "BadObject()" + + bad_obj = BadObject() + + # This should not raise an exception + span.set_tag("bad_object", bad_obj) + + # The tag should not be set + span_json = span.to_json() + tags = span_json.get("tags", {}) + + assert tags["bad_object"] == "BadObject()" + + +def test_span_set_tag_handles_broken_repr(): + """Test that Span.set_tag handles objects with broken __str__ and __repr__.""" + span = Span() + + # Create an object that raises exceptions for both __str__ and __repr__ + class BadObject: + def __str__(self): + raise NotImplementedError("Cannot convert to string") + + def __repr__(self): + raise NotImplementedError("Cannot get representation") + + bad_obj = BadObject() + + # This should not raise an exception + span.set_tag("bad_object", bad_obj) + + # The tag should be set to a fallback value + span_json = span.to_json() + tags = span_json.get("tags", {}) + + assert tags["bad_object"] == "" + + +@pytest.mark.parametrize( + ("tag_key", "tag_value", "expected_value"), + [ + ("int_tag", 42, "42"), + ("float_tag", 3.14159, "3.14159"), + ("bool_tag_true", True, "True"), + ("bool_tag_false", False, "False"), + ("none_tag", None, "None"), + ("list_tag", [1, "two", 3.0], "[1, 'two', 3.0]"), + ( + "dict_tag", + {"nested": "value", "count": 123}, + "{'nested': 'value', 'count': 123}", + ), + ("string_tag", "already_string", "already_string"), + ], +) +def test_transaction_finish_serializes_tags( + sentry_init, capture_events, tag_key, tag_value, expected_value +): + """Test that Transaction.finish() properly serializes transaction tags to strings.""" + # Create a transaction with span recording enabled + sentry_init(traces_sample_rate=1.0) + events = capture_events() + + with sentry_sdk.start_transaction( + name="test_transaction", sampled=True + ) as transaction: + transaction.set_tag(tag_key, tag_value) + + (event,) = events + tags = event.get("tags", {}) + + assert tags[tag_key] == expected_value