Skip to content

Commit

Permalink
add configs to suppress uris on 4xx error metrics
Browse files Browse the repository at this point in the history
  • Loading branch information
brunobat committed Jan 7, 2025
1 parent 0065a02 commit 7c3c88d
Show file tree
Hide file tree
Showing 9 changed files with 188 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package io.quarkus.micrometer.deployment.binder;

import jakarta.inject.Inject;
import jakarta.inject.Singleton;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.core.Response;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.micrometer.core.instrument.MeterRegistry;
import io.quarkus.micrometer.test.Util;
import io.quarkus.test.QuarkusUnitTest;
import io.restassured.RestAssured;

public class HttpTagExplosionPreventionTest {
@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withConfigurationResource("test-logging.properties")
.overrideConfigKey("quarkus.micrometer.binder-enabled-default", "false")
.overrideConfigKey("quarkus.micrometer.binder.http-client.enabled", "true")
.overrideConfigKey("quarkus.micrometer.binder.http-server.enabled", "true")
.overrideConfigKey("quarkus.micrometer.binder.vertx.enabled", "true")
.overrideConfigKey("pingpong/mp-rest/url", "${test.url}")
.overrideConfigKey("quarkus.redis.devservices.enabled", "false")
.overrideConfigKey("quarkus.micrometer.binder.http-server.suppress4xx-errors", "true")
.withApplicationRoot((jar) -> jar
.addClasses(Util.class,
Resource.class));

@Inject
MeterRegistry registry;

@Test
public void test() throws Exception {
RestAssured.get("/api/hello").then().statusCode(200);
Util.waitForMeters(registry.find("http.server.requests").timers(), 1);
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/api/hello").timers().size());

RestAssured.get("/api/hello/foo").then().statusCode(200);
Util.waitForMeters(registry.find("http.server.requests").timers(), 2);
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/api/hello/{message}").timers().size());

RestAssured.delete("/api/hello").then().statusCode(405);
Util.waitForMeters(registry.find("http.server.requests").timers(), 3);
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "UNKNOWN").timers().size());

RestAssured.delete("/api/hello/foo").then().statusCode(405);
RestAssured.delete("/api/hello/bar").then().statusCode(405);
RestAssured.delete("/api/hello/baz").then().statusCode(405);
Util.waitForMeters(registry.find("http.server.requests").timers(), 6);
Assertions.assertEquals(4,
registry.find("http.server.requests").tag("uri", "UNKNOWN").timers().iterator().next().count());

for (int i = 0; i < 10; i++) {
RestAssured.get("/api/failure").then().statusCode(500);
RestAssured.get("/api/failure/bar" + i).then().statusCode(500);
}
Util.waitForMeters(registry.find("http.server.requests").timers(), 6 + 10);

Assertions.assertEquals(2, registry.find("http.server.requests").tag("uri", "UNKNOWN").timers().size()); // 2 different set of tags
Assertions.assertEquals(1, registry.find("http.server.requests").tag("uri", "/api/failure/{message}").timers().size());
}

@Path("/")
@Singleton
public static class Resource {

@GET
@Path("api/hello/{message}")
public String hello(@PathParam("message") String message) {
return message;
}

@GET
@Path("api/hello/")
public String hello() {
return "hello";
}

@GET
@Path("api/failure")
public Response failure() {
return Response.status(500).build();
}

@GET
@Path("api/failure/{message}")
public Response failure(@PathParam("message") String message) {
return Response.status(500).build();
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ public class HttpBinderConfiguration {
List<Pattern> clientIgnorePatterns = Collections.emptyList();
Map<Pattern, String> clientMatchPatterns = Collections.emptyMap();

boolean serverSuppress4xxErrors = false;
boolean clientSuppress4xxErrors = false;

private HttpBinderConfiguration() {
}

Expand All @@ -47,6 +50,8 @@ public HttpBinderConfiguration(boolean httpServerMetrics, boolean httpClientMetr

serverEnabled = httpServerMetrics;
clientEnabled = httpClientMetrics;
serverSuppress4xxErrors = serverConfig.suppress4xxErrors;
clientSuppress4xxErrors = clientConfig.suppress4xxErrors;

if (serverEnabled) {
Pattern defaultIgnore = null;
Expand Down Expand Up @@ -104,6 +109,14 @@ public Map<Pattern, String> getClientMatchPatterns() {
return clientMatchPatterns;
}

public boolean isServerSuppress4xxErrors() {
return serverSuppress4xxErrors;
}

public boolean isClientSuppress4xxErrors() {
return clientSuppress4xxErrors;
}

List<Pattern> getIgnorePatterns(Optional<List<String>> configInput, Pattern defaultIgnore) {
if (configInput.isPresent()) {
List<String> input = configInput.get();
Expand Down Expand Up @@ -186,6 +199,8 @@ public HttpBinderConfiguration unwrap() {
// not dev-mode changeable
result.clientEnabled = this.clientEnabled;
result.serverEnabled = this.serverEnabled;
result.serverSuppress4xxErrors = this.serverSuppress4xxErrors;
result.clientSuppress4xxErrors = this.clientSuppress4xxErrors;
return result.update(this);
}

Expand All @@ -194,6 +209,8 @@ public HttpBinderConfiguration update(HttpBinderConfiguration httpConfig) {
this.serverMatchPatterns = httpConfig.serverMatchPatterns;
this.clientIgnorePatterns = httpConfig.clientIgnorePatterns;
this.serverIgnorePatterns = httpConfig.serverIgnorePatterns;
this.serverSuppress4xxErrors = httpConfig.serverSuppress4xxErrors;
this.clientSuppress4xxErrors = httpConfig.clientSuppress4xxErrors;
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public class HttpCommonTags {
public static final Tag URI_NOT_FOUND = Tag.of("uri", "NOT_FOUND");
public static final Tag URI_REDIRECTION = Tag.of("uri", "REDIRECTION");
public static final Tag URI_ROOT = Tag.of("uri", "root");
public static final Tag URI_UNKNOWN = Tag.of("uri", "UNKNOWN");
static final Tag URI_UNKNOWN = Tag.of("uri", "UNKNOWN");

static final Tag STATUS_UNKNOWN = Tag.of("status", "UNKNOWN");
public static final Tag STATUS_RESET = Tag.of("status", "RESET");
Expand Down Expand Up @@ -59,7 +59,7 @@ public static Tag outcome(int statusCode) {
* @param code status code of the response
* @return the uri tag derived from the request
*/
public static Tag uri(String pathInfo, String initialPath, int code) {
public static Tag uri(String pathInfo, String initialPath, int code, boolean suppress4xxErrors) {
if (pathInfo == null) {
return URI_UNKNOWN;
}
Expand All @@ -80,6 +80,17 @@ public static Tag uri(String pathInfo, String initialPath, int code) {
} else {
return URI_NOT_FOUND;
}
} else if (code >= 400) {
if (!suppress4xxErrors) {
// legacy behaviour
return Tag.of("uri", pathInfo);
} else if (isTemplatedPath(pathInfo, initialPath)) {
return Tag.of("uri", pathInfo);
} else {
// Do not return the path info as it can lead to a metrics explosion
// for 4xx and 5xx responses
return URI_UNKNOWN;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ public void filter(final ClientRequestContext requestContext, final ClientRespon
sample.stop(timer
.withTags(Tags.of(
HttpCommonTags.method(requestContext.getMethod()),
HttpCommonTags.uri(requestPath, requestContext.getUri().getPath(), statusCode),
HttpCommonTags.uri(requestPath, requestContext.getUri().getPath(), statusCode,
httpMetricsConfig.isClientSuppress4xxErrors()),
HttpCommonTags.outcome(statusCode),
HttpCommonTags.status(statusCode),
clientName(requestContext))));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public static class RequestTracker extends RequestMetricInfo {
this.tags = origin.and(
Tag.of("address", address),
HttpCommonTags.method(request.method().name()),
HttpCommonTags.uri(request.uri(), null, -1));
HttpCommonTags.uri(request.uri(), null, -1, false));
}

void requestReset() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ public HttpRequestMetric responsePushed(LongTaskTimer.Sample socketMetric, HttpM
if (path != null) {
pushCounter
.withTags(Tags.of(
HttpCommonTags.uri(path, requestMetric.initialPath, response.statusCode()),
HttpCommonTags.uri(path, requestMetric.initialPath, response.statusCode(),
config.isServerSuppress4xxErrors()),
VertxMetricsTags.method(method),
VertxMetricsTags.outcome(response),
HttpCommonTags.status(response.statusCode())))
Expand Down Expand Up @@ -173,7 +174,7 @@ public void requestReset(HttpRequestMetric requestMetric) {
sample::stop,
requestsTimer.withTags(Tags.of(
VertxMetricsTags.method(requestMetric.request().method()),
HttpCommonTags.uri(path, requestMetric.initialPath, 0),
HttpCommonTags.uri(path, requestMetric.initialPath, 0, false),
Outcome.CLIENT_ERROR.asTag(),
HttpCommonTags.STATUS_RESET)),
requestMetric.request().context());
Expand All @@ -199,7 +200,8 @@ public void responseEnd(HttpRequestMetric requestMetric, HttpResponse response,
Timer.Sample sample = requestMetric.getSample();
Tags allTags = Tags.of(
VertxMetricsTags.method(requestMetric.request().method()),
HttpCommonTags.uri(path, requestMetric.initialPath, response.statusCode()),
HttpCommonTags.uri(path, requestMetric.initialPath, response.statusCode(),
config.isServerSuppress4xxErrors()),
VertxMetricsTags.outcome(response),
HttpCommonTags.status(response.statusCode()));
if (!httpServerMetricsTagsContributors.isEmpty()) {
Expand Down Expand Up @@ -237,7 +239,7 @@ public LongTaskTimer.Sample connected(LongTaskTimer.Sample sample, HttpRequestMe
config.getServerIgnorePatterns());
if (path != null) {
return websocketConnectionTimer
.withTags(Tags.of(HttpCommonTags.uri(path, requestMetric.initialPath, 0)))
.withTags(Tags.of(HttpCommonTags.uri(path, requestMetric.initialPath, 0, false)))
.start();
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@ public class HttpClientConfig {
@ConfigItem
public Optional<List<String>> ignorePatterns = Optional.empty();

/**
* Suppress 4xx errors from metrics collection for unmatched templates.
* This configuration exists to limit cardinality explosion from caller side errors. Does not apply to 404 errors.
*
* Suppressing 4xx errors is disabled by default.
*
* @asciidoclet
*/
@ConfigItem(defaultValue = "false")
public boolean suppress4xxErrors;

/**
* Maximum number of unique URI tag values allowed. After the max number of
* tag values is reached, metrics with additional tag values are denied by
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,17 @@ public class HttpServerConfig {
@ConfigItem(defaultValue = "true")
public boolean suppressNonApplicationUris;

/**
* Suppress 4xx errors from metrics collection for unmatched templates.
* This configuration exists to limit cardinality explosion from caller side error. Does not apply to 404 errors.
*
* Suppressing 4xx errors is disabled by default.
*
* @asciidoclet
*/
@ConfigItem(defaultValue = "false")
public boolean suppress4xxErrors;

/**
* Maximum number of unique URI tag values allowed. After the max number of
* tag values is reached, metrics with additional tag values are denied by
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,39 @@ public void testStatus() {

@Test
public void testUriRedirect() {
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));
Assertions.assertEquals(HttpCommonTags.URI_REDIRECTION, HttpCommonTags.uri("/moved", null, 301, false));
Assertions.assertEquals(HttpCommonTags.URI_REDIRECTION, HttpCommonTags.uri("/moved", null, 302, false));
Assertions.assertEquals(HttpCommonTags.URI_REDIRECTION, HttpCommonTags.uri("/moved", null, 304, false));
Assertions.assertEquals(Tag.of("uri", "/moved/{id}"), HttpCommonTags.uri("/moved/{id}", "/moved/111", 304, false));
Assertions.assertEquals(HttpCommonTags.URI_ROOT, HttpCommonTags.uri("/", null, 304, false));
}

@Test
public void testUriDefaults() {
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));
Assertions.assertEquals(HttpCommonTags.URI_ROOT, HttpCommonTags.uri("/", null, 200, false));
Assertions.assertEquals(HttpCommonTags.URI_ROOT, HttpCommonTags.uri("/", null, 404, false));
Assertions.assertEquals(Tag.of("uri", "/known/ok"), HttpCommonTags.uri("/known/ok", null, 200, false));
Assertions.assertEquals(HttpCommonTags.URI_NOT_FOUND, HttpCommonTags.uri("/invalid", null, 404, false));
Assertions.assertEquals(Tag.of("uri", "/invalid/{id}"),
HttpCommonTags.uri("/invalid/{id}", "/invalid/111", 404, false));
Assertions.assertEquals(Tag.of("uri", "/known/bad/request"),
HttpCommonTags.uri("/known/bad/request", null, 400, false));
Assertions.assertEquals(Tag.of("uri", "/known/bad/{request}"),
HttpCommonTags.uri("/known/bad/{request}", "/known/bad/request", 400, false));
Assertions.assertEquals(Tag.of("uri", "/known/server/error"),
HttpCommonTags.uri("/known/server/error", null, 500, false));
}

@Test
public void testUriDefaultsWithSuppression() {
Assertions.assertEquals(HttpCommonTags.URI_ROOT, HttpCommonTags.uri("/", null, 200, true));
Assertions.assertEquals(HttpCommonTags.URI_ROOT, HttpCommonTags.uri("/", null, 404, true));
Assertions.assertEquals(Tag.of("uri", "/known/ok"), HttpCommonTags.uri("/known/ok", null, 200, true));
Assertions.assertEquals(HttpCommonTags.URI_NOT_FOUND, HttpCommonTags.uri("/invalid", null, 404, true));
Assertions.assertEquals(Tag.of("uri", "/invalid/{id}"), HttpCommonTags.uri("/invalid/{id}", "/invalid/111", 404, true));
Assertions.assertEquals(HttpCommonTags.URI_UNKNOWN, HttpCommonTags.uri("/known/bad/request", null, 400, true));
Assertions.assertEquals(Tag.of("uri", "/known/bad/{request}"),
HttpCommonTags.uri("/known/bad/{request}", "/known/bad/request", 400, true));
Assertions.assertEquals(HttpCommonTags.URI_UNKNOWN, HttpCommonTags.uri("/known/server/error", null, 500, true));
}
}

0 comments on commit 7c3c88d

Please sign in to comment.