fix: httpjson callables to trace attempts (started, failed)

gax-java/gax-httpjson/src/main/java/com/google/api/gax/httpjson/HttpJsonCallableFactory.java

@@ -84,6 +84,11 @@ static <RequestT, ResponseT> UnaryCallable<RequestT, ResponseT> 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 <RequestT, ResponseT> UnaryCallable<RequestT, ResponseT> createUna
     UnaryCallable<RequestT, ResponseT> innerCallable =
         createDirectUnaryCallable(httpJsonCallSettings);
 
-    innerCallable =
-        new TracedUnaryCallable<>(
-            innerCallable,
-            clientContext.getTracerFactory(),
-            getSpanName(httpJsonCallSettings.getMethodDescriptor()));
-
     return createUnaryCallable(innerCallable, callSettings, httpJsonCallSettings, clientContext);
   }
 

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<Integer, Integer> httpJsonCallSettings =
       HttpJsonCallSettings.<Integer, Integer>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<StatusCode.Code> retryable = ImmutableSet.of(Code.UNAVAILABLE);
     Mockito.when(callInt.futureCall((Integer) any(), (ApiCallContext) any()))
         .thenReturn(ApiFutures.<Integer>immediateFailedFuture(HTTP_SERVICE_UNAVAILABLE_EXCEPTION))
@@ -143,6 +149,10 @@ void retry() {
         HttpJsonCallableFactory.createUnaryCallable(
             callInt, callSettings, httpJsonCallSettings, clientContext);
     assertThat(callable.call(initialRequest)).isEqualTo(2);
+    assertThat(tracerFactory.getTracerAttemptsFailed().get()).isEqualTo(3);
+    assertThat(tracerFactory.getTracerAttempts().get()).isEqualTo(4);
+    assertThat(tracerFactory.getTracerOperationFailed().get()).isEqualTo(false);
+    assertThat(tracerFactory.getTracerFailedRetriesExhausted().get()).isEqualTo(false);
 
     // Capture the argument passed to futureCall
     ArgumentCaptor<Integer> argumentCaptor = ArgumentCaptor.forClass(Integer.class);
@@ -180,6 +190,9 @@ void retryTotalTimeoutExceeded() {
         HttpJsonCallableFactory.createUnaryCallable(
             callInt, callSettings, httpJsonCallSettings, clientContext);
     assertThrows(ApiException.class, () -> callable.call(initialRequest));
+    assertThat(tracerFactory.getTracerAttempts().get()).isEqualTo(1);
+    assertThat(tracerFactory.getTracerAttemptsFailed().get()).isEqualTo(0);
+    assertThat(tracerFactory.getTracerOperationFailed().get()).isEqualTo(true);
     // Capture the argument passed to futureCall
     ArgumentCaptor<Integer> argumentCaptor = ArgumentCaptor.forClass(Integer.class);
     verify(callInt, atLeastOnce()).futureCall(argumentCaptor.capture(), any(ApiCallContext.class));
@@ -200,6 +213,9 @@ void retryMaxAttemptsExceeded() {
         HttpJsonCallableFactory.createUnaryCallable(
             callInt, callSettings, httpJsonCallSettings, clientContext);
     assertThrows(ApiException.class, () -> callable.call(initialRequest));
+    assertThat(tracerFactory.getTracerAttempts().get()).isEqualTo(2);
+    assertThat(tracerFactory.getTracerAttemptsFailed().get()).isEqualTo(1);
+    assertThat(tracerFactory.getTracerFailedRetriesExhausted().get()).isEqualTo(true);
     // Capture the argument passed to futureCall
     ArgumentCaptor<Integer> argumentCaptor = ArgumentCaptor.forClass(Integer.class);
     verify(callInt, atLeastOnce()).futureCall(argumentCaptor.capture(), any(ApiCallContext.class));
@@ -220,6 +236,9 @@ void retryWithinMaxAttempts() {
         HttpJsonCallableFactory.createUnaryCallable(
             callInt, callSettings, httpJsonCallSettings, clientContext);
     assertThat(callable.call(initialRequest)).isEqualTo(2);
+    assertThat(tracerFactory.getTracerAttempts().get()).isEqualTo(3);
+    assertThat(tracerFactory.getTracerAttemptsFailed().get()).isEqualTo(2);
+    assertThat(tracerFactory.getTracerOperationFailed().get()).isEqualTo(false);
     // Capture the argument passed to futureCall
     ArgumentCaptor<Integer> argumentCaptor = ArgumentCaptor.forClass(Integer.class);
     verify(callInt, atLeastOnce()).futureCall(argumentCaptor.capture(), any(ApiCallContext.class));
@@ -246,6 +265,9 @@ void retryOnStatusUnknown() {
         HttpJsonCallableFactory.createUnaryCallable(
             callInt, callSettings, httpJsonCallSettings, clientContext);
     assertThat(callable.call(initialRequest)).isEqualTo(2);
+    assertThat(tracerFactory.getTracerAttempts().get()).isEqualTo(4);
+    assertThat(tracerFactory.getTracerAttemptsFailed().get()).isEqualTo(3);
+    assertThat(tracerFactory.getTracerOperationFailed().get()).isEqualTo(false);
     // Capture the argument passed to futureCall
     ArgumentCaptor<Integer> argumentCaptor = ArgumentCaptor.forClass(Integer.class);
     verify(callInt, atLeastOnce()).futureCall(argumentCaptor.capture(), any(ApiCallContext.class));
@@ -264,6 +286,9 @@ void retryOnUnexpectedException() {
         HttpJsonCallableFactory.createUnaryCallable(
             callInt, callSettings, httpJsonCallSettings, clientContext);
     ApiException exception = assertThrows(ApiException.class, () -> callable.call(initialRequest));
+    assertThat(tracerFactory.getTracerAttempts().get()).isEqualTo(1);
+    assertThat(tracerFactory.getTracerAttemptsFailed().get()).isEqualTo(0);
+    assertThat(tracerFactory.getTracerOperationFailed().get()).isEqualTo(true);
     assertThat(exception).hasCauseThat().isSameInstanceAs(throwable);
     // Capture the argument passed to futureCall
     ArgumentCaptor<Integer> argumentCaptor = ArgumentCaptor.forClass(Integer.class);
@@ -293,6 +318,9 @@ void retryNoRecover() {
         HttpJsonCallableFactory.createUnaryCallable(
             callInt, callSettings, httpJsonCallSettings, clientContext);
     ApiException exception = assertThrows(ApiException.class, () -> callable.call(initialRequest));
+    assertThat(tracerFactory.getTracerAttempts().get()).isEqualTo(1);
+    assertThat(tracerFactory.getTracerAttemptsFailed().get()).isEqualTo(0);
+    assertThat(tracerFactory.getTracerOperationFailed().get()).isEqualTo(true);
     assertThat(exception).isSameInstanceAs(apiException);
     // Capture the argument passed to futureCall
     ArgumentCaptor<Integer> argumentCaptor = ArgumentCaptor.forClass(Integer.class);
@@ -319,6 +347,13 @@ void retryKeepFailing() {
 
     UncheckedExecutionException exception =
         assertThrows(UncheckedExecutionException.class, () -> Futures.getUnchecked(future));
+    // the number of attempts varies. Here we just make sure that all of them except the last are
+    // considered as failed
+    // attempts and that the operation was considered as failed.
+    assertThat(tracerFactory.getTracerAttemptsFailed().get()).isGreaterThan(0);
+    assertThat(tracerFactory.getTracerAttemptsFailed().get())
+        .isEqualTo(tracerFactory.getTracerAttempts().get() - 1);
+    assertThat(tracerFactory.getTracerFailedRetriesExhausted().get()).isEqualTo(true);
     assertThat(exception).hasCauseThat().isInstanceOf(ApiException.class);
     assertThat(exception).hasCauseThat().hasMessageThat().contains("Unavailable");
     // Capture the argument passed to futureCall
@@ -359,6 +394,9 @@ void testKnownStatusCode() {
             callInt, callSettings, httpJsonCallSettings, clientContext);
     ApiException exception =
         assertThrows(FailedPreconditionException.class, () -> callable.call(initialRequest));
+    assertThat(tracerFactory.getTracerAttempts().get()).isEqualTo(1);
+    assertThat(tracerFactory.getTracerAttemptsFailed().get()).isEqualTo(0);
+    assertThat(tracerFactory.getTracerOperationFailed().get()).isEqualTo(true);
     assertThat(exception.getStatusCode().getTransportCode())
         .isEqualTo(HTTP_CODE_PRECONDITION_FAILED);
     assertThat(exception).hasMessageThat().contains("precondition failed");
@@ -383,6 +421,9 @@ void testUnknownStatusCode() {
     UnknownException exception =
         assertThrows(UnknownException.class, () -> callable.call(initialRequest));
     assertThat(exception).hasMessageThat().isEqualTo("java.lang.RuntimeException: unknown");
+    assertThat(tracerFactory.getTracerAttempts().get()).isEqualTo(1);
+    assertThat(tracerFactory.getTracerAttemptsFailed().get()).isEqualTo(0);
+    assertThat(tracerFactory.getTracerOperationFailed().get()).isEqualTo(true);
     // Capture the argument passed to futureCall
     ArgumentCaptor<Integer> argumentCaptor = ArgumentCaptor.forClass(Integer.class);
     verify(callInt, atLeastOnce()).futureCall(argumentCaptor.capture(), any(ApiCallContext.class));

gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/testing/TestApiTracer.java

 metricsRecorder.recordAttemptCount(1, attributes); 
 metricsRecorder.recordAttemptCount(1, attributes); 
@@ -0,0 +1,82 @@
+/*
+ * 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.BaseApiTracer;
+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 extends BaseApiTracer {
+
+  private final AtomicInteger tracerAttempts;
+  private final AtomicInteger tracerAttemptsFailed;
+  private final AtomicBoolean tracerOperationFailed;
+  private final AtomicBoolean tracerFailedRetriesExhausted;
+
+  public TestApiTracer(
+      AtomicInteger tracerAttempts,
+      AtomicInteger tracerAttemptsFailed,
+      AtomicBoolean tracerOperationFailed,
+      AtomicBoolean tracerFailedRetriesExhausted) {
+    this.tracerAttempts = tracerAttempts;
+    this.tracerAttemptsFailed = tracerAttemptsFailed;
+    this.tracerOperationFailed = tracerOperationFailed;
+    this.tracerFailedRetriesExhausted = tracerFailedRetriesExhausted;
+  }
+
+  @Override
+  public void attemptFailed(Throwable error, Duration delay) {
+    tracerAttemptsFailed.incrementAndGet();
+    super.attemptFailed(error, delay);
+  }
+
+  @Override
+  public void attemptStarted(int attemptNumber) {
+    tracerAttempts.incrementAndGet();
+    super.attemptStarted(attemptNumber);
+  }
+
+  @Override
+  public void operationFailed(Throwable error) {
+    tracerOperationFailed.set(true);
+    super.operationFailed(error);
+  }
+
+  @Override
+  public void attemptFailedRetriesExhausted(Throwable error) {
+    tracerFailedRetriesExhausted.set(true);
+    super.attemptFailedRetriesExhausted(error);
+  }
+};

gax-java/gax-httpjson/src/test/java/com/google/api/gax/httpjson/testing/TestApiTracerFactory.java

@@ -0,0 +1,75 @@
+/*
+ * 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;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+
+/**
+ * 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 AtomicInteger tracerAttempts = new AtomicInteger();
+  private final AtomicInteger tracerAttemptsFailed = new AtomicInteger();
+  private final AtomicBoolean tracerOperationFailed = new AtomicBoolean(false);
+  private final AtomicBoolean tracerFailedRetriesExhausted = new AtomicBoolean(false);
+  private final TestApiTracer instance =
+      new TestApiTracer(
+          tracerAttempts,
+          tracerAttemptsFailed,
+          tracerOperationFailed,
+          tracerFailedRetriesExhausted);
+
+  public AtomicInteger getTracerAttempts() {
+    return tracerAttempts;
+  }
+
+  public AtomicInteger getTracerAttemptsFailed() {
+    return tracerAttemptsFailed;
+  }
+
+  public AtomicBoolean getTracerOperationFailed() {
+    return tracerOperationFailed;
+  }
+
+  public AtomicBoolean getTracerFailedRetriesExhausted() {
+    return tracerFailedRetriesExhausted;
+  }
+
+  @Override
+  public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) {
+    return instance;
+  }
+}