Skip to content

Commit

Permalink
Keep the URIs in the metrics tag if they match a client or server pat…
Browse files Browse the repository at this point in the history
…tern

Closes quarkusio#39581

Co-authored-by: Erin Schnabel <ebullientworks@gmail.com>
(cherry picked from commit 2b0b1f2)
  • Loading branch information
ahus1 authored and gsmet committed Mar 26, 2024
1 parent b14d63a commit 828599d
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 24 deletions.
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())))
Expand Down Expand Up @@ -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));

Expand All @@ -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()) {
Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

0 comments on commit 828599d

Please sign in to comment.