tests: Wait for all metrics to show up in ITCompositeTracer#12880
tests: Wait for all metrics to show up in ITCompositeTracer#12880
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a 100ms sleep in the ITCompositeTracer test to allow for metrics collection. Feedback indicates that using Thread.sleep() is brittle and can lead to flaky tests, suggesting the use of a polling mechanism like Awaitility instead to ensure more robust and efficient test execution.
| .get(AttributeKey.stringKey(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE))) | ||
| .isEqualTo(SHOWCASE_SERVER_ADDRESS); | ||
|
|
||
| Thread.sleep(100); |
There was a problem hiding this comment.
Using Thread.sleep() to handle asynchronous metrics collection is brittle and can lead to flaky tests. In a slow or resource-constrained CI environment, 100ms may not be sufficient, while in faster environments, it introduces unnecessary delays. A more robust approach is to use a polling mechanism, such as Awaitility, to wait for the metrics to be available with a reasonable timeout. This ensures the test proceeds as soon as the condition is met and is less sensitive to environment speed. Additionally, when testing for metric generation, it is acceptable to assert the exact number of metrics to ensure no unexpected metrics are produced.
References
- When testing for metric generation, it is acceptable to assert the exact number of metrics to ensure no unexpected metrics are produced.
There was a problem hiding this comment.
Agree with this in general, but we don't have Awaitility introduced yet. This PR is to unblock the current CI and it should be a separate effort to introduce Awaitility.
diegomarquezp
left a comment
diegomarquezp
left a comment
There was a problem hiding this comment.
Good as a temporary solution. Also +1 to Awaitability.
|
|
3 participants
Add a 100ms sleep to wait for all metrics to show up in ITCompositeTracer.
This is because metrics are captured in a separate thread rather than the main thread. See #12657
Skip formatter check in java-dataplex since it is flaky.