-
Couldn't load subscription status.
- Fork 849
[Infra/OpenTelemetry] Fix flaky test #6622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Simplify `if` statement. - Fix typo in variable name.
Assert that the metrics are flushed successfully.
- Assert more exports succeed. - Simplify `while` loop. - Remove redundant `!`. - Use collection expression.
Use `Interlocked.Add()` to avoid potential races updating `Sum` if two exports overlap.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6622 +/- ##
==========================================
+ Coverage 86.70% 86.82% +0.11%
==========================================
Files 258 258
Lines 11955 11955
==========================================
+ Hits 10366 10380 +14
+ Misses 1589 1575 -14
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors MetricPointReclaimTests to fix a flaky test by addressing potential race conditions and improving code quality. The changes focus on thread-safety improvements and code modernization.
Key Changes:
- Added thread-safe sum accumulation using
Interlocked.Addto prevent race conditions - Added assertions on
ForceFlush()return values to verify export success - Modernized code syntax (collection expressions, simplified conditionals)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
I'm not 100% sure these changes definitely fix the issue observed, but there's been no reoccurrence in 5 re-runs. |
Fix flaky test observed in #6621.
Changes
Refactor
MetricPointReclaimTeststests to avoid possible race condition, extend assertions and improve the code.Will run a few times in draft to check for lack of failures.
Merge requirement checklist
AppropriateCHANGELOG.mdfiles updated for non-trivial changesChanges in public API reviewed (if applicable)