tests: Add retry metrics tests to ITOtelGoldenMetrics#12672
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 integration tests for OpenTelemetry metrics across gRPC and HTTP/JSON transports, covering deadline exceeded scenarios and successful retries. It also adds utility methods to TestClientInitializer for configuring clients with custom retry settings and interceptors. The review feedback recommends replacing Thread.sleep() with polling mechanisms to prevent test flakiness, adding safety checks before accessing metric data points to avoid potential exceptions, avoiding fragile assertions on total metric counts, and explicitly specifying the UTF-8 charset when converting strings to bytes.
...howcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelGoldenMetrics.java
Show resolved
Hide resolved
...howcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelGoldenMetrics.java
Show resolved
Hide resolved
...howcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelGoldenMetrics.java
Show resolved
Hide resolved
...howcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelGoldenMetrics.java
Outdated
Show resolved
Hide resolved
...howcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelGoldenMetrics.java
Show resolved
Hide resolved
...howcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelGoldenMetrics.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| @Test | ||
| void testMetrics_zeroDeadline_grpc() throws Exception { |
There was a problem hiding this comment.
what does zeroDeadline mean? Is this testing that the RPC returns deadline_exceeded status? I think it may be helpful if the test name could be a bit more specific
There was a problem hiding this comment.
Updated to make it more specific
| } | ||
|
|
||
| @Test | ||
| void testMetrics_retryAndSucceed_grpc() throws Exception { |
There was a problem hiding this comment.
same here. Is this test intended to test that that multiple retry attempts results in one value for gcp.client.request.duration and that the final status is OK?
There was a problem hiding this comment.
Yes, exactly. Making sure that there is only one metric instead of multiple in case of retries.
There was a problem hiding this comment.
Also updated to make it more specific
2 participants
Add showcase tests for metrics for the following two cases per test plan: