Use correct config nodes for metrics settings; when using global mete…

…r registry set metrics config correctly (helidon-io#8008)

Signed-off-by: Tim Quinn <[email protected]>

metrics/api/src/main/java/io/helidon/metrics/api/MetricsFactoryManager.java

@@ -122,7 +122,8 @@ static MetricsFactory getMetricsFactory(Config metricsConfigNode) {
      */
     static MetricsFactory getMetricsFactory() {
         return access(() -> {
-            metricsConfigNode = Objects.requireNonNullElseGet(metricsConfigNode, GlobalConfig::config);
+            metricsConfigNode = Objects.requireNonNullElseGet(metricsConfigNode,
+                                                              MetricsFactoryManager::externalMetricsConfig);
             metricsFactory = Objects.requireNonNullElseGet(metricsFactory,
                                                            () -> getMetricsFactory(metricsConfigNode));
             return metricsFactory;
@@ -149,6 +150,14 @@ static void closeAll() {
         metricsFactory = null;
     }
 
+    private static Config externalMetricsConfig() {
+        Config serverFeaturesMetricsConfig = GlobalConfig.config().get("server.features.observe.observers.metrics");
+        if (!serverFeaturesMetricsConfig.exists()) {
+            serverFeaturesMetricsConfig = GlobalConfig.config().get("metrics");
+        }
+        return serverFeaturesMetricsConfig;
+    }
+
     private static MetricsFactory completeGetInstance(MetricsConfig metricsConfig, Config metricsConfigNode) {
 
         metricsConfig = applyOverrides(metricsConfig);

microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/MetricsCdiExtension.java

@@ -551,14 +551,10 @@ private MetricsObserver configure() {
         MetricsFactory metricsFactory = MetricsFactory.getInstance(config);
 
         Contexts.globalContext().register(metricsFactory);
-        MetricsConfig.Builder metricsConfigBuilder = MetricsConfig.builder()
-                .config(config);
-        MetricsConfig metricsConfig = metricsConfigBuilder.build();
+        MetricsConfig metricsConfig = metricsFactory.metricsConfig();
         MeterRegistry meterRegistry = metricsFactory.globalRegistry(metricsConfig);
-        RegistryFactory.getInstance(meterRegistry); // initialize before first use
-        return builder.metricsConfig(metricsConfigBuilder)
+        return builder.metricsConfig(metricsConfig)
                 .meterRegistry(meterRegistry)
-                .metricsConfig(MetricsConfig.builder(metricsConfig))
                 .build();
     }
 

microprofile/metrics/src/main/java/io/helidon/microprofile/metrics/RegistryFactory.java

@@ -76,8 +76,9 @@ static RegistryFactory create(MeterRegistry meterRegistry) {
     }
 
     static RegistryFactory getInstance(MeterRegistry meterRegistry) {
-        REGISTRY_FACTORY.set(create(meterRegistry));
-        return REGISTRY_FACTORY.get();
+        return REGISTRY_FACTORY.updateAndGet(rf -> rf != null && rf.meterRegistry == meterRegistry
+                ? rf
+                : create(meterRegistry));
     }
 
     /**

webserver/observe/metrics/pom.xml

@@ -93,6 +93,11 @@
             <artifactId>helidon-config-yaml</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>io.helidon.webserver.testing.junit5</groupId>
+            <artifactId>helidon-webserver-testing-junit5</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

webserver/observe/metrics/src/main/java/io/helidon/webserver/observe/metrics/MetricsFeature.java

@@ -64,8 +64,8 @@ class MetricsFeature {
     private KeyPerformanceIndicatorSupport.Metrics kpiMetrics;
 
     MetricsFeature(MetricsObserverConfig config) {
-        this.meterRegistry = config.meterRegistry().orElseGet(MetricsFactory.getInstance()::globalRegistry);
         this.metricsConfig = config.metricsConfig();
+        this.meterRegistry = config.meterRegistry().orElseGet(() -> MetricsFactory.getInstance().globalRegistry(metricsConfig));
     }
 
     /**

webserver/observe/metrics/src/main/java/io/helidon/webserver/observe/metrics/MetricsObserver.java

@@ -122,7 +122,7 @@ public MetricsObserverConfig prototype() {
 
     @Override
     public String type() {
-        return "log";
+        return "metrics";
     }
 
     @Override

webserver/observe/metrics/src/test/java/io/helidon/webserver/observe/metrics/TestMetricsConfigPropagation.java

@@ -0,0 +1,61 @@
+/*
+ * Copyright (c) 2023 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.helidon.webserver.observe.metrics;
+
+import io.helidon.common.media.type.MediaTypes;
+import io.helidon.webclient.http1.Http1Client;
+import io.helidon.webclient.http1.Http1ClientResponse;
+import io.helidon.webserver.WebServer;
+import io.helidon.webserver.testing.junit5.ServerTest;
+
+import jakarta.json.JsonObject;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
+
+@ServerTest
+class TestMetricsConfigPropagation {
+
+    private final Http1Client client;
+
+    TestMetricsConfigPropagation(WebServer server, Http1Client client) {
+        this.client = client;
+    }
+
+    @Test
+    void checkExtendedKpiMetrics() {
+        try (Http1ClientResponse response = client.get("/observe/metrics")
+                .accept(MediaTypes.APPLICATION_JSON)
+                .request()) {
+            assertThat("Metrics endpoint", response.status().code(), is(200));
+            JsonObject metricsResponse = response.as(JsonObject.class);
+            JsonObject vendorMeters = metricsResponse.getJsonObject("vendor");
+            assertThat("Vendor meters", vendorMeters, notNullValue());
+
+            // Make sure that the extended KPI metrics were turned on as directed by the configuration.
+            assertThat("Metrics KPI load",
+                       vendorMeters.getJsonNumber("requests.load").intValue(),
+                       greaterThan(0));
+
+            // Make sure that requests.count is absent because of the filtering in the config.
+            assertThat("Metrics KPI requests.count", vendorMeters.get("requests.count"), nullValue());
+        }
+    }
+}

webserver/observe/metrics/src/test/resources/application.yaml

@@ -0,0 +1,27 @@
+#
+# Copyright (c) 2023 Oracle and/or its affiliates.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+server:
+  features:
+    observe:
+      observers:
+        metrics:
+          scoping:
+            scopes:
+              - name: vendor
+                filter:
+                  include: requests.l.*
+          key-performance-indicators:
+            extended: true