Manage failures in micrometer Counter instead of Gauge for consistency

aureamunoz · Sep 29, 2023 · 3be8196 · 3be8196
1 parent bafbe0d
commit 3be8196
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 76 deletions.
diff --git a/...c/test/java/io/quarkus/micrometer/deployment/binder/StorkMetricsLoadBalancerFailTest.java b/...c/test/java/io/quarkus/micrometer/deployment/binder/StorkMetricsLoadBalancerFailTest.java
@@ -15,11 +15,11 @@
 import org.mockito.Mockito;
 
 import io.micrometer.core.instrument.Counter;
-import io.micrometer.core.instrument.Gauge;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.Timer;
 import io.quarkus.micrometer.runtime.binder.stork.StorkObservationCollectorBean;
+import io.quarkus.micrometer.test.GreetingResource;
 import io.quarkus.micrometer.test.MockServiceSelectorConfiguration;
 import io.quarkus.micrometer.test.MockServiceSelectorProvider;
 import io.quarkus.micrometer.test.MockServiceSelectorProviderLoader;
@@ -35,14 +35,19 @@ public class StorkMetricsLoadBalancerFailTest {
     @RegisterExtension
     static final QuarkusUnitTest config = new QuarkusUnitTest()
             .withConfigurationResource("test-logging.properties")
+            .overrideConfigKey("pingpong/mp-rest/url", "stork://pingpong-service")
             .overrideConfigKey("quarkus.stork.pingpong-service.service-discovery.type", "static")
             .overrideConfigKey("quarkus.stork.pingpong-service.service-discovery.address-list", "${test.url}")
-            .overrideConfigKey("pingpong/mp-rest/url", "stork://pingpong-service")
             .overrideConfigKey("quarkus.stork.pingpong-service.load-balancer.type", "mock")
+            .overrideConfigKey("greeting/mp-rest/url", "stork://greeting-service/greeting")
+            .overrideConfigKey("quarkus.stork.greeting-service.service-discovery.type", "static")
+            .overrideConfigKey("quarkus.stork.greeting-service.service-discovery.address-list", "${test.url}")
+            .overrideConfigKey("quarkus.stork.greeting-service.load-balancer.type", "mock")
             .withApplicationRoot((jar) -> jar
                     .addClasses(PingPongResource.class, PingPongResource.PingPongRestClient.class,
                             MockServiceSelectorProvider.class, MockServiceSelectorConfiguration.class,
-                            MockServiceSelectorProviderLoader.class, Util.class));
+                            MockServiceSelectorProviderLoader.class, GreetingResource.class,
+                            GreetingResource.GreetingRestClient.class, Util.class));
 
     @Inject
     MeterRegistry registry;
@@ -56,43 +61,54 @@ public void shouldGetStorkMetricsWhenServiceSelectorFails() {
         Mockito.when(provider.getLoadBalancer().selectServiceInstance(Mockito.anyCollection()))
                 .thenThrow(new RuntimeException("Load Balancer induced failure"));
         RestAssured.when().get("/ping/one").then().statusCode(500);
+        RestAssured.when().get("/greeting/hola").then().statusCode(500);
 
         //Stork metrics
-        assertStorkMetrics();
+        assertStorkMetrics("pingpong-service");
+        assertStorkMetrics("greeting-service");
 
         // Stork metrics exposed to Micrometer
-        Counter instanceCounter = registry.get("stork.instances.count").counter();
-        Timer serviceDiscoveryDuration = registry.get("stork.service-discovery.duration").timer();
-        Timer serviceSelectionDuration = registry.get("stork.service-selection.duration").timer();
-        Timer overallDuration = registry.get("stork.overall.duration").timer();
-        Gauge serviceDiscoveryFailures = registry.get("stork.service-discovery.failures").gauge();
-        Gauge loadBalancerFailures = registry.get("stork.load-balancer.failures").gauge();
-
-        Util.assertTags(Tag.of("service-name", "pingpong-service"), instanceCounter, serviceDiscoveryDuration,
-                serviceSelectionDuration, overallDuration);
-
-        assertThat(instanceCounter.count()).isEqualTo(1);
-        assertThat(serviceDiscoveryFailures.value()).isEqualTo(0);
-        assertThat(loadBalancerFailures.value()).isEqualTo(1);
-        assertThat(serviceDiscoveryDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
-        assertThat(serviceSelectionDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
-        assertThat(overallDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
+        assertStorkMetricsInMicrometerRegistry("pingpong-service");
+        assertStorkMetricsInMicrometerRegistry("greeting-service");
 
     }
 
-    private static void assertStorkMetrics() {
+    private static void assertStorkMetrics(String serviceName) {
         StorkObservationPoints metrics = StorkObservationCollectorBean.STORK_METRICS
-                .get("pingpong-service" + StorkObservationCollectorBean.METRICS_SUFIX);
+                .get(serviceName + StorkObservationCollectorBean.METRICS_SUFIX);
         Assertions.assertThat(metrics.getDiscoveredInstancesCount()).isEqualTo(1);
         Assertions.assertThat(metrics.getSelectedInstanceId()).isEqualTo(-1);
-        Assertions.assertThat(metrics.getServiceName()).isEqualTo("pingpong-service");
+        Assertions.assertThat(metrics.getServiceName()).isEqualTo(serviceName);
         Assertions.assertThat(metrics.isDone()).isTrue();
-        Assertions.assertThat(metrics.failure().getMessage()).isEqualTo("Load Balancer induced failure");
+        Assertions.assertThat(metrics.isServiceDiscoverySuccessful()).isTrue();
+        Assertions.assertThat(metrics.failure().getMessage())
+                .isEqualTo("Load Balancer induced failure");
         Assertions.assertThat(metrics.getOverallDuration()).isNotNull();
         Assertions.assertThat(metrics.getServiceDiscoveryType()).isEqualTo("static");
         Assertions.assertThat(metrics.getServiceSelectionType()).isEqualTo("mock");
         Assertions.assertThat(metrics.getServiceDiscoveryDuration()).isNotNull();
         Assertions.assertThat(metrics.getServiceSelectionDuration()).isNotNull();
     }
 
+    private void assertStorkMetricsInMicrometerRegistry(String serviceName) {
+        Counter instanceCounter = registry.counter("stork.instances.count", "service-name", serviceName);
+        Timer serviceDiscoveryDuration = registry.timer("stork.service-discovery.duration", "service-name", serviceName);
+        Timer serviceSelectionDuration = registry.timer("stork.service-selection.duration", "service-name", serviceName);
+        Timer overallDuration = registry.timer("stork.overall.duration", "service-name", serviceName);
+        Counter serviceDiscoveryFailures = registry.get("stork.service-discovery.failures")
+                .tags("service-name", serviceName).counter();
+        Counter loadBalancerFailures = registry.get("stork.load-balancer.failures").tags("service-name", serviceName)
+                .counter();
+
+        Util.assertTags(Tag.of("service-name", serviceName), instanceCounter, serviceDiscoveryDuration,
+                serviceSelectionDuration, overallDuration);
+
+        Assertions.assertThat(instanceCounter.count()).isEqualTo(1);
+        Assertions.assertThat(loadBalancerFailures.count()).isEqualTo(1);
+        Assertions.assertThat(serviceDiscoveryFailures.count()).isEqualTo(0);
+        Assertions.assertThat(serviceDiscoveryDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
+        Assertions.assertThat(serviceSelectionDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
+        Assertions.assertThat(overallDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
+    }
+
 }
diff --git a/...st/java/io/quarkus/micrometer/deployment/binder/StorkMetricsServiceDiscoveryFailTest.java b/...st/java/io/quarkus/micrometer/deployment/binder/StorkMetricsServiceDiscoveryFailTest.java
@@ -1,6 +1,7 @@
 
 package io.quarkus.micrometer.deployment.binder;
 
+import static io.restassured.RestAssured.when;
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 
 import java.util.concurrent.TimeUnit;
@@ -13,11 +14,11 @@
 import org.mockito.Mockito;
 
 import io.micrometer.core.instrument.Counter;
-import io.micrometer.core.instrument.Gauge;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.Timer;
 import io.quarkus.micrometer.runtime.binder.stork.StorkObservationCollectorBean;
+import io.quarkus.micrometer.test.GreetingResource;
 import io.quarkus.micrometer.test.MockServiceDiscoveryConfiguration;
 import io.quarkus.micrometer.test.MockServiceDiscoveryProvider;
 import io.quarkus.micrometer.test.MockServiceDiscoveryProviderLoader;
@@ -35,10 +36,13 @@ public class StorkMetricsServiceDiscoveryFailTest {
             .withConfigurationResource("test-logging.properties")
             .overrideConfigKey("quarkus.stork.pingpong-service.service-discovery.type", "mock")
             .overrideConfigKey("pingpong/mp-rest/url", "stork://pingpong-service")
+            .overrideConfigKey("greeting/mp-rest/url", "stork://greeting-service/greeting")
+            .overrideConfigKey("quarkus.stork.greeting-service.service-discovery.type", "mock")
             .withApplicationRoot((jar) -> jar
                     .addClasses(PingPongResource.class, PingPongResource.PingPongRestClient.class,
                             MockServiceDiscoveryProvider.class, MockServiceDiscoveryConfiguration.class,
-                            MockServiceDiscoveryProviderLoader.class, Util.class));
+                            MockServiceDiscoveryProviderLoader.class, GreetingResource.class,
+                            GreetingResource.GreetingRestClient.class, Util.class));
 
     @Inject
     MeterRegistry registry;
@@ -52,38 +56,49 @@ public void shouldGetStorkMetricsWhenServiceDiscoveryFails() {
         Mockito.when(provider.getServiceDiscovery().getServiceInstances())
                 .thenReturn(Uni.createFrom().failure(new RuntimeException("Service Discovery induced failure")));
         RestAssured.when().get("/ping/one").then().statusCode(500);
+        when().get("/greeting/hola").then().statusCode(500);
 
         //Stork metrics
-        assertStorkMetrics();
+        assertStorkMetrics("pingpong-service");
+        assertStorkMetrics("greeting-service");
 
         // Stork metrics exposed to Micrometer
-        Counter instanceCounter = registry.get("stork.instances.count").counter();
-        Timer serviceDiscoveryDuration = registry.get("stork.service-discovery.duration").timer();
-        Timer serviceSelectionDuration = registry.get("stork.service-selection.duration").timer();
-        Timer overallDuration = registry.get("stork.overall.duration").timer();
-        Gauge serviceDiscoveryFailures = registry.get("stork.service-discovery.failures").gauge();
-        Gauge loadBalancerFailures = registry.get("stork.load-balancer.failures").gauge();
-
-        Util.assertTags(Tag.of("service-name", "pingpong-service"), instanceCounter, serviceDiscoveryDuration,
-                serviceSelectionDuration, overallDuration);
+        assertStorkMetricsInMicrometerRegistry("pingpong-service");
+        assertStorkMetricsInMicrometerRegistry("greeting-service");
+
+    }
 
-        assertThat(instanceCounter.count()).isEqualTo(0);
-        assertThat(serviceDiscoveryFailures.value()).isEqualTo(1);
-        assertThat(loadBalancerFailures.value()).isEqualTo(0);
-        assertThat(serviceDiscoveryDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
-        assertThat(serviceSelectionDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
-        assertThat(overallDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
+    private void assertStorkMetricsInMicrometerRegistry(String serviceName) {
+        Counter instanceCounter = registry.counter("stork.instances.count", "service-name", serviceName);
+        Timer serviceDiscoveryDuration = registry.timer("stork.service-discovery.duration", "service-name", serviceName);
+        Timer serviceSelectionDuration = registry.timer("stork.service-selection.duration", "service-name", serviceName);
+        Timer overallDuration = registry.timer("stork.overall.duration", "service-name", serviceName);
+        Counter serviceDiscoveryFailures = registry.get("stork.service-discovery.failures")
+                .tags("service-name", serviceName).counter();
+        Counter loadBalancerFailures = registry.get("stork.load-balancer.failures").tags("service-name", serviceName)
+                .counter();
+
+        Util.assertTags(Tag.of("service-name", serviceName), instanceCounter, serviceDiscoveryDuration,
+                serviceSelectionDuration, overallDuration);
 
+        Assertions.assertThat(instanceCounter.count()).isEqualTo(0);
+        Assertions.assertThat(loadBalancerFailures.count()).isEqualTo(0);
+        Assertions.assertThat(serviceDiscoveryFailures.count()).isEqualTo(1);
+        Assertions.assertThat(serviceDiscoveryDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
+        Assertions.assertThat(serviceSelectionDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
+        Assertions.assertThat(overallDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
     }
 
-    private static void assertStorkMetrics() {
+    private static void assertStorkMetrics(String serviceName) {
         StorkObservationPoints metrics = StorkObservationCollectorBean.STORK_METRICS
-                .get("pingpong-service" + StorkObservationCollectorBean.METRICS_SUFIX);
+                .get(serviceName + StorkObservationCollectorBean.METRICS_SUFIX);
         Assertions.assertThat(metrics.getDiscoveredInstancesCount()).isNegative();
         Assertions.assertThat(metrics.getSelectedInstanceId()).isEqualTo(-1);
-        Assertions.assertThat(metrics.getServiceName()).isEqualTo("pingpong-service");
+        Assertions.assertThat(metrics.getServiceName()).isEqualTo(serviceName);
         Assertions.assertThat(metrics.isDone()).isTrue();
-        Assertions.assertThat(metrics.failure().getMessage()).isEqualTo("Service Discovery induced failure");
+        Assertions.assertThat(metrics.isServiceDiscoverySuccessful()).isFalse();
+        Assertions.assertThat(metrics.failure().getMessage())
+                .isEqualTo("Service Discovery induced failure");
         Assertions.assertThat(metrics.getOverallDuration()).isNotNull();
         Assertions.assertThat(metrics.getServiceDiscoveryType()).isEqualTo("mock");
         Assertions.assertThat(metrics.getServiceSelectionType()).isEqualTo("round-robin");

diff --git a/...er/deployment/src/test/java/io/quarkus/micrometer/deployment/binder/StorkMetricsTest.java b/...er/deployment/src/test/java/io/quarkus/micrometer/deployment/binder/StorkMetricsTest.java
@@ -12,7 +12,6 @@
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 import io.micrometer.core.instrument.Counter;
-import io.micrometer.core.instrument.Gauge;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.Timer;
@@ -56,15 +55,15 @@ public void shouldGetStorkMetricsForTwoServicesWhenEverythingSucceded() {
 
     }
 
-    public void assertStorkMetricsInMicrometerRegistry(String serviceName) {
+    private void assertStorkMetricsInMicrometerRegistry(String serviceName) {
         Counter instanceCounter = registry.counter("stork.instances.count", "service-name", serviceName);
         Timer serviceDiscoveryDuration = registry.timer("stork.service-discovery.duration", "service-name", serviceName);
         Timer serviceSelectionDuration = registry.timer("stork.service-selection.duration", "service-name", serviceName);
         Timer overallDuration = registry.timer("stork.overall.duration", "service-name", serviceName);
-        Gauge serviceDiscoveryFailures = registry.get("stork.service-discovery.failures")
-                .tags("service-name", serviceName).gauge();
-        Gauge loadBalancerFailures = registry.get("stork.load-balancer.failures").tags("service-name", serviceName)
-                .gauge();
+        Counter serviceDiscoveryFailures = registry.get("stork.service-discovery.failures")
+                .tags("service-name", serviceName).counter();
+        Counter loadBalancerFailures = registry.get("stork.load-balancer.failures").tags("service-name", serviceName)
+                .counter();
 
         Util.assertTags(Tag.of("service-name", serviceName), instanceCounter, serviceDiscoveryDuration,
                 serviceSelectionDuration, overallDuration);
@@ -73,8 +72,8 @@ public void assertStorkMetricsInMicrometerRegistry(String serviceName) {
         Assertions.assertThat(serviceDiscoveryDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
         Assertions.assertThat(serviceSelectionDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
         Assertions.assertThat(overallDuration.totalTime(TimeUnit.NANOSECONDS)).isGreaterThan(0);
-        Assertions.assertThat(serviceDiscoveryFailures.value()).isEqualTo(0);
-        Assertions.assertThat(loadBalancerFailures.value()).isEqualTo(0);
+        Assertions.assertThat(serviceDiscoveryFailures.count()).isEqualTo(0);
+        Assertions.assertThat(loadBalancerFailures.count()).isEqualTo(0);
     }
 
     public static void assertStorkMetrics(String serviceName) {

diff --git a/...c/main/java/io/quarkus/micrometer/runtime/binder/stork/StorkObservationCollectorBean.java b/...c/main/java/io/quarkus/micrometer/runtime/binder/stork/StorkObservationCollectorBean.java
@@ -1,7 +1,5 @@
 package io.quarkus.micrometer.runtime.binder.stork;
 
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
@@ -10,7 +8,6 @@
 import jakarta.enterprise.inject.Typed;
 
 import io.micrometer.core.instrument.Counter;
-import io.micrometer.core.instrument.Gauge;
 import io.micrometer.core.instrument.MeterRegistry;
 import io.micrometer.core.instrument.Metrics;
 import io.micrometer.core.instrument.Tag;
@@ -38,8 +35,8 @@ public StorkObservationPoints create(String serviceName, String serviceDiscovery
     }
 
     @Override
-    public void complete(StorkObservationPoints event) {
-        Tags tags = Tags.of(Tag.of("service-name", event.getServiceName()));
+    public void complete(StorkObservationPoints observation) {
+        Tags tags = Tags.of(Tag.of("service-name", observation.getServiceName()));
 
         Counter instanceCounter = Counter.builder("stork.instances.count")
                 .description("The number of service instances discovered")
@@ -63,31 +60,26 @@ public void complete(StorkObservationPoints event) {
                 .tags(tags)
                 .register(registry);
 
-        List<Throwable> serviceDiscoveryExceptions = new ArrayList<>();
-
-        Gauge serviceDiscoveryFailures = Gauge
-                .builder("stork.service-discovery.failures", serviceDiscoveryExceptions,
-                        List::size)
+        Counter serviceDiscoveryFailures = Counter
+                .builder("stork.service-discovery.failures")
                 .description("The number of failures during service discovery").tags(tags)
                 .register(registry);
 
-        List<Throwable> serviceSelectionExceptions = new ArrayList<>();
-
-        Gauge serviceSelectionFailures = Gauge
-                .builder("stork.load-balancer.failures", serviceSelectionExceptions, List::size)
+        Counter serviceSelectionFailures = Counter
+                .builder("stork.load-balancer.failures")
                 .description("The number of failures during service selection.").tags(tags)
                 .register(registry);
 
-        instanceCounter.increment(event.getDiscoveredInstancesCount());
-        serviceDiscoveryTimer.record(event.getServiceDiscoveryDuration().getNano(), TimeUnit.NANOSECONDS);
-        serviceSelectionTimer.record(event.getServiceSelectionDuration().getNano(), TimeUnit.NANOSECONDS);
-        overallTimer.record(event.getServiceSelectionDuration().getNano(), TimeUnit.NANOSECONDS);
+        instanceCounter.increment(observation.getDiscoveredInstancesCount());
+        serviceDiscoveryTimer.record(observation.getServiceDiscoveryDuration().getNano(), TimeUnit.NANOSECONDS);
+        serviceSelectionTimer.record(observation.getServiceSelectionDuration().getNano(), TimeUnit.NANOSECONDS);
+        overallTimer.record(observation.getServiceSelectionDuration().getNano(), TimeUnit.NANOSECONDS);
 
-        if (event.failure() != null) {
-            if (event.isServiceDiscoverySuccessful()) {
-                serviceSelectionExceptions.add(event.failure());
+        if (observation.failure() != null) {
+            if (observation.isServiceDiscoverySuccessful()) {
+                serviceSelectionFailures.increment();
             } else {// SD failure
-                serviceDiscoveryExceptions.add(event.failure());
+                serviceDiscoveryFailures.increment();
             }
         }