Serialize span attrs properly (#3668)

Author: sentrivana

sentry_sdk/integrations/asyncpg.py

@@ -8,6 +8,7 @@
 from sentry_sdk.tracing import Span
 from sentry_sdk.tracing_utils import add_query_source, record_sql_queries
 from sentry_sdk.utils import (
+    _serialize_span_attribute,
     ensure_integration_enabled,
     parse_version,
     capture_internal_exceptions,
@@ -147,7 +148,7 @@ def _inner(*args: Any, **kwargs: Any) -> T:  # noqa: N807
         ) as span:
             _set_db_data(span, args[0])
             res = f(*args, **kwargs)
-            span.set_attribute("db.cursor", str(res))
+            span.set_attribute("db.cursor", _serialize_span_attribute(res))
 
         return res
 

sentry_sdk/integrations/clickhouse_driver.py

@@ -3,7 +3,11 @@
 from sentry_sdk.integrations import Integration, DidNotEnable
 from sentry_sdk.tracing import Span
 from sentry_sdk.scope import should_send_default_pii
-from sentry_sdk.utils import capture_internal_exceptions, ensure_integration_enabled
+from sentry_sdk.utils import (
+    _serialize_span_attribute,
+    capture_internal_exceptions,
+    ensure_integration_enabled,
+)
 
 from typing import TYPE_CHECKING, TypeVar
 
@@ -99,7 +103,7 @@ def _inner(*args: P.args, **kwargs: P.kwargs) -> T:
 
         if params and should_send_default_pii():
             connection._sentry_db_params = params
-            span.set_attribute("db.params", str(params))
+            span.set_attribute("db.params", _serialize_span_attribute(params))
 
         # run the original code
         ret = f(*args, **kwargs)
@@ -117,7 +121,7 @@ def _inner_end(*args: P.args, **kwargs: P.kwargs) -> T:
 
         if span is not None:
             if res is not None and should_send_default_pii():
-                span.set_attribute("db.result", str(res))
+                span.set_attribute("db.result", _serialize_span_attribute(res))
 
             with capture_internal_exceptions():
                 query = span.get_attribute("db.query.text")
@@ -159,7 +163,7 @@ def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T:
                     getattr(instance.connection, "_sentry_db_params", None) or []
                 )
                 db_params.extend(data)
-                span.set_attribute("db.params", str(db_params))
+                span.set_attribute("db.params", _serialize_span_attribute(db_params))
                 try:
                     del instance.connection._sentry_db_params
                 except AttributeError:

sentry_sdk/tracing_utils.py

@@ -3,11 +3,11 @@
 import os
 import re
 import sys
+import uuid
 from collections.abc import Mapping
 from datetime import datetime, timedelta, timezone
 from functools import wraps
 from urllib.parse import quote, unquote
-import uuid
 
 import sentry_sdk
 from sentry_sdk.consts import OP, SPANDATA
@@ -23,6 +23,7 @@
     _is_external_source,
     _is_in_project_root,
     _module_in_list,
+    _serialize_span_attribute,
 )
 
 from typing import TYPE_CHECKING
@@ -133,13 +134,13 @@ def record_sql_queries(
 
     data = {}
     if params_list is not None:
-        data["db.params"] = str(params_list)
+        data["db.params"] = _serialize_span_attribute(params_list)
     if paramstyle is not None:
-        data["db.paramstyle"] = str(paramstyle)
+        data["db.paramstyle"] = _serialize_span_attribute(paramstyle)
     if executemany:
         data["db.executemany"] = True
     if record_cursor_repr and cursor is not None:
-        data["db.cursor"] = str(cursor)
+        data["db.cursor"] = _serialize_span_attribute(cursor)
 
     with capture_internal_exceptions():
         sentry_sdk.add_breadcrumb(message=query, category="query", data=data)

sentry_sdk/utils.py

@@ -51,6 +51,7 @@
     )
 
     from gevent.hub import Hub as GeventHub
+    from opentelemetry.util.types import AttributeValue
 
     from sentry_sdk._types import Event, ExcInfo
 
@@ -1831,3 +1832,28 @@ def get_current_thread_meta(thread=None):
 
     # we've tried everything, time to give up
     return None, None
+
+
+def _serialize_span_attribute(value):
+    # type: (Any) -> Optional[AttributeValue]
+    """Serialize an object so that it's OTel-compatible and displays nicely in Sentry."""
+    # check for allowed primitives
+    if isinstance(value, (int, str, float, bool)):
+        return value
+
+    # lists are allowed too, as long as they don't mix types
+    if isinstance(value, (list, tuple)):
+        for type_ in (int, str, float, bool):
+            if all(isinstance(item, type_) for item in value):
+                return list(value)
+
+    # if this is anything else, just try to coerce to string
+    # we prefer json.dumps since this makes things like dictionaries display
+    # nicely in the UI
+    try:
+        return json.dumps(value)
+    except TypeError:
+        try:
+            return str(value)
+        except Exception:
+            return None

tests/integrations/clickhouse_driver/test_clickhouse_driver.py

@@ -16,8 +16,6 @@
 if clickhouse_driver.VERSION < (0, 2, 6):
     EXPECT_PARAMS_IN_SELECT = False
 
-PARAMS_SERIALIZER = str
-
 
 def test_clickhouse_client_breadcrumbs(sentry_init, capture_events) -> None:
     sentry_init(
@@ -144,7 +142,7 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) ->
                 "db.user": "default",
                 "server.address": "localhost",
                 "server.port": 9000,
-                "db.result": str([]),
+                "db.result": [],
             },
             "message": "DROP TABLE IF EXISTS test",
             "type": "default",
@@ -157,7 +155,7 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) ->
                 "db.user": "default",
                 "server.address": "localhost",
                 "server.port": 9000,
-                "db.result": str([]),
+                "db.result": [],
             },
             "message": "CREATE TABLE test (x Int32) ENGINE = Memory",
             "type": "default",
@@ -170,7 +168,7 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) ->
                 "db.user": "default",
                 "server.address": "localhost",
                 "server.port": 9000,
-                "db.params": PARAMS_SERIALIZER([{"x": 100}]),
+                "db.params": '[{"x": 100}]',
             },
             "message": "INSERT INTO test (x) VALUES",
             "type": "default",
@@ -183,7 +181,7 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) ->
                 "db.user": "default",
                 "server.address": "localhost",
                 "server.port": 9000,
-                "db.params": PARAMS_SERIALIZER([[170], [200]]),
+                "db.params": "[[170], [200]]",
             },
             "message": "INSERT INTO test (x) VALUES",
             "type": "default",
@@ -196,8 +194,8 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) ->
                 "db.user": "default",
                 "server.address": "localhost",
                 "server.port": 9000,
-                "db.result": str([[370]]),
-                "db.params": PARAMS_SERIALIZER({"minv": 150}),
+                "db.result": "[[370]]",
+                "db.params": '{"minv": 150}',
             },
             "message": "SELECT sum(x) FROM test WHERE x > 150",
             "type": "default",
@@ -396,7 +394,7 @@ def test_clickhouse_client_spans_with_pii(
                 "server.address": "localhost",
                 "server.port": 9000,
                 "db.query.text": "DROP TABLE IF EXISTS test",
-                "db.result": str([]),
+                "db.result": [],
             },
             "trace_id": transaction_trace_id,
             "parent_span_id": transaction_span_id,
@@ -413,7 +411,7 @@ def test_clickhouse_client_spans_with_pii(
                 "db.name": "",
                 "db.user": "default",
                 "db.query.text": "CREATE TABLE test (x Int32) ENGINE = Memory",
-                "db.result": str([]),
+                "db.result": [],
                 "server.address": "localhost",
                 "server.port": 9000,
             },
@@ -432,7 +430,7 @@ def test_clickhouse_client_spans_with_pii(
                 "db.name": "",
                 "db.user": "default",
                 "db.query.text": "INSERT INTO test (x) VALUES",
-                "db.params": PARAMS_SERIALIZER([{"x": 100}]),
+                "db.params": '[{"x": 100}]',
                 "server.address": "localhost",
                 "server.port": 9000,
             },
@@ -468,9 +466,9 @@ def test_clickhouse_client_spans_with_pii(
                 "db.system": "clickhouse",
                 "db.name": "",
                 "db.user": "default",
-                "db.params": PARAMS_SERIALIZER({"minv": 150}),
+                "db.params": '{"minv": 150}',
                 "db.query.text": "SELECT sum(x) FROM test WHERE x > 150",
-                "db.result": str([(370,)]),
+                "db.result": "[[370]]",
                 "server.address": "localhost",
                 "server.port": 9000,
             },
@@ -622,7 +620,7 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N
                 "db.user": "default",
                 "server.address": "localhost",
                 "server.port": 9000,
-                "db.result": str([[], []]),
+                "db.result": "[[], []]",
             },
             "message": "DROP TABLE IF EXISTS test",
             "type": "default",
@@ -635,7 +633,7 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N
                 "db.user": "default",
                 "server.address": "localhost",
                 "server.port": 9000,
-                "db.result": str([[], []]),
+                "db.result": "[[], []]",
             },
             "message": "CREATE TABLE test (x Int32) ENGINE = Memory",
             "type": "default",
@@ -648,7 +646,7 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N
                 "db.user": "default",
                 "server.address": "localhost",
                 "server.port": 9000,
-                "db.params": PARAMS_SERIALIZER([{"x": 100}]),
+                "db.params": '[{"x": 100}]',
             },
             "message": "INSERT INTO test (x) VALUES",
             "type": "default",
@@ -661,7 +659,7 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N
                 "db.user": "default",
                 "server.address": "localhost",
                 "server.port": 9000,
-                "db.params": PARAMS_SERIALIZER([[170], [200]]),
+                "db.params": "[[170], [200]]",
             },
             "message": "INSERT INTO test (x) VALUES",
             "type": "default",
@@ -674,8 +672,8 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N
                 "db.user": "default",
                 "server.address": "localhost",
                 "server.port": 9000,
-                "db.params": PARAMS_SERIALIZER({"minv": 150}),
-                "db.result": str([[["370"]], [["'sum(x)'", "'Int64'"]]]),
+                "db.params": '{"minv": 150}',
+                "db.result": '[[["370"]], [["\'sum(x)\'", "\'Int64\'"]]]',
             },
             "message": "SELECT sum(x) FROM test WHERE x > 150",
             "type": "default",
@@ -869,7 +867,7 @@ def test_clickhouse_dbapi_spans_with_pii(
                 "db.name": "",
                 "db.user": "default",
                 "db.query.text": "DROP TABLE IF EXISTS test",
-                "db.result": str(([], [])),
+                "db.result": "[[], []]",
                 "server.address": "localhost",
                 "server.port": 9000,
             },
@@ -888,7 +886,7 @@ def test_clickhouse_dbapi_spans_with_pii(
                 "db.name": "",
                 "db.user": "default",
                 "db.query.text": "CREATE TABLE test (x Int32) ENGINE = Memory",
-                "db.result": str(([], [])),
+                "db.result": "[[], []]",
                 "server.address": "localhost",
                 "server.port": 9000,
             },
@@ -907,7 +905,7 @@ def test_clickhouse_dbapi_spans_with_pii(
                 "db.name": "",
                 "db.user": "default",
                 "db.query.text": "INSERT INTO test (x) VALUES",
-                "db.params": PARAMS_SERIALIZER([{"x": 100}]),
+                "db.params": '[{"x": 100}]',
                 "server.address": "localhost",
                 "server.port": 9000,
             },
@@ -926,7 +924,7 @@ def test_clickhouse_dbapi_spans_with_pii(
                 "db.name": "",
                 "db.user": "default",
                 "db.query.text": "INSERT INTO test (x) VALUES",
-                "db.params": PARAMS_SERIALIZER([[170], [200]]),
+                "db.params": "[[170], [200]]",
                 "server.address": "localhost",
                 "server.port": 9000,
             },
@@ -945,8 +943,8 @@ def test_clickhouse_dbapi_spans_with_pii(
                 "db.name": "",
                 "db.user": "default",
                 "db.query.text": "SELECT sum(x) FROM test WHERE x > 150",
-                "db.params": PARAMS_SERIALIZER({"minv": 150}),
-                "db.result": str(([(370,)], [("sum(x)", "Int64")])),
+                "db.params": '{"minv": 150}',
+                "db.result": '[[[370]], [["sum(x)", "Int64"]]]',
                 "server.address": "localhost",
                 "server.port": 9000,
             },

tests/integrations/django/test_basic.py

@@ -376,7 +376,7 @@ def test_sql_queries(sentry_init, capture_events, with_integration):
         crumb = event["breadcrumbs"]["values"][-1]
 
         assert crumb["message"] == "SELECT count(*) FROM people_person WHERE foo = %s"
-        assert crumb["data"]["db.params"] == "[123]"
+        assert crumb["data"]["db.params"] == [123]
 
 
 @pytest.mark.forked
@@ -411,7 +411,7 @@ def test_sql_dict_query_params(sentry_init, capture_events):
     assert crumb["message"] == (
         "SELECT count(*) FROM people_person WHERE foo = %(my_foo)s"
     )
-    assert crumb["data"]["db.params"] == str({"my_foo": 10})
+    assert crumb["data"]["db.params"] == '{"my_foo": 10}'
 
 
 @pytest.mark.forked
@@ -473,7 +473,7 @@ def test_sql_psycopg2_string_composition(sentry_init, capture_events, query):
     (event,) = events
     crumb = event["breadcrumbs"]["values"][-1]
     assert crumb["message"] == ('SELECT %(my_param)s FROM "foobar"')
-    assert crumb["data"]["db.params"] == str({"my_param": 10})
+    assert crumb["data"]["db.params"] == '{"my_param": 10}'
 
 
 @pytest.mark.forked
@@ -526,7 +526,7 @@ def test_sql_psycopg2_placeholders(sentry_init, capture_events):
         {
             "category": "query",
             "data": {
-                "db.params": str({"first_var": "fizz", "second_var": "not a date"}),
+                "db.params": '{"first_var": "fizz", "second_var": "not a date"}',
                 "db.paramstyle": "format",
             },
             "message": 'insert into my_test_table ("foo", "bar") values (%(first_var)s, '

tests/test_utils.py

@@ -30,6 +30,7 @@
     _get_installed_modules,
     _generate_installed_modules,
     ensure_integration_enabled,
+    _serialize_span_attribute,
 )
 
 
@@ -901,3 +902,34 @@ def test_format_timestamp_naive():
     # Ensure that some timestamp is returned, without error. We currently treat these as local time, but this is an
     # implementation detail which we should not assert here.
     assert re.fullmatch(timestamp_regex, format_timestamp(datetime_object))
+
+
+class NoStr:
+    def __str__(self):
+        1 / 0
+
+
+@pytest.mark.parametrize(
+    ("value", "result"),
+    (
+        ("meow", "meow"),
+        (1, 1),
+        (47.0, 47.0),
+        (True, True),
+        (["meow", "bark"], ["meow", "bark"]),
+        ([True, False], [True, False]),
+        ([1, 2, 3], [1, 2, 3]),
+        ([46.5, 47.0, 47.5], [46.5, 47.0, 47.5]),
+        (["meow", 47], '["meow", 47]'),  # mixed types not allowed in a list
+        (None, "null"),
+        (
+            {"cat": "meow", "dog": ["bark", "woof"]},
+            '{"cat": "meow", "dog": ["bark", "woof"]}',
+        ),
+        (datetime(2024, 1, 1), "2024-01-01 00:00:00"),
+        (("meow", "purr"), ["meow", "purr"]),
+        (NoStr(), None),
+    ),
+)
+def test_serialize_span_attribute(value, result):
+    assert _serialize_span_attribute(value) == result