From f3ad269552666c6a05ac5335535c810214026a97 Mon Sep 17 00:00:00 2001 From: Martin Imre Date: Mon, 12 Jun 2023 18:12:23 +0200 Subject: [PATCH 01/21] feat(integrations): Add integration for clickhouse-driver>=0.2.0 Adds an integration that automatically facilitates tracing/recording of all queries, their parameters, data, and results. See https://github.com/getsentry/sentry-python/issues/2154 --- sentry_sdk/integrations/clickhouse_driver.py | 103 +++++++++++++ setup.py | 1 + tests/integrations/clickhouse/__init__.py | 3 + .../clickhouse/test_clickhouse.py | 137 ++++++++++++++++++ 4 files changed, 244 insertions(+) create mode 100644 sentry_sdk/integrations/clickhouse_driver.py create mode 100644 tests/integrations/clickhouse/__init__.py create mode 100644 tests/integrations/clickhouse/test_clickhouse.py diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py new file mode 100644 index 0000000000..e2f490cf42 --- /dev/null +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -0,0 +1,103 @@ +from sentry_sdk import Hub +from sentry_sdk.consts import OP +from sentry_sdk.integrations import Integration, DidNotEnable +from sentry_sdk.utils import capture_internal_exceptions + + +try: + import clickhouse_driver + +except ImportError: + raise DidNotEnable("clickhouse-driver not installed.") + +if clickhouse_driver.VERSION < (0, 2, 0): + raise DidNotEnable("clickhouse-driver >= 0.2.0 required") + + +class ClickhouseDriverIntegration(Integration): + identifier = "clickhouse_driver" + + @staticmethod + def setup_once(): + # Every query is done using the Connection's `send_query` function + clickhouse_driver.connection.Connection.send_query = _wrap_start( + clickhouse_driver.connection.Connection.send_query + ) + + # If the query contains parameters then the send_data function is used to send those parameters to clickhouse + clickhouse_driver.client.Client.send_data = _wrap_send_data( + clickhouse_driver.client.Client.send_data + ) + + # Every query ends either with the Client's `receive_end_of_query` (no result expected) + # or its `receive_result` (result expected) + clickhouse_driver.client.Client.receive_end_of_query = _wrap_end( + clickhouse_driver.client.Client.receive_end_of_query + ) + clickhouse_driver.client.Client.receive_result = _wrap_end( + clickhouse_driver.client.Client.receive_result + ) + + +def _wrap_start(f): + def _inner(*args, **kwargs): + hub = Hub.current + if hub.get_integration(ClickhouseDriverIntegration) is None: + return + instance = args[0] + query = args[1] + query_id = args[2] if len(args) > 2 else kwargs.get("query_id") + params = args[3] if len(args) > 3 else kwargs.get("params") + + span = hub.start_span(op=OP.DB, description=query) + + instance._sentry_span = span + + span.set_data("query", query) + + if query_id: + span.set_data("db.query_id", query_id) + + if params: + span.set_data("db.params", params) + + # run the original code + ret = f(*args, **kwargs) + + return ret + + return _inner + + +def _wrap_end(f): + def _inner_end(*args, **kwargs): + res = f(*args, **kwargs) + instance = args[0] + span = instance.connection._sentry_span + + if span is not None: + if res is not None: + span.set_data("db.result", res) + with capture_internal_exceptions(): + span.hub.add_breadcrumb( + message=span._data.pop("query"), category="query", data=span._data + ) + span.finish() + + return res + + return _inner_end + + +def _wrap_send_data(f): + def _inner_send_data(*args, **kwargs): + instance = args[0] + data = args[2] + span = instance.connection._sentry_span + + db_params = span._data.get("db.params", []) + db_params.extend(data) + span.set_data("db.params", db_params) + return f(*args, **kwargs) + + return _inner_send_data diff --git a/setup.py b/setup.py index 372866fc01..cdc94eeb0d 100644 --- a/setup.py +++ b/setup.py @@ -69,6 +69,7 @@ def get_file_text(file_name): "opentelemetry": ["opentelemetry-distro>=0.35b0"], "grpcio": ["grpcio>=1.21.1"], "loguru": ["loguru>=0.5"], + "clickhouse-driver": ["clickhouse-driver>=0.2.0"], }, classifiers=[ "Development Status :: 5 - Production/Stable", diff --git a/tests/integrations/clickhouse/__init__.py b/tests/integrations/clickhouse/__init__.py new file mode 100644 index 0000000000..602c4e553c --- /dev/null +++ b/tests/integrations/clickhouse/__init__.py @@ -0,0 +1,3 @@ +import pytest + +pytest.importorskip("clickhouse_driver") diff --git a/tests/integrations/clickhouse/test_clickhouse.py b/tests/integrations/clickhouse/test_clickhouse.py new file mode 100644 index 0000000000..003b12de85 --- /dev/null +++ b/tests/integrations/clickhouse/test_clickhouse.py @@ -0,0 +1,137 @@ +""" +Tests need a local clickhouse instance running, this can best be done using +```sh +docker run -d -p 18123:8123 -p9000:9000 --name clickhouse-test --ulimit nofile=262144:262144 --rm clickhouse/clickhouse-server +``` +""" +import clickhouse_driver +from clickhouse_driver import Client, connect + +from sentry_sdk import capture_message +from sentry_sdk.integrations.clickhouse_driver import ClickhouseDriverIntegration + +EXPECT_PARAMS_IN_SELECT = True +if clickhouse_driver.VERSION < (0, 2, 6): + EXPECT_PARAMS_IN_SELECT = False + + +def test_clickhouse_client(sentry_init, capture_events) -> None: + sentry_init( + integrations=[ClickhouseDriverIntegration()], + _experiments={"record_sql_params": True}, + ) + events = capture_events() + + client = Client("localhost") + client.execute("DROP TABLE IF EXISTS test") + client.execute("CREATE TABLE test (x Int32) ENGINE = Memory") + client.execute("INSERT INTO test (x) VALUES", [{"x": 100}]) + client.execute("INSERT INTO test (x) VALUES", [[170], [200]]) + + res = client.execute("SELECT sum(x) FROM test WHERE x > %(minv)i", {"minv": 150}) + assert res[0][0] == 370 + + capture_message("hi") + + (event,) = events + + for crumb in event["breadcrumbs"]["values"]: + del crumb["timestamp"] + + assert event["breadcrumbs"]["values"] == [ + { + "category": "query", + "data": {"db.result": []}, + "message": "DROP TABLE IF EXISTS test", + "type": "default", + }, + { + "category": "query", + "data": {"db.result": []}, + "message": "CREATE TABLE test (x Int32) ENGINE = Memory", + "type": "default", + }, + { + "category": "query", + "data": {"db.params": [{"x": 100}]}, + "message": "INSERT INTO test (x) VALUES", + "type": "default", + }, + { + "category": "query", + "data": {"db.params": [[170], [200]]}, + "message": "INSERT INTO test (x) VALUES", + "type": "default", + }, + { + "category": "query", + "data": {"db.params": {"minv": 150}, "db.result": [[370]]} + if EXPECT_PARAMS_IN_SELECT + else {"db.result": [[370]]}, + "message": "SELECT sum(x) FROM test WHERE x > 150", + "type": "default", + }, + ] + + +def test_clickhouse_dbapi(sentry_init, capture_events) -> None: + sentry_init( + integrations=[ClickhouseDriverIntegration()], + ) + events = capture_events() + + conn = connect("clickhouse://localhost") + cursor = conn.cursor() + cursor.execute("DROP TABLE IF EXISTS test") + cursor.execute("CREATE TABLE test (x Int32) ENGINE = Memory") + cursor.executemany("INSERT INTO test (x) VALUES", [{"x": 100}]) + cursor.executemany("INSERT INTO test (x) VALUES", [[170], [200]]) + cursor.execute("SELECT sum(x) FROM test WHERE x > %(minv)i", {"minv": 150}) + res = cursor.fetchall() + + assert res[0][0] == 370 + + capture_message("hi") + + (event,) = events + + for crumb in event["breadcrumbs"]["values"]: + del crumb["timestamp"] + + assert event["breadcrumbs"]["values"] == [ + { + "category": "query", + "data": {"db.result": [[], []]}, + "message": "DROP TABLE IF EXISTS test", + "type": "default", + }, + { + "category": "query", + "data": {"db.result": [[], []]}, + "message": "CREATE TABLE test (x Int32) ENGINE = Memory", + "type": "default", + }, + { + "category": "query", + "data": {"db.params": [{"x": 100}]}, + "message": "INSERT INTO test (x) VALUES", + "type": "default", + }, + { + "category": "query", + "data": {"db.params": [[170], [200]]}, + "message": "INSERT INTO test (x) VALUES", + "type": "default", + }, + { + "category": "query", + "data": { + "db.params": {"minv": 150}, + "db.result": [[["370"]], [["'sum(x)'", "'Int64'"]]], + } + if EXPECT_PARAMS_IN_SELECT + else {"db.result": [[["370"]], [["'sum(x)'", "'Int64'"]]]}, + "message": "SELECT sum(x) FROM test WHERE x > 150", + "type": "default", + }, + ] From 7428b93de220c61ce93a0f0403c0dfd9ba1a2693 Mon Sep 17 00:00:00 2001 From: Martin Imre Date: Tue, 13 Jun 2023 17:10:28 +0200 Subject: [PATCH 02/21] lint(integrations): Add type hints for clickhouse-driver integration --- sentry_sdk/integrations/clickhouse_driver.py | 30 ++++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index e2f490cf42..d775b930f5 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -1,3 +1,5 @@ +from typing import ParamSpec, TypeVar, Callable + from sentry_sdk import Hub from sentry_sdk.consts import OP from sentry_sdk.integrations import Integration, DidNotEnable @@ -5,7 +7,7 @@ try: - import clickhouse_driver + import clickhouse_driver # type: ignore[import] except ImportError: raise DidNotEnable("clickhouse-driver not installed.") @@ -18,7 +20,7 @@ class ClickhouseDriverIntegration(Integration): identifier = "clickhouse_driver" @staticmethod - def setup_once(): + def setup_once() -> None: # Every query is done using the Connection's `send_query` function clickhouse_driver.connection.Connection.send_query = _wrap_start( clickhouse_driver.connection.Connection.send_query @@ -39,11 +41,15 @@ def setup_once(): ) -def _wrap_start(f): - def _inner(*args, **kwargs): +P = ParamSpec("P") +T = TypeVar("T") + + +def _wrap_start(f: Callable[P, T]) -> Callable[P, T]: + def _inner(*args: P.args, **kwargs: P.kwargs) -> T: hub = Hub.current if hub.get_integration(ClickhouseDriverIntegration) is None: - return + return f(*args, **kwargs) instance = args[0] query = args[1] query_id = args[2] if len(args) > 2 else kwargs.get("query_id") @@ -51,7 +57,7 @@ def _inner(*args, **kwargs): span = hub.start_span(op=OP.DB, description=query) - instance._sentry_span = span + instance._sentry_span = span # type: ignore[attr-defined] span.set_data("query", query) @@ -69,11 +75,11 @@ def _inner(*args, **kwargs): return _inner -def _wrap_end(f): - def _inner_end(*args, **kwargs): +def _wrap_end(f: Callable[P, T]) -> Callable[P, T]: + def _inner_end(*args: P.args, **kwargs: P.kwargs) -> T: res = f(*args, **kwargs) instance = args[0] - span = instance.connection._sentry_span + span = instance.connection._sentry_span # type: ignore[attr-defined] if span is not None: if res is not None: @@ -89,11 +95,11 @@ def _inner_end(*args, **kwargs): return _inner_end -def _wrap_send_data(f): - def _inner_send_data(*args, **kwargs): +def _wrap_send_data(f: Callable[P, T]) -> Callable[P, T]: + def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: instance = args[0] data = args[2] - span = instance.connection._sentry_span + span = instance.connection._sentry_span # type: ignore[attr-defined] db_params = span._data.get("db.params", []) db_params.extend(data) From 7f20f3747edd9b19a5697e954fb05f5da964b54f Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 12 Sep 2023 15:01:28 +0200 Subject: [PATCH 03/21] Make sure db params and result are only added to spans if send_default_pii is True. --- sentry_sdk/integrations/clickhouse_driver.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index d775b930f5..1bff3dfb6c 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -2,6 +2,7 @@ from sentry_sdk import Hub from sentry_sdk.consts import OP +from sentry_sdk.hub import _should_send_default_pii from sentry_sdk.integrations import Integration, DidNotEnable from sentry_sdk.utils import capture_internal_exceptions @@ -64,7 +65,7 @@ def _inner(*args: P.args, **kwargs: P.kwargs) -> T: if query_id: span.set_data("db.query_id", query_id) - if params: + if params and _should_send_default_pii(): span.set_data("db.params", params) # run the original code @@ -82,12 +83,14 @@ def _inner_end(*args: P.args, **kwargs: P.kwargs) -> T: span = instance.connection._sentry_span # type: ignore[attr-defined] if span is not None: - if res is not None: + if res is not None and _should_send_default_pii(): span.set_data("db.result", res) + with capture_internal_exceptions(): span.hub.add_breadcrumb( message=span._data.pop("query"), category="query", data=span._data ) + span.finish() return res @@ -101,9 +104,11 @@ def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: data = args[2] span = instance.connection._sentry_span # type: ignore[attr-defined] - db_params = span._data.get("db.params", []) - db_params.extend(data) - span.set_data("db.params", db_params) + if _should_send_default_pii(): + db_params = span._data.get("db.params", []) + db_params.extend(data) + span.set_data("db.params", db_params) + return f(*args, **kwargs) return _inner_send_data From 7d6f91f6c1b7142b88f9e010eda9bbb40ae71e81 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 12 Sep 2023 15:27:02 +0200 Subject: [PATCH 04/21] Added db data to span --- sentry_sdk/integrations/clickhouse_driver.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index 1bff3dfb6c..0048a41bb0 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -1,9 +1,10 @@ from typing import ParamSpec, TypeVar, Callable from sentry_sdk import Hub -from sentry_sdk.consts import OP +from sentry_sdk.consts import OP, SPANDATA from sentry_sdk.hub import _should_send_default_pii from sentry_sdk.integrations import Integration, DidNotEnable +from sentry_sdk.tracing import Span from sentry_sdk.utils import capture_internal_exceptions @@ -60,6 +61,8 @@ def _inner(*args: P.args, **kwargs: P.kwargs) -> T: instance._sentry_span = span # type: ignore[attr-defined] + _set_db_data(span, instance) + span.set_data("query", query) if query_id: @@ -104,6 +107,8 @@ def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: data = args[2] span = instance.connection._sentry_span # type: ignore[attr-defined] + _set_db_data(span, instance) + if _should_send_default_pii(): db_params = span._data.get("db.params", []) db_params.extend(data) @@ -112,3 +117,11 @@ def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: return f(*args, **kwargs) return _inner_send_data + + +def _set_db_data(span: Span, instance: clickhouse_driver.connection.Connection) -> None: + span.set_data(SPANDATA.DB_SYSTEM, "clickhouse") + span.set_data(SPANDATA.SERVER_ADDRESS, instance.host) + span.set_data(SPANDATA.SERVER_PORT, instance.port) + span.set_data(SPANDATA.DB_NAME, instance.database) + span.set_data(SPANDATA.DB_USER, instance.user) From 70e91448a83776f280ff4539146f0bf6c9d255dc Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 12 Sep 2023 15:41:16 +0200 Subject: [PATCH 05/21] Adding db connection data to span --- sentry_sdk/integrations/clickhouse_driver.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index 0048a41bb0..b57b020554 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -52,16 +52,16 @@ def _inner(*args: P.args, **kwargs: P.kwargs) -> T: hub = Hub.current if hub.get_integration(ClickhouseDriverIntegration) is None: return f(*args, **kwargs) - instance = args[0] + connection = args[0] query = args[1] query_id = args[2] if len(args) > 2 else kwargs.get("query_id") params = args[3] if len(args) > 3 else kwargs.get("params") span = hub.start_span(op=OP.DB, description=query) - instance._sentry_span = span # type: ignore[attr-defined] + connection._sentry_span = span # type: ignore[attr-defined] - _set_db_data(span, instance) + _set_db_data(span, connection) span.set_data("query", query) @@ -107,7 +107,7 @@ def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: data = args[2] span = instance.connection._sentry_span # type: ignore[attr-defined] - _set_db_data(span, instance) + _set_db_data(span, instance.connection) if _should_send_default_pii(): db_params = span._data.get("db.params", []) @@ -119,9 +119,11 @@ def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: return _inner_send_data -def _set_db_data(span: Span, instance: clickhouse_driver.connection.Connection) -> None: +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, instance.host) - span.set_data(SPANDATA.SERVER_PORT, instance.port) - span.set_data(SPANDATA.DB_NAME, instance.database) - span.set_data(SPANDATA.DB_USER, instance.user) + 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) From f61797e9df0a779c816fffc49dddde4b7ad09aa6 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 12 Sep 2023 15:47:05 +0200 Subject: [PATCH 06/21] Type fix --- sentry_sdk/integrations/clickhouse_driver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index b57b020554..65b81a340f 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -103,7 +103,7 @@ def _inner_end(*args: P.args, **kwargs: P.kwargs) -> T: def _wrap_send_data(f: Callable[P, T]) -> Callable[P, T]: def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: - instance = args[0] + instance = args[0] # type: clickhouse_driver.client.Client data = args[2] span = instance.connection._sentry_span # type: ignore[attr-defined] From 4b553ade846f4e2efe9081e94e92da7988ca975e Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 12 Sep 2023 15:49:35 +0200 Subject: [PATCH 07/21] Type fix --- sentry_sdk/integrations/clickhouse_driver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index 65b81a340f..28e364256b 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -105,7 +105,7 @@ def _wrap_send_data(f: Callable[P, T]) -> Callable[P, T]: def _inner_send_data(*args: P.args, **kwargs: P.kwargs) -> T: instance = args[0] # type: clickhouse_driver.client.Client data = args[2] - span = instance.connection._sentry_span # type: ignore[attr-defined] + span = instance.connection._sentry_span _set_db_data(span, instance.connection) From e20c50cd5d71551c60147c1474b2f9e8f51bb08c Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 12 Sep 2023 16:02:24 +0200 Subject: [PATCH 08/21] Renamed test file for consistency (did not rename the directory because maybe we add some other clickhouse integrations in the future --- .../clickhouse/{test_clickhouse.py => test_clickhouse_driver.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/integrations/clickhouse/{test_clickhouse.py => test_clickhouse_driver.py} (100%) diff --git a/tests/integrations/clickhouse/test_clickhouse.py b/tests/integrations/clickhouse/test_clickhouse_driver.py similarity index 100% rename from tests/integrations/clickhouse/test_clickhouse.py rename to tests/integrations/clickhouse/test_clickhouse_driver.py From b4236482ef635c2f1eef233d4f5898c087a1d143 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 12 Sep 2023 16:09:29 +0200 Subject: [PATCH 09/21] Added clickhouse driver to the test matrix --- .../test-integration-clickhouse_driver.yml | 83 +++++++++++++++++++ tox.ini | 10 +++ 2 files changed, 93 insertions(+) create mode 100644 .github/workflows/test-integration-clickhouse_driver.yml diff --git a/.github/workflows/test-integration-clickhouse_driver.yml b/.github/workflows/test-integration-clickhouse_driver.yml new file mode 100644 index 0000000000..e3bb869095 --- /dev/null +++ b/.github/workflows/test-integration-clickhouse_driver.yml @@ -0,0 +1,83 @@ +name: Test clickhouse_driver + +on: + push: + branches: + - master + - release/** + + pull_request: + +# Cancel in progress workflows on pull_requests. +# https://docs.github.com/en/actions/using-jobs/using-concurrency#example-using-a-fallback-value +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +permissions: + contents: read + +env: + BUILD_CACHE_KEY: ${{ github.sha }} + CACHED_BUILD_PATHS: | + ${{ github.workspace }}/dist-serverless + +jobs: + test: + name: clickhouse_driver, python ${{ matrix.python-version }}, ${{ matrix.os }} + runs-on: ${{ matrix.os }} + timeout-minutes: 30 + + strategy: + fail-fast: false + matrix: + python-version: ["3.8","3.9","3.10","3.11"] + # python3.6 reached EOL and is no longer being supported on + # new versions of hosted runners on Github Actions + # ubuntu-20.04 is the last version that supported python3.6 + # see https://github.com/actions/setup-python/issues/544#issuecomment-1332535877 + os: [ubuntu-20.04] + + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + + - name: Setup Test Env + run: | + pip install coverage "tox>=3,<4" + + - name: Test clickhouse_driver + uses: nick-fields/retry@v2 + with: + timeout_minutes: 15 + max_attempts: 2 + retry_wait_seconds: 5 + shell: bash + command: | + set -x # print commands that are executed + coverage erase + + # Run tests + ./scripts/runtox.sh "py${{ matrix.python-version }}-clickhouse_driver" --cov=tests --cov=sentry_sdk --cov-report= --cov-branch && + coverage combine .coverage* && + coverage xml -i + + - uses: codecov/codecov-action@v3 + with: + token: ${{ secrets.CODECOV_TOKEN }} + files: coverage.xml + + + check_required_tests: + name: All clickhouse_driver tests passed or skipped + needs: test + # Always run this, even if a dependent job failed + if: always() + runs-on: ubuntu-20.04 + steps: + - name: Check for failures + if: contains(needs.test.result, 'failure') + run: | + echo "One of the dependent jobs has failed. You may need to re-run it." && exit 1 diff --git a/tox.ini b/tox.ini index fd9a0ca5a4..083942361b 100644 --- a/tox.ini +++ b/tox.ini @@ -55,6 +55,9 @@ envlist = # Chalice {py3.6,py3.7,py3.8}-chalice-v{1.18,1.20,1.22,1.24} + # Clickhouse Driver + {py3.8,py3.9,py3.10,py3.11}-clickhouse_driver-v{0.2.1,0.2.4,0.2.5,0.2.6} + # Cloud Resource Context {py3.6,py3.7,py3.8,py3.9,py3.10,py3.11}-cloud_resource_context @@ -248,6 +251,12 @@ deps = {py3.7}-chalice: botocore~=1.31 {py3.8}-chalice: botocore~=1.31 + # Clickhouse Driver + clickhouse_driver-v0.2.1: clickhouse_driver>=0.2.1,<0.2.2 + clickhouse_driver-v0.2.4: clickhouse_driver>=0.2.4,<0.2.5 + clickhouse_driver-v0.2.5: clickhouse_driver>=0.2.5,<0.2.6 + clickhouse_driver-v0.2.6: clickhouse_driver>=0.2.6,<0.2.7 + # Django django: psycopg2-binary django: Werkzeug<2.1.0 @@ -474,6 +483,7 @@ setenv = bottle: TESTPATH=tests/integrations/bottle celery: TESTPATH=tests/integrations/celery chalice: TESTPATH=tests/integrations/chalice + clickhouse_driver: TESTPATH=tests/integrations/clickhouse_driver cloud_resource_context: TESTPATH=tests/integrations/cloud_resource_context django: TESTPATH=tests/integrations/django falcon: TESTPATH=tests/integrations/falcon From 81c0c4197d70124c422bd66cb71faefb3df70173 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 12 Sep 2023 16:31:47 +0200 Subject: [PATCH 10/21] Fixed testpath --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 083942361b..b2a2dbcb79 100644 --- a/tox.ini +++ b/tox.ini @@ -483,7 +483,7 @@ setenv = bottle: TESTPATH=tests/integrations/bottle celery: TESTPATH=tests/integrations/celery chalice: TESTPATH=tests/integrations/chalice - clickhouse_driver: TESTPATH=tests/integrations/clickhouse_driver + clickhouse_driver: TESTPATH=tests/integrations/clickhouse cloud_resource_context: TESTPATH=tests/integrations/cloud_resource_context django: TESTPATH=tests/integrations/django falcon: TESTPATH=tests/integrations/falcon From 6f76024ba109a0a9a84e73f197810d64d3d21d90 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 13 Sep 2023 09:07:08 +0200 Subject: [PATCH 11/21] Trying to run ClickHouse in CI --- .github/workflows/test-integration-clickhouse_driver.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/test-integration-clickhouse_driver.yml b/.github/workflows/test-integration-clickhouse_driver.yml index e3bb869095..49b26e1803 100644 --- a/.github/workflows/test-integration-clickhouse_driver.yml +++ b/.github/workflows/test-integration-clickhouse_driver.yml @@ -44,6 +44,8 @@ jobs: with: python-version: ${{ matrix.python-version }} + - uses: getsentry/action-clickhouse-in-ci@v1 + - name: Setup Test Env run: | pip install coverage "tox>=3,<4" From 9200c4a284896728e03c50913fea188576e6daa4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 13 Sep 2023 10:17:59 +0200 Subject: [PATCH 12/21] More tests --- .../clickhouse/test_clickhouse_driver.py | 725 +++++++++++++++++- 1 file changed, 708 insertions(+), 17 deletions(-) diff --git a/tests/integrations/clickhouse/test_clickhouse_driver.py b/tests/integrations/clickhouse/test_clickhouse_driver.py index 003b12de85..583baf82ed 100644 --- a/tests/integrations/clickhouse/test_clickhouse_driver.py +++ b/tests/integrations/clickhouse/test_clickhouse_driver.py @@ -7,7 +7,7 @@ import clickhouse_driver from clickhouse_driver import Client, connect -from sentry_sdk import capture_message +from sentry_sdk import start_transaction, capture_message from sentry_sdk.integrations.clickhouse_driver import ClickhouseDriverIntegration EXPECT_PARAMS_IN_SELECT = True @@ -15,7 +15,7 @@ EXPECT_PARAMS_IN_SELECT = False -def test_clickhouse_client(sentry_init, capture_events) -> None: +def test_clickhouse_client_breadcrumbs(sentry_init, capture_events) -> None: sentry_init( integrations=[ClickhouseDriverIntegration()], _experiments={"record_sql_params": True}, @@ -41,40 +41,388 @@ def test_clickhouse_client(sentry_init, capture_events) -> None: assert event["breadcrumbs"]["values"] == [ { "category": "query", - "data": {"db.result": []}, + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + }, "message": "DROP TABLE IF EXISTS test", "type": "default", }, { "category": "query", - "data": {"db.result": []}, + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + }, "message": "CREATE TABLE test (x Int32) ENGINE = Memory", "type": "default", }, { "category": "query", - "data": {"db.params": [{"x": 100}]}, + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + }, "message": "INSERT INTO test (x) VALUES", "type": "default", }, { "category": "query", - "data": {"db.params": [[170], [200]]}, + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + }, "message": "INSERT INTO test (x) VALUES", "type": "default", }, { "category": "query", - "data": {"db.params": {"minv": 150}, "db.result": [[370]]} - if EXPECT_PARAMS_IN_SELECT - else {"db.result": [[370]]}, + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + }, "message": "SELECT sum(x) FROM test WHERE x > 150", "type": "default", }, ] -def test_clickhouse_dbapi(sentry_init, capture_events) -> None: +def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) -> None: + sentry_init( + integrations=[ClickhouseDriverIntegration()], + send_default_pii=True, + _experiments={"record_sql_params": True}, + ) + events = capture_events() + + client = Client("localhost") + client.execute("DROP TABLE IF EXISTS test") + client.execute("CREATE TABLE test (x Int32) ENGINE = Memory") + client.execute("INSERT INTO test (x) VALUES", [{"x": 100}]) + client.execute("INSERT INTO test (x) VALUES", [[170], [200]]) + + res = client.execute("SELECT sum(x) FROM test WHERE x > %(minv)i", {"minv": 150}) + assert res[0][0] == 370 + + capture_message("hi") + + (event,) = events + + for crumb in event["breadcrumbs"]["values"]: + del crumb["timestamp"] + + assert event["breadcrumbs"]["values"] == [ + { + "category": "query", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + "db.result": [], + }, + "message": "DROP TABLE IF EXISTS test", + "type": "default", + }, + { + "category": "query", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + "db.result": [], + }, + "message": "CREATE TABLE test (x Int32) ENGINE = Memory", + "type": "default", + }, + { + "category": "query", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + "db.params": [{"x": 100}], + }, + "message": "INSERT INTO test (x) VALUES", + "type": "default", + }, + { + "category": "query", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + "db.params": [[170], [200]], + }, + "message": "INSERT INTO test (x) VALUES", + "type": "default", + }, + { + "category": "query", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + "db.result": [[370]], + "db.params": {"minv": 150}, + }, + # if EXPECT_PARAMS_IN_SELECT + "message": "SELECT sum(x) FROM test WHERE x > 150", + "type": "default", + }, + ] + + +def test_clickhouse_client_spans( + sentry_init, capture_events, capture_envelopes +) -> None: + sentry_init( + integrations=[ClickhouseDriverIntegration()], + _experiments={"record_sql_params": True}, + traces_sample_rate=1.0, + ) + events = capture_events() + + transaction_trace_id = None + transaction_span_id = None + + with start_transaction(name="test_clickhouse_transaction") as transaction: + transaction_trace_id = transaction.trace_id + transaction_span_id = transaction.span_id + + client = Client("localhost") + client.execute("DROP TABLE IF EXISTS test") + client.execute("CREATE TABLE test (x Int32) ENGINE = Memory") + client.execute("INSERT INTO test (x) VALUES", [{"x": 100}]) + client.execute("INSERT INTO test (x) VALUES", [[170], [200]]) + + res = client.execute( + "SELECT sum(x) FROM test WHERE x > %(minv)i", {"minv": 150} + ) + assert res[0][0] == 370 + + (event,) = events + + for span in event["spans"]: + del span["span_id"] + del span["start_timestamp"] + del span["timestamp"] + + assert event["spans"] == [ + { + "op": "db", + "description": "DROP TABLE IF EXISTS test", + "data": { + "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, + }, + { + "op": "db", + "description": "CREATE TABLE test (x Int32) ENGINE = Memory", + "data": { + "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, + }, + { + "op": "db", + "description": "INSERT INTO test (x) VALUES", + "data": { + "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, + }, + { + "op": "db", + "description": "INSERT INTO test (x) VALUES", + "data": { + "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, + }, + { + "op": "db", + "description": "SELECT sum(x) FROM test WHERE x > 150", + "data": { + "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, + }, + ] + + +def test_clickhouse_client_spans_with_pii( + sentry_init, capture_events, capture_envelopes +) -> None: + sentry_init( + integrations=[ClickhouseDriverIntegration()], + _experiments={"record_sql_params": True}, + traces_sample_rate=1.0, + send_default_pii=True, + ) + events = capture_events() + + transaction_trace_id = None + transaction_span_id = None + + with start_transaction(name="test_clickhouse_transaction") as transaction: + transaction_trace_id = transaction.trace_id + transaction_span_id = transaction.span_id + + client = Client("localhost") + client.execute("DROP TABLE IF EXISTS test") + client.execute("CREATE TABLE test (x Int32) ENGINE = Memory") + client.execute("INSERT INTO test (x) VALUES", [{"x": 100}]) + client.execute("INSERT INTO test (x) VALUES", [[170], [200]]) + + res = client.execute( + "SELECT sum(x) FROM test WHERE x > %(minv)i", {"minv": 150} + ) + assert res[0][0] == 370 + + (event,) = events + + for span in event["spans"]: + del span["span_id"] + del span["start_timestamp"] + del span["timestamp"] + + assert event["spans"] == [ + { + "op": "db", + "description": "DROP TABLE IF EXISTS test", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + "db.result": [], + }, + "same_process_as_parent": True, + "trace_id": transaction_trace_id, + "parent_span_id": transaction_span_id, + }, + { + "op": "db", + "description": "CREATE TABLE test (x Int32) ENGINE = Memory", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + "db.result": [], + }, + "same_process_as_parent": True, + "trace_id": transaction_trace_id, + "parent_span_id": transaction_span_id, + }, + { + "op": "db", + "description": "INSERT INTO test (x) VALUES", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "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, + }, + { + "op": "db", + "description": "INSERT INTO test (x) VALUES", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "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, + }, + { + "op": "db", + "description": "SELECT sum(x) FROM test WHERE x > 150", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "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, + }, + ] + + +def test_clickhouse_dbapi_breadcrumbs(sentry_init, capture_events) -> None: sentry_init( integrations=[ClickhouseDriverIntegration()], ) @@ -101,37 +449,380 @@ def test_clickhouse_dbapi(sentry_init, capture_events) -> None: assert event["breadcrumbs"]["values"] == [ { "category": "query", - "data": {"db.result": [[], []]}, + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + }, + "message": "DROP TABLE IF EXISTS test", + "type": "default", + }, + { + "category": "query", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + }, + "message": "CREATE TABLE test (x Int32) ENGINE = Memory", + "type": "default", + }, + { + "category": "query", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + }, + "message": "INSERT INTO test (x) VALUES", + "type": "default", + }, + { + "category": "query", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + }, + "message": "INSERT INTO test (x) VALUES", + "type": "default", + }, + { + "category": "query", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + }, + "message": "SELECT sum(x) FROM test WHERE x > 150", + "type": "default", + }, + ] + + +def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> None: + sentry_init( + integrations=[ClickhouseDriverIntegration()], + send_default_pii=True, + ) + events = capture_events() + + conn = connect("clickhouse://localhost") + cursor = conn.cursor() + cursor.execute("DROP TABLE IF EXISTS test") + cursor.execute("CREATE TABLE test (x Int32) ENGINE = Memory") + cursor.executemany("INSERT INTO test (x) VALUES", [{"x": 100}]) + cursor.executemany("INSERT INTO test (x) VALUES", [[170], [200]]) + cursor.execute("SELECT sum(x) FROM test WHERE x > %(minv)i", {"minv": 150}) + res = cursor.fetchall() + + assert res[0][0] == 370 + + capture_message("hi") + + (event,) = events + + for crumb in event["breadcrumbs"]["values"]: + del crumb["timestamp"] + + assert event["breadcrumbs"]["values"] == [ + { + "category": "query", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + "db.result": [[], []], + }, "message": "DROP TABLE IF EXISTS test", "type": "default", }, { "category": "query", - "data": {"db.result": [[], []]}, + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + "db.result": [[], []], + }, "message": "CREATE TABLE test (x Int32) ENGINE = Memory", "type": "default", }, { "category": "query", - "data": {"db.params": [{"x": 100}]}, + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + "db.params": [{"x": 100}], + }, "message": "INSERT INTO test (x) VALUES", "type": "default", }, { "category": "query", - "data": {"db.params": [[170], [200]]}, + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + "db.params": [[170], [200]], + }, "message": "INSERT INTO test (x) VALUES", "type": "default", }, { "category": "query", "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, "db.params": {"minv": 150}, "db.result": [[["370"]], [["'sum(x)'", "'Int64'"]]], - } - if EXPECT_PARAMS_IN_SELECT - else {"db.result": [[["370"]], [["'sum(x)'", "'Int64'"]]]}, + }, "message": "SELECT sum(x) FROM test WHERE x > 150", "type": "default", }, ] + + +def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes) -> None: + sentry_init( + integrations=[ClickhouseDriverIntegration()], + _experiments={"record_sql_params": True}, + traces_sample_rate=1.0, + ) + events = capture_events() + + transaction_trace_id = None + transaction_span_id = None + + with start_transaction(name="test_clickhouse_transaction") as transaction: + transaction_trace_id = transaction.trace_id + transaction_span_id = transaction.span_id + + conn = connect("clickhouse://localhost") + cursor = conn.cursor() + cursor.execute("DROP TABLE IF EXISTS test") + cursor.execute("CREATE TABLE test (x Int32) ENGINE = Memory") + cursor.executemany("INSERT INTO test (x) VALUES", [{"x": 100}]) + cursor.executemany("INSERT INTO test (x) VALUES", [[170], [200]]) + cursor.execute("SELECT sum(x) FROM test WHERE x > %(minv)i", {"minv": 150}) + res = cursor.fetchall() + + assert res[0][0] == 370 + + (event,) = events + + for span in event["spans"]: + del span["span_id"] + del span["start_timestamp"] + del span["timestamp"] + + assert event["spans"] == [ + { + "op": "db", + "description": "DROP TABLE IF EXISTS test", + "data": { + "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, + }, + { + "op": "db", + "description": "CREATE TABLE test (x Int32) ENGINE = Memory", + "data": { + "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, + }, + { + "op": "db", + "description": "INSERT INTO test (x) VALUES", + "data": { + "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, + }, + { + "op": "db", + "description": "INSERT INTO test (x) VALUES", + "data": { + "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, + }, + { + "op": "db", + "description": "SELECT sum(x) FROM test WHERE x > 150", + "data": { + "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, + }, + ] + + +def test_clickhouse_dbapi_spans_with_pii( + sentry_init, capture_events, capture_envelopes +) -> None: + sentry_init( + integrations=[ClickhouseDriverIntegration()], + _experiments={"record_sql_params": True}, + traces_sample_rate=1.0, + send_default_pii=True, + ) + events = capture_events() + + transaction_trace_id = None + transaction_span_id = None + + with start_transaction(name="test_clickhouse_transaction") as transaction: + transaction_trace_id = transaction.trace_id + transaction_span_id = transaction.span_id + + conn = connect("clickhouse://localhost") + cursor = conn.cursor() + cursor.execute("DROP TABLE IF EXISTS test") + cursor.execute("CREATE TABLE test (x Int32) ENGINE = Memory") + cursor.executemany("INSERT INTO test (x) VALUES", [{"x": 100}]) + cursor.executemany("INSERT INTO test (x) VALUES", [[170], [200]]) + cursor.execute("SELECT sum(x) FROM test WHERE x > %(minv)i", {"minv": 150}) + res = cursor.fetchall() + + assert res[0][0] == 370 + + (event,) = events + + for span in event["spans"]: + del span["span_id"] + del span["start_timestamp"] + del span["timestamp"] + + assert event["spans"] == [ + { + "op": "db", + "description": "DROP TABLE IF EXISTS test", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + "db.result": [[], []], + }, + "same_process_as_parent": True, + "trace_id": transaction_trace_id, + "parent_span_id": transaction_span_id, + }, + { + "op": "db", + "description": "CREATE TABLE test (x Int32) ENGINE = Memory", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "server.address": "localhost", + "server.port": 9000, + "db.result": [[], []], + }, + "same_process_as_parent": True, + "trace_id": transaction_trace_id, + "parent_span_id": transaction_span_id, + }, + { + "op": "db", + "description": "INSERT INTO test (x) VALUES", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "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, + }, + { + "op": "db", + "description": "INSERT INTO test (x) VALUES", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "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, + }, + { + "op": "db", + "description": "SELECT sum(x) FROM test WHERE x > 150", + "data": { + "db.system": "clickhouse", + "db.name": "", + "db.user": "default", + "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, + }, + ] From 8e0eb78143b02715db31eaf4858e17a335647eee Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 13 Sep 2023 11:13:51 +0200 Subject: [PATCH 13/21] Refactoring --- .../clickhouse/test_clickhouse_driver.py | 97 +++++++++++-------- 1 file changed, 56 insertions(+), 41 deletions(-) diff --git a/tests/integrations/clickhouse/test_clickhouse_driver.py b/tests/integrations/clickhouse/test_clickhouse_driver.py index 583baf82ed..cd170d3f73 100644 --- a/tests/integrations/clickhouse/test_clickhouse_driver.py +++ b/tests/integrations/clickhouse/test_clickhouse_driver.py @@ -35,10 +35,7 @@ def test_clickhouse_client_breadcrumbs(sentry_init, capture_events) -> None: (event,) = events - for crumb in event["breadcrumbs"]["values"]: - del crumb["timestamp"] - - assert event["breadcrumbs"]["values"] == [ + expected_breadcrumbs = [ { "category": "query", "data": { @@ -101,6 +98,11 @@ def test_clickhouse_client_breadcrumbs(sentry_init, capture_events) -> None: }, ] + for crumb in event["breadcrumbs"]["values"]: + del crumb["timestamp"] + + assert event["breadcrumbs"]["values"] == expected_breadcrumbs + def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) -> None: sentry_init( @@ -123,10 +125,7 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) -> (event,) = events - for crumb in event["breadcrumbs"]["values"]: - del crumb["timestamp"] - - assert event["breadcrumbs"]["values"] == [ + expected_breadcrumbs = [ { "category": "query", "data": { @@ -190,12 +189,16 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) -> "db.result": [[370]], "db.params": {"minv": 150}, }, - # if EXPECT_PARAMS_IN_SELECT "message": "SELECT sum(x) FROM test WHERE x > 150", "type": "default", }, ] + for crumb in event["breadcrumbs"]["values"]: + del crumb["timestamp"] + + assert event["breadcrumbs"]["values"] == expected_breadcrumbs + def test_clickhouse_client_spans( sentry_init, capture_events, capture_envelopes @@ -227,12 +230,7 @@ def test_clickhouse_client_spans( (event,) = events - for span in event["spans"]: - del span["span_id"] - del span["start_timestamp"] - del span["timestamp"] - - assert event["spans"] == [ + expected_spans = [ { "op": "db", "description": "DROP TABLE IF EXISTS test", @@ -305,6 +303,13 @@ def test_clickhouse_client_spans( }, ] + for span in event["spans"]: + del span["span_id"] + del span["start_timestamp"] + del span["timestamp"] + + assert event["spans"] == expected_spans + def test_clickhouse_client_spans_with_pii( sentry_init, capture_events, capture_envelopes @@ -337,12 +342,7 @@ def test_clickhouse_client_spans_with_pii( (event,) = events - for span in event["spans"]: - del span["span_id"] - del span["start_timestamp"] - del span["timestamp"] - - assert event["spans"] == [ + expected_spans = [ { "op": "db", "description": "DROP TABLE IF EXISTS test", @@ -421,6 +421,13 @@ def test_clickhouse_client_spans_with_pii( }, ] + for span in event["spans"]: + del span["span_id"] + del span["start_timestamp"] + del span["timestamp"] + + assert event["spans"] == expected_spans + def test_clickhouse_dbapi_breadcrumbs(sentry_init, capture_events) -> None: sentry_init( @@ -443,10 +450,7 @@ def test_clickhouse_dbapi_breadcrumbs(sentry_init, capture_events) -> None: (event,) = events - for crumb in event["breadcrumbs"]["values"]: - del crumb["timestamp"] - - assert event["breadcrumbs"]["values"] == [ + expected_breadcrumbs = [ { "category": "query", "data": { @@ -509,6 +513,11 @@ def test_clickhouse_dbapi_breadcrumbs(sentry_init, capture_events) -> None: }, ] + for crumb in event["breadcrumbs"]["values"]: + del crumb["timestamp"] + + assert event["breadcrumbs"]["values"] == expected_breadcrumbs + def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> None: sentry_init( @@ -532,10 +541,7 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N (event,) = events - for crumb in event["breadcrumbs"]["values"]: - del crumb["timestamp"] - - assert event["breadcrumbs"]["values"] == [ + expected_breadcrumbs = [ { "category": "query", "data": { @@ -604,6 +610,11 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N }, ] + for crumb in event["breadcrumbs"]["values"]: + del crumb["timestamp"] + + assert event["breadcrumbs"]["values"] == expected_breadcrumbs + def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes) -> None: sentry_init( @@ -633,12 +644,7 @@ def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes) (event,) = events - for span in event["spans"]: - del span["span_id"] - del span["start_timestamp"] - del span["timestamp"] - - assert event["spans"] == [ + expected_spans = [ { "op": "db", "description": "DROP TABLE IF EXISTS test", @@ -711,6 +717,13 @@ def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes) }, ] + for span in event["spans"]: + del span["span_id"] + del span["start_timestamp"] + del span["timestamp"] + + assert event["spans"] == expected_spans + def test_clickhouse_dbapi_spans_with_pii( sentry_init, capture_events, capture_envelopes @@ -743,12 +756,7 @@ def test_clickhouse_dbapi_spans_with_pii( (event,) = events - for span in event["spans"]: - del span["span_id"] - del span["start_timestamp"] - del span["timestamp"] - - assert event["spans"] == [ + expected_spans = [ { "op": "db", "description": "DROP TABLE IF EXISTS test", @@ -826,3 +834,10 @@ def test_clickhouse_dbapi_spans_with_pii( "parent_span_id": transaction_span_id, }, ] + + for span in event["spans"]: + del span["span_id"] + del span["start_timestamp"] + del span["timestamp"] + + assert event["spans"] == expected_spans From b902d9627ba8815465a5977c91cf920d276cfda1 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 13 Sep 2023 11:17:24 +0200 Subject: [PATCH 14/21] Refactoring --- .../clickhouse/test_clickhouse_driver.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/integrations/clickhouse/test_clickhouse_driver.py b/tests/integrations/clickhouse/test_clickhouse_driver.py index cd170d3f73..c367e748fb 100644 --- a/tests/integrations/clickhouse/test_clickhouse_driver.py +++ b/tests/integrations/clickhouse/test_clickhouse_driver.py @@ -194,6 +194,10 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) -> }, ] + if not EXPECT_PARAMS_IN_SELECT: + for crumb in expected_breadcrumbs: + del crumb["data"]["db.params"] + for crumb in event["breadcrumbs"]["values"]: del crumb["timestamp"] @@ -421,6 +425,10 @@ def test_clickhouse_client_spans_with_pii( }, ] + if not EXPECT_PARAMS_IN_SELECT: + for span in expected_spans: + del span["data"]["db.params"] + for span in event["spans"]: del span["span_id"] del span["start_timestamp"] @@ -610,6 +618,10 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N }, ] + if not EXPECT_PARAMS_IN_SELECT: + for crumb in expected_breadcrumbs: + del crumb["data"]["db.params"] + for crumb in event["breadcrumbs"]["values"]: del crumb["timestamp"] @@ -835,6 +847,10 @@ def test_clickhouse_dbapi_spans_with_pii( }, ] + if not EXPECT_PARAMS_IN_SELECT: + for span in expected_spans: + del span["data"]["db.params"] + for span in event["spans"]: del span["span_id"] del span["start_timestamp"] From 301eb70beac8f87700b3815aa3b8d293c1c4b391 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 13 Sep 2023 11:28:21 +0200 Subject: [PATCH 15/21] Fixed tests for older Clickhouses --- .../clickhouse/test_clickhouse_driver.py | 56 +++++++++++-------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/tests/integrations/clickhouse/test_clickhouse_driver.py b/tests/integrations/clickhouse/test_clickhouse_driver.py index c367e748fb..6b0fa566d4 100644 --- a/tests/integrations/clickhouse/test_clickhouse_driver.py +++ b/tests/integrations/clickhouse/test_clickhouse_driver.py @@ -98,8 +98,11 @@ def test_clickhouse_client_breadcrumbs(sentry_init, capture_events) -> None: }, ] + if not EXPECT_PARAMS_IN_SELECT: + expected_breadcrumbs[-1]["data"].pop("db.params", None) + for crumb in event["breadcrumbs"]["values"]: - del crumb["timestamp"] + crumb.pop("timestamp", None) assert event["breadcrumbs"]["values"] == expected_breadcrumbs @@ -195,11 +198,10 @@ def test_clickhouse_client_breadcrumbs_with_pii(sentry_init, capture_events) -> ] if not EXPECT_PARAMS_IN_SELECT: - for crumb in expected_breadcrumbs: - del crumb["data"]["db.params"] + expected_breadcrumbs[-1]["data"].pop("db.params", None) for crumb in event["breadcrumbs"]["values"]: - del crumb["timestamp"] + crumb.pop("timestamp", None) assert event["breadcrumbs"]["values"] == expected_breadcrumbs @@ -307,10 +309,13 @@ def test_clickhouse_client_spans( }, ] + if not EXPECT_PARAMS_IN_SELECT: + expected_spans[-1]["data"].pop("db.params", None) + for span in event["spans"]: - del span["span_id"] - del span["start_timestamp"] - del span["timestamp"] + span.pop("span_id", None) + span.pop("start_timestamp", None) + span.pop("timestamp", None) assert event["spans"] == expected_spans @@ -426,13 +431,12 @@ def test_clickhouse_client_spans_with_pii( ] if not EXPECT_PARAMS_IN_SELECT: - for span in expected_spans: - del span["data"]["db.params"] + expected_spans[-1]["data"].pop("db.params", None) for span in event["spans"]: - del span["span_id"] - del span["start_timestamp"] - del span["timestamp"] + span.pop("span_id", None) + span.pop("start_timestamp", None) + span.pop("timestamp", None) assert event["spans"] == expected_spans @@ -521,8 +525,11 @@ def test_clickhouse_dbapi_breadcrumbs(sentry_init, capture_events) -> None: }, ] + if not EXPECT_PARAMS_IN_SELECT: + expected_breadcrumbs[-1]["data"].pop("db.params", None) + for crumb in event["breadcrumbs"]["values"]: - del crumb["timestamp"] + crumb.pop("timestamp", None) assert event["breadcrumbs"]["values"] == expected_breadcrumbs @@ -619,11 +626,10 @@ def test_clickhouse_dbapi_breadcrumbs_with_pii(sentry_init, capture_events) -> N ] if not EXPECT_PARAMS_IN_SELECT: - for crumb in expected_breadcrumbs: - del crumb["data"]["db.params"] + expected_breadcrumbs[-1]["data"].pop("db.params", None) for crumb in event["breadcrumbs"]["values"]: - del crumb["timestamp"] + crumb.pop("timestamp", None) assert event["breadcrumbs"]["values"] == expected_breadcrumbs @@ -729,10 +735,13 @@ def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes) }, ] + if not EXPECT_PARAMS_IN_SELECT: + expected_spans[-1]["data"].pop("db.params", None) + for span in event["spans"]: - del span["span_id"] - del span["start_timestamp"] - del span["timestamp"] + span.pop("span_id", None) + span.pop("start_timestamp", None) + span.pop("timestamp", None) assert event["spans"] == expected_spans @@ -848,12 +857,11 @@ def test_clickhouse_dbapi_spans_with_pii( ] if not EXPECT_PARAMS_IN_SELECT: - for span in expected_spans: - del span["data"]["db.params"] + expected_spans[-1]["data"].pop("db.params", None) for span in event["spans"]: - del span["span_id"] - del span["start_timestamp"] - del span["timestamp"] + span.pop("span_id", None) + span.pop("start_timestamp", None) + span.pop("timestamp", None) assert event["spans"] == expected_spans From 7ce699a84e826295a3b7911c3a82a517325bf6e4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 13 Sep 2023 11:42:59 +0200 Subject: [PATCH 16/21] Removed old clickhouse_driver from tests --- tox.ini | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index b2a2dbcb79..713384b63e 100644 --- a/tox.ini +++ b/tox.ini @@ -56,7 +56,7 @@ envlist = {py3.6,py3.7,py3.8}-chalice-v{1.18,1.20,1.22,1.24} # Clickhouse Driver - {py3.8,py3.9,py3.10,py3.11}-clickhouse_driver-v{0.2.1,0.2.4,0.2.5,0.2.6} + {py3.8,py3.9,py3.10,py3.11}-clickhouse_driver-v{0.2.4,0.2.5,0.2.6} # Cloud Resource Context {py3.6,py3.7,py3.8,py3.9,py3.10,py3.11}-cloud_resource_context @@ -252,7 +252,6 @@ deps = {py3.8}-chalice: botocore~=1.31 # Clickhouse Driver - clickhouse_driver-v0.2.1: clickhouse_driver>=0.2.1,<0.2.2 clickhouse_driver-v0.2.4: clickhouse_driver>=0.2.4,<0.2.5 clickhouse_driver-v0.2.5: clickhouse_driver>=0.2.5,<0.2.6 clickhouse_driver-v0.2.6: clickhouse_driver>=0.2.6,<0.2.7 From 84a92a25e3b96f8ec348d54f967fda9a1558d9e2 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 13 Sep 2023 12:01:48 +0200 Subject: [PATCH 17/21] Added clickhouse to github actions yaml generation script --- .../split-tox-gh-actions/ci-yaml-test-snippet.txt | 1 + .../split-tox-gh-actions/split-tox-gh-actions.py | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/scripts/split-tox-gh-actions/ci-yaml-test-snippet.txt b/scripts/split-tox-gh-actions/ci-yaml-test-snippet.txt index 8a60a70167..c2d10596ea 100644 --- a/scripts/split-tox-gh-actions/ci-yaml-test-snippet.txt +++ b/scripts/split-tox-gh-actions/ci-yaml-test-snippet.txt @@ -10,6 +10,7 @@ - uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} +{{ additional_uses }} - name: Setup Test Env run: | diff --git a/scripts/split-tox-gh-actions/split-tox-gh-actions.py b/scripts/split-tox-gh-actions/split-tox-gh-actions.py index 3b40178082..15f85391ed 100755 --- a/scripts/split-tox-gh-actions/split-tox-gh-actions.py +++ b/scripts/split-tox-gh-actions/split-tox-gh-actions.py @@ -36,6 +36,10 @@ "asyncpg", ] +FRAMEWORKS_NEEDING_CLICKHOUSE = [ + "clickhouse_driver", +] + MATRIX_DEFINITION = """ strategy: fail-fast: false @@ -48,6 +52,11 @@ os: [ubuntu-20.04] """ +ADDITIONAL_USES_CLICKHOUSE = """\ + + - uses: getsentry/action-clickhouse-in-ci@v1 +""" + CHECK_NEEDS = """\ needs: test """ @@ -119,6 +128,10 @@ def write_yaml_file( f = open(TEMPLATE_FILE_SETUP_DB, "r") out += "".join(f.readlines()) + elif template_line.strip() == "{{ additional_uses }}": + if current_framework in FRAMEWORKS_NEEDING_CLICKHOUSE: + out += ADDITIONAL_USES_CLICKHOUSE + elif template_line.strip() == "{{ check_needs }}": if py27_supported: out += CHECK_NEEDS_PY27 From a8a662a5c35bcf5ac778fb6e8c692f7f3c7ca33c Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 13 Sep 2023 12:04:56 +0200 Subject: [PATCH 18/21] Renamed dir for consistency --- .../integrations/{clickhouse => clickhouse_driver}/__init__.py | 0 .../{clickhouse => clickhouse_driver}/test_clickhouse_driver.py | 0 tox.ini | 2 +- 3 files changed, 1 insertion(+), 1 deletion(-) rename tests/integrations/{clickhouse => clickhouse_driver}/__init__.py (100%) rename tests/integrations/{clickhouse => clickhouse_driver}/test_clickhouse_driver.py (100%) diff --git a/tests/integrations/clickhouse/__init__.py b/tests/integrations/clickhouse_driver/__init__.py similarity index 100% rename from tests/integrations/clickhouse/__init__.py rename to tests/integrations/clickhouse_driver/__init__.py diff --git a/tests/integrations/clickhouse/test_clickhouse_driver.py b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py similarity index 100% rename from tests/integrations/clickhouse/test_clickhouse_driver.py rename to tests/integrations/clickhouse_driver/test_clickhouse_driver.py diff --git a/tox.ini b/tox.ini index 713384b63e..9e1c7a664f 100644 --- a/tox.ini +++ b/tox.ini @@ -482,7 +482,7 @@ setenv = bottle: TESTPATH=tests/integrations/bottle celery: TESTPATH=tests/integrations/celery chalice: TESTPATH=tests/integrations/chalice - clickhouse_driver: TESTPATH=tests/integrations/clickhouse + clickhouse_driver: TESTPATH=tests/integrations/clickhouse_driver cloud_resource_context: TESTPATH=tests/integrations/cloud_resource_context django: TESTPATH=tests/integrations/django falcon: TESTPATH=tests/integrations/falcon From 6d06fdc489abe681c5bc8f928976aba4533a61da Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 13 Sep 2023 12:15:39 +0200 Subject: [PATCH 19/21] Make ParamSpec work in older Python (hopefully) --- sentry_sdk/integrations/clickhouse_driver.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index 28e364256b..8a5f07f40b 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -1,12 +1,22 @@ -from typing import ParamSpec, TypeVar, Callable - from sentry_sdk import Hub from sentry_sdk.consts import OP, SPANDATA from sentry_sdk.hub import _should_send_default_pii from sentry_sdk.integrations import Integration, DidNotEnable from sentry_sdk.tracing import Span +from sentry_sdk._types import TYPE_CHECKING from sentry_sdk.utils import capture_internal_exceptions +from typing import TypeVar, Callable + +if TYPE_CHECKING: + from typing import ParamSpec +else: + # Fake ParamSpec + class ParamSpec: + def __init__(self, _): + self.args = None + self.kwargs = None + try: import clickhouse_driver # type: ignore[import] From 030623a90ec11f7accb0b79539fe9c36178f1637 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 13 Sep 2023 12:19:54 +0200 Subject: [PATCH 20/21] Next try --- sentry_sdk/integrations/clickhouse_driver.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index 8a5f07f40b..cea05de4d0 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -6,10 +6,10 @@ from sentry_sdk._types import TYPE_CHECKING from sentry_sdk.utils import capture_internal_exceptions -from typing import TypeVar, Callable +from typing import TypeVar if TYPE_CHECKING: - from typing import ParamSpec + from typing import ParamSpec, Callable else: # Fake ParamSpec class ParamSpec: @@ -17,6 +17,14 @@ def __init__(self, _): self.args = None self.kwargs = None + # Callable[anything] will return None + class _Callable: + def __getitem__(self, _): + return None + + # Make instances + Callable = _Callable() + try: import clickhouse_driver # type: ignore[import] From 360e09df9b87a9c7e2aed9c4e0880c851e542198 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 13 Sep 2023 12:33:48 +0200 Subject: [PATCH 21/21] Added comment about typing hack. --- sentry_sdk/integrations/clickhouse_driver.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sentry_sdk/integrations/clickhouse_driver.py b/sentry_sdk/integrations/clickhouse_driver.py index cea05de4d0..8a436022be 100644 --- a/sentry_sdk/integrations/clickhouse_driver.py +++ b/sentry_sdk/integrations/clickhouse_driver.py @@ -8,6 +8,9 @@ from typing import TypeVar +# Hack to get new Python features working in older versions +# without introducing a hard dependency on `typing_extensions` +# from: https://stackoverflow.com/a/71944042/300572 if TYPE_CHECKING: from typing import ParamSpec, Callable else: