Repository navigation
Reproduction for sentry-python#8031 - #62
Closed
sentry-junior[bot] wants to merge 1 commit into
Closed
sentry-junior[bot] wants to merge 1 commit into
sentry-junior[bot] wants to merge 1 commit into
Conversation
Co-Authored-By: Johannes Daxböck <johannes.daxboeck@sentry.io>
sentrivana
pushed a commit
to getsentry/sentry-python
that referenced
this pull request
Oct 15, 2026
…monitor (#8032) ### Description This fixes the flaky `tests/test_monitor.py::test_segment_uses_downsampled_rate` (`assert 0.25 == 0.5`, [job](https://github.com/getsentry/sentry-python/actions/runs/38393542918/job/115282966684)). 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 * resolves: #8031 <!-- junior-request-attribution:start --> via **johdax**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent-dispatch%3Adispatch_6df7374375b237816e130534b6439833 --> -- [View Junior Session](https://junior-prod.sentry.dev/conversations/agent-dispatch%3Adispatch_6df7374375b237816e130534b6439833) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent-dispatch%3Adispatch_6df7374375b237816e130534b6439833/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Johannes Daxböck <johannes.daxboeck@sentry.io>
|
Upstream issue getsentry/sentry-python#8031 has been closed. Closing this reproduction PR. Re-open if the issue resurfaces! |
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.
Reproduction for getsentry/sentry-python#8031: the flaky
tests/test_monitor.py::test_segment_uses_downsampled_rate.What it shows: the test sets
monitor.interval = 0.1and callsmonitor.run()once by hand, but the monitor's own background thread also callsrun()every 0.1 s. If the sampling decision comes more than 0.1 s after the thread starts,downsample_factoris 2 instead of 1, and the sample rate is 0.25 instead of 0.5. The script waits a random 0–--delay-maxseconds before sampling, to stand in for a slow CI runner.Run:
Tested with sentry-sdk 2.72.0 on Python 3.12. See
sentry-python/8031/README.mdfor the full setup.via johdax.
--
View Junior Session [Sentry]