diff --git a/sentry_sdk/integrations/asyncpg.py b/sentry_sdk/integrations/asyncpg.py index 71740cb3aa..c1f2557a20 100644 --- a/sentry_sdk/integrations/asyncpg.py +++ b/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 diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index 4ee18d0e54..245ea0ef71 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/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: diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index b8f7288374..28b301c397 100644 --- a/sentry_sdk/tracing_utils.py +++ b/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) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 80a5df9700..61ba34d956 100644 --- a/sentry_sdk/utils.py +++ b/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 diff --git a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py index 6378919b06..5da77ce13d 100644 --- a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py +++ b/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, }, diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index ed39dda350..8d0dbb9e32 100644 --- a/tests/integrations/django/test_basic.py +++ b/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, ' diff --git a/tests/test_utils.py b/tests/test_utils.py index 6745e2a966..5011662f05 100644 --- a/tests/test_utils.py +++ b/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