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 new file mode 100644 index 000000000..cab01d288 --- /dev/null +++ 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 listeners; + + public CompoundRetryListener(final RetryListener... listeners) { + this(Arrays.asList(listeners)); + } + + public CompoundRetryListener(final Collection 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 index 79868fb36..058953ebf 100644 --- 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 failsafe() { return circuitBreaker == null ? failsafe : failsafe.with(circuitBreaker); } - private static final class RetryListenersAdapter extends Listeners { + @VisibleForTesting + static final class RetryListenersAdapter extends Listeners { 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 new file mode 100644 index 000000000..04c41764e --- /dev/null +++ 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 new file mode 100644 index 000000000..62a2afad1 --- /dev/null +++ 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 new file mode 100644 index 000000000..86fd5045c --- /dev/null +++ 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 index 2577af245..044a8ab29 100644 --- 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 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(); } }