Refactored TaggedTimeLimiterMetrics to be similar to TaggedCircuitBreakerMetrics (ReactiveX#601)

Author: RobWin

resilience4j-micrometer/src/main/java/io/github/resilience4j/micrometer/tagged/TaggedTimeLimiterMetrics.java

@@ -34,6 +34,10 @@
  */
 public class TaggedTimeLimiterMetrics extends AbstractMetrics implements MeterBinder {
 
+    private static final String KIND_FAILED = "failed";
+    private static final String KIND_SUCCESSFUL = "successful";
+    private static final String KIND_TIMEOUT = "timeout";
+
     /**
      * Creates a new binder that uses given {@code registry} as source of time limiters.
      *
@@ -78,17 +82,20 @@ public void bindTo(MeterRegistry registry) {
     }
 
     private void addMetrics(MeterRegistry registry, TimeLimiter timeLimiter) {
-        Counter successes = Counter.builder(names.getSuccessfulMetricName())
+        Counter successes = Counter.builder(names.getCallsMetricName())
                 .description("The number of successful calls")
                 .tag(TagNames.NAME, timeLimiter.getName())
+                .tag(TagNames.KIND, KIND_SUCCESSFUL)
                 .register(registry);
-        Counter failures = Counter.builder(names.getFailedMetricName())
+        Counter failures = Counter.builder(names.getCallsMetricName())
                 .description("The number of failed calls")
                 .tag(TagNames.NAME, timeLimiter.getName())
+                .tag(TagNames.KIND, KIND_FAILED)
                 .register(registry);
-        Counter timeouts = Counter.builder(names.getTimeoutMetricName())
+        Counter timeouts = Counter.builder(names.getCallsMetricName())
                 .description("The number of timed out calls")
                 .tag(TagNames.NAME, timeLimiter.getName())
+                .tag(TagNames.KIND, KIND_TIMEOUT)
                 .register(registry);
 
         timeLimiter.getEventPublisher()
@@ -106,9 +113,9 @@ private void addMetrics(MeterRegistry registry, TimeLimiter timeLimiter) {
     public static class MetricNames {
 
         private static final String DEFAULT_PREFIX = "resilience4j.timelimiter";
-        public static final String SUCCESSFUL_METRIC_NAME = DEFAULT_PREFIX + ".successful";
-        public static final String FAILED_METRIC_NAME = DEFAULT_PREFIX + ".failed";
-        public static final String TIMEOUT_METRIC_NAME = DEFAULT_PREFIX + ".timeout";
+        public static final String DEFAULT_TIME_LIMITER_CALLS = DEFAULT_PREFIX + ".calls";
+
+        private String callsMetricName = DEFAULT_TIME_LIMITER_CALLS;
 
         /**
          * Returns a builder for creating custom metric names.
@@ -129,35 +136,11 @@ public static MetricNames ofDefaults() {
             return new MetricNames();
         }
 
-        private String successfulMetricName = SUCCESSFUL_METRIC_NAME;
-        private String failedMetricName = FAILED_METRIC_NAME;
-        private String timeoutMetricName = TIMEOUT_METRIC_NAME;
-
-        /**
-         * Returns the metric name for successful calls, defaults to {@value SUCCESSFUL_METRIC_NAME}.
-         *
-         * @return The successful calls metric name.
-         */
-        public String getSuccessfulMetricName() {
-            return successfulMetricName;
-        }
-
-        /**
-         * Returns the metric name for failed calls, defaults to {@value FAILED_METRIC_NAME}.
-         *
-         * @return The failed calls metric name.
-         */
-        public String getFailedMetricName() {
-            return failedMetricName;
-        }
-
-        /**
-         * Returns the metric name for timed out calls, defaults to {@value TIMEOUT_METRIC_NAME}.
-         *
-         * @return The timed out calls metric name.
+        /** Returns the metric name for circuit breaker calls, defaults to {@value DEFAULT_TIME_LIMITER_CALLS}.
+         * @return The circuit breaker calls metric name.
          */
-        public String getTimeoutMetricName() {
-            return timeoutMetricName;
+        public String getCallsMetricName() {
+            return callsMetricName;
         }
 
         /**
@@ -167,36 +150,11 @@ public static class Builder {
 
             private final MetricNames metricNames = new MetricNames();
 
-            /**
-             * Overrides the default metric name {@value MetricNames#SUCCESSFUL_METRIC_NAME} with a given one.
-             *
-             * @param successfulMetricName The successful calls metric name.
-             * @return The builder.
-             */
-            public Builder successfulMetricName(String successfulMetricName) {
-                metricNames.successfulMetricName = requireNonNull(successfulMetricName);
-                return this;
-            }
-
-            /**
-             * Overrides the default metric name {@value MetricNames#FAILED_METRIC_NAME} with a given one.
-             *
-             * @param failedMetricName The failed calls metric name.
-             * @return The builder.
-             */
-            public Builder failedMetricName(String failedMetricName) {
-                metricNames.failedMetricName = requireNonNull(failedMetricName);
-                return this;
-            }
-
-            /**
-             * Overrides the default metric name {@value MetricNames#TIMEOUT_METRIC_NAME} with a given one.
-             *
-             * @param timeoutMetricName The timed out calls metric name.
-             * @return The builder.
-             */
-            public Builder timeoutMetricName(String timeoutMetricName) {
-                metricNames.timeoutMetricName = requireNonNull(timeoutMetricName);
+            /** Overrides the default metric name {@value TaggedTimeLimiterMetrics.MetricNames#DEFAULT_TIME_LIMITER_CALLS} with a given one.
+             * @param callsMetricName The calls metric name.
+             * @return The builder.*/
+            public TaggedTimeLimiterMetrics.MetricNames.Builder callsMetricName(String callsMetricName) {
+                metricNames.callsMetricName = requireNonNull(callsMetricName);
                 return this;
             }
 

resilience4j-micrometer/src/test/java/io/github/resilience4j/micrometer/tagged/TaggedTimeLimiterMetricsTest.java

@@ -28,21 +28,20 @@
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.stream.Collectors;
 
 import org.junit.Before;
 import org.junit.Test;
 
-import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.findCounterByNamesTag;
+import static io.github.resilience4j.micrometer.tagged.MetricsTestHelper.*;
+import static io.github.resilience4j.micrometer.tagged.TaggedCircuitBreakerMetrics.MetricNames.DEFAULT_CIRCUIT_BREAKER_BUFFERED_CALLS;
+import static io.github.resilience4j.micrometer.tagged.TaggedTimeLimiterMetrics.MetricNames.DEFAULT_TIME_LIMITER_CALLS;
 import static org.assertj.core.api.Assertions.assertThat;
 
 public class TaggedTimeLimiterMetricsTest {
 
-    private static final String DEFAULT_PREFIX = "resilience4j.timelimiter";
-    private static final String SUCCESSFUL = DEFAULT_PREFIX + ".successful";
-    private static final String FAILED = DEFAULT_PREFIX + ".failed";
-    private static final String TIMEOUT = DEFAULT_PREFIX + ".timeout";
-
     private MeterRegistry meterRegistry;
     private TimeLimiter timeLimiter;
     private TimeLimiterRegistry timeLimiterRegistry;
@@ -61,17 +60,18 @@ public void setUp() {
     @Test
     public void shouldAddMetricsForANewlyCreatedTimeLimiter() {
         TimeLimiter newTimeLimiter = timeLimiterRegistry.timeLimiter("backendB");
+        newTimeLimiter.onSuccess();
 
         assertThat(taggedTimeLimiterMetrics.meterIdMap).containsKeys("backendA", "backendB");
         assertThat(taggedTimeLimiterMetrics.meterIdMap.get("backendA")).hasSize(3);
         assertThat(taggedTimeLimiterMetrics.meterIdMap.get("backendB")).hasSize(3);
 
         assertThat(meterRegistry.getMeters()).hasSize(6);
 
-        Collection<Counter> counters = meterRegistry.get(SUCCESSFUL).counters();
+        Collection<Counter> counters = meterRegistry.get(DEFAULT_TIME_LIMITER_CALLS).counters();
 
-        Optional<Counter> successful = findCounterByNamesTag(counters, newTimeLimiter.getName());
-        assertThat(successful).map(Counter::count).contains(0d);
+        Optional<Counter> successful = findCounterByKindAndNameTags(counters, "successful", newTimeLimiter.getName());
+        assertThat(successful).map(Counter::count).contains(1d);
     }
 
     @Test
@@ -88,69 +88,53 @@ public void shouldRemovedMetricsForRemovedRetry() {
 
     @Test
     public void shouldReplaceMetrics() {
-        Counter before = meterRegistry.get(SUCCESSFUL).counter();
+        Counter before = meterRegistry.get(DEFAULT_TIME_LIMITER_CALLS).counter();
         assertThat(before).isNotNull();
         assertThat(before.count()).isEqualTo(0);
         assertThat(before.getId().getTag(TagNames.NAME)).isEqualTo(timeLimiter.getName());
 
         timeLimiterRegistry.replace(timeLimiter.getName(), TimeLimiter.ofDefaults());
 
-        Counter after = meterRegistry.get(SUCCESSFUL).counter();
+        Counter after = meterRegistry.get(DEFAULT_TIME_LIMITER_CALLS).counter();
         assertThat(after).isNotNull();
         assertThat(after.count()).isEqualTo(0);
         assertThat(after.getId().getTag(TagNames.NAME)).isEqualTo(TimeLimiter.ofDefaults().getName());
     }
 
     @Test
     public void successfulCounterIsRegistered() {
-        Counter successful = meterRegistry.get(SUCCESSFUL).counter();
+        Collection<Counter> counters = meterRegistry.get(DEFAULT_TIME_LIMITER_CALLS).counters();
+        timeLimiter.onSuccess();
 
-        assertThat(successful).isNotNull();
-        assertThat(successful.count()).isEqualTo(0);
-        assertThat(successful.getId().getTag(TagNames.NAME)).isEqualTo(timeLimiter.getName());
+        Optional<Counter> successful = findCounterByKindAndNameTags(counters, "successful", timeLimiter.getName());
+        assertThat(successful).map(Counter::count).contains(1d);
     }
 
     @Test
     public void failedCounterIsRegistered() {
-        Counter failed = meterRegistry.get(FAILED).counter();
+        Collection<Counter> counters = meterRegistry.get(DEFAULT_TIME_LIMITER_CALLS).counters();
+        timeLimiter.onError(new RuntimeException());
 
-        assertThat(failed).isNotNull();
-        assertThat(failed.count()).isEqualTo(0);
-        assertThat(failed.getId().getTag(TagNames.NAME)).isEqualTo(timeLimiter.getName());
+        Optional<Counter> failed = findCounterByKindAndNameTags(counters, "failed", timeLimiter.getName());
+        assertThat(failed).map(Counter::count).contains(1d);
     }
 
     @Test
     public void timoutCounterIsRegistered() {
-        Counter timout = meterRegistry.get(TIMEOUT).counter();
+        Collection<Counter> counters = meterRegistry.get(DEFAULT_TIME_LIMITER_CALLS).counters();
+        timeLimiter.onError(new TimeoutException());
 
-        assertThat(timout).isNotNull();
-        assertThat(timout.count()).isEqualTo(0);
-        assertThat(timout.getId().getTag(TagNames.NAME)).isEqualTo(timeLimiter.getName());
+        Optional<Counter> timeout = findCounterByKindAndNameTags(counters, "timeout", timeLimiter.getName());
+        assertThat(timeout).map(Counter::count).contains(1d);
     }
-
-    @Test
-    public void successfulCounterIsIncremented() throws Exception {
-        Callable<String> decoratedSupplier = timeLimiter.decorateFutureSupplier(() ->
-                CompletableFuture.completedFuture("Hello world"));
-
-        decoratedSupplier.call();
-        decoratedSupplier.call();
-
-        assertThat(meterRegistry.get(SUCCESSFUL).counter().count()).isEqualTo(2);
-        assertThat(meterRegistry.get(FAILED).counter().count()).isEqualTo(0);
-        assertThat(meterRegistry.get(TIMEOUT).counter().count()).isEqualTo(0);
-    }
-
     @Test
     public void customMetricNamesGetApplied() {
         MeterRegistry meterRegistry = new SimpleMeterRegistry();
         TimeLimiterRegistry timeLimiterRegistry = TimeLimiterRegistry.ofDefaults();
         timeLimiterRegistry.timeLimiter("backendA");
         TaggedTimeLimiterMetrics.ofTimeLimiterRegistry(
                 TaggedTimeLimiterMetrics.MetricNames.custom()
-                        .successfulMetricName("custom_successful")
-                        .failedMetricName("custom_failed")
-                        .timeoutMetricName("custom_timeout")
+                        .callsMetricName("custom_calls")
                         .build(),
                 timeLimiterRegistry
         ).bindTo(meterRegistry);
@@ -162,9 +146,7 @@ public void customMetricNamesGetApplied() {
                 .collect(Collectors.toSet());
 
         assertThat(metricNames).hasSameElementsAs(Arrays.asList(
-                "custom_successful",
-                "custom_failed",
-                "custom_timeout"
+                "custom_calls"
         ));
     }
 }