From 95798265b6fed27e6603f73f8b792b771a434645 Mon Sep 17 00:00:00 2001 From: Jon Schneider Date: Tue, 21 Apr 2020 11:52:16 -0400 Subject: [PATCH 1/2] Update validation of Micrometer configuration See gh-21067 --- ...er.java => ValidationFailureAnalyzer.java} | 26 ++++++++++--------- .../export/elastic/ElasticProperties.java | 6 ++--- .../export/ganglia/GangliaProperties.java | 10 +++++-- .../main/resources/META-INF/spring.factories | 2 +- ...va => ValidationFailureAnalyzerTests.java} | 10 ++++--- .../wavefront/WavefrontPropertiesTests.java | 4 +-- 6 files changed, 34 insertions(+), 24 deletions(-) rename spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/{MissingRequiredConfigurationFailureAnalyzer.java => ValidationFailureAnalyzer.java} (60%) rename spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/{MissingRequiredConfigurationFailureAnalyzerTests.java => ValidationFailureAnalyzerTests.java} (82%) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MissingRequiredConfigurationFailureAnalyzer.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/ValidationFailureAnalyzer.java similarity index 60% rename from spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MissingRequiredConfigurationFailureAnalyzer.java rename to spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/ValidationFailureAnalyzer.java index 597461dcd4..80e0841004 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/MissingRequiredConfigurationFailureAnalyzer.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/ValidationFailureAnalyzer.java @@ -16,28 +16,30 @@ package org.springframework.boot.actuate.autoconfigure.metrics; -import io.micrometer.core.instrument.config.MissingRequiredConfigurationException; - +import io.micrometer.core.instrument.config.validate.ValidationException; import org.springframework.boot.diagnostics.AbstractFailureAnalyzer; import org.springframework.boot.diagnostics.FailureAnalysis; +import java.util.stream.Collectors; + /** * An {@link AbstractFailureAnalyzer} that performs analysis of failures caused by a - * {@link MissingRequiredConfigurationException}. + * {@link ValidationException}. * * @author Andy Wilkinson */ -class MissingRequiredConfigurationFailureAnalyzer - extends AbstractFailureAnalyzer { +class ValidationFailureAnalyzer + extends AbstractFailureAnalyzer { @Override - protected FailureAnalysis analyze(Throwable rootFailure, MissingRequiredConfigurationException cause) { - StringBuilder description = new StringBuilder(); - description.append(cause.getMessage()); - if (!cause.getMessage().endsWith(".")) { - description.append("."); - } - return new FailureAnalysis(description.toString(), + protected FailureAnalysis analyze(Throwable rootFailure, ValidationException cause) { + String description = cause.getValidation().failures().stream() + .map(failure -> "management.metrics.export." + failure.getProperty() + " was '" + + (failure.getValue() == null ? "null" : failure.getValue().toString()) + + "' but it " + failure.getMessage() + ".") + .collect(Collectors.joining("\n")); + + return new FailureAnalysis(description, "Update your application to provide the missing configuration.", cause); } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/elastic/ElasticProperties.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/elastic/ElasticProperties.java index 2a94556cda..5cbb8c4b56 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/elastic/ElasticProperties.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/elastic/ElasticProperties.java @@ -62,17 +62,17 @@ public class ElasticProperties extends StepRegistryProperties { /** * Login user of the Elastic server. */ - private String userName = ""; + private String userName; /** * Login password of the Elastic server. */ - private String password = ""; + private String password; /** * Ingest pipeline name. By default, events are not pre-processed. */ - private String pipeline = ""; + private String pipeline; public String getHost() { return this.host; diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/ganglia/GangliaProperties.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/ganglia/GangliaProperties.java index f00dad931c..de6ed541e9 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/ganglia/GangliaProperties.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/ganglia/GangliaProperties.java @@ -46,8 +46,11 @@ public class GangliaProperties { /** * Base time unit used to report rates. + * + * @deprecated No longer used by Micrometer. */ - private TimeUnit rateUnits = TimeUnit.SECONDS; + @Deprecated + private TimeUnit rateUnits; /** * Base time unit used to report durations. @@ -56,8 +59,11 @@ public class GangliaProperties { /** * Ganglia protocol version. Must be either 3.1 or 3.0. + * + * @deprecated No longer used by Micrometer. */ - private String protocolVersion = "3.1"; + @Deprecated + private String protocolVersion; /** * UDP addressing mode, either unicast or multicast. diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/spring.factories b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/spring.factories index 0771aebd53..e297520518 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/spring.factories +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/resources/META-INF/spring.factories @@ -105,4 +105,4 @@ org.springframework.boot.actuate.autoconfigure.web.servlet.ServletManagementChil org.springframework.boot.actuate.autoconfigure.web.servlet.WebMvcEndpointChildContextConfiguration org.springframework.boot.diagnostics.FailureAnalyzer=\ -org.springframework.boot.actuate.autoconfigure.metrics.MissingRequiredConfigurationFailureAnalyzer +org.springframework.boot.actuate.autoconfigure.metrics.ValidationFailureAnalyzer diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MissingRequiredConfigurationFailureAnalyzerTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/ValidationFailureAnalyzerTests.java similarity index 82% rename from spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MissingRequiredConfigurationFailureAnalyzerTests.java rename to spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/ValidationFailureAnalyzerTests.java index e8ed900191..f300ddb858 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/MissingRequiredConfigurationFailureAnalyzerTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/ValidationFailureAnalyzerTests.java @@ -30,18 +30,20 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.fail; /** - * Tests for {@link MissingRequiredConfigurationFailureAnalyzer}. + * Tests for {@link ValidationFailureAnalyzer}. * * @author Andy Wilkinson */ -class MissingRequiredConfigurationFailureAnalyzerTests { +class ValidationFailureAnalyzerTests { @Test void analyzesMissingRequiredConfiguration() { - FailureAnalysis analysis = new MissingRequiredConfigurationFailureAnalyzer() + FailureAnalysis analysis = new ValidationFailureAnalyzer() .analyze(createFailure(MissingAccountIdConfiguration.class)); assertThat(analysis).isNotNull(); - assertThat(analysis.getDescription()).isEqualTo("accountId must be set to report metrics to New Relic."); + assertThat(analysis.getDescription()).isEqualTo( + "management.metrics.export.newrelic.apiKey was 'null' but it is required when publishing to Insights API.\n" + + "management.metrics.export.newrelic.accountId was 'null' but it is required when publishing to Insights API."); assertThat(analysis.getAction()).isEqualTo("Update your application to provide the missing configuration."); } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/wavefront/WavefrontPropertiesTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/wavefront/WavefrontPropertiesTests.java index 1be810c36e..8d2258edfb 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/wavefront/WavefrontPropertiesTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/wavefront/WavefrontPropertiesTests.java @@ -16,12 +16,14 @@ package org.springframework.boot.actuate.autoconfigure.metrics.export.wavefront; +import io.micrometer.core.instrument.config.validate.ValidationException; import io.micrometer.wavefront.WavefrontConfig; import org.junit.jupiter.api.Test; import org.springframework.boot.actuate.autoconfigure.metrics.export.properties.PushRegistryPropertiesTests; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** * Tests for {@link WavefrontProperties}. @@ -36,8 +38,6 @@ class WavefrontPropertiesTests extends PushRegistryPropertiesTests { WavefrontConfig config = WavefrontConfig.DEFAULT_DIRECT; assertStepRegistryDefaultValues(properties, config); assertThat(properties.getUri().toString()).isEqualTo(config.uri()); - // source has no static default value - assertThat(properties.getApiToken()).isEqualTo(config.apiToken()); assertThat(properties.getGlobalPrefix()).isEqualTo(config.globalPrefix()); } From 60a76ce6e7c75f384d7aba625dd9eb1d4ea8522d Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 21 Apr 2020 17:39:21 +0100 Subject: [PATCH 2/2] Polish "Update validation of Micrometer configuration" See gh-21067 --- .../metrics/ValidationFailureAnalyzer.java | 24 +++++++++---------- .../export/ganglia/GangliaProperties.java | 15 ++++++------ .../PushRegistryPropertiesConfigAdapter.java | 5 ---- .../ValidationFailureAnalyzerTests.java | 13 +++++----- .../ganglia/GangliaPropertiesTests.java | 3 ++- .../wavefront/WavefrontPropertiesTests.java | 4 +--- 6 files changed, 28 insertions(+), 36 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/ValidationFailureAnalyzer.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/ValidationFailureAnalyzer.java index 80e0841004..fa9e41bd02 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/ValidationFailureAnalyzer.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/ValidationFailureAnalyzer.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,31 +16,29 @@ package org.springframework.boot.actuate.autoconfigure.metrics; +import io.micrometer.core.instrument.config.validate.Validated.Invalid; import io.micrometer.core.instrument.config.validate.ValidationException; + import org.springframework.boot.diagnostics.AbstractFailureAnalyzer; import org.springframework.boot.diagnostics.FailureAnalysis; -import java.util.stream.Collectors; - /** * An {@link AbstractFailureAnalyzer} that performs analysis of failures caused by a * {@link ValidationException}. * * @author Andy Wilkinson */ -class ValidationFailureAnalyzer - extends AbstractFailureAnalyzer { +class ValidationFailureAnalyzer extends AbstractFailureAnalyzer { @Override protected FailureAnalysis analyze(Throwable rootFailure, ValidationException cause) { - String description = cause.getValidation().failures().stream() - .map(failure -> "management.metrics.export." + failure.getProperty() + " was '" + - (failure.getValue() == null ? "null" : failure.getValue().toString()) + - "' but it " + failure.getMessage() + ".") - .collect(Collectors.joining("\n")); - - return new FailureAnalysis(description, - "Update your application to provide the missing configuration.", cause); + StringBuilder description = new StringBuilder(String.format("Invalid Micrometer configuration detected:%n")); + for (Invalid failure : cause.getValidation().failures()) { + description.append(String.format("%n - management.metrics.export.%s was '%s' but it %s", + failure.getProperty(), failure.getValue(), failure.getMessage())); + } + return new FailureAnalysis(description.toString(), + "Update your application to correct the invalid configuration.", cause); } } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/ganglia/GangliaProperties.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/ganglia/GangliaProperties.java index de6ed541e9..4461e44c44 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/ganglia/GangliaProperties.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/ganglia/GangliaProperties.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,7 @@ import java.util.concurrent.TimeUnit; import info.ganglia.gmetric4j.gmetric.GMetric; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.DeprecatedConfigurationProperty; /** * {@link ConfigurationProperties @ConfigurationProperties} for configuring Ganglia @@ -46,10 +47,7 @@ public class GangliaProperties { /** * Base time unit used to report rates. - * - * @deprecated No longer used by Micrometer. */ - @Deprecated private TimeUnit rateUnits; /** @@ -59,10 +57,7 @@ public class GangliaProperties { /** * Ganglia protocol version. Must be either 3.1 or 3.0. - * - * @deprecated No longer used by Micrometer. */ - @Deprecated private String protocolVersion; /** @@ -102,10 +97,13 @@ public class GangliaProperties { this.step = step; } + @Deprecated + @DeprecatedConfigurationProperty(reason = "No longer used by Micormeter") public TimeUnit getRateUnits() { return this.rateUnits; } + @Deprecated public void setRateUnits(TimeUnit rateUnits) { this.rateUnits = rateUnits; } @@ -118,10 +116,13 @@ public class GangliaProperties { this.durationUnits = durationUnits; } + @Deprecated + @DeprecatedConfigurationProperty(reason = "No longer used by Micormeter") public String getProtocolVersion() { return this.protocolVersion; } + @Deprecated public void setProtocolVersion(String protocolVersion) { this.protocolVersion = protocolVersion; } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/properties/PushRegistryPropertiesConfigAdapter.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/properties/PushRegistryPropertiesConfigAdapter.java index e05037ca8f..779859ceb7 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/properties/PushRegistryPropertiesConfigAdapter.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/properties/PushRegistryPropertiesConfigAdapter.java @@ -36,11 +36,6 @@ public abstract class PushRegistryPropertiesConfigAdapter configuration) { @@ -58,7 +57,7 @@ class ValidationFailureAnalyzerTests { } @Configuration(proxyBeanMethods = false) - static class MissingAccountIdConfiguration { + static class MissingAccountIdAndApiKeyConfiguration { @Bean NewRelicMeterRegistry meterRegistry() { diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/ganglia/GangliaPropertiesTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/ganglia/GangliaPropertiesTests.java index a840f38078..6d4fd0d0f5 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/ganglia/GangliaPropertiesTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/ganglia/GangliaPropertiesTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,7 @@ import static org.assertj.core.api.Assertions.assertThat; class GangliaPropertiesTests { @Test + @SuppressWarnings("deprecation") void defaultValuesAreConsistent() { GangliaProperties properties = new GangliaProperties(); GangliaConfig config = GangliaConfig.DEFAULT; diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/wavefront/WavefrontPropertiesTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/wavefront/WavefrontPropertiesTests.java index 8d2258edfb..4ec6050d46 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/wavefront/WavefrontPropertiesTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/wavefront/WavefrontPropertiesTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,14 +16,12 @@ package org.springframework.boot.actuate.autoconfigure.metrics.export.wavefront; -import io.micrometer.core.instrument.config.validate.ValidationException; import io.micrometer.wavefront.WavefrontConfig; import org.junit.jupiter.api.Test; import org.springframework.boot.actuate.autoconfigure.metrics.export.properties.PushRegistryPropertiesTests; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; /** * Tests for {@link WavefrontProperties}.