From 828599d2fc2b191e2c1f9f0a5865c59c5d840ebd Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Wed, 20 Mar 2024 12:56:18 +0100 Subject: [PATCH] Keep the URIs in the metrics tag if they match a client or server pattern Closes #39581 Co-authored-by: Erin Schnabel (cherry picked from commit 2b0b1f2c2d67ee954f310d9a913ef233a456526d) --- .../runtime/binder/HttpCommonTags.java | 39 ++++++++++++++----- .../binder/RestClientMetricsFilter.java | 2 +- .../binder/vertx/VertxHttpClientMetrics.java | 2 +- .../binder/vertx/VertxHttpServerMetrics.java | 8 ++-- .../runtime/binder/HttpCommonTagsTest.java | 20 ++++++---- 5 files changed, 47 insertions(+), 24 deletions(-) diff --git a/extensions/micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/HttpCommonTags.java b/extensions/micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/HttpCommonTags.java index 656736a6d345c..7cab35f4f0446 100644 --- a/extensions/micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/HttpCommonTags.java +++ b/extensions/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,22 +48,18 @@ 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; } @@ -69,7 +67,28 @@ public static Tag uri(String pathInfo, int code) { 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/extensions/micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/RestClientMetricsFilter.java b/extensions/micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/RestClientMetricsFilter.java index 004aa63e2d162..f1c6acb745eed 100644 --- a/extensions/micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/RestClientMetricsFilter.java +++ b/extensions/micrometer/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/extensions/micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/vertx/VertxHttpClientMetrics.java b/extensions/micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/vertx/VertxHttpClientMetrics.java index 8e3fc3474b8e1..5d346eee3428a 100644 --- a/extensions/micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/vertx/VertxHttpClientMetrics.java +++ b/extensions/micrometer/runtime/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/extensions/micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/vertx/VertxHttpServerMetrics.java b/extensions/micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/vertx/VertxHttpServerMetrics.java index 6f22060edc250..23b605d546109 100644 --- a/extensions/micrometer/runtime/src/main/java/io/quarkus/micrometer/runtime/binder/vertx/VertxHttpServerMetrics.java +++ b/extensions/micrometer/runtime/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/extensions/micrometer/runtime/src/test/java/io/quarkus/micrometer/runtime/binder/HttpCommonTagsTest.java b/extensions/micrometer/runtime/src/test/java/io/quarkus/micrometer/runtime/binder/HttpCommonTagsTest.java index 2474a9f228c6e..e6bf10ee83dbc 100644 --- a/extensions/micrometer/runtime/src/test/java/io/quarkus/micrometer/runtime/binder/HttpCommonTagsTest.java +++ b/extensions/micrometer/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)); } }