From fe3cddae84eac2f1952c285966b18980de8b69f5 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 9 Oct 2024 14:28:55 +0200 Subject: [PATCH 01/10] Fix data in clickhouse-driver integration --- sentry_sdk/integrations/clickhouse_driver.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index daf4c2257c..d76122736a 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -91,7 +91,7 @@ def _inner(*args: P.args, **kwargs: P.kwargs) -> T: _set_db_data(span, connection) - span.set_data("query", query) + span.set_data("db.query.text", query) if query_id: span.set_data("db.query_id", query_id) @@ -118,9 +118,16 @@ def _inner_end(*args: P.args, **kwargs: P.kwargs) -> T: span.set_data("db.result", res) with capture_internal_exceptions(): - span.scope.add_breadcrumb( - message=span._data.pop("query"), category="query", data=span._data - ) + query = span._get_attribute("db.query.text") + data = {} + for attr in ("db.query_id", "db.params", "db.result"): + if span._get_attribute(attr): + data[attr] = span._get_attribute(attr) + + if query: + sentry_sdk.add_breadcrumb( + message=query, category="query", data=data + ) span.finish() @@ -139,7 +146,7 @@ def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: _set_db_data(span, instance.connection) if should_send_default_pii(): - db_params = span._data.get("db.params", []) + db_params = span._get_attribute("db.params") or [] db_params.extend(data) span.set_data("db.params", db_params) From 35b7015e9a75d8f70eb4d7d3af814367c978290e Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 9 Oct 2024 15:07:16 +0200 Subject: [PATCH 02/10] add more attrs --- sentry_sdk/integrations/clickhouse_driver.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index d76122736a..89fbf99b6a 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -120,7 +120,15 @@ def _inner_end(*args: P.args, **kwargs: P.kwargs) -> T: with capture_internal_exceptions(): query = span._get_attribute("db.query.text") data = {} - for attr in ("db.query_id", "db.params", "db.result"): + for attr in ( + "db.query_id", + "db.params", + "db.result", + "db.system", + "db.user", + "server.address", + "server.port", + ): if span._get_attribute(attr): data[attr] = span._get_attribute(attr) From 66cb16ae9933c830efd6c00fb821dab989fd765e Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 9 Oct 2024 15:24:26 +0200 Subject: [PATCH 03/10] expose get_attribute --- sentry_sdk/integrations/clickhouse_driver.py | 8 ++++---- sentry_sdk/tracing.py | 20 ++++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index 89fbf99b6a..e53a5d3c61 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -118,7 +118,7 @@ def _inner_end(*args: P.args, **kwargs: P.kwargs) -> T: span.set_data("db.result", res) with capture_internal_exceptions(): - query = span._get_attribute("db.query.text") + query = span.get_attribute("db.query.text") data = {} for attr in ( "db.query_id", @@ -129,8 +129,8 @@ def _inner_end(*args: P.args, **kwargs: P.kwargs) -> T: "server.address", "server.port", ): - if span._get_attribute(attr): - data[attr] = span._get_attribute(attr) + if span.get_attribute(attr): + data[attr] = span.get_attribute(attr) if query: sentry_sdk.add_breadcrumb( @@ -154,7 +154,7 @@ def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: _set_db_data(span, instance.connection) if should_send_default_pii(): - db_params = span._get_attribute("db.params") or [] + db_params = span.get_attribute("db.params") or [] db_params.extend(data) span.set_data("db.params", db_params) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 2e560ff3d7..42a1d4d7c9 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -1277,18 +1277,12 @@ def __exit__(self, ty, value, tb): # XXX set status to error if unset and an exception occurred? context.detach(self._ctx_token) - def _get_attribute(self, name): - # type: (str) -> Optional[Any] - if not isinstance(self._otel_span, ReadableSpan): - return None - return self._otel_span.attributes.get(name) - @property def description(self): # type: () -> Optional[str] from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute - return self._get_attribute(SentrySpanAttribute.DESCRIPTION) + return self.get_attribute(SentrySpanAttribute.DESCRIPTION) @description.setter def description(self, value): @@ -1303,7 +1297,7 @@ def origin(self): # type: () -> Optional[str] from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute - return self._get_attribute(SentrySpanAttribute.ORIGIN) + return self.get_attribute(SentrySpanAttribute.ORIGIN) @origin.setter def origin(self, value): @@ -1376,7 +1370,7 @@ def op(self): # type: () -> Optional[str] from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute - return self._get_attribute(SentrySpanAttribute.OP) + return self.get_attribute(SentrySpanAttribute.OP) @op.setter def op(self, value): @@ -1391,7 +1385,7 @@ def name(self): # type: () -> Optional[str] from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute - return self._get_attribute(SentrySpanAttribute.NAME) + return self.get_attribute(SentrySpanAttribute.NAME) @name.setter def name(self, value): @@ -1507,6 +1501,12 @@ def set_data(self, key, value): # TODO-neel-potel we cannot add dicts here self.set_attribute(key, value) + def get_attribute(self, name): + # type: (str) -> Optional[Any] + if not isinstance(self._otel_span, ReadableSpan): + return None + return self._otel_span.attributes.get(name) + def set_attribute(self, key, value): # type: (str, Any) -> None self._otel_span.set_attribute(key, value) From cd22cbd92097bd7881ff617574e856d4f6599783 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 9 Oct 2024 17:06:24 +0200 Subject: [PATCH 04/10] more fixes --- MIGRATION_GUIDE.md | 1 + sentry_sdk/integrations/clickhouse_driver.py | 33 ++-- .../test_clickhouse_driver.py | 144 ++++++++++++------ 3 files changed, 118 insertions(+), 60 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index e90f63cbaf..61d5fe3cd7 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -18,6 +18,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh - A `Profile` object does not have a `.hub` property anymore. - `sentry_sdk.continue_trace` no longer returns a `Transaction` and is now a context manager. - Redis integration: In Redis pipeline spans there is no `span["data"]["redis.commands"]` that contains a dict `{"count": 3, "first_ten": ["cmd1", "cmd2", ...]}` but instead `span["data"]["redis.commands.count"]` (containing `3`) and `span["data"]["redis.commands.first_ten"]` (containing `["cmd1", "cmd2", ...]`). +- clickhouse-driver integration: The query is now available under the `db.query.text` span attribute (only if `send_default_pii` is `True`). ### Removed diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index e53a5d3c61..61be0c960f 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -91,13 +91,14 @@ def _inner(*args: P.args, **kwargs: P.kwargs) -> T: _set_db_data(span, connection) - span.set_data("db.query.text", query) + if should_send_default_pii(): + span.set_data("db.query.text", query) if query_id: span.set_data("db.query_id", query_id) if params and should_send_default_pii(): - span.set_data("db.params", params) + span.set_data("db.params", str(params)) # run the original code ret = f(*args, **kwargs) @@ -115,24 +116,24 @@ 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_data("db.result", res) + span.set_data("db.result", str(res)) with capture_internal_exceptions(): query = span.get_attribute("db.query.text") - data = {} - for attr in ( - "db.query_id", - "db.params", - "db.result", - "db.system", - "db.user", - "server.address", - "server.port", - ): - if span.get_attribute(attr): - data[attr] = span.get_attribute(attr) - if query: + data = {} + for attr in ( + "db.query_id", + "db.params", + "db.result", + "db.system", + "db.user", + "server.address", + "server.port", + ): + if span.get_attribute(attr): + data[attr] = span.get_attribute(attr) + sentry_sdk.add_breadcrumb( message=query, category="query", data=data ) diff --git a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py index 3b07a82f03..e3dc226c87 100644 --- a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py +++ b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py @@ -142,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": [], + "db.result": str([]), }, "message": "DROP TABLE IF EXISTS test", "type": "default", @@ -155,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": [], + "db.result": str([]), }, "message": "CREATE TABLE test (x Int32) ENGINE = Memory", "type": "default", @@ -168,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": [{"x": 100}], + "db.params": str([{"x": 100}]), }, "message": "INSERT INTO test (x) VALUES", "type": "default", @@ -181,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": [[170], [200]], + "db.params": str([[170], [200]]), }, "message": "INSERT INTO test (x) VALUES", "type": "default", @@ -194,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": [[370]], - "db.params": {"minv": 150}, + "db.result": str([[370]]), + "db.params": str({"minv": 150}), }, "message": "SELECT sum(x) FROM test WHERE x > 150", "type": "default", @@ -250,13 +250,15 @@ def test_clickhouse_client_spans( "origin": "auto.db.clickhouse_driver", "description": "DROP TABLE IF EXISTS test", "data": { + "sentry.name": "DROP TABLE IF EXISTS test", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", "server.address": "localhost", "server.port": 9000, }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -265,13 +267,15 @@ def test_clickhouse_client_spans( "origin": "auto.db.clickhouse_driver", "description": "CREATE TABLE test (x Int32) ENGINE = Memory", "data": { + "sentry.name": "CREATE TABLE test (x Int32) ENGINE = Memory", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", "server.address": "localhost", "server.port": 9000, }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -280,13 +284,15 @@ def test_clickhouse_client_spans( "origin": "auto.db.clickhouse_driver", "description": "INSERT INTO test (x) VALUES", "data": { + "sentry.name": "INSERT INTO test (x) VALUES", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", "server.address": "localhost", "server.port": 9000, }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -295,13 +301,15 @@ def test_clickhouse_client_spans( "origin": "auto.db.clickhouse_driver", "description": "INSERT INTO test (x) VALUES", "data": { + "sentry.name": "INSERT INTO test (x) VALUES", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", "server.address": "localhost", "server.port": 9000, }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -310,13 +318,15 @@ def test_clickhouse_client_spans( "origin": "auto.db.clickhouse_driver", "description": "SELECT sum(x) FROM test WHERE x > 150", "data": { + "sentry.name": "SELECT sum(x) FROM test WHERE x > 150", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", "server.address": "localhost", "server.port": 9000, }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -332,6 +342,8 @@ def test_clickhouse_client_spans( span.pop("span_id", None) span.pop("start_timestamp", None) span.pop("timestamp", None) + span.pop("same_process_as_parent", None) + span.pop("status", None) assert event["spans"] == expected_spans @@ -373,14 +385,17 @@ def test_clickhouse_client_spans_with_pii( "origin": "auto.db.clickhouse_driver", "description": "DROP TABLE IF EXISTS test", "data": { + "sentry.name": "DROP TABLE IF EXISTS test", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", "server.address": "localhost", "server.port": 9000, - "db.result": [], + "db.query.text": "DROP TABLE IF EXISTS test", + "db.result": str([]), }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -389,14 +404,17 @@ def test_clickhouse_client_spans_with_pii( "origin": "auto.db.clickhouse_driver", "description": "CREATE TABLE test (x Int32) ENGINE = Memory", "data": { + "sentry.name": "CREATE TABLE test (x Int32) ENGINE = Memory", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", + "db.query.text": "CREATE TABLE test (x Int32) ENGINE = Memory", + "db.result": str([]), "server.address": "localhost", "server.port": 9000, - "db.result": [], }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -405,14 +423,17 @@ def test_clickhouse_client_spans_with_pii( "origin": "auto.db.clickhouse_driver", "description": "INSERT INTO test (x) VALUES", "data": { + "sentry.name": "INSERT INTO test (x) VALUES", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", + "db.query.text": "INSERT INTO test (x) VALUES", + "db.params": str([{"x": 100}]), "server.address": "localhost", "server.port": 9000, - "db.params": [{"x": 100}], }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -421,14 +442,16 @@ def test_clickhouse_client_spans_with_pii( "origin": "auto.db.clickhouse_driver", "description": "INSERT INTO test (x) VALUES", "data": { + "sentry.name": "INSERT INTO test (x) VALUES", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", + "db.query.text": "INSERT INTO test (x) VALUES", "server.address": "localhost", "server.port": 9000, - "db.params": [[170], [200]], }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -437,15 +460,18 @@ def test_clickhouse_client_spans_with_pii( "origin": "auto.db.clickhouse_driver", "description": "SELECT sum(x) FROM test WHERE x > 150", "data": { + "sentry.name": "SELECT sum(x) FROM test WHERE x > 150", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", + "db.params": str({"minv": 150}), + "db.query.text": "SELECT sum(x) FROM test WHERE x > 150", + "db.result": str([(370,)]), "server.address": "localhost", "server.port": 9000, - "db.params": {"minv": 150}, - "db.result": [[370]], }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -461,6 +487,8 @@ def test_clickhouse_client_spans_with_pii( span.pop("span_id", None) span.pop("start_timestamp", None) span.pop("timestamp", None) + span.pop("same_process_as_parent", None) + span.pop("status", None) assert event["spans"] == expected_spans @@ -592,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": [[], []], + "db.result": str([[], []]), }, "message": "DROP TABLE IF EXISTS test", "type": "default", @@ -605,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": [[], []], + "db.result": str([[], []]), }, "message": "CREATE TABLE test (x Int32) ENGINE = Memory", "type": "default", @@ -618,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": [{"x": 100}], + "db.params": str([{"x": 100}]), }, "message": "INSERT INTO test (x) VALUES", "type": "default", @@ -631,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": [[170], [200]], + "db.params": str([[170], [200]]), }, "message": "INSERT INTO test (x) VALUES", "type": "default", @@ -644,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": {"minv": 150}, - "db.result": [[["370"]], [["'sum(x)'", "'Int64'"]]], + "db.params": str({"minv": 150}), + "db.result": str([[["370"]], [["'sum(x)'", "'Int64'"]]]), }, "message": "SELECT sum(x) FROM test WHERE x > 150", "type": "default", @@ -698,13 +726,15 @@ def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes) "origin": "auto.db.clickhouse_driver", "description": "DROP TABLE IF EXISTS test", "data": { + "sentry.name": "DROP TABLE IF EXISTS test", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", "server.address": "localhost", "server.port": 9000, }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -713,13 +743,15 @@ def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes) "origin": "auto.db.clickhouse_driver", "description": "CREATE TABLE test (x Int32) ENGINE = Memory", "data": { + "sentry.name": "CREATE TABLE test (x Int32) ENGINE = Memory", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", "server.address": "localhost", "server.port": 9000, }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -728,13 +760,15 @@ def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes) "origin": "auto.db.clickhouse_driver", "description": "INSERT INTO test (x) VALUES", "data": { + "sentry.name": "INSERT INTO test (x) VALUES", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", "server.address": "localhost", "server.port": 9000, }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -743,13 +777,15 @@ def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes) "origin": "auto.db.clickhouse_driver", "description": "INSERT INTO test (x) VALUES", "data": { + "sentry.name": "INSERT INTO test (x) VALUES", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", "server.address": "localhost", "server.port": 9000, }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -758,13 +794,15 @@ def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes) "origin": "auto.db.clickhouse_driver", "description": "SELECT sum(x) FROM test WHERE x > 150", "data": { + "sentry.name": "SELECT sum(x) FROM test WHERE x > 150", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", "server.address": "localhost", "server.port": 9000, }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -780,6 +818,7 @@ def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes) span.pop("span_id", None) span.pop("start_timestamp", None) span.pop("timestamp", None) + span.pop("status") assert event["spans"] == expected_spans @@ -821,14 +860,17 @@ def test_clickhouse_dbapi_spans_with_pii( "origin": "auto.db.clickhouse_driver", "description": "DROP TABLE IF EXISTS test", "data": { + "sentry.name": "DROP TABLE IF EXISTS test", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", + "db.query.text": "DROP TABLE IF EXISTS test", + "db.result": str(([], [])), "server.address": "localhost", "server.port": 9000, - "db.result": [[], []], }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -837,14 +879,17 @@ def test_clickhouse_dbapi_spans_with_pii( "origin": "auto.db.clickhouse_driver", "description": "CREATE TABLE test (x Int32) ENGINE = Memory", "data": { + "sentry.name": "CREATE TABLE test (x Int32) ENGINE = Memory", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", + "db.query.text": "CREATE TABLE test (x Int32) ENGINE = Memory", + "db.result": str(([], [])), "server.address": "localhost", "server.port": 9000, - "db.result": [[], []], }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -853,14 +898,17 @@ def test_clickhouse_dbapi_spans_with_pii( "origin": "auto.db.clickhouse_driver", "description": "INSERT INTO test (x) VALUES", "data": { + "sentry.name": "INSERT INTO test (x) VALUES", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", + "db.query.text": "INSERT INTO test (x) VALUES", + "db.params": str([{"x": 100}]), "server.address": "localhost", "server.port": 9000, - "db.params": [{"x": 100}], }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -869,14 +917,17 @@ def test_clickhouse_dbapi_spans_with_pii( "origin": "auto.db.clickhouse_driver", "description": "INSERT INTO test (x) VALUES", "data": { + "sentry.name": "INSERT INTO test (x) VALUES", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", + "db.query.text": "INSERT INTO test (x) VALUES", + "db.params": str([[170], [200]]), "server.address": "localhost", "server.port": 9000, - "db.params": [[170], [200]], }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -885,15 +936,18 @@ def test_clickhouse_dbapi_spans_with_pii( "origin": "auto.db.clickhouse_driver", "description": "SELECT sum(x) FROM test WHERE x > 150", "data": { + "sentry.name": "SELECT sum(x) FROM test WHERE x > 150", + "sentry.origin": "auto.db.clickhouse_driver", + "sentry.op": "db", "db.system": "clickhouse", "db.name": "", "db.user": "default", + "db.query.text": "SELECT sum(x) FROM test WHERE x > 150", + "db.params": str({"minv": 150}), + "db.result": str(([(370,)], [("sum(x)", "Int64")])), "server.address": "localhost", "server.port": 9000, - "db.params": {"minv": 150}, - "db.result": [[[370]], [["sum(x)", "Int64"]]], }, - "same_process_as_parent": True, "trace_id": transaction_trace_id, "parent_span_id": transaction_span_id, }, @@ -909,6 +963,8 @@ def test_clickhouse_dbapi_spans_with_pii( span.pop("span_id", None) span.pop("start_timestamp", None) span.pop("timestamp", None) + span.pop("same_process_as_parent", None) + span.pop("status", None) assert event["spans"] == expected_spans From b9b02e6ce8aae2ca34d41292892e385ac771737a Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 9 Oct 2024 17:19:36 +0200 Subject: [PATCH 05/10] fixes --- sentry_sdk/integrations/clickhouse_driver.py | 24 ++++++++++--------- .../test_clickhouse_driver.py | 20 +++++++++------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index 61be0c960f..425a6e3782 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -1,3 +1,5 @@ +import json + import sentry_sdk from sentry_sdk.consts import OP, SPANDATA from sentry_sdk.integrations import Integration, DidNotEnable @@ -92,13 +94,13 @@ def _inner(*args: P.args, **kwargs: P.kwargs) -> T: _set_db_data(span, connection) if should_send_default_pii(): - span.set_data("db.query.text", query) + span.set_attribute("db.query.text", query) if query_id: - span.set_data("db.query_id", query_id) + span.set_attribute("db.query_id", query_id) if params and should_send_default_pii(): - span.set_data("db.params", str(params)) + span.set_attribute("db.params", json.dumps(params)) # run the original code ret = f(*args, **kwargs) @@ -116,7 +118,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_data("db.result", str(res)) + span.set_attribute("db.result", str(res)) with capture_internal_exceptions(): query = span.get_attribute("db.query.text") @@ -155,9 +157,9 @@ def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: _set_db_data(span, instance.connection) if should_send_default_pii(): - db_params = span.get_attribute("db.params") or [] + db_params = json.loads(span.get_attribute("db.params") or "[]") db_params.extend(data) - span.set_data("db.params", db_params) + span.set_attribute("db.params", json.dumps(db_params)) return f(*args, **kwargs) @@ -167,8 +169,8 @@ def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: def _set_db_data( span: Span, connection: clickhouse_driver.connection.Connection ) -> None: - span.set_data(SPANDATA.DB_SYSTEM, "clickhouse") - span.set_data(SPANDATA.SERVER_ADDRESS, connection.host) - span.set_data(SPANDATA.SERVER_PORT, connection.port) - span.set_data(SPANDATA.DB_NAME, connection.database) - span.set_data(SPANDATA.DB_USER, connection.user) + span.set_attribute(SPANDATA.DB_SYSTEM, "clickhouse") + span.set_attribute(SPANDATA.SERVER_ADDRESS, connection.host) + span.set_attribute(SPANDATA.SERVER_PORT, connection.port) + span.set_attribute(SPANDATA.DB_NAME, connection.database) + span.set_attribute(SPANDATA.DB_USER, connection.user) diff --git a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py index e3dc226c87..c47bc3e18f 100644 --- a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py +++ b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py @@ -5,6 +5,8 @@ ``` """ +import json + import clickhouse_driver from clickhouse_driver import Client, connect @@ -181,7 +183,7 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) -> "db.user": "default", "server.address": "localhost", "server.port": 9000, - "db.params": str([[170], [200]]), + "db.params": json.dumps([[170], [200]]), }, "message": "INSERT INTO test (x) VALUES", "type": "default", @@ -430,7 +432,7 @@ def test_clickhouse_client_spans_with_pii( "db.name": "", "db.user": "default", "db.query.text": "INSERT INTO test (x) VALUES", - "db.params": str([{"x": 100}]), + "db.params": json.dumps([{"x": 100}]), "server.address": "localhost", "server.port": 9000, }, @@ -466,7 +468,7 @@ def test_clickhouse_client_spans_with_pii( "db.system": "clickhouse", "db.name": "", "db.user": "default", - "db.params": str({"minv": 150}), + "db.params": json.dumps({"minv": 150}), "db.query.text": "SELECT sum(x) FROM test WHERE x > 150", "db.result": str([(370,)]), "server.address": "localhost", @@ -646,7 +648,7 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N "db.user": "default", "server.address": "localhost", "server.port": 9000, - "db.params": str([{"x": 100}]), + "db.params": json.dumps([{"x": 100}]), }, "message": "INSERT INTO test (x) VALUES", "type": "default", @@ -659,7 +661,7 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N "db.user": "default", "server.address": "localhost", "server.port": 9000, - "db.params": str([[170], [200]]), + "db.params": json.dumps([[170], [200]]), }, "message": "INSERT INTO test (x) VALUES", "type": "default", @@ -672,7 +674,7 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N "db.user": "default", "server.address": "localhost", "server.port": 9000, - "db.params": str({"minv": 150}), + "db.params": json.dumps({"minv": 150}), "db.result": str([[["370"]], [["'sum(x)'", "'Int64'"]]]), }, "message": "SELECT sum(x) FROM test WHERE x > 150", @@ -905,7 +907,7 @@ def test_clickhouse_dbapi_spans_with_pii( "db.name": "", "db.user": "default", "db.query.text": "INSERT INTO test (x) VALUES", - "db.params": str([{"x": 100}]), + "db.params": json.dumps([{"x": 100}]), "server.address": "localhost", "server.port": 9000, }, @@ -924,7 +926,7 @@ def test_clickhouse_dbapi_spans_with_pii( "db.name": "", "db.user": "default", "db.query.text": "INSERT INTO test (x) VALUES", - "db.params": str([[170], [200]]), + "db.params": json.dumps([[170], [200]]), "server.address": "localhost", "server.port": 9000, }, @@ -943,7 +945,7 @@ 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": str({"minv": 150}), + "db.params": json.dumps({"minv": 150}), "db.result": str(([(370,)], [("sum(x)", "Int64")])), "server.address": "localhost", "server.port": 9000, From d96ec3cc1f292b33be6289ac9a97efd2b231fdf1 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Wed, 9 Oct 2024 17:22:25 +0200 Subject: [PATCH 06/10] . --- .../integrations/clickhouse_driver/test_clickhouse_driver.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py index c47bc3e18f..b81c9fe6b0 100644 --- a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py +++ b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py @@ -170,7 +170,7 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) -> "db.user": "default", "server.address": "localhost", "server.port": 9000, - "db.params": str([{"x": 100}]), + "db.params": json.dumps([{"x": 100}]), }, "message": "INSERT INTO test (x) VALUES", "type": "default", @@ -197,7 +197,7 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) -> "server.address": "localhost", "server.port": 9000, "db.result": str([[370]]), - "db.params": str({"minv": 150}), + "db.params": json.dumps({"minv": 150}), }, "message": "SELECT sum(x) FROM test WHERE x > 150", "type": "default", From 0d3185381672a522418fccb9707bb1dda1c269a4 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Thu, 10 Oct 2024 10:02:54 +0200 Subject: [PATCH 07/10] save stuff on conn --- sentry_sdk/integrations/clickhouse_driver.py | 11 ++++---- .../test_clickhouse_driver.py | 26 +++++++++---------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index 425a6e3782..a37c0cd678 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -1,5 +1,3 @@ -import json - import sentry_sdk from sentry_sdk.consts import OP, SPANDATA from sentry_sdk.integrations import Integration, DidNotEnable @@ -100,7 +98,8 @@ def _inner(*args: P.args, **kwargs: P.kwargs) -> T: span.set_attribute("db.query_id", query_id) if params and should_send_default_pii(): - span.set_attribute("db.params", json.dumps(params)) + connection._sentry_db_params = params + span.set_attribute("db.params", str(params)) # run the original code ret = f(*args, **kwargs) @@ -157,9 +156,11 @@ def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: _set_db_data(span, instance.connection) if should_send_default_pii(): - db_params = json.loads(span.get_attribute("db.params") or "[]") + db_params = ( + getattr(instance.connection, "_sentry_db_params", None) or [] + ) db_params.extend(data) - span.set_attribute("db.params", json.dumps(db_params)) + span.set_attribute("db.params", str(db_params)) return f(*args, **kwargs) diff --git a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py index b81c9fe6b0..6378919b06 100644 --- a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py +++ b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py @@ -5,8 +5,6 @@ ``` """ -import json - import clickhouse_driver from clickhouse_driver import Client, connect @@ -18,6 +16,8 @@ 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( @@ -170,7 +170,7 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) -> "db.user": "default", "server.address": "localhost", "server.port": 9000, - "db.params": json.dumps([{"x": 100}]), + "db.params": PARAMS_SERIALIZER([{"x": 100}]), }, "message": "INSERT INTO test (x) VALUES", "type": "default", @@ -183,7 +183,7 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) -> "db.user": "default", "server.address": "localhost", "server.port": 9000, - "db.params": json.dumps([[170], [200]]), + "db.params": PARAMS_SERIALIZER([[170], [200]]), }, "message": "INSERT INTO test (x) VALUES", "type": "default", @@ -197,7 +197,7 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) -> "server.address": "localhost", "server.port": 9000, "db.result": str([[370]]), - "db.params": json.dumps({"minv": 150}), + "db.params": PARAMS_SERIALIZER({"minv": 150}), }, "message": "SELECT sum(x) FROM test WHERE x > 150", "type": "default", @@ -432,7 +432,7 @@ def test_clickhouse_client_spans_with_pii( "db.name": "", "db.user": "default", "db.query.text": "INSERT INTO test (x) VALUES", - "db.params": json.dumps([{"x": 100}]), + "db.params": PARAMS_SERIALIZER([{"x": 100}]), "server.address": "localhost", "server.port": 9000, }, @@ -468,7 +468,7 @@ def test_clickhouse_client_spans_with_pii( "db.system": "clickhouse", "db.name": "", "db.user": "default", - "db.params": json.dumps({"minv": 150}), + "db.params": PARAMS_SERIALIZER({"minv": 150}), "db.query.text": "SELECT sum(x) FROM test WHERE x > 150", "db.result": str([(370,)]), "server.address": "localhost", @@ -648,7 +648,7 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N "db.user": "default", "server.address": "localhost", "server.port": 9000, - "db.params": json.dumps([{"x": 100}]), + "db.params": PARAMS_SERIALIZER([{"x": 100}]), }, "message": "INSERT INTO test (x) VALUES", "type": "default", @@ -661,7 +661,7 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N "db.user": "default", "server.address": "localhost", "server.port": 9000, - "db.params": json.dumps([[170], [200]]), + "db.params": PARAMS_SERIALIZER([[170], [200]]), }, "message": "INSERT INTO test (x) VALUES", "type": "default", @@ -674,7 +674,7 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N "db.user": "default", "server.address": "localhost", "server.port": 9000, - "db.params": json.dumps({"minv": 150}), + "db.params": PARAMS_SERIALIZER({"minv": 150}), "db.result": str([[["370"]], [["'sum(x)'", "'Int64'"]]]), }, "message": "SELECT sum(x) FROM test WHERE x > 150", @@ -907,7 +907,7 @@ def test_clickhouse_dbapi_spans_with_pii( "db.name": "", "db.user": "default", "db.query.text": "INSERT INTO test (x) VALUES", - "db.params": json.dumps([{"x": 100}]), + "db.params": PARAMS_SERIALIZER([{"x": 100}]), "server.address": "localhost", "server.port": 9000, }, @@ -926,7 +926,7 @@ def test_clickhouse_dbapi_spans_with_pii( "db.name": "", "db.user": "default", "db.query.text": "INSERT INTO test (x) VALUES", - "db.params": json.dumps([[170], [200]]), + "db.params": PARAMS_SERIALIZER([[170], [200]]), "server.address": "localhost", "server.port": 9000, }, @@ -945,7 +945,7 @@ 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": json.dumps({"minv": 150}), + "db.params": PARAMS_SERIALIZER({"minv": 150}), "db.result": str(([(370,)], [("sum(x)", "Int64")])), "server.address": "localhost", "server.port": 9000, From 64f77b2824cf3c10fcc25fbe4bec51df44496477 Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Thu, 10 Oct 2024 10:24:25 +0200 Subject: [PATCH 08/10] clean up after ourselves --- sentry_sdk/integrations/clickhouse_driver.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index a37c0cd678..a17f4e643e 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -161,6 +161,10 @@ def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: ) db_params.extend(data) span.set_attribute("db.params", str(db_params)) + try: + del instance.connection._sentry_db_params + except AttributeError: + pass return f(*args, **kwargs) From 1aab2f32abf4f8b157b7069b470c2ed651dcebaa Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Thu, 10 Oct 2024 10:39:15 +0200 Subject: [PATCH 09/10] no query id --- sentry_sdk/integrations/clickhouse_driver.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index a17f4e643e..48bff5e458 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -124,7 +124,6 @@ def _inner_end(*args: P.args, **kwargs: P.kwargs) -> T: if query: data = {} for attr in ( - "db.query_id", "db.params", "db.result", "db.system", From 5f440df8bd62877ad3e2a3e99caaa09c89201e0e Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Thu, 10 Oct 2024 11:59:28 +0200 Subject: [PATCH 10/10] use spandata --- sentry_sdk/integrations/clickhouse_driver.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index 48bff5e458..4ee18d0e54 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -126,10 +126,10 @@ def _inner_end(*args: P.args, **kwargs: P.kwargs) -> T: for attr in ( "db.params", "db.result", - "db.system", - "db.user", - "server.address", - "server.port", + SPANDATA.DB_SYSTEM, + SPANDATA.DB_USER, + SPANDATA.SERVER_ADDRESS, + SPANDATA.SERVER_PORT, ): if span.get_attribute(attr): data[attr] = span.get_attribute(attr)