test(compute): use Awaitility for trace polling in golden signals test to prevent quota exhaustion - #14295
Conversation
In ITComputeGoldenSignals, custom RetrySettings had 0ms backoff and added NOT_FOUND as retryable, causing tight-loop retries that exhausted the Cloud Trace read quota (RESOURCE_EXHAUSTED). Reverted TraceServiceClient to default retry settings and used Awaitility with a 3-second poll interval and 2-minute timeout to poll for ingested traces.
There was a problem hiding this comment.
Code Review
This pull request integrates the Awaitility library into the integration tests to poll Cloud Trace, replacing manual try-catch blocks and custom retry settings. Feedback points out a critical issue in the Awaitility configuration: chaining .ignoreExceptionsMatching after .ignoreExceptionsInstanceOf overwrites the previous configuration, and the client throws GAX exceptions instead of StatusRuntimeException. A combined exception-matching check is suggested to fix this.
| settingsBuilder | ||
| .getTraceSettings() | ||
| .setRetrySettings( | ||
| RetrySettings.newBuilder() | ||
| .setTotalTimeoutDuration(Duration.ofMinutes(5)) | ||
| .setInitialRpcTimeoutDuration(Duration.ofSeconds(5)) | ||
| .setMaxRpcTimeoutDuration(Duration.ofSeconds(10)) | ||
| .build()) | ||
| .setRetryableCodes( | ||
| StatusCode.Code.NOT_FOUND, StatusCode.Code.INTERNAL, StatusCode.Code.DEADLINE_EXCEEDED); |
There was a problem hiding this comment.
IIUC, getTrace() should have default values configured from stubsettings (this is just removing our overrides)?
Since we are using awaitillity, we can probably just set the default to be 1 attempt and like 5s timeout or something.
There was a problem hiding this comment.
Yes this is just removing our overrides so we can rely on awaitility for the retries on server errors. The default configuration still makes sense for transient network errors and we can keep them as they are.
There was a problem hiding this comment.
Reframing the question: Do we want/ need to account for transient network issues if we are polling every 3 seconds?
getTrace() is called every 3 seconds with its own retry configurations. Each invocation from awaitility theoretically can retry multiple times
…mcrest reflection in GraalVM
Fixes b/558226581
Problem
In
ITComputeGoldenSignals, integration tests were failing with:Root Cause
TraceServiceClientwas configured with customRetrySettingswhere retry delays were initialized to 0ms backoff, while simultaneously addingStatusCode.Code.NOT_FOUNDas a retryable code.traceClient.getTrace(...)initially returnedNOT_FOUND(404).Solution
TraceServiceClientto default retry settings: Uses the default GAX retry configuration (which does not retryNOT_FOUNDand applies exponential backoff for transient gRPC errors likeUNAVAILABLE).Awaitility.await(), polling every 3 seconds with a 3-second initial delay up to a 2-minute timeout, ignoring transientNOT_FOUNDandRESOURCE_EXHAUSTEDerrors during ingestion.org.awaitility:awaitilityin test scope and returnedTracedirectly from Awaitility'suntil(Callable<T>, Predicate<T>).Verification
mvn test-compile -DskipTestsinjava-compute/google-cloud-compute: PASSED (0 checkstyle violations, clean compilation).