diff --git a/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/ServerRequestMeterRegistryFilter.java b/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/ServerRequestMeterRegistryFilter.java index 4f6de5cc7..4d2fcfc8e 100644 --- a/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/ServerRequestMeterRegistryFilter.java +++ b/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/ServerRequestMeterRegistryFilter.java @@ -27,6 +27,7 @@ import java.util.Optional; +import static io.micronaut.configuration.metrics.binder.web.WebMetricsPublisher.UNKNOWN; import static io.micronaut.core.util.StringUtils.FALSE; import static io.micronaut.http.HttpAttributes.URI_TEMPLATE; @@ -34,7 +35,7 @@ * Registers the timers and meters for each request. * *

The default is to intercept all paths /**, but using the - * property micronaut.metrics.http.path, this can be changed.

+ * property micronaut.metrics.http.path, this can be changed.

* * @author Christian Oestreich * @author graemerocher @@ -58,7 +59,7 @@ public ServerRequestMeterRegistryFilter(MeterRegistry meterRegistry) { private String resolvePath(HttpRequest request) { Optional route = request.getAttribute(URI_TEMPLATE, String.class); - return route.orElseGet(request::getPath); + return route.orElse(UNKNOWN); } @Override diff --git a/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/WebMetricsPublisher.java b/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/WebMetricsPublisher.java index 4972bcea4..1c2ec0345 100644 --- a/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/WebMetricsPublisher.java +++ b/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/WebMetricsPublisher.java @@ -58,7 +58,7 @@ public class WebMetricsPublisher> extends Flux { private static final Tag URI_NOT_FOUND = Tag.of("uri", "NOT_FOUND"); private static final Tag URI_REDIRECTION = Tag.of("uri", "REDIRECTION"); - private static final String UNKNOWN = "UNKNOWN"; + static final String UNKNOWN = "UNKNOWN"; private static final String METHOD = "method"; private static final String STATUS = "status"; private static final String URI = "uri"; diff --git a/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/NotAcceptableRouteWithPathVariableDoesNotPolluteMetricsSpec.groovy b/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/NotAcceptableRouteWithPathVariableDoesNotPolluteMetricsSpec.groovy new file mode 100644 index 000000000..11dbcdbef --- /dev/null +++ b/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/NotAcceptableRouteWithPathVariableDoesNotPolluteMetricsSpec.groovy @@ -0,0 +1,91 @@ +package io.micronaut.configuration.metrics + +import io.micronaut.context.annotation.Property +import io.micronaut.context.annotation.Requires +import io.micronaut.core.type.Argument +import io.micronaut.http.HttpRequest +import io.micronaut.http.HttpStatus +import io.micronaut.http.MediaType +import io.micronaut.http.annotation.Controller +import io.micronaut.http.annotation.Get +import io.micronaut.http.annotation.PathVariable +import io.micronaut.http.annotation.Status +import io.micronaut.http.client.BlockingHttpClient +import io.micronaut.http.client.HttpClient +import io.micronaut.http.client.annotation.Client +import io.micronaut.http.client.exceptions.HttpClientResponseException +import io.micronaut.test.extensions.spock.annotation.MicronautTest +import jakarta.inject.Inject +import spock.lang.Specification + +@Property(name = "spec.name", value = "RoutePathVariableNotFoundMetricsSpec") +@MicronautTest +class NotAcceptableRouteWithPathVariableDoesNotPolluteMetricsSpec extends Specification { + + @Inject + @Client("/") + HttpClient httpClient + + void 'sending multiple requests to a route with a path variable does not pollute the metrics uri'() { + given: + BlockingHttpClient client = httpClient.toBlocking() + + when: + client.exchange(HttpRequest.GET("/books/1")) + + then: + noExceptionThrown() + + when: + client.exchange(HttpRequest.GET("/books/2")) + + then: + noExceptionThrown() + + when: + List uris = fetchHttpServerRequestMetricsUris(client) + + then: + noExceptionThrown() + uris == ['/books/{id}'] + + when: + client.exchange(HttpRequest.GET("/books/1").accept(MediaType.TEXT_PLAIN)) + + then: + HttpClientResponseException e = thrown() + HttpStatus.NOT_ACCEPTABLE == e.status + + when: + client.exchange(HttpRequest.GET("/books/2").accept(MediaType.TEXT_PLAIN)) + + then: + e = thrown() + HttpStatus.NOT_ACCEPTABLE == e.status + + when: + uris = fetchHttpServerRequestMetricsUris(client) + + then: + noExceptionThrown() + ['/metrics/{name}','/books/{id}','UNKNOWN'] == uris + } + + private static List fetchHttpServerRequestMetricsUris(BlockingHttpClient client) { + fetchHttpServerRequestMetrics(client)["availableTags"].find { it.tag == "uri" }?.values + } + + private static Map fetchHttpServerRequestMetrics(BlockingHttpClient client) { + client.retrieve(HttpRequest.GET("/metrics/http.server.requests"), Argument.mapOf(String, Object)) + } + + @Requires(property = "spec.name", value = "RoutePathVariableNotFoundMetricsSpec") + @Controller + static class BookController { + @Get("/books/{id}") + @Status(HttpStatus.OK) + void index(@PathVariable String id) { + } + } +} +