diff --git a/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallableFactory.java b/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallableFactory.java index 33e2ff886e..2a160d5d0a 100644 --- a/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallableFactory.java +++ b/gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallableFactory.java @@ -84,6 +84,11 @@ static UnaryCallable createUnaryCalla callable = Callables.retrying( callable, callSettings, clientContext, httpJsonCallSettings.getRequestMutator()); + callable = + new TracedUnaryCallable<>( + callable, + clientContext.getTracerFactory(), + getSpanName(httpJsonCallSettings.getMethodDescriptor())); return callable.withDefaultCallContext(clientContext.getDefaultCallContext()); } @@ -136,12 +141,6 @@ public static UnaryCallable createUna UnaryCallable innerCallable = createDirectUnaryCallable(httpJsonCallSettings); - innerCallable = - new TracedUnaryCallable<>( - innerCallable, - clientContext.getTracerFactory(), - getSpanName(httpJsonCallSettings.getMethodDescriptor())); - return createUnaryCallable(innerCallable, callSettings, httpJsonCallSettings, clientContext); } diff --git a/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/RetryingTest.java b/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/RetryingTest.java index 892fc9b8f8..63f826bdbe 100644 --- a/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/RetryingTest.java +++ b/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/RetryingTest.java @@ -43,6 +43,7 @@ import com.google.api.core.ApiFutures; import com.google.api.gax.core.FakeApiClock; import com.google.api.gax.core.RecordingScheduler; +import com.google.api.gax.httpjson.testing.TestApiTracerFactory; import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.ApiCallContext; import com.google.api.gax.rpc.ApiException; @@ -80,6 +81,7 @@ class RetryingTest { private final Integer initialRequest = 1; private final Integer modifiedRequest = 0; + private TestApiTracerFactory tracerFactory; private final HttpJsonCallSettings httpJsonCallSettings = HttpJsonCallSettings.newBuilder() @@ -115,8 +117,11 @@ class RetryingTest { void resetClock() { fakeClock = new FakeApiClock(System.nanoTime()); executor = RecordingScheduler.create(fakeClock); + tracerFactory = new TestApiTracerFactory(); clientContext = ClientContext.newBuilder() + // we use a custom tracer to confirm whether the retrials are being recorded. + .setTracerFactory(tracerFactory) .setExecutor(executor) .setClock(fakeClock) .setDefaultCallContext(HttpJsonCallContext.createDefault()) @@ -130,6 +135,7 @@ void teardown() { @Test void retry() { + // set a retriable that will fail 3 times before returning "2" ImmutableSet retryable = ImmutableSet.of(Code.UNAVAILABLE); Mockito.when(callInt.futureCall((Integer) any(), (ApiCallContext) any())) .thenReturn(ApiFutures.immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION)) @@ -143,6 +149,9 @@ void retry() { HttpJsonCallableFactory.createUnaryCallable( callInt, callSettings, httpJsonCallSettings, clientContext); assertThat(callable.call(initialRequest)).isEqualTo(2); + assertThat(tracerFactory.getInstance().getAttemptsFailed().get()).isEqualTo(3); + assertThat(tracerFactory.getInstance().getAttemptsStarted().get()).isEqualTo(4); + assertThat(tracerFactory.getInstance().getRetriesExhausted().get()).isFalse(); // Capture the argument passed to futureCall ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Integer.class); @@ -180,6 +189,9 @@ void retryTotalTimeoutExceeded() { HttpJsonCallableFactory.createUnaryCallable( callInt, callSettings, httpJsonCallSettings, clientContext); assertThrows(ApiException.class, () -> callable.call(initialRequest)); + assertThat(tracerFactory.getInstance().getAttemptsStarted().get()).isEqualTo(1); + assertThat(tracerFactory.getInstance().getAttemptsFailed().get()).isEqualTo(0); + assertThat(tracerFactory.getInstance().getRetriesExhausted().get()).isFalse(); // Capture the argument passed to futureCall ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Integer.class); verify(callInt, atLeastOnce()).futureCall(argumentCaptor.capture(), any(ApiCallContext.class)); @@ -200,6 +212,9 @@ void retryMaxAttemptsExceeded() { HttpJsonCallableFactory.createUnaryCallable( callInt, callSettings, httpJsonCallSettings, clientContext); assertThrows(ApiException.class, () -> callable.call(initialRequest)); + assertThat(tracerFactory.getInstance().getAttemptsStarted().get()).isEqualTo(2); + assertThat(tracerFactory.getInstance().getAttemptsFailed().get()).isEqualTo(2); + assertThat(tracerFactory.getInstance().getRetriesExhausted().get()).isTrue(); // Capture the argument passed to futureCall ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Integer.class); verify(callInt, atLeastOnce()).futureCall(argumentCaptor.capture(), any(ApiCallContext.class)); @@ -220,6 +235,9 @@ void retryWithinMaxAttempts() { HttpJsonCallableFactory.createUnaryCallable( callInt, callSettings, httpJsonCallSettings, clientContext); assertThat(callable.call(initialRequest)).isEqualTo(2); + assertThat(tracerFactory.getInstance().getAttemptsStarted().get()).isEqualTo(3); + assertThat(tracerFactory.getInstance().getAttemptsFailed().get()).isEqualTo(2); + assertThat(tracerFactory.getInstance().getRetriesExhausted().get()).isFalse(); // Capture the argument passed to futureCall ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Integer.class); verify(callInt, atLeastOnce()).futureCall(argumentCaptor.capture(), any(ApiCallContext.class)); @@ -246,6 +264,9 @@ void retryOnStatusUnknown() { HttpJsonCallableFactory.createUnaryCallable( callInt, callSettings, httpJsonCallSettings, clientContext); assertThat(callable.call(initialRequest)).isEqualTo(2); + assertThat(tracerFactory.getInstance().getAttemptsStarted().get()).isEqualTo(4); + assertThat(tracerFactory.getInstance().getAttemptsFailed().get()).isEqualTo(3); + assertThat(tracerFactory.getInstance().getRetriesExhausted().get()).isFalse(); // Capture the argument passed to futureCall ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Integer.class); verify(callInt, atLeastOnce()).futureCall(argumentCaptor.capture(), any(ApiCallContext.class)); @@ -264,6 +285,9 @@ void retryOnUnexpectedException() { HttpJsonCallableFactory.createUnaryCallable( callInt, callSettings, httpJsonCallSettings, clientContext); ApiException exception = assertThrows(ApiException.class, () -> callable.call(initialRequest)); + assertThat(tracerFactory.getInstance().getAttemptsStarted().get()).isEqualTo(1); + assertThat(tracerFactory.getInstance().getAttemptsFailed().get()).isEqualTo(0); + assertThat(tracerFactory.getInstance().getRetriesExhausted().get()).isFalse(); assertThat(exception).hasCauseThat().isSameInstanceAs(throwable); // Capture the argument passed to futureCall ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Integer.class); @@ -293,6 +317,9 @@ void retryNoRecover() { HttpJsonCallableFactory.createUnaryCallable( callInt, callSettings, httpJsonCallSettings, clientContext); ApiException exception = assertThrows(ApiException.class, () -> callable.call(initialRequest)); + assertThat(tracerFactory.getInstance().getAttemptsStarted().get()).isEqualTo(1); + assertThat(tracerFactory.getInstance().getAttemptsFailed().get()).isEqualTo(0); + assertThat(tracerFactory.getInstance().getRetriesExhausted().get()).isFalse(); assertThat(exception).isSameInstanceAs(apiException); // Capture the argument passed to futureCall ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Integer.class); @@ -319,6 +346,10 @@ void retryKeepFailing() { UncheckedExecutionException exception = assertThrows(UncheckedExecutionException.class, () -> Futures.getUnchecked(future)); + assertThat(tracerFactory.getInstance().getAttemptsFailed().get()).isGreaterThan(0); + assertThat(tracerFactory.getInstance().getAttemptsFailed().get()) + .isEqualTo(tracerFactory.getInstance().getAttemptsStarted().get()); + assertThat(tracerFactory.getInstance().getRetriesExhausted().get()).isTrue(); assertThat(exception).hasCauseThat().isInstanceOf(ApiException.class); assertThat(exception).hasCauseThat().hasMessageThat().contains("Unavailable"); // Capture the argument passed to futureCall @@ -359,6 +390,9 @@ void testKnownStatusCode() { callInt, callSettings, httpJsonCallSettings, clientContext); ApiException exception = assertThrows(FailedPreconditionException.class, () -> callable.call(initialRequest)); + assertThat(tracerFactory.getInstance().getAttemptsStarted().get()).isEqualTo(1); + assertThat(tracerFactory.getInstance().getAttemptsFailed().get()).isEqualTo(0); + assertThat(tracerFactory.getInstance().getRetriesExhausted().get()).isFalse(); assertThat(exception.getStatusCode().getTransportCode()) .isEqualTo(HTTP_CODE_PRECONDITION_FAILED); assertThat(exception).hasMessageThat().contains("precondition failed"); @@ -383,6 +417,9 @@ void testUnknownStatusCode() { UnknownException exception = assertThrows(UnknownException.class, () -> callable.call(initialRequest)); assertThat(exception).hasMessageThat().isEqualTo("java.lang.RuntimeException: unknown"); + assertThat(tracerFactory.getInstance().getAttemptsStarted().get()).isEqualTo(1); + assertThat(tracerFactory.getInstance().getAttemptsFailed().get()).isEqualTo(0); + assertThat(tracerFactory.getInstance().getRetriesExhausted().get()).isFalse(); // Capture the argument passed to futureCall ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(Integer.class); verify(callInt, atLeastOnce()).futureCall(argumentCaptor.capture(), any(ApiCallContext.class)); diff --git a/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/testing/TestApiTracer.java b/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/testing/TestApiTracer.java new file mode 100644 index 0000000000..7a95e94949 --- /dev/null +++ b/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/testing/TestApiTracer.java @@ -0,0 +1,81 @@ +/* + * Copyright 2024 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.httpjson.testing; + +import com.google.api.gax.tracing.ApiTracer; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import org.threeten.bp.Duration; + +/** + * Test tracer that keeps count of different events. See {@link TestApiTracerFactory} for more + * details. + */ +public class TestApiTracer implements ApiTracer { + + private final AtomicInteger attemptsStarted = new AtomicInteger(); + private final AtomicInteger attemptsFailed = new AtomicInteger(); + private final AtomicBoolean retriesExhausted = new AtomicBoolean(false); + + public TestApiTracer() {} + + public AtomicInteger getAttemptsStarted() { + return attemptsStarted; + } + + public AtomicInteger getAttemptsFailed() { + return attemptsFailed; + } + + public AtomicBoolean getRetriesExhausted() { + return retriesExhausted; + } + + @Override + public void attemptStarted(int attemptNumber) { + attemptsStarted.incrementAndGet(); + } + + @Override + public void attemptStarted(Object request, int attemptNumber) { + attemptsStarted.incrementAndGet(); + } + + @Override + public void attemptFailed(Throwable error, Duration delay) { + attemptsFailed.incrementAndGet(); + } + + @Override + public void attemptFailedRetriesExhausted(Throwable error) { + attemptsFailed.incrementAndGet(); + retriesExhausted.set(true); + } +}; diff --git a/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/testing/TestApiTracerFactory.java b/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/testing/TestApiTracerFactory.java new file mode 100644 index 0000000000..77f05a6cd2 --- /dev/null +++ b/gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/testing/TestApiTracerFactory.java @@ -0,0 +1,52 @@ +/* + * Copyright 2024 Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package com.google.api.gax.httpjson.testing; + +import com.google.api.gax.tracing.ApiTracer; +import com.google.api.gax.tracing.ApiTracerFactory; +import com.google.api.gax.tracing.SpanName; + +/** + * Produces a {@link TestApiTracer}, which keeps count of the attempts made and attempts + * made-and-failed. It also keeps count of the operations failed and when the retries have been + * exhausted. + */ +public class TestApiTracerFactory implements ApiTracerFactory { + private final TestApiTracer instance = new TestApiTracer(); + + public TestApiTracer getInstance() { + return instance; + } + + @Override + public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) { + return instance; + } +} diff --git a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java index 55b07a851b..f386ba2e73 100644 --- a/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java +++ b/showcase/gapic-showcase/src/test/java/com/google/showcase/v1beta1/it/ITOtelMetrics.java @@ -82,7 +82,6 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; /** @@ -320,12 +319,10 @@ void testGrpc_operationSucceeded_recordsMetrics() throws InterruptedException { verifyStatusAttribute(actualMetricDataList, statusCountList); } - @Disabled("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test void testHttpJson_operationSucceeded_recordsMetrics() throws InterruptedException { int attemptCount = 1; - EchoRequest echoRequest = - EchoRequest.newBuilder().setContent("test_http_operation_succeeded").build(); + EchoRequest echoRequest = EchoRequest.newBuilder().setContent("content").build(); httpClient.echo(echoRequest); List actualMetricDataList = getMetricDataList(); @@ -373,7 +370,6 @@ void testGrpc_operationCancelled_recordsMetrics() throws Exception { verifyStatusAttribute(actualMetricDataList, statusCountList); } - @Disabled("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test void testHttpJson_operationCancelled_recordsMetrics() throws Exception { int attemptCount = 1; @@ -430,7 +426,6 @@ void testGrpc_operationFailed_recordsMetrics() throws InterruptedException { verifyStatusAttribute(actualMetricDataList, statusCountList); } - @Disabled("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test void testHttpJson_operationFailed_recordsMetrics() throws InterruptedException { int attemptCount = 1; @@ -526,7 +521,6 @@ void testGrpc_attemptFailedRetriesExhausted_recordsMetrics() throws Exception { grpcClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } - @Disabled("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test void testHttpJson_attemptFailedRetriesExhausted_recordsMetrics() throws Exception { int attemptCount = 3; @@ -621,7 +615,6 @@ void testGrpc_attemptPermanentFailure_recordsMetrics() throws InterruptedExcepti verifyStatusAttribute(actualMetricDataList, statusCountList); } - @Disabled("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test void testHttpJson_attemptPermanentFailure_recordsMetrics() throws InterruptedException { int attemptCount = 1; @@ -722,7 +715,6 @@ void testGrpc_multipleFailedAttempts_successfulOperation() throws Exception { grpcClient.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS); } - @Disabled("https://github.com/googleapis/sdk-platform-java/issues/2503") @Test void testHttpJson_multipleFailedAttempts_successfulOperation() throws Exception { int attemptCount = 3; @@ -737,7 +729,7 @@ void testHttpJson_multipleFailedAttempts_successfulOperation() throws Exception EchoStubSettings.Builder httpJsonEchoSettingsBuilder = EchoStubSettings.newHttpJsonBuilder(); httpJsonEchoSettingsBuilder - .echoSettings() + .blockSettings() .setRetrySettings(retrySettings) .setRetryableCodes(ImmutableSet.of(Code.DEADLINE_EXCEEDED)); EchoSettings httpJsonEchoSettings = EchoSettings.create(httpJsonEchoSettingsBuilder.build()); @@ -771,7 +763,7 @@ void testHttpJson_multipleFailedAttempts_successfulOperation() throws Exception .setSuccess(BlockResponse.newBuilder().setContent("httpjsonBlockResponse")) .build(); - grpcClient.block(blockRequest); + httpClient.block(blockRequest); List actualMetricDataList = getMetricDataList(); verifyPointDataSum(actualMetricDataList, attemptCount);