Skip to content

tests: Wait for all metrics to show up in ITCompositeTracer #12880

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.

Sign up for GitHub

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

Merged
blakeli0 merged 4 commits into from
Apr 22, 2026
Merged

tests: Wait for all metrics to show up in ITCompositeTracer #12880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0b19966
tests: Wait for all metrics to show up in ITCompositeTracer
blakeli0 Apr 21, 2026
0a40c1a
Merge branch 'main' into fix-otel-tests
blakeli0 Apr 21, 2026
9658401
Merge branch 'main' into fix-otel-tests
blakeli0 Apr 22, 2026
70a2bdf
fix: skip fmt check for generated grpc module in dataplex
blakeli0 Apr 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions java-dataplex/grpc-google-cloud-dataplex-v1/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,15 @@
<artifactId>guava</artifactId>
</dependency>
</dependencies>
<build>
<plugins>
<plugin>
<groupId>com.spotify.fmt</groupId>
<artifactId>fmt-maven-plugin</artifactId>
<configuration>
<skip>true</skip>
</configuration>
</plugin>
</plugins>
</build>
</project>
1 change: 1 addition & 0 deletions ...owcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITCompositeTracer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@
.get(AttributeKey.stringKey(ObservabilityAttributes.SERVER_ADDRESS_ATTRIBUTE)))
.isEqualTo(SHOWCASE_SERVER_ADDRESS);

Thread.sleep(100);

Check warning on line 127 in sdk-platform-java/java-showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITCompositeTracer.java

View check run for this annotation

SonarQubeCloud / [gapic-generator-java-root] SonarCloud Code Analysis

Remove this use of "Thread.sleep()". 


See more on https://sonarcloud.io/project/issues?id=googleapis_google-cloud-java_showcase&issues=AZ2xj97yDZJsarjiHLR9&open=AZ2xj97yDZJsarjiHLR9&pullRequest=12880
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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
  1. When testing for metric generation, it is acceptable to assert the exact number of metrics to ensure no unexpected metrics are produced.

Copy link
Copy Markdown
Member

@meltsufin meltsufin Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Copy Markdown
Contributor Author

@blakeli0 blakeli0 Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// Verify metric name and one basic attribute server.address
Collection<MetricData> actualMetrics = metricReader.collectAllMetrics();

Expand Down
Loading