From ed2876e931fe84a27ff8d435c6428951ea68bfa5 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 22 May 2014 09:08:31 +0100 Subject: [PATCH] Add a PrefixMetricWriter interface to cleanly separate write/read The PrefixMetricGroupExporter only really makes sesne if the writer is aware of the groups, so it seemed better to use a new interface than mix read/write. --- .../export/AbstractMetricExporter.java | 4 +- .../export/PrefixMetricGroupExporter.java | 22 ++++----- .../repository/InMemoryMetricRepository.java | 8 ++++ .../repository/MultiMetricRepository.java | 20 +-------- .../redis/RedisMultiMetricRepository.java | 7 ++- .../metrics/writer/PrefixMetricWriter.java | 45 +++++++++++++++++++ .../PrefixMetricGroupExporterTests.java | 10 +++++ 7 files changed, 82 insertions(+), 34 deletions(-) create mode 100644 spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/writer/PrefixMetricWriter.java diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/export/AbstractMetricExporter.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/export/AbstractMetricExporter.java index c78eb92cd30..59c7ec28fe3 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/export/AbstractMetricExporter.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/export/AbstractMetricExporter.java @@ -81,7 +81,9 @@ public abstract class AbstractMetricExporter implements Exporter { } values.add(value); } - write(group, values); + if (!values.isEmpty()) { + write(group, values); + } } } finally { diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/export/PrefixMetricGroupExporter.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/export/PrefixMetricGroupExporter.java index 844b4a7b1ce..48c8837d24d 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/export/PrefixMetricGroupExporter.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/export/PrefixMetricGroupExporter.java @@ -23,7 +23,7 @@ import java.util.Set; import org.springframework.boot.actuate.metrics.Metric; import org.springframework.boot.actuate.metrics.reader.PrefixMetricReader; import org.springframework.boot.actuate.metrics.repository.MultiMetricRepository; -import org.springframework.boot.actuate.metrics.writer.MetricWriter; +import org.springframework.boot.actuate.metrics.writer.PrefixMetricWriter; /** * A convenient exporter for a group of metrics from a {@link PrefixMetricReader}. Exports @@ -35,7 +35,7 @@ public class PrefixMetricGroupExporter extends AbstractMetricExporter { private final PrefixMetricReader reader; - private final MetricWriter writer; + private final PrefixMetricWriter writer; private Set groups = new HashSet(); @@ -45,7 +45,7 @@ public class PrefixMetricGroupExporter extends AbstractMetricExporter { * @param reader a reader as the source of metrics * @param writer the writer to send the metrics to */ - public PrefixMetricGroupExporter(PrefixMetricReader reader, MetricWriter writer) { + public PrefixMetricGroupExporter(PrefixMetricReader reader, PrefixMetricWriter writer) { this(reader, writer, ""); } @@ -56,8 +56,8 @@ public class PrefixMetricGroupExporter extends AbstractMetricExporter { * @param writer the writer to send the metrics to * @param prefix the prefix for metrics to export */ - public PrefixMetricGroupExporter(PrefixMetricReader reader, MetricWriter writer, - String prefix) { + public PrefixMetricGroupExporter(PrefixMetricReader reader, + PrefixMetricWriter writer, String prefix) { super(prefix); this.reader = reader; this.writer = writer; @@ -72,6 +72,9 @@ public class PrefixMetricGroupExporter extends AbstractMetricExporter { @Override protected Iterable groups() { + if ((this.reader instanceof MultiMetricRepository) && this.groups.isEmpty()) { + return ((MultiMetricRepository) this.reader).groups(); + } return this.groups; } @@ -82,14 +85,7 @@ public class PrefixMetricGroupExporter extends AbstractMetricExporter { @Override protected void write(String group, Collection> values) { - if (this.writer instanceof MultiMetricRepository && !values.isEmpty()) { - ((MultiMetricRepository) this.writer).save(group, values); - } - else { - for (Metric value : values) { - this.writer.set(value); - } - } + this.writer.save(group, values); } } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/InMemoryMetricRepository.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/InMemoryMetricRepository.java index d95654533e9..72ec83bbc76 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/InMemoryMetricRepository.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/InMemoryMetricRepository.java @@ -72,7 +72,15 @@ public class InMemoryMetricRepository implements MetricRepository, MultiMetricRe @Override public void save(String group, Collection> values) { + String prefix = group; + if (!prefix.endsWith(".")) { + prefix = prefix + "."; + } for (Metric metric : values) { + if (!metric.getName().startsWith(prefix)) { + metric = new Metric(prefix + metric.getName(), metric.getValue(), + metric.getTimestamp()); + } set(metric); } this.groups.add(group); diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/MultiMetricRepository.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/MultiMetricRepository.java index fbdb10808e7..2513b73f08f 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/MultiMetricRepository.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/MultiMetricRepository.java @@ -16,10 +16,8 @@ package org.springframework.boot.actuate.metrics.repository; -import java.util.Collection; - -import org.springframework.boot.actuate.metrics.Metric; import org.springframework.boot.actuate.metrics.reader.PrefixMetricReader; +import org.springframework.boot.actuate.metrics.writer.PrefixMetricWriter; /** * A repository for metrics that allows efficient storage and retrieval of groups of @@ -27,21 +25,7 @@ import org.springframework.boot.actuate.metrics.reader.PrefixMetricReader; * * @author Dave Syer */ -public interface MultiMetricRepository extends PrefixMetricReader { - - /** - * Save some metric values and associate them with a group name. - * @param group the name of the group - * @param values the metric values to save - */ - void save(String group, Collection> values); - - /** - * Rest the values of all metrics in the group. Implementations may choose to discard - * the old values. - * @param group reset the whole group - */ - void reset(String group); +public interface MultiMetricRepository extends PrefixMetricReader, PrefixMetricWriter { /** * The names of all the groups known to this repository diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMultiMetricRepository.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMultiMetricRepository.java index 0586345ce56..5404c37983d 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMultiMetricRepository.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/repository/redis/RedisMultiMetricRepository.java @@ -44,7 +44,7 @@ public class RedisMultiMetricRepository implements MultiMetricRepository { private String prefix = DEFAULT_METRICS_PREFIX; - private String keys = this.prefix + "keys"; + private String keys = "keys." + this.prefix; private final BoundZSetOperations zSetOperations; @@ -61,8 +61,11 @@ public class RedisMultiMetricRepository implements MultiMetricRepository { * @param prefix the prefix to set for all metrics keys */ public void setPrefix(String prefix) { + if (!prefix.endsWith(".")) { + prefix = prefix + "."; + } this.prefix = prefix; - this.keys = this.prefix + "keys"; + this.keys = "keys." + this.prefix; } @Override diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/writer/PrefixMetricWriter.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/writer/PrefixMetricWriter.java new file mode 100644 index 00000000000..d3594e81178 --- /dev/null +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/writer/PrefixMetricWriter.java @@ -0,0 +1,45 @@ +/* + * Copyright 2012-2013 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.metrics.writer; + +import java.util.Collection; + +import org.springframework.boot.actuate.metrics.Metric; + +/** + * A writer for metrics that allows efficient storage of groups of metrics with a common + * name prefix (their group name). + * + * @author Dave Syer + */ +public interface PrefixMetricWriter { + + /** + * Save some metric values and associate them with a group name. + * @param group the name of the group + * @param values the metric values to save + */ + void save(String group, Collection> values); + + /** + * Rest the values of all metrics in the group. Implementations may choose to discard + * the old values. + * @param group reset the whole group + */ + void reset(String group); + +} diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/export/PrefixMetricGroupExporterTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/export/PrefixMetricGroupExporterTests.java index 5f41f3af79f..eb6ee2f3580 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/export/PrefixMetricGroupExporterTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/export/PrefixMetricGroupExporterTests.java @@ -16,6 +16,7 @@ package org.springframework.boot.actuate.metrics.export; +import java.util.Arrays; import java.util.Collections; import org.junit.Test; @@ -57,6 +58,15 @@ public class PrefixMetricGroupExporterTests { assertEquals(0, Iterables.collection(this.writer.groups()).size()); } + @Test + public void multiMetricGroupsCopiedAsDefault() { + this.reader.save("foo", Arrays.> asList(new Metric("bar", 2.3), + new Metric("spam", 1.3))); + this.exporter.export(); + assertEquals(1, this.writer.countGroups()); + assertEquals(2, Iterables.collection(this.writer.findAll("foo")).size()); + } + @Test public void onlyPrefixedMetricsCopied() { this.reader.set(new Metric("foo.bar", 2.3));