Skip to content

Commit dafea47

Browse files
lvthanh03mchen-sentry
authored andcommitted
feat(spans-buffer): track pre-merge oversized parent sets (#108447)
Adds the `parent_span_set_already_oversized` metric to record when the destination span set already exceeds `max_segment_bytes` before sunionstore.
1 parent f00b76f commit dafea47

File tree

4 files changed

+89
-0
lines changed

4 files changed

+89
-0
lines changed

src/sentry/scripts/spans/add-buffer.lua

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,12 @@ table.insert(latency_table, {"sunionstore_args_step_latency_ms", sunionstore_arg
127127
-- Used outside the if statement
128128
local spop_end_time_ms = -1
129129
if #sunionstore_args > 0 then
130+
if (redis.call("memory", "usage", set_key) or 0) > max_segment_bytes then
131+
table.insert(metrics_table, {"parent_span_set_already_oversized", 1})
132+
else
133+
table.insert(metrics_table, {"parent_span_set_already_oversized", 0})
134+
end
135+
130136
local start_output_size = redis.call("scard", set_key)
131137
local scard_end_time_ms = get_time_ms()
132138
table.insert(latency_table, {"scard_step_latency_ms", scard_end_time_ms - sunionstore_args_end_time_ms})

src/sentry/spans/buffer_logger.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ def emit_observability_metrics(
243243
gauge_metrics_dict: dict[
244244
str, tuple[float, float, float, float]
245245
] = {} # metric, min, max, sum, count
246+
oversized_count = 0
246247

247248
for evalsha_latency_metrics, evalsha_gauge_metrics in zip(latency_metrics, gauge_metrics):
248249
for raw_key, value in evalsha_latency_metrics:
@@ -267,6 +268,8 @@ def emit_observability_metrics(
267268
gauge_metrics_dict[key][2] + value,
268269
gauge_metrics_dict[key][3] + 1.0,
269270
)
271+
if raw_key == b"parent_span_set_already_oversized":
272+
oversized_count += int(value)
270273

271274
for metric, (min_value, max_value, sum_value, count) in latency_metrics_dict.items():
272275
tags = {"stage": metric}
@@ -295,3 +298,9 @@ def emit_observability_metrics(
295298
value,
296299
tags={"stage": key},
297300
)
301+
302+
if oversized_count > 0:
303+
metrics.incr(
304+
"spans.buffer.process_spans.parent_span_set_already_oversized.count",
305+
amount=oversized_count,
306+
)

tests/sentry/spans/test_buffer.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,62 @@ def test_observability_metrics(
292292
emit_observability_metrics.assert_called_once()
293293

294294

295+
@mock.patch("sentry.spans.buffer.emit_observability_metrics")
296+
def test_observability_metrics_parent_span_already_oversized(
297+
emit_observability_metrics: mock.MagicMock,
298+
buffer: SpansBuffer,
299+
) -> None:
300+
# Disable compression so payload size in Redis is predictable, then force a
301+
# low max-segment-bytes threshold so the destination set is already too
302+
# large before merge.
303+
oversized_parent_payload = orjson.dumps({"span_id": "b" * 16, "blob": "x" * 2048})
304+
spans = [
305+
Span(
306+
# payload=_payload("a" * 16),
307+
payload=oversized_parent_payload,
308+
trace_id="a" * 32,
309+
span_id="a" * 16,
310+
parent_span_id="b" * 16,
311+
segment_id=None,
312+
project_id=1,
313+
end_timestamp=1700000000.0,
314+
),
315+
Span(
316+
# payload=oversized_parent_payload,
317+
payload=_payload("a" * 16),
318+
trace_id="a" * 32,
319+
span_id="b" * 16,
320+
parent_span_id=None,
321+
segment_id=None,
322+
is_segment_span=True,
323+
project_id=1,
324+
end_timestamp=1700000000.0,
325+
),
326+
]
327+
328+
with override_options(
329+
{"spans.buffer.max-segment-bytes": 200, "spans.buffer.compression.level": -1}
330+
):
331+
process_spans(spans, buffer, now=0)
332+
333+
emit_observability_metrics.assert_called_once()
334+
args, _ = emit_observability_metrics.call_args
335+
gauge_metrics = args[1]
336+
337+
oversized_metric_values = [
338+
value
339+
for evalsha_metrics in gauge_metrics
340+
for metric_name, value in evalsha_metrics
341+
if metric_name == b"parent_span_set_already_oversized"
342+
]
343+
assert oversized_metric_values, (
344+
"Expected parent_span_set_already_oversized metric to be emitted"
345+
)
346+
assert 1 in oversized_metric_values, (
347+
"Expected at least one evalsha call with an already oversized parent set"
348+
)
349+
350+
295351
@pytest.mark.parametrize(
296352
"spans",
297353
list(

tests/sentry/spans/test_buffer_logger.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,3 +474,21 @@ def t(stage):
474474
call(LG, 55.0, tags=t("spopcalls")),
475475
]
476476
)
477+
478+
@mock.patch("sentry.spans.buffer_logger.metrics.incr")
479+
def test_emit_observability_metrics_oversized_count_metric(self, mock_incr):
480+
emit_observability_metrics(
481+
latency_metrics=[[]],
482+
gauge_metrics=[
483+
[
484+
(b"parent_span_set_already_oversized", 1.0),
485+
(b"redirect_depth", 1.0),
486+
],
487+
],
488+
longest_evalsha_data=(0, [], []),
489+
)
490+
491+
mock_incr.assert_called_once_with(
492+
"spans.buffer.process_spans.parent_span_set_already_oversized.count",
493+
amount=1,
494+
)

0 commit comments

Comments
 (0)