feat(batcher): Add global flush trigger based on summed size estimates - #7144
Conversation
Sorry, something went wrong.
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.
@@ 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 |
Sorry, something went wrong.
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.
Sorry, something went wrong.
| 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 as sys.argv length, installed integrations, and span metadata, making a bare span's size environment-dependent.
- The neighboring test_weight_based_flushing_by_attribute_size avoids this exact fragility by calling SpanBatcher._estimate_size(bare_span._to_json()) before setting the flush limit.
- Both test_global_length_based_flushing (line 344) and test_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
Sorry, something went wrong.
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 the test_capture_after_length_based_flushing() to ensure accurate bookkeeping to track the total estimated size of spans in the batcher.
Issues
Closes #7137
Reminders