Backport of Metrics and routings (#3260)

integrations/micrometer/micrometer/src/main/java/io/helidon/integrations/micrometer/MicrometerSupport.java

@@ -95,12 +95,12 @@ public MeterRegistry registry() {
 
     @Override
     public void update(Routing.Rules rules) {
-        configureEndpoint(rules);
+        configureEndpoint(rules, rules);
     }
 
     @Override
-    protected void postConfigureEndpoint(Routing.Rules rules) {
-        rules
+    protected void postConfigureEndpoint(Routing.Rules defaultRules, Routing.Rules serviceEndpointRoutingRules) {
+        defaultRules
                 .any(new MetricsContextHandler(meterRegistryFactory.meterRegistry()))
                 .get(context(), this::getOrOptions)
                 .options(context(), this::getOrOptions);

metrics/metrics/src/main/java/io/helidon/metrics/MetricsSupport.java

@@ -379,30 +379,34 @@ public void configureVendorMetrics(String routingName,
      * {@link #update(io.helidon.webserver.Routing.Rules)} (e.g. you should not
      * use both, as otherwise you would register the endpoint twice)
      *
-     * @param rules routing rules (also accepts
-     * {@link io.helidon.webserver.Routing.Builder}
+     * @param defaultRules routing rules for default routing (also accepts {@link io.helidon.webserver.Routing.Builder})
+     * @param serviceEndpointRoutingRules possibly different rules for the metrics endpoint routing
      */
     @Override
-    protected void postConfigureEndpoint(Routing.Rules rules) {
+    protected void postConfigureEndpoint(Routing.Rules defaultRules, Routing.Rules serviceEndpointRoutingRules) {
         Registry base = rf.getARegistry(MetricRegistry.Type.BASE);
         Registry vendor = rf.getARegistry(MetricRegistry.Type.VENDOR);
         Registry app = rf.getARegistry(MetricRegistry.Type.APPLICATION);
 
         // register the metric registry and factory to be available to all
-        rules.any(new MetricsContextHandler(app, rf));
+        MetricsContextHandler metricsContextHandler = new MetricsContextHandler(app, rf);
+        defaultRules.any(metricsContextHandler);
+        if (defaultRules != serviceEndpointRoutingRules) {
+            serviceEndpointRoutingRules.any(metricsContextHandler);
+        }
 
-        configureVendorMetrics(null, rules);
+        configureVendorMetrics(null, defaultRules);
 
         // routing to root of metrics
-        rules.get(context(), (req, res) -> getMultiple(req, res, base, app, vendor))
+        serviceEndpointRoutingRules.get(context(), (req, res) -> getMultiple(req, res, base, app, vendor))
                 .options(context(), (req, res) -> optionsMultiple(req, res, base, app, vendor));
 
         // routing to each scope
         Stream.of(app, base, vendor)
                 .forEach(registry -> {
                     String type = registry.type();
 
-                    rules.get(context() + "/" + type, (req, res) -> getAll(req, res, registry))
+                    serviceEndpointRoutingRules.get(context() + "/" + type, (req, res) -> getAll(req, res, registry))
                             .get(context() + "/" + type + "/{metric}", (req, res) -> getByName(req, res, registry))
                             .options(context() + "/" + type, (req, res) -> optionsAll(req, res, registry))
                             .options(context() + "/" + type + "/{metric}", (req, res) -> optionsOne(req, res, registry));
@@ -415,15 +419,15 @@ protected void postConfigureEndpoint(Routing.Rules rules) {
      * {@link io.helidon.webserver.Routing.Builder#register(io.helidon.webserver.Service...)}
      * rather than calling this method directly. If multiple sockets (and
      * routings) should be supported, you can use the
-     * {@link #configureEndpoint(io.helidon.webserver.Routing.Rules)}, and
+     * {@link #configureEndpoint(io.helidon.webserver.Routing.Rules, io.helidon.webserver.Routing.Rules)}, and
      * {@link #configureVendorMetrics(String, io.helidon.webserver.Routing.Rules)}
      * methods.
      *
      * @param rules a routing rules to update
      */
     @Override
     public void update(Routing.Rules rules) {
-        configureEndpoint(rules);
+        configureEndpoint(rules, rules);
     }
 
     private static KeyPerformanceIndicatorSupport.Context kpiContext(ServerRequest request) {

microprofile/metrics/src/test/java/io/helidon/microprofile/metrics/TestMetricsOnOwnSocket.java

@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2021 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.microprofile.metrics;
+
+import javax.inject.Inject;
+import javax.json.JsonObject;
+import javax.ws.rs.client.ClientBuilder;
+import javax.ws.rs.client.Invocation;
+import javax.ws.rs.client.WebTarget;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+
+import io.helidon.common.http.Http;
+import io.helidon.microprofile.server.ServerCdiExtension;
+import io.helidon.microprofile.tests.junit5.AddConfig;
+import io.helidon.microprofile.tests.junit5.HelidonTest;
+
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.is;
+
+@HelidonTest()
+// Set up the metrics endpoint on its own socket
+@AddConfig(key = "server.sockets.0.name", value = "metrics")
+// No port setting, so use any available one
+@AddConfig(key = "server.sockets.0.bind-address", value = "0.0.0.0")
+@AddConfig(key = "metrics.routing", value = "metrics")
+@AddConfig(key = "metrics.key-performance-indicators.extended", value = "true")
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
+public class TestMetricsOnOwnSocket {
+
+    private Invocation metricsInvocation= null;
+
+    private static int loadCountBeforePingingMetrics = -1;
+
+    @Inject
+    private ServerCdiExtension serverCdiExtension;
+
+    @Inject
+    private WebTarget webTarget;
+
+    Invocation metricsInvocation() {
+        if (metricsInvocation == null) {
+            int metricsPort = serverCdiExtension.port("metrics");
+            metricsInvocation = ClientBuilder.newClient()
+                    .target(String.format("http://localhost:%d/metrics/vendor", metricsPort))
+                    .request(MediaType.APPLICATION_JSON_TYPE)
+                    .buildGet();
+        }
+        return metricsInvocation;
+    }
+
+    @Order(0)
+    @Test
+    void checkMetricsBeforeRequests() {
+        // Just record the load count value. Other tests might have run earlier so we cannot assume the count is exactly 0.
+        loadCountBeforePingingMetrics = getRequestsLoadCount("Pre-test");
+        assertThat("Pre-test load count", loadCountBeforePingingMetrics, is(greaterThanOrEqualTo(0)));
+
+    }
+
+    @Order(1)
+    @Test
+    void checkMessageFromDefaultRouting() {
+        try (Response r = webTarget
+                .path("helloworld")
+                .request(MediaType.TEXT_PLAIN_TYPE)
+                .get()) {
+
+            assertThat("Response code getting greeting", r.getStatus(), is(Http.Status.OK_200.code()));
+        }
+    }
+
+    @Order(2)
+    @Test
+    void checkMetricsAfterGet() {
+        int load = getRequestsLoadCount("Post-test");
+        assertThat("Change in load count", load - loadCountBeforePingingMetrics, is(1));
+
+    }
+
+    private int getRequestsLoadCount(String descr) {
+        try (Response r = metricsInvocation().invoke()) {
+            assertThat(descr + " metrics sampling response", r.getStatus(), is(Http.Status.OK_200.code()));
+
+            JsonObject metrics = r.readEntity(JsonObject.class);
+            assertThat("Check for requests.load", metrics.containsKey("requests.load"), is(true));
+            JsonObject load = metrics.getJsonObject("requests.load");
+            assertThat("JSON requests.load contains count", load.containsKey("count"), is(true));
+
+            return load.getInt("count");
+        }
+    }
+}

service-common/rest-cdi/src/main/java/io/helidon/servicecommon/restcdi/HelidonRestCdiExtension.java

@@ -46,7 +46,7 @@
 import javax.interceptor.Interceptor;
 
 import io.helidon.config.Config;
-import io.helidon.config.ConfigValue;
+import io.helidon.microprofile.server.RoutingBuilders;
 import io.helidon.microprofile.server.ServerCdiExtension;
 import io.helidon.servicecommon.rest.HelidonRestServiceSupport;
 import io.helidon.webserver.Routing;
@@ -239,23 +239,11 @@ protected Routing.Builder registerService(
         Config config = ((Config) ConfigProvider.getConfig()).get(configPrefix);
         serviceSupport = serviceSupportFactory.apply(config);
 
-        ConfigValue<String> routingNameConfig = config.get("routing")
-                .asString();
-        Routing.Builder defaultRouting = server.serverRoutingBuilder();
+        RoutingBuilders routingBuilders = RoutingBuilders.create(config);
 
-        Routing.Builder endpointRouting = defaultRouting;
+        serviceSupport.configureEndpoint(routingBuilders.defaultRoutingBuilder(), routingBuilders.routingBuilder());
 
-        if (routingNameConfig.isPresent()) {
-            String routingName = routingNameConfig.get();
-            // support for overriding this back to default routing using config
-            if (!"@default".equals(routingName)) {
-                endpointRouting = server.serverNamedRoutingBuilder(routingName);
-            }
-        }
-
-        serviceSupport.configureEndpoint(endpointRouting);
-
-        return defaultRouting;
+        return routingBuilders.defaultRoutingBuilder();
     }
 
     protected T serviceSupport() {

service-common/rest/src/main/java/io/helidon/servicecommon/rest/HelidonRestServiceSupport.java

@@ -35,7 +35,8 @@
  *     </ul>
  *
  * <p>
- *     Concrete implementations must implement {@link #postConfigureEndpoint(Routing.Rules)} to do any service-specific routing.
+ *     Concrete implementations must implement {@link #postConfigureEndpoint(Routing.Rules, Routing.Rules)} to do any
+ *     service-specific routing.
  *     See also the {@link Builder} information for possible additional overrides.
  * </p>
  *
@@ -59,28 +60,44 @@ protected HelidonRestServiceSupport(Logger logger, Builder<?, ?> builder, String
         corsEnabledServiceHelper = CorsEnabledServiceHelper.create(serviceName, builder.crossOriginConfig);
     }
 
+    /**
+     * Avoid using this obsolete method. Use {@link #configureEndpoint(Routing.Rules, Routing.Rules)} instead. (Neither method
+     * should typically invoked directly from user code.)
+     *
+     * @param rules routing rules (also accepts
+     * {@link io.helidon.webserver.Routing.Builder}
+     */
+    @Deprecated
+    public final void configureEndpoint(Routing.Rules rules) {
+        configureEndpoint(rules, rules);
+    }
+
     /**
      * Configures service endpoint on the provided routing rules. This method
      * just adds the endpoint path (as defaulted or configured).
      * This method is exclusive to
      * {@link #update(io.helidon.webserver.Routing.Rules)} (e.g. you should not
      * use both, as otherwise you would register the endpoint twice)
      *
-     * @param rules routing rules (also accepts
-     * {@link io.helidon.webserver.Routing.Builder}
+     * @param defaultRules default routing rules (also accepts {@link io.helidon.webserver.Routing.Builder}
+     * @param serviceEndpointRoutingRules actual rules (if different from default) for the service endpoint
      */
-    public final void configureEndpoint(Routing.Rules rules) {
+    public final void configureEndpoint(Routing.Rules defaultRules, Routing.Rules serviceEndpointRoutingRules) {
         // CORS first
-        rules.any(context, corsEnabledServiceHelper.processor());
-        postConfigureEndpoint(rules);
+        defaultRules.any(context, corsEnabledServiceHelper.processor());
+        if (defaultRules != serviceEndpointRoutingRules) {
+            serviceEndpointRoutingRules.any(context, corsEnabledServiceHelper.processor());
+        }
+        postConfigureEndpoint(defaultRules, serviceEndpointRoutingRules);
     }
 
     /**
      * Concrete implementations override this method to perform any service-specific routing set-up.
      *
-     * @param rules {@code Routing.Rules} to be updated
+     * @param defaultRules default {@code Routing.Rules} to be updated
+     * @param serviceEndpointRoutingRules actual rules (if different from the default ones) to be updated for the service endpoint
      */
-    protected abstract void postConfigureEndpoint(Routing.Rules rules);
+    protected abstract void postConfigureEndpoint(Routing.Rules defaultRules, Routing.Rules serviceEndpointRoutingRules);
 
     protected String context() {
         return context;

webserver/webserver/src/main/java/io/helidon/webserver/KeyPerformanceIndicatorContextFactory.java

@@ -69,15 +69,32 @@ protected Metrics kpiMetrics() {
     private static class DeferrableRequestContext extends ImmediateRequestContext
             implements KeyPerformanceIndicatorSupport.DeferrableRequestContext {
 
+        private boolean isStartRecorded = false;
+
         @Override
         public void requestHandlingStarted(Metrics kpiMetrics) {
             kpiMetrics(kpiMetrics);
+            recordStartTime(); // In case no handler in the chain manages the start-of-processing moment.
             kpiMetrics.onRequestReceived();
         }
 
         @Override
         public void requestProcessingStarted() {
-            recordStartTime();
+            recordStartTime(); // Overwrite the previously-recorded, provisional start time, now that we have a real one.
+            recordProcessingStarted();
+        }
+
+        @Override
+        public void requestProcessingCompleted(boolean isSuccessful) {
+            // No handler explicitly dealt with start-of-processing, so approximate it based on request receipt time.
+            if (!isStartRecorded) {
+                recordProcessingStarted();
+            }
+            super.requestProcessingCompleted(isSuccessful);
+        }
+
+        private void recordProcessingStarted() {
+            isStartRecorded = true;
             Metrics kpiMetrics = kpiMetrics();
             if (kpiMetrics != null) {
                 kpiMetrics().onRequestStarted();