From ccb964e88649d4d35eab783aa74ec295be3ebd84 Mon Sep 17 00:00:00 2001 From: artsiom Date: Thu, 4 Oct 2018 13:55:51 +0300 Subject: [PATCH] Drop support for "all" from management.metrics.distribution.sla See gh-14684 --- .../metrics/MetricsProperties.java | 7 ++-- .../metrics/PropertiesMeterFilter.java | 26 +++++++++--- .../metrics/PropertiesMeterFilterTests.java | 40 +------------------ .../appendix-application-properties.adoc | 2 +- 4 files changed, 26 insertions(+), 49 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsProperties.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsProperties.java index 2eeabbdf4d..3bcbd3d0f0 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsProperties.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MetricsProperties.java @@ -192,10 +192,9 @@ public class MetricsProperties { /** * Specific SLA boundaries for meter IDs starting-with the specified name. The - * longest match wins, the key `all` can also be used to configure all meters. - * Counters will be published for each specified boundary. Values can be specified - * as a long or as a Duration value (for timer meters, defaulting to ms if no unit - * specified). + * longest match wins. Counters will be published for each specified boundary. + * Values can be specified as a long or as a Duration value (for timer meters, + * defaulting to ms if no unit specified). */ private final Map sla = new LinkedHashMap<>(); diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/PropertiesMeterFilter.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/PropertiesMeterFilter.java index eacbe66e89..55f5c80c5f 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/PropertiesMeterFilter.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/PropertiesMeterFilter.java @@ -19,6 +19,7 @@ package org.springframework.boot.actuate.autoconfigure.metrics; import java.util.Arrays; import java.util.Map; import java.util.Objects; +import java.util.function.Supplier; import java.util.stream.Collectors; import io.micrometer.core.instrument.Meter; @@ -39,6 +40,7 @@ import org.springframework.util.StringUtils; * @author Jon Schneider * @author Phillip Webb * @author Stephane Nicoll + * @author Artsiom Yudovin * @since 2.0.0 */ public class PropertiesMeterFilter implements MeterFilter { @@ -66,7 +68,7 @@ public class PropertiesMeterFilter implements MeterFilter { @Override public MeterFilterReply accept(Meter.Id id) { - boolean enabled = lookup(this.properties.getEnable(), id, true); + boolean enabled = lookupWithFallbackToAll(this.properties.getEnable(), id, true); return enabled ? MeterFilterReply.NEUTRAL : MeterFilterReply.DENY; } @@ -80,9 +82,10 @@ public class PropertiesMeterFilter implements MeterFilter { DistributionStatisticConfig config) { Distribution distribution = this.properties.getDistribution(); return DistributionStatisticConfig.builder() - .percentilesHistogram( - lookup(distribution.getPercentilesHistogram(), id, null)) - .percentiles(lookup(distribution.getPercentiles(), id, null)) + .percentilesHistogram(lookupWithFallbackToAll( + distribution.getPercentilesHistogram(), id, null)) + .percentiles( + lookupWithFallbackToAll(distribution.getPercentiles(), id, null)) .sla(convertSla(id.getType(), lookup(distribution.getSla(), id, null))) .build().merge(config); } @@ -101,6 +104,18 @@ public class PropertiesMeterFilter implements MeterFilter { if (values.isEmpty()) { return defaultValue; } + return this.baseLookup(values, id, () -> defaultValue); + } + + private T lookupWithFallbackToAll(Map values, Id id, T defaultValue) { + if (values.isEmpty()) { + return defaultValue; + } + return this.baseLookup(values, id, + () -> values.getOrDefault("all", defaultValue)); + } + + private T baseLookup(Map values, Id id, Supplier defaultValue) { String name = id.getName(); while (StringUtils.hasLength(name)) { T result = values.get(name); @@ -110,7 +125,8 @@ public class PropertiesMeterFilter implements MeterFilter { int lastDot = name.lastIndexOf('.'); name = (lastDot != -1) ? name.substring(0, lastDot) : ""; } - return values.getOrDefault("all", defaultValue); + + return defaultValue.get(); } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/PropertiesMeterFilterTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/PropertiesMeterFilterTests.java index 6e7c23f8eb..355b8edd28 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/PropertiesMeterFilterTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/PropertiesMeterFilterTests.java @@ -42,6 +42,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException * * @author Phillip Webb * @author Jon Schneider + * @author Artsiom Yudovin */ public class PropertiesMeterFilterTests { @@ -261,45 +262,6 @@ public class PropertiesMeterFilterTests { .containsExactly(4000000, 5000000, 6000000); } - @Test - public void configureWhenAllSlaSetShouldSetSlaToValue() { - PropertiesMeterFilter filter = new PropertiesMeterFilter( - createProperties("distribution.sla.all=1,2,3")); - assertThat(filter.configure(createMeterId("spring.boot"), - DistributionStatisticConfig.DEFAULT).getSlaBoundaries()) - .containsExactly(1000000, 2000000, 3000000); - } - - @Test - public void configureWhenSlaDurationShouldOnlyApplyToTimer() { - PropertiesMeterFilter filter = new PropertiesMeterFilter( - createProperties("distribution.sla.all=1ms,2ms,3ms")); - Meter.Id timer = createMeterId("spring.boot", Meter.Type.TIMER); - Meter.Id summary = createMeterId("spring.boot", Meter.Type.DISTRIBUTION_SUMMARY); - Meter.Id counter = createMeterId("spring.boot", Meter.Type.COUNTER); - assertThat(filter.configure(timer, DistributionStatisticConfig.DEFAULT) - .getSlaBoundaries()).containsExactly(1000000, 2000000, 3000000); - assertThat(filter.configure(summary, DistributionStatisticConfig.DEFAULT) - .getSlaBoundaries()).isNullOrEmpty(); - assertThat(filter.configure(counter, DistributionStatisticConfig.DEFAULT) - .getSlaBoundaries()).isNullOrEmpty(); - } - - @Test - public void configureWhenSlaLongShouldOnlyApplyToTimerAndDistributionSummary() { - PropertiesMeterFilter filter = new PropertiesMeterFilter( - createProperties("distribution.sla.all=1,2,3")); - Meter.Id timer = createMeterId("spring.boot", Meter.Type.TIMER); - Meter.Id summary = createMeterId("spring.boot", Meter.Type.DISTRIBUTION_SUMMARY); - Meter.Id counter = createMeterId("spring.boot", Meter.Type.COUNTER); - assertThat(filter.configure(timer, DistributionStatisticConfig.DEFAULT) - .getSlaBoundaries()).containsExactly(1000000, 2000000, 3000000); - assertThat(filter.configure(summary, DistributionStatisticConfig.DEFAULT) - .getSlaBoundaries()).containsExactly(1, 2, 3); - assertThat(filter.configure(counter, DistributionStatisticConfig.DEFAULT) - .getSlaBoundaries()).isNullOrEmpty(); - } - private Id createMeterId(String name) { Meter.Type meterType = Type.TIMER; return createMeterId(name, meterType); diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc index 4546300149..0fbd8547be 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc @@ -1382,7 +1382,7 @@ content into your application. Rather, pick only the properties that you need. # METRICS management.metrics.distribution.percentiles-histogram.*= # Whether meter IDs starting with the specified name should publish percentile histograms. management.metrics.distribution.percentiles.*= # Specific computed non-aggregable percentiles to ship to the backend for meter IDs starting-with the specified name. - management.metrics.distribution.sla.*= # Specific SLA boundaries for meter IDs starting-with the specified name. The longest match wins, the key `all` can also be used to configure all meters. + management.metrics.distribution.sla.*= # Specific SLA boundaries for meter IDs starting-with the specified name. The longest match wins. management.metrics.enable.*= # Whether meter IDs starting-with the specified name should be enabled. The longest match wins, the key `all` can also be used to configure all meters. management.metrics.export.atlas.batch-size=10000 # Number of measurements per request to use for this backend. If more measurements are found, then multiple requests will be made. management.metrics.export.atlas.config-refresh-frequency=10s # Frequency for refreshing config settings from the LWC service.