From 612665490ca6bd91fde1f461272de18efb972a15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Serralheiro?= Date: Wed, 17 Apr 2024 17:53:15 +0100 Subject: [PATCH 1/3] Adding support for histograms - Added a new HttpHistogramMeterFilterFactory class to contribute to the configuration of the 'http.server.requests' and 'http.client.requests' meters. - Renamed the existing HttpMeterFilterFactory to HttpPercentilesMeterFilterFactory in order for the naming of both classes to be consistent. - Added test to validate configuration takes effect. --- .../web/HttpHistogramMeterFilterFactory.java | 79 +++++++++++++++++++ ...=> HttpPercentilesMeterFilterFactory.java} | 23 +++--- .../metrics/binder/web/HttpMetricsSpec.groovy | 17 +++- 3 files changed, 104 insertions(+), 15 deletions(-) create mode 100644 micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpHistogramMeterFilterFactory.java rename micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/{HttpMeterFilterFactory.java => HttpPercentilesMeterFilterFactory.java} (74%) diff --git a/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpHistogramMeterFilterFactory.java b/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpHistogramMeterFilterFactory.java new file mode 100644 index 000000000..a1d48a253 --- /dev/null +++ b/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpHistogramMeterFilterFactory.java @@ -0,0 +1,79 @@ +/* + * Copyright 2017-2024 original authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.micronaut.configuration.metrics.binder.web; + +import io.micrometer.core.instrument.Meter; +import io.micrometer.core.instrument.config.MeterFilter; +import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; +import io.micronaut.configuration.metrics.annotation.RequiresMetrics; +import io.micronaut.context.annotation.*; +import jakarta.inject.Singleton; + +import static io.micronaut.configuration.metrics.micrometer.MeterRegistryFactory.MICRONAUT_METRICS_BINDERS; +import static io.micronaut.core.util.StringUtils.FALSE; + +/** + * Optional filter for adding histograms to HTTP metrics. + * + * @author lcavadas + */ +@Factory +@RequiresMetrics +@Requires(property = WebMetricsPublisher.ENABLED, notEquals = FALSE) +@Primary +public class HttpHistogramMeterFilterFactory { + + /** + * Configure new MeterFilter for http.server.requests metrics. + * + * @param histogram If a histogram should be published + * @return A MeterFilter + */ + @Bean + @Singleton + @Requires(property = MICRONAUT_METRICS_BINDERS + ".web.server.histogram") + MeterFilter addServerPercentileMeterFilter(@Value("${" + MICRONAUT_METRICS_BINDERS + ".web.server.histogram}") Boolean histogram) { + return getMeterFilter(histogram, WebMetricsPublisher.METRIC_HTTP_SERVER_REQUESTS); + } + + /** + * Configure new MeterFilter for http.client.requests metrics. + * + * @param histogram If a histogram should be published + * @return A MeterFilter + */ + @Bean + @Singleton + @Requires(property = MICRONAUT_METRICS_BINDERS + ".web.client.histogram") + MeterFilter addClientPercentileMeterFilter(@Value("${" + MICRONAUT_METRICS_BINDERS + ".web.client.histogram}") Boolean histogram) { + return getMeterFilter(histogram, WebMetricsPublisher.METRIC_HTTP_CLIENT_REQUESTS); + } + + private MeterFilter getMeterFilter(Boolean histogram, String metricNamePrefix) { + return new MeterFilter() { + @Override + public DistributionStatisticConfig configure(Meter.Id id, DistributionStatisticConfig config) { + if (id.getName().startsWith(metricNamePrefix)) { + return DistributionStatisticConfig.builder() + .percentilesHistogram(histogram) + .build() + .merge(config); + } + return config; + } + }; + } +} diff --git a/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpMeterFilterFactory.java b/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpPercentilesMeterFilterFactory.java similarity index 74% rename from micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpMeterFilterFactory.java rename to micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpPercentilesMeterFilterFactory.java index d00bc3535..d216ad110 100644 --- a/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpMeterFilterFactory.java +++ b/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpPercentilesMeterFilterFactory.java @@ -26,8 +26,6 @@ import io.micronaut.core.util.ArrayUtils; import jakarta.inject.Singleton; -import static io.micronaut.configuration.metrics.binder.web.WebMetricsPublisher.METRIC_HTTP_CLIENT_REQUESTS; -import static io.micronaut.configuration.metrics.binder.web.WebMetricsPublisher.METRIC_HTTP_SERVER_REQUESTS; import static io.micronaut.configuration.metrics.micrometer.MeterRegistryFactory.MICRONAUT_METRICS_BINDERS; import static io.micronaut.core.util.StringUtils.FALSE; @@ -39,43 +37,46 @@ @Factory @RequiresMetrics @Requires(property = WebMetricsPublisher.ENABLED, notEquals = FALSE) -public class HttpMeterFilterFactory { +public class HttpPercentilesMeterFilterFactory { /** * Configure new MeterFilter for http.server.requests metrics. * * @param percentiles The percentiles + * @param histogram If a histogram should be published * @return A MeterFilter */ @Bean @Singleton @Requires(property = MICRONAUT_METRICS_BINDERS + ".web.server.percentiles") - MeterFilter addServerPercentileMeterFilter(@Value("${" + MICRONAUT_METRICS_BINDERS + ".web.server.percentiles}") Double[] percentiles) { - return getMeterFilter(percentiles, METRIC_HTTP_SERVER_REQUESTS); + MeterFilter addServerPercentileMeterFilter(@Value("${" + MICRONAUT_METRICS_BINDERS + ".web.server.percentiles}") Double[] percentiles, @Value("${" + MICRONAUT_METRICS_BINDERS + ".web.server.histogram:false}") Boolean histogram) { + return getMeterFilter(percentiles, histogram, WebMetricsPublisher.METRIC_HTTP_SERVER_REQUESTS); } /** * Configure new MeterFilter for http.client.requests metrics. * * @param percentiles The percentiles + * @param histogram If a histogram should be published * @return A MeterFilter */ @Bean @Singleton @Requires(property = MICRONAUT_METRICS_BINDERS + ".web.client.percentiles") - MeterFilter addClientPercentileMeterFilter(@Value("${" + MICRONAUT_METRICS_BINDERS + ".web.client.percentiles}") Double[] percentiles) { - return getMeterFilter(percentiles, METRIC_HTTP_CLIENT_REQUESTS); + MeterFilter addClientPercentileMeterFilter(@Value("${" + MICRONAUT_METRICS_BINDERS + ".web.client.percentiles}") Double[] percentiles, @Value("${" + MICRONAUT_METRICS_BINDERS + ".web.client.histogram:false}") Boolean histogram) { + return getMeterFilter(percentiles, histogram, WebMetricsPublisher.METRIC_HTTP_CLIENT_REQUESTS); } - private MeterFilter getMeterFilter(Double[] percentiles, String metricNamePrefix) { + private MeterFilter getMeterFilter(Double[] percentiles, Boolean histogram, String metricNamePrefix) { return new MeterFilter() { @Override public DistributionStatisticConfig configure(Meter.Id id, DistributionStatisticConfig config) { if (id.getName().startsWith(metricNamePrefix)) { return DistributionStatisticConfig.builder() - .percentiles((double[]) ArrayUtils.toPrimitiveArray(percentiles)) - .build() - .merge(config); + .percentiles((double[]) ArrayUtils.toPrimitiveArray(percentiles)) + .percentilesHistogram(histogram) + .build() + .merge(config); } return config; } diff --git a/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/binder/web/HttpMetricsSpec.groovy b/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/binder/web/HttpMetricsSpec.groovy index b0a135f23..423988725 100644 --- a/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/binder/web/HttpMetricsSpec.groovy +++ b/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/binder/web/HttpMetricsSpec.groovy @@ -55,6 +55,8 @@ class HttpMetricsSpec extends Specification { clientTimer.count() == 1 serverSnapshot.percentileValues().length == serverPercentilesCount clientSnapshot.percentileValues().length == clientPercentilesCount + serverTimer.getMetaPropertyValues().find { it.name.equals('distributionStatisticConfig') }.value.percentileHistogram == serverHistogram + clientTimer.getMetaPropertyValues().find { it.name.equals('distributionStatisticConfig') }.value.percentileHistogram == clientHistogram when: "A request is sent to the root route" @@ -135,9 +137,13 @@ class HttpMetricsSpec extends Specification { embeddedServer.close() where: - cfg | setting | serverPercentilesCount | clientPercentilesCount - MICRONAUT_METRICS_BINDERS + ".web.client.percentiles" | "0.95,0.99" | 0 | 2 - MICRONAUT_METRICS_BINDERS + ".web.server.percentiles" | "0.95,0.99" | 2 | 0 + cfg | setting | serverPercentilesCount | clientPercentilesCount| serverHistogram | clientHistogram + MICRONAUT_METRICS_BINDERS + ".web.client.percentiles" | "0.95,0.99" | 0 | 2 | null | false + MICRONAUT_METRICS_BINDERS + ".web.server.percentiles" | "0.95,0.99" | 2 | 0 | false | null + MICRONAUT_METRICS_BINDERS + ".web.server.histogram" | "true" | 0 | 0 | true | null + MICRONAUT_METRICS_BINDERS + ".web.server.histogram" | "false" | 0 | 0 | false | null + MICRONAUT_METRICS_BINDERS + ".web.client.histogram" | "true" | 0 | 0 | null | true + MICRONAUT_METRICS_BINDERS + ".web.client.histogram" | "false" | 0 | 0 | null | false } void "test client / server metrics ignored uris for client errors"() { @@ -303,7 +309,10 @@ class HttpMetricsSpec extends Specification { String root() { "root" } @Get('/test-http-metrics') - String index() { "ok" } + String index() { + Thread.sleep(1000) + "ok" + } @Get("/test-http-metrics/{id}") String template(String id) { "ok " + id } From 4b166d736168d5e74c137a2115d6c0c79ce10201 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Serralheiro?= Date: Fri, 19 Apr 2024 08:26:20 +0100 Subject: [PATCH 2/3] CR Fix Leave the original percentiles class alone so this PR can be released as a patch --- ...ctory.java => HttpMeterFilterFactory.java} | 23 +++++++++---------- .../metrics/binder/web/HttpMetricsSpec.groovy | 4 ++-- 2 files changed, 13 insertions(+), 14 deletions(-) rename micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/{HttpPercentilesMeterFilterFactory.java => HttpMeterFilterFactory.java} (74%) diff --git a/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpPercentilesMeterFilterFactory.java b/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpMeterFilterFactory.java similarity index 74% rename from micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpPercentilesMeterFilterFactory.java rename to micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpMeterFilterFactory.java index d216ad110..d00bc3535 100644 --- a/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpPercentilesMeterFilterFactory.java +++ b/micrometer-core/src/main/java/io/micronaut/configuration/metrics/binder/web/HttpMeterFilterFactory.java @@ -26,6 +26,8 @@ import io.micronaut.core.util.ArrayUtils; import jakarta.inject.Singleton; +import static io.micronaut.configuration.metrics.binder.web.WebMetricsPublisher.METRIC_HTTP_CLIENT_REQUESTS; +import static io.micronaut.configuration.metrics.binder.web.WebMetricsPublisher.METRIC_HTTP_SERVER_REQUESTS; import static io.micronaut.configuration.metrics.micrometer.MeterRegistryFactory.MICRONAUT_METRICS_BINDERS; import static io.micronaut.core.util.StringUtils.FALSE; @@ -37,46 +39,43 @@ @Factory @RequiresMetrics @Requires(property = WebMetricsPublisher.ENABLED, notEquals = FALSE) -public class HttpPercentilesMeterFilterFactory { +public class HttpMeterFilterFactory { /** * Configure new MeterFilter for http.server.requests metrics. * * @param percentiles The percentiles - * @param histogram If a histogram should be published * @return A MeterFilter */ @Bean @Singleton @Requires(property = MICRONAUT_METRICS_BINDERS + ".web.server.percentiles") - MeterFilter addServerPercentileMeterFilter(@Value("${" + MICRONAUT_METRICS_BINDERS + ".web.server.percentiles}") Double[] percentiles, @Value("${" + MICRONAUT_METRICS_BINDERS + ".web.server.histogram:false}") Boolean histogram) { - return getMeterFilter(percentiles, histogram, WebMetricsPublisher.METRIC_HTTP_SERVER_REQUESTS); + MeterFilter addServerPercentileMeterFilter(@Value("${" + MICRONAUT_METRICS_BINDERS + ".web.server.percentiles}") Double[] percentiles) { + return getMeterFilter(percentiles, METRIC_HTTP_SERVER_REQUESTS); } /** * Configure new MeterFilter for http.client.requests metrics. * * @param percentiles The percentiles - * @param histogram If a histogram should be published * @return A MeterFilter */ @Bean @Singleton @Requires(property = MICRONAUT_METRICS_BINDERS + ".web.client.percentiles") - MeterFilter addClientPercentileMeterFilter(@Value("${" + MICRONAUT_METRICS_BINDERS + ".web.client.percentiles}") Double[] percentiles, @Value("${" + MICRONAUT_METRICS_BINDERS + ".web.client.histogram:false}") Boolean histogram) { - return getMeterFilter(percentiles, histogram, WebMetricsPublisher.METRIC_HTTP_CLIENT_REQUESTS); + MeterFilter addClientPercentileMeterFilter(@Value("${" + MICRONAUT_METRICS_BINDERS + ".web.client.percentiles}") Double[] percentiles) { + return getMeterFilter(percentiles, METRIC_HTTP_CLIENT_REQUESTS); } - private MeterFilter getMeterFilter(Double[] percentiles, Boolean histogram, String metricNamePrefix) { + private MeterFilter getMeterFilter(Double[] percentiles, String metricNamePrefix) { return new MeterFilter() { @Override public DistributionStatisticConfig configure(Meter.Id id, DistributionStatisticConfig config) { if (id.getName().startsWith(metricNamePrefix)) { return DistributionStatisticConfig.builder() - .percentiles((double[]) ArrayUtils.toPrimitiveArray(percentiles)) - .percentilesHistogram(histogram) - .build() - .merge(config); + .percentiles((double[]) ArrayUtils.toPrimitiveArray(percentiles)) + .build() + .merge(config); } return config; } diff --git a/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/binder/web/HttpMetricsSpec.groovy b/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/binder/web/HttpMetricsSpec.groovy index 423988725..43418312f 100644 --- a/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/binder/web/HttpMetricsSpec.groovy +++ b/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/binder/web/HttpMetricsSpec.groovy @@ -138,8 +138,8 @@ class HttpMetricsSpec extends Specification { where: cfg | setting | serverPercentilesCount | clientPercentilesCount| serverHistogram | clientHistogram - MICRONAUT_METRICS_BINDERS + ".web.client.percentiles" | "0.95,0.99" | 0 | 2 | null | false - MICRONAUT_METRICS_BINDERS + ".web.server.percentiles" | "0.95,0.99" | 2 | 0 | false | null + MICRONAUT_METRICS_BINDERS + ".web.client.percentiles" | "0.95,0.99" | 0 | 2 | null | null + MICRONAUT_METRICS_BINDERS + ".web.server.percentiles" | "0.95,0.99" | 2 | 0 | null | null MICRONAUT_METRICS_BINDERS + ".web.server.histogram" | "true" | 0 | 0 | true | null MICRONAUT_METRICS_BINDERS + ".web.server.histogram" | "false" | 0 | 0 | false | null MICRONAUT_METRICS_BINDERS + ".web.client.histogram" | "true" | 0 | 0 | null | true From a9d88dd861365d69160a4d49b29be4389c2334a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Serralheiro?= Date: Fri, 19 Apr 2024 08:28:07 +0100 Subject: [PATCH 3/3] CR Fix Leave the original percentiles class alone so this PR can be released as a patch --- .../configuration/metrics/binder/web/HttpMetricsSpec.groovy | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/binder/web/HttpMetricsSpec.groovy b/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/binder/web/HttpMetricsSpec.groovy index 43418312f..16645a255 100644 --- a/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/binder/web/HttpMetricsSpec.groovy +++ b/micrometer-core/src/test/groovy/io/micronaut/configuration/metrics/binder/web/HttpMetricsSpec.groovy @@ -309,10 +309,7 @@ class HttpMetricsSpec extends Specification { String root() { "root" } @Get('/test-http-metrics') - String index() { - Thread.sleep(1000) - "ok" - } + String index() { "ok" } @Get("/test-http-metrics/{id}") String template(String id) { "ok " + id }