feat(clickhouse_driver): Set breadcrumbs in the streaming trace lifecycle - #7325
3 issues
code-review: Found 3 issues (1 high, 2 medium)
High
Streaming ClickHouse breadcrumbs include query results when database query data is disabled - `sentry_sdk/integrations/clickhouse_driver.py:18`
In _wrap_end, the streaming breadcrumb always passes {"db.result": res, **breadcrumb_data}. This bypasses the preceding database_query_data/send_default_pii check, causing ClickHouse query results to be attached to breadcrumbs even when database query data collection is disabled. Pass only breadcrumb_data after the conditional merge.
Also found at:
sentry_sdk/integrations/clickhouse_driver.py:161-165tests/integrations/clickhouse_driver/test_clickhouse_driver.py:543-644tests/integrations/clickhouse_driver/test_clickhouse_driver.py:748-753tests/integrations/clickhouse_driver/test_clickhouse_driver.py:945-951tests/integrations/clickhouse_driver/test_clickhouse_driver.py:1345-1376tests/integrations/clickhouse_driver/test_clickhouse_driver.py:134-241tests/integrations/clickhouse_driver/test_clickhouse_driver.py:225-241tests/integrations/clickhouse_driver/test_clickhouse_driver.py:339-445
Medium
Streaming breadcrumb tests never assert db.result absence - `sentry_sdk/integrations/clickhouse_driver.py:134-239`
Streaming breadcrumb cases rely on ApproxDict subset matching and omit explicit db.result checks, so they cannot catch the unconditional result leak; mirror the non-streaming tests’ absence assertions.
Also found at:
tests/integrations/clickhouse_driver/test_clickhouse_driver.py:24-31tests/integrations/clickhouse_driver/test_clickhouse_driver.py:770-819tests/integrations/clickhouse_driver/test_clickhouse_driver.py:1034-1037tests/integrations/clickhouse_driver/test_clickhouse_driver.py:2145-2151
Streaming breadcrumb path never records db.params - `tests/integrations/clickhouse_driver/test_clickhouse_driver.py:547-552`
Unlike the non-streaming breadcrumb path, streaming never copies query/insert params into _breadcrumb_data, so even with database_query_data: True this test cannot observe db.params; collect params in the streaming start/send_data path when data collection is enabled.
⏱ 11m · 5.9M in / 245.0k out · $3.31
Annotations
Check failure on line 18 in sentry_sdk/integrations/clickhouse_driver.py
sentry-warden / warden: code-review
Streaming ClickHouse breadcrumbs include query results when database query data is disabled
In `_wrap_end`, the streaming breadcrumb always passes `{"db.result": res, **breadcrumb_data}`. This bypasses the preceding `database_query_data`/`send_default_pii` check, causing ClickHouse query results to be attached to breadcrumbs even when database query data collection is disabled. Pass only `breadcrumb_data` after the conditional merge.
Check failure on line 165 in sentry_sdk/integrations/clickhouse_driver.py
sentry-warden / warden: code-review
[VEU-T9U] Streaming ClickHouse breadcrumbs include query results when database query data is disabled (additional location)
In `_wrap_end`, the streaming breadcrumb always passes `{"db.result": res, **breadcrumb_data}`. This bypasses the preceding `database_query_data`/`send_default_pii` check, causing ClickHouse query results to be attached to breadcrumbs even when database query data collection is disabled. Pass only `breadcrumb_data` after the conditional merge.
Check failure on line 644 in tests/integrations/clickhouse_driver/test_clickhouse_driver.py
sentry-warden / warden: code-review
[VEU-T9U] Streaming ClickHouse breadcrumbs include query results when database query data is disabled (additional location)
In `_wrap_end`, the streaming breadcrumb always passes `{"db.result": res, **breadcrumb_data}`. This bypasses the preceding `database_query_data`/`send_default_pii` check, causing ClickHouse query results to be attached to breadcrumbs even when database query data collection is disabled. Pass only `breadcrumb_data` after the conditional merge.
Check failure on line 753 in tests/integrations/clickhouse_driver/test_clickhouse_driver.py
sentry-warden / warden: code-review
[VEU-T9U] Streaming ClickHouse breadcrumbs include query results when database query data is disabled (additional location)
In `_wrap_end`, the streaming breadcrumb always passes `{"db.result": res, **breadcrumb_data}`. This bypasses the preceding `database_query_data`/`send_default_pii` check, causing ClickHouse query results to be attached to breadcrumbs even when database query data collection is disabled. Pass only `breadcrumb_data` after the conditional merge.
Check failure on line 951 in tests/integrations/clickhouse_driver/test_clickhouse_driver.py
sentry-warden / warden: code-review
[VEU-T9U] Streaming ClickHouse breadcrumbs include query results when database query data is disabled (additional location)
In `_wrap_end`, the streaming breadcrumb always passes `{"db.result": res, **breadcrumb_data}`. This bypasses the preceding `database_query_data`/`send_default_pii` check, causing ClickHouse query results to be attached to breadcrumbs even when database query data collection is disabled. Pass only `breadcrumb_data` after the conditional merge.
Check failure on line 1376 in tests/integrations/clickhouse_driver/test_clickhouse_driver.py
sentry-warden / warden: code-review
[VEU-T9U] Streaming ClickHouse breadcrumbs include query results when database query data is disabled (additional location)
In `_wrap_end`, the streaming breadcrumb always passes `{"db.result": res, **breadcrumb_data}`. This bypasses the preceding `database_query_data`/`send_default_pii` check, causing ClickHouse query results to be attached to breadcrumbs even when database query data collection is disabled. Pass only `breadcrumb_data` after the conditional merge.
Check failure on line 241 in tests/integrations/clickhouse_driver/test_clickhouse_driver.py
sentry-warden / warden: code-review
[VEU-T9U] Streaming ClickHouse breadcrumbs include query results when database query data is disabled (additional location)
In `_wrap_end`, the streaming breadcrumb always passes `{"db.result": res, **breadcrumb_data}`. This bypasses the preceding `database_query_data`/`send_default_pii` check, causing ClickHouse query results to be attached to breadcrumbs even when database query data collection is disabled. Pass only `breadcrumb_data` after the conditional merge.
Check failure on line 241 in tests/integrations/clickhouse_driver/test_clickhouse_driver.py
sentry-warden / warden: code-review
[VEU-T9U] Streaming ClickHouse breadcrumbs include query results when database query data is disabled (additional location)
In `_wrap_end`, the streaming breadcrumb always passes `{"db.result": res, **breadcrumb_data}`. This bypasses the preceding `database_query_data`/`send_default_pii` check, causing ClickHouse query results to be attached to breadcrumbs even when database query data collection is disabled. Pass only `breadcrumb_data` after the conditional merge.
Check failure on line 445 in tests/integrations/clickhouse_driver/test_clickhouse_driver.py
sentry-warden / warden: code-review
[VEU-T9U] Streaming ClickHouse breadcrumbs include query results when database query data is disabled (additional location)
In `_wrap_end`, the streaming breadcrumb always passes `{"db.result": res, **breadcrumb_data}`. This bypasses the preceding `database_query_data`/`send_default_pii` check, causing ClickHouse query results to be attached to breadcrumbs even when database query data collection is disabled. Pass only `breadcrumb_data` after the conditional merge.
Check warning on line 239 in sentry_sdk/integrations/clickhouse_driver.py
sentry-warden / warden: code-review
Streaming breadcrumb tests never assert db.result absence
Streaming breadcrumb cases rely on ApproxDict subset matching and omit explicit `db.result` checks, so they cannot catch the unconditional result leak; mirror the non-streaming tests’ absence assertions.
Check warning on line 31 in tests/integrations/clickhouse_driver/test_clickhouse_driver.py
sentry-warden / warden: code-review
[S3K-UXU] Streaming breadcrumb tests never assert db.result absence (additional location)
Streaming breadcrumb cases rely on ApproxDict subset matching and omit explicit `db.result` checks, so they cannot catch the unconditional result leak; mirror the non-streaming tests’ absence assertions.
Check warning on line 819 in tests/integrations/clickhouse_driver/test_clickhouse_driver.py
sentry-warden / warden: code-review
[S3K-UXU] Streaming breadcrumb tests never assert db.result absence (additional location)
Streaming breadcrumb cases rely on ApproxDict subset matching and omit explicit `db.result` checks, so they cannot catch the unconditional result leak; mirror the non-streaming tests’ absence assertions.
Check warning on line 1037 in tests/integrations/clickhouse_driver/test_clickhouse_driver.py
sentry-warden / warden: code-review
[S3K-UXU] Streaming breadcrumb tests never assert db.result absence (additional location)
Streaming breadcrumb cases rely on ApproxDict subset matching and omit explicit `db.result` checks, so they cannot catch the unconditional result leak; mirror the non-streaming tests’ absence assertions.
Check warning on line 2151 in tests/integrations/clickhouse_driver/test_clickhouse_driver.py
sentry-warden / warden: code-review
[S3K-UXU] Streaming breadcrumb tests never assert db.result absence (additional location)
Streaming breadcrumb cases rely on ApproxDict subset matching and omit explicit `db.result` checks, so they cannot catch the unconditional result leak; mirror the non-streaming tests’ absence assertions.
Check warning on line 552 in tests/integrations/clickhouse_driver/test_clickhouse_driver.py
sentry-warden / warden: code-review
Streaming breadcrumb path never records db.params
Unlike the non-streaming breadcrumb path, streaming never copies query/insert params into `_breadcrumb_data`, so even with `database_query_data: True` this test cannot observe `db.params`; collect params in the streaming start/send_data path when data collection is enabled.