Skip to content

Commit 42cac90

Browse files
authored
Fix clickhouse logic for generator case (#4722)
we need to call the original function first for `db_params` to be correctly populated in the generator case (see diff without whitespace)
1 parent 8d64be5 commit 42cac90

File tree

2 files changed

+32
-25
lines changed

2 files changed

+32
-25
lines changed

sentry_sdk/integrations/clickhouse_driver.py

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -146,40 +146,47 @@ def _inner_send_data(
146146
**kwargs: Any,
147147
) -> Any:
148148
span = getattr(self.connection, "_sentry_span", None)
149+
if span is None:
150+
return original_send_data(
151+
self, sample_block, data, types_check, columnar, *args, **kwargs
152+
)
149153

150-
if span is not None:
151-
data = _get_db_data(self.connection)
152-
_set_on_span(span, data)
153-
154-
if should_send_default_pii():
155-
saved_db_data: dict[str, Any] = getattr(
156-
self.connection, "_sentry_db_data", {}
157-
)
158-
db_params: list[Any] = saved_db_data.get("db.params") or []
154+
db_data = _get_db_data(self.connection)
155+
_set_on_span(span, db_data)
159156

160-
if isinstance(data, (list, tuple)):
161-
db_params.extend(data)
157+
saved_db_data: dict[str, Any] = getattr(self.connection, "_sentry_db_data", {})
158+
db_params: list[Any] = saved_db_data.get("db.params") or []
162159

163-
else: # data is a generic iterator
164-
orig_data = data
160+
if should_send_default_pii():
161+
if isinstance(data, (list, tuple)):
162+
db_params.extend(data)
165163

166-
# Wrap the generator to add items to db.params as they are yielded.
167-
# This allows us to send the params to Sentry without needing to allocate
168-
# memory for the entire generator at once.
169-
def wrapped_generator() -> "Iterator[Any]":
170-
for item in orig_data:
171-
db_params.append(item)
172-
yield item
164+
else: # data is a generic iterator
165+
orig_data = data
173166

174-
# Replace the original iterator with the wrapped one.
175-
data = wrapped_generator()
167+
# Wrap the generator to add items to db.params as they are yielded.
168+
# This allows us to send the params to Sentry without needing to allocate
169+
# memory for the entire generator at once.
170+
def wrapped_generator() -> "Iterator[Any]":
171+
for item in orig_data:
172+
db_params.append(item)
173+
yield item
176174

177-
span.set_attribute("db.params", _serialize_span_attribute(db_params))
175+
# Replace the original iterator with the wrapped one.
176+
data = wrapped_generator()
178177

179-
return original_send_data(
178+
rv = original_send_data(
180179
self, sample_block, data, types_check, columnar, *args, **kwargs
181180
)
182181

182+
if should_send_default_pii() and db_params:
183+
# need to do this after the original function call to make sure
184+
# db_params is populated correctly
185+
saved_db_data["db.params"] = db_params
186+
span.set_attribute("db.params", _serialize_span_attribute(db_params))
187+
188+
return rv
189+
183190
clickhouse_driver.client.Client.send_data = _inner_send_data
184191

185192

tests/integrations/clickhouse_driver/test_clickhouse_driver.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ def test_clickhouse_spans_with_generator(sentry_init, capture_events):
383383
span for span in spans if span["description"] == "INSERT INTO test (x) VALUES"
384384
]
385385

386-
assert span["data"]["db.params"] == [{"x": 0}, {"x": 1}, {"x": 2}]
386+
assert span["data"]["db.params"] == '[{"x": 0}, {"x": 1}, {"x": 2}]'
387387

388388

389389
def test_clickhouse_client_spans_with_pii(

0 commit comments

Comments
 (0)