Fix matching logic and tests for path entries

Author: tjquinno

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

@@ -69,16 +69,14 @@ static boolean isMeasured(AutoHttpMetricsConfig config, Method method, UriPath u
                 return true;
             }
 
-            boolean matchedPath = false;
             boolean latestResult = true;
 
             for (AutoHttpMetricsPathConfig cfg : config.effectivePathConfigs()) {
                 if (cfg.matchesPath(uriPath)) {
-                    matchedPath = true;
-                    latestResult = cfg.matchesMethod(method) && cfg.enabled();
+                    latestResult = cfg.matchesMethod(method) == cfg.enabled();
                 }
             }
-            return !matchedPath || latestResult;
+            return latestResult;
         }
 
         /**

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

@@ -42,7 +42,10 @@ void testAutoMetricsConfig() {
                               - path: "/greet"
                                 methods: ["GET","HEAD"]
                               - path: "/greet/{name}"
-                                methods: ["GET","OPTION"]
+                                methods: ["GET","OPTIONS"]
+                              - path: "/stuff"
+                                methods: ["HEAD","OPTIONS"]
+                                enabled: false
                               - path: "/hi"
                                 enabled: false
                 """;
@@ -59,6 +62,8 @@ void testAutoMetricsConfig() {
 
         assertThat("GET /metrics", config.isMeasured(Method.GET, UriPath.create("/metrics")), is(false));
 
+        assertThat("GET /stuff", config.isMeasured(Method.GET, UriPath.create("/stuff")), is(true));
+        assertThat("HEAD /stuff", config.isMeasured(Method.HEAD, UriPath.create("/stuff")), is(false));
 
     }
 

webserver/observe/telemetry/metrics/src/main/java/io/helidon/webserver/observe/telemetry/metrics/OpenTelemetryMetricsHttpSemanticConventions.java

@@ -162,11 +162,11 @@ private void updateMetricsIfMeasured(RoutingRequest req,
             /*
             If we search for the meter and find it we avoid constructing the builder unnecessarily.
              */
-            meterRegistry.timer(TIMER_NAME, tags)
+            var timer = meterRegistry.timer(TIMER_NAME, tags)
                     .orElse(meterRegistry.getOrCreate(Timer.builder(TIMER_NAME)
                                                               .tags(tags)
-                                                              .buckets(BUCKET_BOUNDARIES)))
-                    .record(endTime - startTime, TimeUnit.NANOSECONDS);
+                                                              .buckets(BUCKET_BOUNDARIES)));
+            timer.record(endTime - startTime, TimeUnit.NANOSECONDS);
         }
 
         private String errorType(RoutingResponse resp, Exception exception) {

webserver/observe/telemetry/metrics/src/test/java/io/helidon/webserver/observe/telemetry/metrics/TestOpenTelemetrySemanticConventions.java

@@ -28,9 +28,9 @@
 import io.helidon.common.testing.junit5.MatcherWithRetry;
 import io.helidon.config.Config;
 import io.helidon.config.ConfigSources;
+import io.helidon.http.Method;
 import io.helidon.metrics.api.MeterRegistry;
-import io.helidon.metrics.api.Timer;
-import io.helidon.service.registry.Services;
+import io.helidon.metrics.api.Metrics;import io.helidon.metrics.api.Timer;
 import io.helidon.webclient.http1.Http1Client;
 import io.helidon.webclient.http1.Http1ClientResponse;
 import io.helidon.webserver.WebServer;
@@ -88,6 +88,10 @@ static void setupServer(WebServerConfig.Builder serverBuilder) {
                       observers:
                         metrics:
                           auto:
+                            paths:
+                              - path: /greet
+                                methods: ["OPTIONS"]
+                                enabled: false
                             sockets: ["@default","private"]
                             opt-in: ["http.server.request.duration:server.address"]
                 """;
@@ -98,15 +102,20 @@ static void setupServer(WebServerConfig.Builder serverBuilder) {
                 .routing(r -> r.get("/greet/{name}",
                                     (req, resp) ->
                                             resp.send("Hello, " + req.path().segments().get(1).value() + "!")))
-                .routing("private", r -> r.get("/greet",
-                                               (req, resp) ->
-                                                       resp.send("Hello, World!")));
+                .routing("private", r -> r.any("/greet",
+                                               (req, resp) -> {
+                                                   switch (req.prologue().method().text()) {
+                                                   case Method.GET_NAME -> resp.send("Hello, World!");
+                                                   case Method.OPTIONS_NAME -> resp.send("Options, World!");
+                                                   default -> resp.next();
+                                                   }
+                                               }));
 
     }
 
     @Test
     void checkAutoMetrics() {
-        var meterRegistry = Services.get(MeterRegistry.class);
+        var meterRegistry = Metrics.globalRegistry();
 
         // Use default socket.
         try (Http1ClientResponse defaultResponse = defaultClient.get("/greet/Joe")
@@ -120,15 +129,20 @@ void checkAutoMetrics() {
                         .request();
                 Http1ClientResponse metricsOnDefaultResponse = defaultClient.get("/observe/metrics")
                         .accept(MediaTypes.APPLICATION_JSON)
+                        .request();
+                Http1ClientResponse greetOptionsResponse = privateClient.options("/greet")
+                        .accept(MediaTypes.TEXT_PLAIN)
                         .request()) {
 
             assertThat("Greet endpoint", defaultResponse.status().code(), is(200));
             assertThat("Private endpoint", privateResponse.status().code(), is(200));
             assertThat("Admin endpoint", adminResponse.status().code(), is(200));
             assertThat("Metrics endpoint via default socket", metricsOnDefaultResponse.status().code(), is(404));
+            assertThat("Private endpoint HEAD", greetOptionsResponse.status().code(), is(200));
 
             /*
             Helidon registers and updates the timer asynchronously, so tolerate some delay.
+            Despite all the requests we sent, we expect only two timers to have been created (and updated).
              */
             AtomicReference<List<Timer>> timersRef = requestTimers(meterRegistry, 2);
 
@@ -205,7 +219,6 @@ void checkAutoMetrics() {
             socketNamesInTimers.forEach(unexpectedlyUntimedSockets::remove);
             assertThat("Unexpectedly untimed sockets", unexpectedlyUntimedSockets, hasSize(0));
 
-
         }
 
     }