Repository navigation
test(monitor): Stop background health checks in tests that drive the monitor - #8032
Merged
Merged
Conversation
…monitor The monitor tests call monitor.run() by hand and assert exact downsample_factor values, but they also set monitor.interval = 0.1, so the monitor's background thread runs extra health checks every 0.1 s. On a slow runner that bumps the factor between the asserts (0.25 instead of 0.5). Kill the background loop in these tests instead. Fixes #8031 Co-Authored-By: Johannes Daxböck <johannes.daxboeck@sentry.io>
Contributor
Coverage Results 📊✅ 133666 passed | ⏭️ 7276 skipped | Total: 140942 | Pass Rate: 94.84% | Execution Time: 447m 45s 📊 Comparison with Base Branch
✨ Test counts unchanged from base. All tests are passing successfully. ✅ Patch coverage is 100.00% (no changed executable lines found; target 80%). Coverage diff@@ Coverage Diff @@
## master #8032 +/-##
==========================================
- Coverage 90.26% 90.23% -0.03%
==========================================
Files 202 202 —
Tracked lines 26660 26660 —
Branches 9926 9926 —
==========================================
- Hits 24063 24055 -8
- Misses 2597 2605 +8
Partials 1510 1510 —Generated by Coverage Action |
sentrivana
approved these changes
Oct 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This fixes the flaky
tests/test_monitor.py::test_segment_uses_downsampled_rate(assert 0.25 == 0.5, job). Its siblingstest_transaction_uses_downsampled_rateandtest_monitor_unhealthyhave the same race.Root cause
These tests call
monitor.run()by hand and assert exactdownsample_factorvalues. They also setmonitor.interval = 0.1. The firstmonitor.is_healthy()starts the monitor's background thread, which then callsrun()every 0.1 s as well. On a slow runner (the failure was in Gevent 3.10, where the test took about 0.34 s), the thread runs an extra unhealthy check before the sampling decision. The factor goes from 1 to 2, so the sample rate is 0.25.Fix
These tests now call
monitor.kill()instead of shortening the interval. The threadis_healthy()starts then exits right away, so only the test's ownrun()calls change the factor. The assertions are unchanged, and there are no retries, sleeps or skips. SDK code is unchanged; this was a test bug.Reproduction: getsentry/repro#62 (sentry-sdk 2.72.0, Python 3.12). With random gaps of 0–0.2 s before sampling, the rate was wrong in 55 of 100 iterations. With gaps under 0.1 s it was 0 of 100, and with the background thread stopped it was 0 of 100.
Original tests, local runs (
py3.14-common). Each run covered the 3 affected tests. A test-only-pplugin added a random wait beforestart_transaction/traces.start_spanto imitate a slow runner; no test or SDK code was changed by it.master, no wait: 10 of 10 runs passed. The race doesn't show up locally on its own.master, 0–200 ms wait: 11 of 15 runs failed.test_segment_uses_downsampled_ratefailed 6 times andtest_transaction_uses_downsampled_rate8 times.tests/test_monitor.py,tests/tracing/test_sampling.pyandtests/tracing/test_decorator.pyall passed (109 tests).ruffis clean. I haven't run the full common suite or the gevent setup locally; CI covers those.Not changed here:
test_trace_decorator_no_trxalso failed once, with leftover monitor logs (downsampling with a factor of 10). The likely cause is intests/conftest.py::sentry_init:inner()assignsclientwithoutnonlocal, so theclient.close()in teardown never runs. Monitor threads from earlier tests (with the 0.1 s interval) then keep running. I'll handle that separately, because closing every test client affects the whole suite.Issues
via johdax.
--
View Junior Session [Sentry]