From cc169c5009dc9c299c70e1ae9b3b7617ecdf7e4c Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Fri, 5 Jun 2015 11:54:06 +0100 Subject: [PATCH] Change the way the AggregateMetricReader works to make it easier for users to get started. It also makes it more flexible if different aggregation keys are needed depending on the environment. The most important new feature is the spring.metrics.export.redis.aggregateKeyPattern configuration, which fits the *.redis.key and prefix defaults. The aggregate reader uses a prefix based on the key by default, with a naming convention that the key starts with "keys.". --- .../MetricExportAutoConfiguration.java | 6 ++- .../aggregate/AggregateMetricReader.java | 51 +++++++++++++------ .../export/MetricExportProperties.java | 36 +++++++++---- .../aggregate/AggregateMetricReaderTests.java | 39 +++++++++++--- .../asciidoc/production-ready-features.adoc | 9 ++-- 5 files changed, 103 insertions(+), 38 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricExportAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricExportAutoConfiguration.java index 4737e3543c..897893fc97 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricExportAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/MetricExportAutoConfiguration.java @@ -85,14 +85,18 @@ public class MetricExportAutoConfiguration { @Configuration protected static class MetricExportPropertiesConfiguration { - @Value("spring.metrics.${random.value:0000}.${spring.application.name:application}") + @Value("spring.metrics.${spring.application.name:application}.${random.value:0000}") private String prefix = "spring.metrics"; + @Value("d.d.k.d") + private String aggregateKeyPattern = ""; + @Bean(name = "spring.metrics.export.CONFIGURATION_PROPERTIES") @ConditionalOnMissingBean public MetricExportProperties metricExportProperties() { MetricExportProperties export = new MetricExportProperties(); export.getRedis().setPrefix(this.prefix); + export.getRedis().setAggregateKeyPattern(this.aggregateKeyPattern); return export; } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/aggregate/AggregateMetricReader.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/aggregate/AggregateMetricReader.java index cb6b7a3f58..6028321e2c 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/aggregate/AggregateMetricReader.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/aggregate/AggregateMetricReader.java @@ -42,7 +42,7 @@ public class AggregateMetricReader implements MetricReader { private MetricReader source; - private int truncate = 2; + private String keyPattern = "d.d"; private String prefix = "aggregate."; @@ -51,21 +51,32 @@ public class AggregateMetricReader implements MetricReader { } /** - * The number of period-separated keys to remove from the start of the input metric - * names before aggregating. - * @param truncate length of source metric prefixes + * Pattern that tells the aggregator what to do with the keys from the source + * repository. The keys in the source repository are assumed to be period separated, + * and the pattern is in the same format, e.g. "d.d.k.d". The pattern segments are + * matched against the source keys and a rule is applied: + * + * Default is "d.d" (we assume there is a global prefix of length 2). + * + * @param keyPattern the keyPattern to set */ - public void setTruncateKeyLength(int truncate) { - this.truncate = truncate; + public void setKeyPattern(String keyPattern) { + this.keyPattern = keyPattern; } /** - * Prefix to apply to all output metrics. A period will be appended if no present in - * the provided value. - * @param prefix the prefix to use default "aggregator.") + * Prefix to apply to all output metrics. A period will be appended if not present in the + * provided value. + * + * @param prefix the prefix to use (default "aggregator.") */ public void setPrefix(String prefix) { - this.prefix = prefix.endsWith(".") ? prefix : prefix + "."; + this.prefix = prefix.endsWith(".") ? prefix : (StringUtils.hasText(prefix) ? prefix + "." : ""); } @Override @@ -128,12 +139,22 @@ public class AggregateMetricReader implements MetricReader { private String getSourceKey(String name) { String[] keys = StringUtils.delimitedListToStringArray(name, "."); - if (keys.length <= this.truncate) { - return null; + String[] patterns = StringUtils.delimitedListToStringArray(this.keyPattern, "."); + StringBuilder builder = new StringBuilder(); + int index = 0; + for (index = 0; index < patterns.length; index++) { + if ("k".equals(patterns[index])) { + if (builder.length() > 0) { + builder.append("."); + } + builder.append(keys[index]); + } } - StringBuilder builder = new StringBuilder(keys[this.truncate]); - for (int i = this.truncate + 1; i < keys.length; i++) { - builder.append(".").append(keys[i]); + for (; index < keys.length; index++) { + if (builder.length() > 0) { + builder.append("."); + } + builder.append(keys[index]); } return builder.toString(); } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/export/MetricExportProperties.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/export/MetricExportProperties.java index b4aff1b066..d51ee7e948 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/export/MetricExportProperties.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/export/MetricExportProperties.java @@ -24,7 +24,6 @@ import javax.annotation.PostConstruct; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.util.PatternMatchUtils; -import org.springframework.util.StringUtils; /** * Configuration properties for metrics export. @@ -105,8 +104,8 @@ public class MetricExportProperties extends TriggerProperties { /** * Prefix for redis repository if active. Should be unique for this JVM, but most * useful if it also has the form "x.y.a.b" where "x.y" is globally unique across - * all processes sharing the same repository, "a" is unique to this physical - * process and "b" is unique to this logical process (this application). If you + * all processes sharing the same repository, "a" is unique to this logical + * process (this application) and "b" is unique to this physical process. If you * set spring.application.name elsewhere, then the default will be in the right * form. */ @@ -118,6 +117,15 @@ public class MetricExportProperties extends TriggerProperties { */ private String key = "keys.spring.metrics"; + /** + * Pattern that tells the aggregator what to do with the keys from the source + * repository. The keys in the source repository are assumed to be period + * separated, and the pattern is in the same format, e.g. "d.d.k.d". Here "d" + * means "discard" and "k" means "keep" the key segment in the corresponding + * position in the source. + */ + private String aggregateKeyPattern = ""; + public String getPrefix() { return this.prefix; } @@ -134,14 +142,22 @@ public class MetricExportProperties extends TriggerProperties { this.key = key; } + public String getAggregateKeyPattern() { + return this.aggregateKeyPattern; + } + + public void setAggregateKeyPattern(String keyPattern) { + this.aggregateKeyPattern = keyPattern; + } + public String getAggregatePrefix() { - String[] tokens = StringUtils.delimitedListToStringArray(this.prefix, "."); - if (tokens.length > 1) { - if (StringUtils.hasText(tokens[1])) { - // If the prefix has 2 or more non-trivial parts, use the first 1 - // (the aggregator strips a further 2 by default). - return tokens[0]; - } + if (this.key.startsWith("keys.")) { + return this.key.substring("keys.".length()); + } + // Something that is safe (not empty) but not the whole prefix (on the + // assumption that it contains dimension keys) + if (this.prefix.contains(".") && !this.prefix.endsWith(".")) { + return this.prefix.substring(this.prefix.indexOf("." + 1)); } return this.prefix; } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/aggregate/AggregateMetricReaderTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/aggregate/AggregateMetricReaderTests.java index 3f87f1fc45..c4412a7ad1 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/aggregate/AggregateMetricReaderTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/aggregate/AggregateMetricReaderTests.java @@ -16,6 +16,9 @@ package org.springframework.boot.actuate.metrics.aggregate; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + import java.util.Date; import org.junit.Test; @@ -23,9 +26,6 @@ import org.springframework.boot.actuate.metrics.Metric; import org.springframework.boot.actuate.metrics.repository.InMemoryMetricRepository; import org.springframework.boot.actuate.metrics.writer.Delta; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; - /** * Tests for {@link AggregateMetricReader}. * @@ -44,16 +44,39 @@ public class AggregateMetricReaderTests { } @Test - public void writeAndReadLatestValue() { - this.source.set(new Metric("foo.bar.spam", 2.3, new Date(100L))); - this.source.set(new Metric("oof.rab.spam", 2.4, new Date(0L))); - assertEquals(2.3, this.reader.findOne("aggregate.spam").getValue()); + public void defaultKeyPattern() { + this.source.set(new Metric("foo.bar.spam.bucket.wham", 2.3)); + assertEquals(2.3, this.reader.findOne("aggregate.spam.bucket.wham").getValue()); + } + + @Test + public void addKeyPattern() { + this.source.set(new Metric("foo.bar.spam.bucket.wham", 2.3)); + this.reader.setKeyPattern("d.d.k.d"); + assertEquals(2.3, this.reader.findOne("aggregate.spam.wham").getValue()); + } + + @Test + public void addPrefix() { + this.source.set(new Metric("foo.bar.spam.bucket.wham", 2.3)); + this.source.set(new Metric("off.bar.spam.bucket.wham", 2.4)); + this.reader.setPrefix("www"); + this.reader.setKeyPattern("k.d.k.d"); + assertEquals(2.3, this.reader.findOne("www.foo.spam.wham").getValue()); + assertEquals(2, this.reader.count()); } @Test public void writeAndReadExtraLong() { this.source.set(new Metric("blee.foo.bar.spam", 2.3)); - this.reader.setTruncateKeyLength(3); + this.reader.setKeyPattern("d.d.d.k"); + assertEquals(2.3, this.reader.findOne("aggregate.spam").getValue()); + } + + @Test + public void writeAndReadLatestValue() { + this.source.set(new Metric("foo.bar.spam", 2.3, new Date(100L))); + this.source.set(new Metric("oof.rab.spam", 2.4, new Date(0L))); assertEquals(2.3, this.reader.findOne("aggregate.spam").getValue()); } diff --git a/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc b/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc index d63459aa76..f0c678e9aa 100644 --- a/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc +++ b/spring-boot-docs/src/main/asciidoc/production-ready-features.adoc @@ -985,11 +985,11 @@ MetricWriter metricWriter(MetricExportProperties export) { .application.properties [source,properties] ---- -spring.metrics.export.redis.prefix: metrics.mysystem.${random.value:0000}.${spring.application.name:application} -spring.metrics.export.redis.key: keys.mysystem +spring.metrics.export.redis.prefix: metrics.mysystem.${spring.application.name:application}.${random.value:0000} +spring.metrics.export.redis.key: keys.metrics.mysystem ---- -The prefix is constructed with the application name at the end, so it can easily be used +The prefix is constructed with the application name and id at the end, so it can easily be used to identify a group of processes with the same logical name later. NOTE: it's important to set both the key and the prefix. The key is used for all @@ -1128,7 +1128,8 @@ results to the "/metrics" endpoint. Example: NOTE: the example above uses `MetricExportProperties` to inject and extract the key and prefix. This is provided to you as a convenience -by Spring Boot, and the defaults for that will be sensible. +by Spring Boot, and the defaults will be sensible. They are set up in +`MetricExportAutoConfiguration`. NOTE: the `MetricReaders` above are not `@Beans` and are not marked as `@ExportMetricReader` because they are just collecting and analysing