Make SE metrics default scope application instead of nothing (#7666)

helidon-io · Sep 26, 2023 · af48313 · af48313
1 parent c6f976b
commit af48313
Show file tree

Hide file tree

Showing 14 changed files with 43 additions and 43 deletions.
diff --git a/archetypes/helidon/src/main/archetype/common/observability.xml b/archetypes/helidon/src/main/archetype/common/observability.xml
@@ -203,7 +203,7 @@ curl -H 'Accept: application/json' -X GET http://localhost:8080/metrics
 
         String output = response.as(String.class);
 
-        String expected = MetricsService.PERSONALIZED_GETS_COUNTER_NAME + "_total 1.0";
+        String expected = MetricsService.PERSONALIZED_GETS_COUNTER_NAME + "_total{scope=\"application\",} 1.0";
         assertThat("Unable to find expected counter result " + expected + "; output is " + output,
                 output, containsString(expected));
         response.close();
@@ -385,13 +385,13 @@ allRequests_total 0.0
         assertThat(response.status().code(), is(200));
 
         String output = response.as(String.class);
-        String expected = MetricsService.ALL_GETS_TIMER_NAME + "_seconds_count " + 2;
+        String expected = MetricsService.ALL_GETS_TIMER_NAME + "_seconds_count{scope=\"application\",} " + 2;
         assertThat("Unable to find expected all-gets timer count " + expected + "; output is " + output,
                 output, containsString(expected));
 
         assertThat("Unable to find expected all-gets timer sum", output,
                 containsString(MetricsService.ALL_GETS_TIMER_NAME + "_seconds_sum"));
-        expected = MetricsService.PERSONALIZED_GETS_COUNTER_NAME + "_total " + 1;
+        expected = MetricsService.PERSONALIZED_GETS_COUNTER_NAME + "_total{scope=\"application\",} " + 1;
         assertThat("Unable to find expected counter result " + expected + "; output is " + output,
                 output, containsString(expected));
         response.close();

diff --git a/examples/metrics/exemplar/src/test/java/io/helidon/examples/metrics/exemplar/MainTest.java b/examples/metrics/exemplar/src/test/java/io/helidon/examples/metrics/exemplar/MainTest.java
@@ -94,31 +94,25 @@ public void testMetrics() {
             assertThat(response.as(String.class), containsString("Hello Joe!"));
         }
 
-        try (Http1ClientResponse response = client.get("/metrics/application").request()) {
+        try (Http1ClientResponse response = client.get("/observe/metrics/application").request()) {
 
             String openMetricsOutput = response.as(String.class);
             LineNumberReader reader = new LineNumberReader(new StringReader(openMetricsOutput));
             List<String> returnedLines = reader.lines()
                                                .collect(Collectors.toList());
 
-            List<String> expected = List.of(">> skip to timer total >>",
-                    "# TYPE application_" + GreetService.TIMER_FOR_GETS + "_mean_seconds gauge",
-                    valueMatcher("mean"),
-                    ">> end of output >>");
-            assertLinesMatch(expected, returnedLines, GreetService.TIMER_FOR_GETS + "_mean_seconds TYPE and value");
-
-            expected = List.of(">> skip to max >>",
-                    "# TYPE application_" + GreetService.TIMER_FOR_GETS + "_max_seconds gauge",
+            List<String> expected = List.of(">> skip to max >>",
+                    "# TYPE " + GreetService.TIMER_FOR_GETS + "_seconds_max gauge",
                     valueMatcher("max"),
                     ">> end of output >>");
             assertLinesMatch(expected, returnedLines, GreetService.TIMER_FOR_GETS + "_max_seconds TYPE and value");
         }
     }
 
     private static String valueMatcher(String statName) {
-        // application_timerForGets_mean_seconds 0.010275403147594316 # {trace_id="cfd13196e6a9fb0c"} 0.002189822 1617799841.963000
-        return "application_" + GreetService.TIMER_FOR_GETS
-                + "_" + statName + "_seconds [\\d\\.]+ # \\{trace_id=\"[^\"]+\"\\} [\\d\\.]+ [\\d\\.]+";
+        // timerForGets_mean_seconds 0.010275403147594316 # {scope="application",trace_id="cfd13196e6a9fb0c"} 0.002189822 1617799841.963000
+        return GreetService.TIMER_FOR_GETS
+                + "_" + statName + "_seconds [\\d\\.]+ # \\{scope=\"application\",trace_id=\"[^\"]+\"\\} [\\d\\.]+ [\\d\\.]+";
     }
 
 }
diff --git a/...ics/filtering/se/src/main/java/io/helidon/examples/metrics/filtering/se/GreetService.java b/...ics/filtering/se/src/main/java/io/helidon/examples/metrics/filtering/se/GreetService.java
@@ -77,7 +77,7 @@ public class GreetService implements HttpService {
      */
     @Override
     public void routing(HttpRules rules) {
-        rules.get("/", this::timeGet, this::getDefaultMessageHandler)
+        rules.get("/", this::timeGet)
                 .get("/{name}", this::countPersonalized, this::getMessageHandler)
                 .put("/greeting", this::updateGreetingHandler);
     }
@@ -142,7 +142,7 @@ private void updateGreetingHandler(ServerRequest request,
     }
 
     private void timeGet(ServerRequest request, ServerResponse response) {
-        timerForGets.record((Runnable) response::next);
+        timerForGets.record(() -> getDefaultMessageHandler(request, response));
     }
 
     private void countPersonalized(ServerRequest request, ServerResponse response) {

diff --git a/...les/metrics/filtering/se/src/main/java/io/helidon/examples/metrics/filtering/se/Main.java b/...les/metrics/filtering/se/src/main/java/io/helidon/examples/metrics/filtering/se/Main.java
@@ -96,7 +96,7 @@ static void setup(WebServerConfig.Builder server) {
     static void routing(HttpRouting.Builder routing, MetricsConfig.Builder metricsConfigBuilder) {
         Config config = Config.global();
 
-        MeterRegistry meterRegistry = MetricsFactory.getInstance(config).globalRegistry();
+        MeterRegistry meterRegistry = MetricsFactory.getInstance(config).globalRegistry(metricsConfigBuilder.build());
 
         MetricsObserver metrics = MetricsObserver.builder()
                 .metricsConfig(metricsConfigBuilder)

diff --git a/...metrics/filtering/se/src/test/java/io/helidon/examples/metrics/filtering/se/MainTest.java b/...metrics/filtering/se/src/test/java/io/helidon/examples/metrics/filtering/se/MainTest.java
@@ -28,7 +28,6 @@
 import jakarta.json.JsonBuilderFactory;
 import jakarta.json.JsonObject;
 import org.hamcrest.CoreMatchers;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -78,7 +77,6 @@ public void testHelloWorld() {
     }
 
     @Test
-    @Disabled // application metrics returns 404
     public void testMetrics() {
         try (Http1ClientResponse response = client.get("/greet").request()) {
             assertThat(response.as(String.class), containsString("Hello World!"));

diff --git a/...ount-se/src/main/java/io/helidon/examples/se/httpstatuscount/HttpStatusMetricService.java b/...ount-se/src/main/java/io/helidon/examples/se/httpstatuscount/HttpStatusMetricService.java
@@ -74,11 +74,10 @@ static boolean isInProgress() {
         return IN_PROGRESS.get() != 0;
     }
 
-    // Edited to adopt Ciaran's fix later in the thread.
     private void updateRange(ServerRequest request, ServerResponse response) {
         IN_PROGRESS.incrementAndGet();
+        response.whenSent(() -> logMetric(response));
         response.next();
-        logMetric(response);
     }
 
     private void logMetric(ServerResponse response) {

diff --git a/...trics/http-status-count-se/src/main/java/io/helidon/examples/se/httpstatuscount/Main.java b/...trics/http-status-count-se/src/main/java/io/helidon/examples/se/httpstatuscount/Main.java
@@ -67,10 +67,8 @@ static void setup(WebServerConfig.Builder server) {
      * Set up routing.
      *
      * @param routing routing builder
-     * @param config  configuration of this server
      */
     static void routing(HttpRouting.Builder routing) {
-        Config config = Config.global();
         routing.addFeature(ObserveFeature.create())
                 .register(HttpStatusMetricService.create()) // no endpoint, just metrics updates
                 .register("/simple-greet", new SimpleGreetService())

diff --git a/...s/http-status-count-se/src/test/java/io/helidon/examples/se/httpstatuscount/MainTest.java b/...s/http-status-count-se/src/test/java/io/helidon/examples/se/httpstatuscount/MainTest.java
@@ -26,7 +26,6 @@
 import jakarta.json.Json;
 import jakarta.json.JsonBuilderFactory;
 import jakarta.json.JsonObject;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.MethodOrderer;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestMethodOrder;
@@ -56,14 +55,13 @@ public static void setup(WebServerConfig.Builder server) {
     }
 
     @Test
-    @Disabled
     public void testMicroprofileMetrics() {
         try (Http1ClientResponse response = client.get("/simple-greet/greet-count").request()) {
             assertThat(response.as(String.class), containsString("Hello World!"));
         }
 
         try (Http1ClientResponse response = client.get("/observe/metrics").request()) {
-            assertThat("Metrics output", response.as(String.class), containsString("application_accessctr_total"));
+            assertThat("Metrics output", response.as(String.class), containsString("accessctr_total"));
         }
     }
 

diff --git a/...http-status-count-se/src/test/java/io/helidon/examples/se/httpstatuscount/StatusTest.java b/...http-status-count-se/src/test/java/io/helidon/examples/se/httpstatuscount/StatusTest.java
@@ -31,7 +31,6 @@
 import io.helidon.webserver.testing.junit5.SetUpServer;
 
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import static io.helidon.examples.se.httpstatuscount.HttpStatusMetricService.STATUS_COUNTER_NAME;
@@ -71,7 +70,6 @@ void findStatusMetrics() {
     }
 
     @Test
-    @Disabled
     void checkStatusMetrics() throws InterruptedException {
         checkAfterStatus(Status.create(171));
         checkAfterStatus(Status.OK_200);
@@ -105,15 +103,16 @@ void checkAfterStatus(Status status) throws InterruptedException {
         }
         try (Http1ClientResponse response = client.get("/status/" + status.code())
                 .accept(MediaTypes.APPLICATION_JSON)
+                .followRedirects(false)
                 .request()) {
-            assertThat("Response status", response.status(), is(status));
+            assertThat("Response status", response.status().code(), is(status.code()));
             checkCounters(status, before);
         }
     }
 
     @SuppressWarnings("BusyWait")
     private void checkCounters(Status status, long[] before) throws InterruptedException {
-        // first make sure we do not have a request in progress
+        // Make sure the server has updated the counter(s).
         long now = System.currentTimeMillis();
 
         while (HttpStatusMetricService.isInProgress()) {

diff --git a/examples/metrics/kpi/src/test/java/io/helidon/examples/metrics/kpi/MainTest.java b/examples/metrics/kpi/src/test/java/io/helidon/examples/metrics/kpi/MainTest.java
@@ -28,7 +28,6 @@
 import jakarta.json.Json;
 import jakarta.json.JsonBuilderFactory;
 import jakarta.json.JsonObject;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.CoreMatchers.is;
@@ -79,7 +78,6 @@ public void testHelloWorld() {
     }
 
     @Test
-    @Disabled
     public void testMetrics() {
         try (Http1ClientResponse response = client.get("/greet").request()) {
             assertThat(response.as(String.class), containsString("Hello World!"));
@@ -92,7 +90,7 @@ public void testMetrics() {
 
         try (Http1ClientResponse response = client.get("/observe/metrics/" + KPI_REGISTRY_TYPE).request()) {
             assertThat("Returned metrics output", response.as(String.class),
-                    containsString("# TYPE " + KPI_REGISTRY_TYPE + "_requests_inFlight_current"));
+                    containsString("# TYPE " + "requests_inFlight"));
         }
     }
 }
diff --git a/metrics/api/src/main/java/io/helidon/metrics/api/NoOpMeter.java b/metrics/api/src/main/java/io/helidon/metrics/api/NoOpMeter.java
@@ -563,22 +563,22 @@ public <T> T record(Supplier<T> f) {
 
         @Override
         public <T> T record(Callable<T> f) throws Exception {
-            return null;
+            return f.call();
         }
 
         @Override
         public void record(Runnable f) {
-
+            f.run();
         }
 
         @Override
         public Runnable wrap(Runnable f) {
-            return null;
+            return f;
         }
 
         @Override
         public <T> Callable<T> wrap(Callable<T> f) {
-            return null;
+            return f;
         }
 
         @Override

diff --git a/metrics/api/src/main/java/io/helidon/metrics/api/ScopingConfigBlueprint.java b/metrics/api/src/main/java/io/helidon/metrics/api/ScopingConfigBlueprint.java
@@ -34,11 +34,11 @@ interface ScopingConfigBlueprint {
 
     /**
      * Default scope value to associate with meters that are registered without an explicit setting; no setting means meters
-     * receive no default scope value.
+     * are assigned scope {@value io.helidon.metrics.api.Meter.Scope#DEFAULT}.
      *
      * @return default scope value
      */
-    @ConfiguredOption(key = "default")
+    @ConfiguredOption(key = "default", value = Meter.Scope.DEFAULT)
     Optional<String> defaultValue();
 
     /**

diff --git a/...s/provider-tests/src/main/java/io/helidon/metrics/provider/tests/TestScopeManagement.java b/...s/provider-tests/src/main/java/io/helidon/metrics/provider/tests/TestScopeManagement.java
@@ -19,21 +19,25 @@
 import java.util.List;
 import java.util.Set;
 
+import io.helidon.common.testing.junit5.OptionalMatcher;
 import io.helidon.metrics.api.Counter;
 import io.helidon.metrics.api.Meter;
 import io.helidon.metrics.api.MeterRegistry;
+import io.helidon.metrics.api.Metrics;
 import io.helidon.metrics.api.MetricsConfig;
 import io.helidon.metrics.api.MetricsFactory;
 import io.helidon.metrics.api.ScopingConfig;
 import io.helidon.metrics.api.SystemTagsManager;
 import io.helidon.metrics.api.Timer;
 
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
 
 class TestScopeManagement {
@@ -42,7 +46,8 @@ class TestScopeManagement {
     @ValueSource(booleans = {true, false})
     void testExplicitScopeOnMetersWithNoDefaultScope(boolean scopeTagEnabled) {
         MetricsConfig metricsConfig = MetricsConfig.builder()
-                .scoping(ScopingConfig.builder())
+                .scoping(ScopingConfig.builder()
+                                 .clearDefaultValue())
                 .build();
         MeterRegistry reg = MetricsFactory.getInstance().globalRegistry();
         SystemTagsManager.instance(metricsConfig);
@@ -127,4 +132,14 @@ void testExplicitScopeOnMetersWithDefaultScope(boolean scopeTagEnabled) {
                            hasItem((Meter) t1)
                    ));
     }
+
+    @Test
+    void checkDefaultScope() {
+        MetricsConfig metricsConfig = MetricsConfig.create(); // Make sure to use the defaults, not leftovers from earlier tests
+        MetricsFactory.getInstance().globalRegistry(metricsConfig);
+        SystemTagsManager.instance(metricsConfig);
+
+        Counter counter = Metrics.getOrCreate(Counter.builder("defaultScopedCounter"));
+        assertThat("Unspecified scope", counter.scope(), OptionalMatcher.optionalValue(is(Meter.Scope.DEFAULT)));
+    }
 }
diff --git a/...ders/micrometer/src/main/java/io/helidon/metrics/providers/micrometer/MMeterRegistry.java b/...ders/micrometer/src/main/java/io/helidon/metrics/providers/micrometer/MMeterRegistry.java
@@ -247,9 +247,10 @@ public Iterable<String> scopes() {
 
     @Override
     public boolean isMeterEnabled(String name, Map<String, String> tags, Optional<String> scope) {
+        String effectiveScope = scope.orElse(SystemTagsManager.instance().effectiveScope(scope)
+                                                     .orElse(io.helidon.metrics.api.Meter.Scope.DEFAULT));
         return metricsConfig.enabled()
-                && (scope.isEmpty()
-                    || metricsConfig.isMeterEnabled(name, scope.get()));
+                && metricsConfig.isMeterEnabled(name, effectiveScope);
     }
 
     @Override