Keep the URIs in the metrics tag if they match a client or server pat…

…tern Closes quarkusio#39581 Co-authored-by: Erin Schnabel <[email protected]> (cherry picked from commit 2b0b1f2)
gsmet · Jun 4, 2024 · 1bc33c4 · 1bc33c4
1 parent a0e04e6
commit 1bc33c4
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 24 deletions.
diff --git a/...micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/HttpCommonTags.java b/...micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/HttpCommonTags.java
@@ -1,5 +1,7 @@
 package io.quarkus.micrometer.runtime.binder;
 
+import java.util.Objects;
+
 import io.micrometer.core.instrument.Tag;
 import io.micrometer.core.instrument.binder.http.Outcome;
 
@@ -46,30 +48,47 @@ public static Tag outcome(int statusCode) {
 
     /**
      * Creates a {@code uri} tag based on the URI of the given {@code request}.
-     * Falling back to {@code REDIRECTION} for 3xx responses, {@code NOT_FOUND}
-     * for 404 responses, {@code root} for requests with no path info, and {@code UNKNOWN}
+     * Falling back to {@code REDIRECTION} for 3xx responses if there wasn't a matched path pattern, {@code NOT_FOUND}
+     * for 404 responses if there wasn't a matched path pattern, {@code root} for requests with no path info, and
+     * {@code UNKNOWN}
      * for all other requests.
      *
      * @param pathInfo request path
+     * @param initialPath initial path before request pattern matching took place. Pass in null if there is pattern matching
+     *        done in the caller.
      * @param code status code of the response
      * @return the uri tag derived from the request
      */
-    public static Tag uri(String pathInfo, int code) {
-        if (code > 0) {
-            if (code / 100 == 3) {
-                return URI_REDIRECTION;
-            } else if (code == 404) {
-                return URI_NOT_FOUND;
-            }
-        }
+    public static Tag uri(String pathInfo, String initialPath, int code) {
         if (pathInfo == null) {
             return URI_UNKNOWN;
         }
         if (pathInfo.isEmpty() || "/".equals(pathInfo)) {
             return URI_ROOT;
         }
 
+        if (code > 0) {
+            if (code / 100 == 3) {
+                if (isTemplatedPath(pathInfo, initialPath)) {
+                    return Tag.of("uri", pathInfo);
+                } else {
+                    return URI_REDIRECTION;
+                }
+            } else if (code == 404) {
+                if (isTemplatedPath(pathInfo, initialPath)) {
+                    return Tag.of("uri", pathInfo);
+                } else {
+                    return URI_NOT_FOUND;
+                }
+            }
+        }
+
         // Use first segment of request path
         return Tag.of("uri", pathInfo);
     }
+
+    private static boolean isTemplatedPath(String pathInfo, String initialPath) {
+        // only include the path info if it has been matched to a template (initialPath != pathInfo) to avoid a metrics explosion with lots of entries
+        return initialPath != null && !Objects.equals(initialPath, pathInfo);
+    }
 }
diff --git a/...r/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/RestClientMetricsFilter.java b/...r/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/RestClientMetricsFilter.java
@@ -72,7 +72,7 @@ public void filter(final ClientRequestContext requestContext, final ClientRespon
                 Timer.Builder builder = Timer.builder(httpMetricsConfig.getHttpClientRequestsName())
                         .tags(Tags.of(
                                 HttpCommonTags.method(requestContext.getMethod()),
-                                HttpCommonTags.uri(requestPath, statusCode),
+                                HttpCommonTags.uri(requestPath, requestContext.getUri().getPath(), statusCode),
                                 HttpCommonTags.outcome(statusCode),
                                 HttpCommonTags.status(statusCode),
                                 clientName(requestContext)));

diff --git a/...time/src/main/java/io/quarkus/micrometer/runtime/binder/vertx/VertxHttpClientMetrics.java b/...time/src/main/java/io/quarkus/micrometer/runtime/binder/vertx/VertxHttpClientMetrics.java
@@ -183,7 +183,7 @@ public static class RequestTracker extends RequestMetricInfo {
             this.tags = origin.and(
                     Tag.of("address", address),
                     HttpCommonTags.method(method),
-                    HttpCommonTags.uri(path, -1));
+                    HttpCommonTags.uri(path, null, -1));
         }
 
         void requestReset() {

diff --git a/...time/src/main/java/io/quarkus/micrometer/runtime/binder/vertx/VertxHttpServerMetrics.java b/...time/src/main/java/io/quarkus/micrometer/runtime/binder/vertx/VertxHttpServerMetrics.java
@@ -99,7 +99,7 @@ public HttpRequestMetric responsePushed(LongTaskTimer.Sample socketMetric, HttpM
                 config.getServerIgnorePatterns());
         if (path != null) {
             registry.counter(nameHttpServerPush, Tags.of(
-                    HttpCommonTags.uri(path, response.statusCode()),
+                    HttpCommonTags.uri(path, requestMetric.initialPath, response.statusCode()),
                     VertxMetricsTags.method(method),
                     VertxMetricsTags.outcome(response),
                     HttpCommonTags.status(response.statusCode())))
@@ -153,7 +153,7 @@ public void requestReset(HttpRequestMetric requestMetric) {
             Timer.Builder builder = Timer.builder(nameHttpServerRequests)
                     .tags(Tags.of(
                             VertxMetricsTags.method(requestMetric.request().method()),
-                            HttpCommonTags.uri(path, 0),
+                            HttpCommonTags.uri(path, requestMetric.initialPath, 0),
                             Outcome.CLIENT_ERROR.asTag(),
                             HttpCommonTags.STATUS_RESET));
 
@@ -180,7 +180,7 @@ public void responseEnd(HttpRequestMetric requestMetric, HttpResponse response,
             Timer.Sample sample = requestMetric.getSample();
             Tags allTags = Tags.of(
                     VertxMetricsTags.method(requestMetric.request().method()),
-                    HttpCommonTags.uri(path, response.statusCode()),
+                    HttpCommonTags.uri(path, requestMetric.initialPath, response.statusCode()),
                     VertxMetricsTags.outcome(response),
                     HttpCommonTags.status(response.statusCode()));
             if (!httpServerMetricsTagsContributors.isEmpty()) {
@@ -217,7 +217,7 @@ public LongTaskTimer.Sample connected(LongTaskTimer.Sample sample, HttpRequestMe
                 config.getServerIgnorePatterns());
         if (path != null) {
             return LongTaskTimer.builder(nameWebsocketConnections)
-                    .tags(Tags.of(HttpCommonTags.uri(path, 0)))
+                    .tags(Tags.of(HttpCommonTags.uri(path, requestMetric.initialPath, 0)))
                     .register(registry)
                     .start();
         }

diff --git a/...ometer/runtime/src/test/java/io/quarkus/micrometer/runtime/binder/HttpCommonTagsTest.java b/...ometer/runtime/src/test/java/io/quarkus/micrometer/runtime/binder/HttpCommonTagsTest.java
@@ -21,17 +21,21 @@ public void testStatus() {
 
     @Test
     public void testUriRedirect() {
-        Assertions.assertEquals(HttpCommonTags.URI_REDIRECTION, HttpCommonTags.uri("/moved", 301));
-        Assertions.assertEquals(HttpCommonTags.URI_REDIRECTION, HttpCommonTags.uri("/moved", 302));
-        Assertions.assertEquals(HttpCommonTags.URI_REDIRECTION, HttpCommonTags.uri("/moved", 304));
+        Assertions.assertEquals(HttpCommonTags.URI_REDIRECTION, HttpCommonTags.uri("/moved", null, 301));
+        Assertions.assertEquals(HttpCommonTags.URI_REDIRECTION, HttpCommonTags.uri("/moved", null, 302));
+        Assertions.assertEquals(HttpCommonTags.URI_REDIRECTION, HttpCommonTags.uri("/moved", null, 304));
+        Assertions.assertEquals(Tag.of("uri", "/moved/{id}"), HttpCommonTags.uri("/moved/{id}", "/moved/111", 304));
+        Assertions.assertEquals(HttpCommonTags.URI_ROOT, HttpCommonTags.uri("/", null, 304));
     }
 
     @Test
     public void testUriDefaults() {
-        Assertions.assertEquals(HttpCommonTags.URI_ROOT, HttpCommonTags.uri("/", 200));
-        Assertions.assertEquals(Tag.of("uri", "/known/ok"), HttpCommonTags.uri("/known/ok", 200));
-        Assertions.assertEquals(HttpCommonTags.URI_NOT_FOUND, HttpCommonTags.uri("/invalid", 404));
-        Assertions.assertEquals(Tag.of("uri", "/known/bad/request"), HttpCommonTags.uri("/known/bad/request", 400));
-        Assertions.assertEquals(Tag.of("uri", "/known/server/error"), HttpCommonTags.uri("/known/server/error", 500));
+        Assertions.assertEquals(HttpCommonTags.URI_ROOT, HttpCommonTags.uri("/", null, 200));
+        Assertions.assertEquals(HttpCommonTags.URI_ROOT, HttpCommonTags.uri("/", null, 404));
+        Assertions.assertEquals(Tag.of("uri", "/known/ok"), HttpCommonTags.uri("/known/ok", null, 200));
+        Assertions.assertEquals(HttpCommonTags.URI_NOT_FOUND, HttpCommonTags.uri("/invalid", null, 404));
+        Assertions.assertEquals(Tag.of("uri", "/invalid/{id}"), HttpCommonTags.uri("/invalid/{id}", "/invalid/111", 404));
+        Assertions.assertEquals(Tag.of("uri", "/known/bad/request"), HttpCommonTags.uri("/known/bad/request", null, 400));
+        Assertions.assertEquals(Tag.of("uri", "/known/server/error"), HttpCommonTags.uri("/known/server/error", null, 500));
     }
 }