Commit 1eca70a8 authored by Stephane Nicoll's avatar Stephane Nicoll

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
parent e533ce38
......@@ -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;
}
......
......@@ -78,7 +78,7 @@ class GraphitePropertiesConfigAdapter extends PropertiesConfigAdapter<GraphitePr
@Override
public boolean graphiteTagsEnabled() {
return get(GraphiteProperties::isGraphiteTagsEnabled, GraphiteConfig.super::graphiteTagsEnabled);
return get(GraphiteProperties::getGraphiteTagsEnabled, GraphiteConfig.super::graphiteTagsEnabled);
}
@Override
......
......@@ -47,26 +47,26 @@ class GraphiteMetricsExportAutoConfigurationTests {
}
@Test
void autoConfiguresUseTagsAsPrefixIsIgnoredByDefault() {
void autoConfiguresUseTagsAsPrefix() {
this.contextRunner.withUserConfiguration(BaseConfiguration.class)
.withPropertyValues("management.metrics.export.graphite.tags-as-prefix=ignored").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"));
assertThat(registry.getDropwizardRegistry().getMeters()).containsOnlyKeys("test.count;app=myapp");
assertThat(registry.getDropwizardRegistry().getMeters()).containsOnlyKeys("myapp.testCount");
});
}
@Test
void autoConfiguresUseTagsAsPrefix() {
void autoConfiguresWithTagsAsPrefixCanBeDisabled() {
this.contextRunner.withUserConfiguration(BaseConfiguration.class)
.withPropertyValues("management.metrics.export.graphite.tags-as-prefix=app",
"management.metrics.export.graphite.graphite-tags-enabled=false")
"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("myapp.testCount");
assertThat(registry.getDropwizardRegistry().getMeters()).containsOnlyKeys("test.count;app=myapp");
});
}
......
......@@ -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();
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment