chore(ddtrace/tracer): fix flaky TestHealthMetricsRaceCondition#3971
Conversation
Switch wg.Wait() and flush(5), otherwise we might flush before the wait group is done. Stop relying on the total number of reported statsd metrics as an indicator for the statsd client having received the metrics we are looking for. In my testing, I've seen the client receive 200+ metrics in this test (should it?), so performing our check after only 10 metrics have reported seemed brittle. Once we are on go1.25, we should start using the synctest package for this test. The changes in this patch are expected to fix the flakiness in a practical sense. But scheduling delays of > 5s will still cause this test to flake. [1] https://tip.golang.org/doc/go1.25#new-testingsynctest-package
BenchmarksBenchmark execution time: 2025-09-16 18:00:42 Comparing candidate commit 1ff7d1c in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 24 metrics, 0 unstable metrics. |
|
/merge |
|
View all feedbacks in Devflow UI.
This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
The expected merge time in
|
2 participants
What does this PR do?
Switch wg.Wait() and flush(5), otherwise we might flush before the wait group is done.
Stop relying on the total number of reported statsd metrics as an indicator for the statsd client having received the metrics we are looking for. In my testing, I've seen the client receive 200+ metrics in this test (should it?), so performing our check after only 10 metrics have reported seemed brittle.
Once we are on go1.25, we should start using the synctest package for this test. The changes in this patch are expected to fix the flakiness in a practical sense. But scheduling delays of > 5s will still cause this test to flake.
[1] https://tip.golang.org/doc/go1.25#new-testingsynctest-package
Motivation
This test keeps flaking. See this link.
Reviewer's Checklist
./scripts/lint.shlocally.Unsure? Have a question? Request a review!