feat: add toString to futures returned by operations (#3140)

igorbernstein2 · web-flow · commit afecb8c43c48 · 2024-08-30T16:47:31.000-04:00
Sometimes an operation can get stuck indefinitely. The underlying reasons can vary significantly: - the underlying attempt rpc can get stuck due to a bug in grpc (ie grpc/grpc-java#11026) - the operation can get stuck in layers above gax: googleapis/java-bigtable#1939 - or it can get stuck in gax itself (dont have a pointer handy) Guava futures provide some observability for ListenableFutures, but in creating the custom ApiFutures in gax, we lose that functionality. This PR sprinkles a few to toString to allow callers to inspect the internal state of the operation. For example with these changes, the toString() of the future returned from bigtableDataClient.mutateRows() changes from > TransformFuture@652ce654[status=PENDING, info=[inputFuture=[com.google.api.core.ApiFutureToListenableFuture@522ba524], function=[com.google.api.core.ApiFutures$ApiFunctionToGuavaFunction@29c5ee1d]]] to > ListenableFutureToApiFuture{delegate=TransformFuture@7ac9af2a[status=PENDING, info=[inputFuture=[ApiFutureToListenableFuture{apiFuture=CallbackChainRetryingFuture{super=com.google.api.gax.retrying.CallbackChainRetryingFuture@7bb004b8[status=PENDING], latestCompletedAttemptResult=null, attemptResult=null, attemptSettings=TimedAttemptSettings{globalSettings=RetrySettings{totalTimeout=PT10M, initialRetryDelay=PT0.01S, retryDelayMultiplier=2.0, maxRetryDelay=PT1M, maxAttempts=0, jittered=true, initialRpcTimeout=PT1M, rpcTimeoutMultiplier=1.0, maxRpcTimeout=PT1M}, retryDelay=PT0S, rpcTimeout=PT1M, randomizedRetryDelay=PT0S, attemptCount=0, overallAttemptCount=0, firstAttemptStartTimeNanos=635709620001791}}}], function=[com.google.api.core.ApiFutures$ApiFunctionToGuavaFunction@652ce654]]]} This allows us to reason about whats stuck. I'm working another PR that will add a close(timeout) to the Batcher that will use this functionality to identify why batcher.close() timed out
diff --git a/api-common-java/src/main/java/com/google/api/core/ApiFutureToListenableFuture.java b/api-common-java/src/main/java/com/google/api/core/ApiFutureToListenableFuture.java
@@ -29,6 +29,7 @@
  */
 package com.google.api.core;
 
+import com.google.common.base.MoreObjects;
 import com.google.common.util.concurrent.ListenableFuture;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
@@ -74,4 +75,11 @@ public V get(long l, TimeUnit timeUnit)
       throws InterruptedException, ExecutionException, TimeoutException {
     return apiFuture.get(l, timeUnit);
   }
+
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(ApiFutureToListenableFuture.class.getSimpleName())
+        .add("apiFuture", apiFuture)
+        .toString();
+  }
 }
diff --git a/api-common-java/src/main/java/com/google/api/core/ListenableFutureToApiFuture.java b/api-common-java/src/main/java/com/google/api/core/ListenableFutureToApiFuture.java
@@ -29,6 +29,7 @@
  */
 package com.google.api.core;
 
+import com.google.common.base.MoreObjects;
 import com.google.common.util.concurrent.ForwardingListenableFuture.SimpleForwardingListenableFuture;
 import com.google.common.util.concurrent.ListenableFuture;
 
@@ -39,4 +40,10 @@ public class ListenableFutureToApiFuture<V> extends SimpleForwardingListenableFu
   public ListenableFutureToApiFuture(ListenableFuture<V> delegate) {
     super(delegate);
   }
+
+  public String toString() {
+    return MoreObjects.toStringHelper(ListenableFutureToApiFuture.class)
+        .add("delegate", delegate())
+        .toString();
+  }
 }
diff --git a/api-common-java/src/test/java/com/google/api/core/ApiFutureToListenableFutureTest.java b/api-common-java/src/test/java/com/google/api/core/ApiFutureToListenableFutureTest.java
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2024, Google Inc.
+ *
+ * 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 Inc. 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.core;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import org.junit.jupiter.api.Test;
+
+class ApiFutureToListenableFutureTest {
+  @Test
+  void testThatInnerToStringIsNotLost() {
+    String customInnerToString = "my-custom-inner-tostring";
+    ApiFuture<String> apiFuture =
+        new AbstractApiFuture<String>() {
+          @Override
+          public String toString() {
+            return customInnerToString;
+          }
+        };
+    ApiFutureToListenableFuture<String> listenableFuture =
+        new ApiFutureToListenableFuture<>(apiFuture);
+    assertThat(listenableFuture.toString()).contains(customInnerToString);
+  }
+}
diff --git a/api-common-java/src/test/java/com/google/api/core/ListenableFutureToApiFutureTest.java b/api-common-java/src/test/java/com/google/api/core/ListenableFutureToApiFutureTest.java
@@ -30,6 +30,8 @@
 package com.google.api.core;
 
 import com.google.common.truth.Truth;
+import com.google.common.util.concurrent.AbstractFuture;
+import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.SettableFuture;
 import org.junit.jupiter.api.Test;
 
@@ -42,4 +44,20 @@ void testGet() throws Exception {
     future.set(3);
     Truth.assertThat(apiFuture.get()).isEqualTo(3);
   }
+
+  @Test
+  void testToStringShowsUnderlyingFutureToString() {
+    String customInnerFutureDesc = "my-custom-toString-impl";
+    ListenableFuture<String> listenableFuture =
+        new AbstractFuture<String>() {
+          @Override
+          public String toString() {
+            return customInnerFutureDesc;
+          }
+        };
+
+    ListenableFutureToApiFuture<String> apiFuture =
+        new ListenableFutureToApiFuture<>(listenableFuture);
+    Truth.assertThat(apiFuture.toString()).contains(customInnerFutureDesc);
+  }
 }
diff --git a/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java b/gax-java/gax/src/main/java/com/google/api/gax/retrying/BasicRetryingFuture.java
@@ -35,6 +35,7 @@
 import com.google.api.core.ApiFuture;
 import com.google.api.core.ApiFutures;
 import com.google.api.gax.tracing.ApiTracer;
+import com.google.common.base.MoreObjects;
 import com.google.common.util.concurrent.AbstractFuture;
 import com.google.common.util.concurrent.MoreExecutors;
 import java.util.concurrent.Callable;
@@ -265,6 +266,16 @@ private void setAttemptResult(Throwable throwable, ResponseT response, boolean s
     }
   }
 
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this.getClass())
+        .add("super", pendingToString())
+        .add("latestCompletedAttemptResult", this.latestCompletedAttemptResult)
+        .add("attemptResult", this.attemptResult)
+        .add("attemptSettings", this.attemptSettings)
+        .toString();
+  }
+
   private class CompletionListener implements Runnable {
     @Override
     public void run() {
