feat(batcher): Add global flush trigger based on summed size estimates - #7144
Conversation
Codecov Results 📊✅ 99536 passed | ⏭️ 6623 skipped | Total: 106159 | Pass Rate: 93.76% | Execution Time: 348m 56s 📊 Comparison with Base Branch
All tests are passing successfully. ✅ Patch coverage is 88.89%. Project has 2500 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 90.02% 90.01% -0.01%
==========================================
Files 193 193 —
Lines 25014 25020 +6
Branches 9032 9032 —
==========================================
+ Hits 22516 22520 +4
- Misses 2498 2500 +2
- Partials 1435 1436 +1Generated by Codecov Action |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f6acf15. Configure here.
| monkeypatch.setattr(SpanBatcher, "GLOBAL_MAX_BYTES_BEFORE_FLUSH", 2_000) | ||
| # set the time-based flush limit to something huge so that it doesn't | ||
| # interfere | ||
| monkeypatch.setattr(SpanBatcher, "FLUSH_WAIT_TIME", 100000) | ||
|
|
||
| sentry_init( | ||
| traces_sample_rate=1.0, | ||
| trace_lifecycle="stream", | ||
| ) | ||
|
|
||
| items = capture_items("span") | ||
|
|
||
| with sentry_sdk.traces.start_span(name="span"): | ||
| pass | ||
|
|
||
| sentry_sdk.traces.new_trace() | ||
| with sentry_sdk.traces.start_span(name="span"): | ||
| pass | ||
|
|
||
| time.sleep(0.1) | ||
|
|
||
| assert len(items) == 2 | ||
| assert items[0].payload["name"] == "span" | ||
|
|
||
|
|
||
| def test_total_size_reset_after_length_based_flushing( | ||
| sentry_init, capture_items, monkeypatch | ||
| ): | ||
| """Span is not flushed after a flush reduces the combined span size in bytes below the global limit.""" | ||
| # Limit of 2_000 is just above the size of a bare span. | ||
| monkeypatch.setattr(SpanBatcher, "GLOBAL_MAX_BYTES_BEFORE_FLUSH", 2_000) | ||
| # set the time-based flush limit to something huge so that it doesn't | ||
| # interfere | ||
| monkeypatch.setattr(SpanBatcher, "FLUSH_WAIT_TIME", 100000) | ||
|
|
||
| sentry_init( |
There was a problem hiding this comment.
Global byte limit tests hardcode span size assumption without dynamic measurement
Hardcoding GLOBAL_MAX_BYTES_BEFORE_FLUSH = 2_000 relies on a specific bare span size that varies with runtime environment; the test should dynamically compute span size like the adjacent test_weight_based_flushing_by_attribute_size.
Evidence
SpanBatcher._estimate_size()computes size from runtime attributes such assys.argvlength, installed integrations, and span metadata, making a bare span's size environment-dependent.- The neighboring
test_weight_based_flushing_by_attribute_sizeavoids this exact fragility by callingSpanBatcher._estimate_size(bare_span._to_json())before setting the flush limit. - Both
test_global_length_based_flushing(line 344) andtest_total_size_reset_after_length_based_flushing(line 379) assume a span size between 1,000 and 2,000 bytes. If the actual estimate falls outside this range, the global flush will not trigger as expected and the assertions will fail.
Identified by Warden · find-bugs · GYU-MGF

Description
Add a global size limit to trigger a batcher flush. This mirrors the per-bucket logic.
Adapt the
test_weight_based_flushing()test which tests the per-bucket bytes limit to a test which tests the global bytes limit. Also add thetest_capture_after_length_based_flushing()to ensure accurate bookkeeping to track the total estimated size of spans in the batcher.Issues
Closes #7137
Reminders
uv run ruff.feat:,fix:,ref:,meta:)