Merge pull request #441 from zalando/feature/logging-retry-listener

Added logging retry listener
zalando · Jul 13, 2018 · 5241d13 · 5241d13
2 parents 26ec5f4 + 509d028
commit 5241d13
Show file tree

Hide file tree

Showing 6 changed files with 198 additions and 7 deletions.
diff --git a/riptide-failsafe/src/main/java/org/zalando/riptide/failsafe/CompoundRetryListener.java b/riptide-failsafe/src/main/java/org/zalando/riptide/failsafe/CompoundRetryListener.java
@@ -0,0 +1,35 @@
+package org.zalando.riptide.failsafe;
+
+import net.jodah.failsafe.ExecutionContext;
+import org.apiguardian.api.API;
+import org.springframework.http.client.ClientHttpResponse;
+import org.zalando.riptide.RequestArguments;
+
+import javax.annotation.Nullable;
+import java.util.Arrays;
+import java.util.Collection;
+
+import static org.apiguardian.api.API.Status.EXPERIMENTAL;
+
+@API(status = EXPERIMENTAL)
+public final class CompoundRetryListener implements RetryListener {
+
+    private final Collection<RetryListener> listeners;
+
+    public CompoundRetryListener(final RetryListener... listeners) {
+        this(Arrays.asList(listeners));
+    }
+
+    public CompoundRetryListener(final Collection<RetryListener> listeners) {
+        this.listeners = listeners;
+    }
+
+    @Override
+    public void onRetry(final RequestArguments arguments, @Nullable final ClientHttpResponse result,
+            @Nullable final Throwable failure, final ExecutionContext context) {
+
+        listeners.forEach(listener ->
+            listener.onRetry(arguments, result, failure, context));
+    }
+
+}
diff --git a/riptide-failsafe/src/main/java/org/zalando/riptide/failsafe/FailsafePlugin.java b/riptide-failsafe/src/main/java/org/zalando/riptide/failsafe/FailsafePlugin.java
@@ -1,5 +1,6 @@
 package org.zalando.riptide.failsafe;
 
+import com.google.common.annotations.VisibleForTesting;
 import net.jodah.failsafe.CircuitBreaker;
 import net.jodah.failsafe.ExecutionContext;
 import net.jodah.failsafe.Listeners;
@@ -74,20 +75,21 @@ SyncFailsafe<Object> failsafe() {
         return circuitBreaker == null ? failsafe : failsafe.with(circuitBreaker);
     }
 
-    private static final class RetryListenersAdapter extends Listeners<ClientHttpResponse> {
+    @VisibleForTesting
+    static final class RetryListenersAdapter extends Listeners<ClientHttpResponse> {
 
         private final RequestArguments arguments;
-        private RetryListener listeners;
+        private RetryListener listener;
 
-        public RetryListenersAdapter(final RetryListener listeners, final RequestArguments arguments) {
+        public RetryListenersAdapter(final RetryListener listener, final RequestArguments arguments) {
             this.arguments = arguments;
-            this.listeners = listeners;
+            this.listener = listener;
         }
 
         @Override
         public void onRetry(final ClientHttpResponse result, final Throwable failure,
                 final ExecutionContext context) {
-            listeners.onRetry(arguments, result, failure, context);
+            listener.onRetry(arguments, result, failure, context);
         }
 
     }

diff --git a/riptide-failsafe/src/main/java/org/zalando/riptide/failsafe/LoggingRetryListener.java b/riptide-failsafe/src/main/java/org/zalando/riptide/failsafe/LoggingRetryListener.java
@@ -0,0 +1,36 @@
+package org.zalando.riptide.failsafe;
+
+import net.jodah.failsafe.ExecutionContext;
+import org.apiguardian.api.API;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.http.client.ClientHttpResponse;
+import org.zalando.riptide.RequestArguments;
+
+import javax.annotation.Nullable;
+
+import static org.apiguardian.api.API.Status.EXPERIMENTAL;
+
+@API(status = EXPERIMENTAL)
+public final class LoggingRetryListener implements RetryListener {
+
+    private final Logger logger;
+
+    public LoggingRetryListener() {
+        this(LoggerFactory.getLogger(LoggingRetryListener.class));
+    }
+
+    public LoggingRetryListener(final Logger logger) {
+        this.logger = logger;
+    }
+
+    @Override
+    public void onRetry(final RequestArguments arguments, @Nullable final ClientHttpResponse result,
+            @Nullable final Throwable failure, final ExecutionContext context) {
+
+        if (failure != null) {
+            logger.warn("Retrying failure", failure);
+        }
+    }
+
+}
diff --git a/riptide-failsafe/src/test/java/org/zalando/riptide/failsafe/CompoundRetryListenerTest.java b/riptide-failsafe/src/test/java/org/zalando/riptide/failsafe/CompoundRetryListenerTest.java
@@ -0,0 +1,43 @@
+package org.zalando.riptide.failsafe;
+
+import net.jodah.failsafe.Failsafe;
+import net.jodah.failsafe.RetryPolicy;
+import org.junit.Test;
+import org.zalando.riptide.RequestArguments;
+import org.zalando.riptide.failsafe.FailsafePlugin.RetryListenersAdapter;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+public final class CompoundRetryListenerTest {
+
+    private final RetryListener first = mock(RetryListener.class);
+    private final RetryListener second = mock(RetryListener.class);
+
+    private final RetryListener unit = new CompoundRetryListener(first, second);
+
+    @Test
+    public void shouldPropagateRetryToEveryListener() {
+        final AtomicBoolean success = new AtomicBoolean(false);
+
+        final RequestArguments arguments = RequestArguments.create();
+        final IllegalStateException exception = new IllegalStateException();
+
+        Failsafe.with(new RetryPolicy().withMaxRetries(3))
+                .with(new RetryListenersAdapter(unit, arguments))
+                .run(() -> {
+                    if (!success.getAndSet(true)) {
+                        throw exception;
+                    }
+                });
+
+        verify(first).onRetry(eq(arguments), isNull(), eq(exception), any());
+        verify(second).onRetry(eq(arguments), isNull(), eq(exception), any());
+    }
+
+}
diff --git a/riptide-failsafe/src/test/java/org/zalando/riptide/failsafe/LoggingRetryListenerTest.java b/riptide-failsafe/src/test/java/org/zalando/riptide/failsafe/LoggingRetryListenerTest.java
@@ -0,0 +1,70 @@
+package org.zalando.riptide.failsafe;
+
+import com.google.gag.annotation.remark.Hack;
+import net.jodah.failsafe.Failsafe;
+import net.jodah.failsafe.RetryPolicy;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.zalando.riptide.RequestArguments;
+import org.zalando.riptide.failsafe.FailsafePlugin.RetryListenersAdapter;
+
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+
+public final class LoggingRetryListenerTest {
+
+    private final Logger logger = mock(Logger.class);
+    private final RetryListener unit = new LoggingRetryListener(logger);
+
+    @Test
+    public void shouldLogFailure() {
+        final AtomicBoolean success = new AtomicBoolean(false);
+
+        final RequestArguments arguments = RequestArguments.create();
+        final IllegalStateException exception = new IllegalStateException();
+
+        Failsafe.with(new RetryPolicy().withMaxRetries(3))
+                .with(new RetryListenersAdapter(unit, arguments))
+                .run(() -> {
+                    if (!success.getAndSet(true)) {
+                        throw exception;
+                    }
+                });
+
+        verify(logger).warn(any(), eq(exception));
+    }
+
+    @Test
+    public void shouldNotLogResults() {
+        final AtomicBoolean success = new AtomicBoolean(false);
+
+        final RequestArguments arguments = RequestArguments.create();
+
+        Failsafe.with(new RetryPolicy()
+                    .withMaxRetries(3)
+                    .retryIf(Objects::isNull))
+                .with(new RetryListenersAdapter(unit, arguments))
+                .get(() -> {
+                    if (!success.getAndSet(true)) {
+                        return null;
+                    }
+
+                    return "not null";
+                });
+
+        verifyNoMoreInteractions(logger);
+    }
+
+    @Hack("We're not really testing anything here, since we don't want to clutter the logs.")
+    @Test
+    public void shouldUseDefaultLogger() {
+        new LoggingRetryListener();
+    }
+
+}
diff --git a/riptide-spring-boot-starter/src/main/java/org/zalando/riptide/spring/Metrics.java b/riptide-spring-boot-starter/src/main/java/org/zalando/riptide/spring/Metrics.java
@@ -5,6 +5,8 @@
 import io.micrometer.core.instrument.Tag;
 import org.zalando.riptide.Plugin;
 import org.zalando.riptide.failsafe.CircuitBreakerListener;
+import org.zalando.riptide.failsafe.CompoundRetryListener;
+import org.zalando.riptide.failsafe.LoggingRetryListener;
 import org.zalando.riptide.failsafe.RetryListener;
 import org.zalando.riptide.failsafe.metrics.MetricsCircuitBreakerListener;
 import org.zalando.riptide.failsafe.metrics.MetricsRetryListener;
@@ -32,10 +34,13 @@ public static CircuitBreakerListener getDefaultCircuitBreakerListener() {
 
     public static RetryListener createRetryListener(final MeterRegistry registry,
             final ImmutableList<Tag> defaultTags) {
-        return new MetricsRetryListener(registry).withDefaultTags(defaultTags);
+        return new CompoundRetryListener(
+                new MetricsRetryListener(registry).withDefaultTags(defaultTags),
+                new LoggingRetryListener()
+        );
     }
 
     public static RetryListener getDefaultRetryListener() {
-        return RetryListener.DEFAULT;
+        return new LoggingRetryListener();
     }
 }