Skip to content

test(monitor): Stop background health checks in tests that drive the monitor - #8032

Draft
sentry-junior[bot] wants to merge 1 commit into
masterfrom
fix/monitor-tests-background-thread-race
Draft

sentry-junior[bot] wants to merge 1 commit into
masterfrom
fix/monitor-tests-background-thread-race

Conversation

@sentry-junior

@sentry-junior sentry-junior Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Description

This fixes the flaky tests/test_monitor.py::test_segment_uses_downsampled_rate (assert 0.25 == 0.5, job). Its siblings test_transaction_uses_downsampled_rate and test_monitor_unhealthy have the same race.

Root cause
These tests call monitor.run() by hand and assert exact downsample_factor values. They also set monitor.interval = 0.1. The first monitor.is_healthy() starts the monitor's background thread, which then calls run() 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 thread is_healthy() starts then exits right away, so only the test's own run() 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 -p plugin added a random wait before start_transaction / traces.start_span to 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_rate failed 6 times and test_transaction_uses_downsampled_rate 8 times.
  • This branch, 0–200 ms wait: 40 of 40 runs passed. The baseline per-test failure rate was about 40%, so 9 clean runs would already give 99% confidence.
  • This branch, 0–500 ms wait: 20 of 20 runs passed.
  • Regression checks: tests/test_monitor.py, tests/tracing/test_sampling.py and tests/tracing/test_decorator.py all passed (109 tests). ruff is clean. I haven't run the full common suite or the gevent setup locally; CI covers those.

Not changed here: test_trace_decorator_no_trx also failed once, with leftover monitor logs (downsampling with a factor of 10). The likely cause is in tests/conftest.py::sentry_init: inner() assigns client without nonlocal, so the client.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]

…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>
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Results 📊

✅ 133666 passed | ⏭️ 7276 skipped | Total: 140942 | Pass Rate: 94.84% | Execution Time: 447m 45s

📊 Comparison with Base Branch

Metric Change
Total Tests —
Passed Tests —
Failed Tests —
Skipped Tests —

✨ Test counts unchanged from base.

All tests are passing successfully.

✅ Patch coverage is 100.00% (no changed executable lines found; target 80%).
Project statement coverage is 90.23% (down 0.03 percentage points from base (c96bbef) to head (2828363)).

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky test: test_segment_uses_downsampled_rate races the monitor's background thread

0 participants