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.
|
|
…el (#12839) Currently, the hermetic code generator can only move files to top level folders. Rather than changing the generator, we will move the generated code to the top level to also make it compatible with librarian. Other fixes: * The hermetic_build scripts had special case logic to force regeneration of the entire repository if the common protos were found in the generation_config.yaml. This is no longer necessary in google-cloud-java as the nightly build regenerates everything * Includes the OTel wait for metrics fix similar to #12880 * Removes configuration for GCS Maven Mirror as that mirror is no longer updated and is missing dependencies Towards #12735 Towards #12702 --------- Co-authored-by: cloud-java-bot <cloud-java-bot@google.com> Co-authored-by: Mike Eltsufin <meltsufin@google.com>
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.