From 1eca70a84b0f20cef4a337bc887edd764f3eae0f Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 20 Apr 2020 12:20:46 +0200 Subject: [PATCH] Restore Graphite tags behaviour unless configured explicitly This commit harmonizes how Graphite is configured. If tagsAsPrefix is set, it is used transparently. Otherwise, the new native tagging system is used. See https://github.com/micrometer-metrics/micrometer/pull/2007 Closes gh-20884 --- .../export/graphite/GraphiteProperties.java | 11 ++++---- .../GraphitePropertiesConfigAdapter.java | 2 +- ...teMetricsExportAutoConfigurationTests.java | 28 +++++++++---------- .../graphite/GraphitePropertiesTests.java | 17 ++++++++++- 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/graphite/GraphiteProperties.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/graphite/GraphiteProperties.java index 3dec132528..bb5629b2e3 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/graphite/GraphiteProperties.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/graphite/GraphiteProperties.java @@ -22,6 +22,7 @@ import java.util.concurrent.TimeUnit; import io.micrometer.graphite.GraphiteProtocol; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.util.ObjectUtils; /** * {@link ConfigurationProperties @ConfigurationProperties} for configuring Graphite @@ -71,9 +72,9 @@ public class GraphiteProperties { /** * Whether Graphite tags should be used, as opposed to a hierarchical naming - * convention. + * convention. Enabled by default unless "tagsAsPrefix" is set. */ - private boolean graphiteTagsEnabled = true; + private Boolean graphiteTagsEnabled; /** * For the hierarchical naming convention, turn the specified tag keys into part of @@ -137,11 +138,11 @@ public class GraphiteProperties { this.protocol = protocol; } - public boolean isGraphiteTagsEnabled() { - return this.graphiteTagsEnabled; + public Boolean getGraphiteTagsEnabled() { + return (this.graphiteTagsEnabled != null) ? this.graphiteTagsEnabled : ObjectUtils.isEmpty(this.tagsAsPrefix); } - public void setGraphiteTagsEnabled(boolean graphiteTagsEnabled) { + public void setGraphiteTagsEnabled(Boolean graphiteTagsEnabled) { this.graphiteTagsEnabled = graphiteTagsEnabled; } diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/graphite/GraphitePropertiesConfigAdapter.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/graphite/GraphitePropertiesConfigAdapter.java index bc89039799..4e3d894b99 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/graphite/GraphitePropertiesConfigAdapter.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/export/graphite/GraphitePropertiesConfigAdapter.java @@ -78,7 +78,7 @@ class GraphitePropertiesConfigAdapter extends PropertiesConfigAdapter assertThat(context).doesNotHaveBean(GraphiteMeterRegistry.class)); } - @Test - void autoConfiguresUseTagsAsPrefixIsIgnoredByDefault() { - this.contextRunner.withUserConfiguration(BaseConfiguration.class) - .withPropertyValues("management.metrics.export.graphite.tags-as-prefix=ignored").run((context) -> { - assertThat(context).hasSingleBean(GraphiteMeterRegistry.class); - GraphiteMeterRegistry registry = context.getBean(GraphiteMeterRegistry.class); - registry.counter("test.count", Tags.of("app", "myapp")); - assertThat(registry.getDropwizardRegistry().getMeters()).containsOnlyKeys("test.count;app=myapp"); - }); - } - @Test void autoConfiguresUseTagsAsPrefix() { this.contextRunner.withUserConfiguration(BaseConfiguration.class) - .withPropertyValues("management.metrics.export.graphite.tags-as-prefix=app", - "management.metrics.export.graphite.graphite-tags-enabled=false") - .run((context) -> { + .withPropertyValues("management.metrics.export.graphite.tags-as-prefix=app").run((context) -> { assertThat(context).hasSingleBean(GraphiteMeterRegistry.class); GraphiteMeterRegistry registry = context.getBean(GraphiteMeterRegistry.class); registry.counter("test.count", Tags.of("app", "myapp")); @@ -70,6 +57,19 @@ class GraphiteMetricsExportAutoConfigurationTests { }); } + @Test + void autoConfiguresWithTagsAsPrefixCanBeDisabled() { + this.contextRunner.withUserConfiguration(BaseConfiguration.class) + .withPropertyValues("management.metrics.export.graphite.tags-as-prefix=app", + "management.metrics.export.graphite.graphite-tags-enabled=true") + .run((context) -> { + assertThat(context).hasSingleBean(GraphiteMeterRegistry.class); + GraphiteMeterRegistry registry = context.getBean(GraphiteMeterRegistry.class); + registry.counter("test.count", Tags.of("app", "myapp")); + assertThat(registry.getDropwizardRegistry().getMeters()).containsOnlyKeys("test.count;app=myapp"); + }); + } + @Test void autoConfiguresItsConfigAndMeterRegistry() { this.contextRunner.withUserConfiguration(BaseConfiguration.class).run((context) -> assertThat(context) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/graphite/GraphitePropertiesTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/graphite/GraphitePropertiesTests.java index 5e0ed18fc5..ea3cf5358c 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/graphite/GraphitePropertiesTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/metrics/export/graphite/GraphitePropertiesTests.java @@ -39,8 +39,23 @@ class GraphitePropertiesTests { assertThat(properties.getHost()).isEqualTo(config.host()); assertThat(properties.getPort()).isEqualTo(config.port()); assertThat(properties.getProtocol()).isEqualTo(config.protocol()); - assertThat(properties.isGraphiteTagsEnabled()).isEqualTo(config.graphiteTagsEnabled()); + assertThat(properties.getGraphiteTagsEnabled()).isEqualTo(config.graphiteTagsEnabled()); assertThat(properties.getTagsAsPrefix()).isEqualTo(config.tagsAsPrefix()); } + @Test + void graphiteTagsAreDisabledIfTagsAsPrefixIsSet() { + GraphiteProperties properties = new GraphiteProperties(); + properties.setTagsAsPrefix(new String[] { "app" }); + assertThat(properties.getGraphiteTagsEnabled()).isFalse(); + } + + @Test + void graphiteTagsCanBeEnabledEvenIfTagsAsPrefixIsSet() { + GraphiteProperties properties = new GraphiteProperties(); + properties.setGraphiteTagsEnabled(true); + properties.setTagsAsPrefix(new String[] { "app" }); + assertThat(properties.getGraphiteTagsEnabled()).isTrue(); + } + }