Add detail to circuit breaker failures

5 files changed, 23 insertions, 7 deletions

diff --git a/vespa-feed-client/abi-spec.json b/vespa-feed-client/abi-spec.json
index e1fddfeefef..07d02c81291 100644
--- a/vespa-feed-client/abi-spec.json
+++ b/vespa-feed-client/abi-spec.json
@@ -81,6 +81,7 @@
     "methods": [
       "public void success()",
       "public void failure()",
+      "public void failure(java.lang.String)",
       "public abstract ai.vespa.feed.client.FeedClient$CircuitBreaker$State state()"
     ],
     "fields": [
@@ -196,7 +197,7 @@
     "methods": [
       "public void <init>(java.time.Duration, java.time.Duration)",
       "public void success()",
-      "public void failure()",
+      "public void failure(java.lang.String)",
       "public ai.vespa.feed.client.FeedClient$CircuitBreaker$State state()"
     ],
     "fields": []
diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/ApacheCluster.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/ApacheCluster.java
index bf407c60075..6b6d7f82597 100644
--- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/ApacheCluster.java
+++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/ApacheCluster.java
@@ -18,12 +18,14 @@ import org.apache.hc.core5.util.Timeout;
 import javax.net.ssl.SSLContext;
 import java.io.IOException;
 import java.net.URI;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.apache.hc.core5.http.ssl.TlsCiphers.excludeH2Blacklisted;
 import static org.apache.hc.core5.http.ssl.TlsCiphers.excludeWeak;
 
@@ -161,6 +163,12 @@ class ApacheCluster implements Cluster {
             return wrapped.getBodyBytes();
         }
 
+        @Override
+        public String toString() {
+            return "HTTP response with code " + code() +
+                   (body() != null ? " and body '" + new String(body(), UTF_8) + "'" : "");
+        }
+
     }
 
 }
diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/FeedClient.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/FeedClient.java
index 65ce8efe107..6e9572ba154 100644
--- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/FeedClient.java
+++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/FeedClient.java
@@ -69,7 +69,10 @@ public interface FeedClient extends Closeable {
         default void success() { }
 
         /** Called by the client whenever a transient or fatal error occurs. */
-        default void failure() { }
+        default void failure() { failure(null); }
+
+        /** Called by the client whenever a transient or fatal error occurs. */
+        default void failure(String message) { }
 
         /** The current state of the circuit breaker. */
         State state();
diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/GracePeriodCircuitBreaker.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/GracePeriodCircuitBreaker.java
index 814d0283140..ed1ab7113df 100644
--- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/GracePeriodCircuitBreaker.java
+++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/GracePeriodCircuitBreaker.java
@@ -4,6 +4,7 @@ package ai.vespa.feed.client;
 import java.time.Duration;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.LongSupplier;
 import java.util.logging.Logger;
 
@@ -26,6 +27,7 @@ public class GracePeriodCircuitBreaker implements FeedClient.CircuitBreaker {
     private final AtomicBoolean halfOpen = new AtomicBoolean(false);
     private final AtomicBoolean open = new AtomicBoolean(false);
     private final LongSupplier clock;
+    private final AtomicReference<String> detail = new AtomicReference<>();
     private final long graceMillis;
     private final long doomMillis;
 
@@ -53,8 +55,9 @@ public class GracePeriodCircuitBreaker implements FeedClient.CircuitBreaker {
     }
 
     @Override
-    public void failure() {
-        failingSinceMillis.compareAndSet(NEVER, clock.getAsLong());
+    public void failure(String detail) {
+        if (failingSinceMillis.compareAndSet(NEVER, clock.getAsLong()))
+            this.detail.set(detail);
     }
 
     @Override
@@ -63,7 +66,8 @@ public class GracePeriodCircuitBreaker implements FeedClient.CircuitBreaker {
         if (failingMillis > graceMillis && halfOpen.compareAndSet(false, true))
             log.log(INFO, "Circuit breaker is now half-open, as no requests have succeeded for the " +
                           "last " + failingMillis + "ms. The server will be pinged to see if it recovers, " +
-                          "but this client will give up if no successes are observed within " + doomMillis + "ms");
+                          "but this client will give up if no successes are observed within " + doomMillis + "ms. " +
+                          "First failure was '" + detail.get() + "'.");
 
         if (failingMillis > doomMillis && open.compareAndSet(false, true))
             log.log(WARNING, "Circuit breaker is now open, after " + doomMillis + "ms of failing request, " +
diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/HttpRequestStrategy.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/HttpRequestStrategy.java
index 667f3489cf7..51facb0fa0d 100644
--- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/HttpRequestStrategy.java
+++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/HttpRequestStrategy.java
@@ -135,7 +135,7 @@ class HttpRequestStrategy implements RequestStrategy {
      * or the user has turned off retries for this type of operation.
      */
     private boolean retry(HttpRequest request, Throwable thrown, int attempt) {
-        breaker.failure();
+        breaker.failure(thrown.getMessage());
         if (   (thrown instanceof IOException)               // General IO problems.
             || (thrown instanceof CancellationException)     // TLS session disconnect.
             || (thrown instanceof CancelledKeyException)) {  // Selection cancelled.
@@ -163,7 +163,7 @@ class HttpRequestStrategy implements RequestStrategy {
             return true;
         }
 
-        breaker.failure();
+        breaker.failure(response.toString());
         logResponse(FINE, response, request, attempt);
         if (response.code() == 500 || response.code() == 502 || response.code() == 504) { // Hopefully temporary errors.
             return retry(request, attempt);