Split MetricWriter into 2 interfaces covering counters and gauges

This way the MetricCopyExporter can make a sensible choice about
what to do with counter metrics, and cache the latest values, so that
they can be properly incremented.

Fixes gh-4305

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/export/MetricCopyExporter.java

@@ -20,6 +20,8 @@ import java.io.Flushable;
 import java.lang.reflect.Method;
 import java.util.Collection;
 import java.util.Iterator;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -27,6 +29,9 @@ import org.apache.commons.logging.LogFactory;
 import org.springframework.boot.actuate.metrics.Metric;
 import org.springframework.boot.actuate.metrics.reader.MetricReader;
 import org.springframework.boot.actuate.metrics.writer.CompositeMetricWriter;
+import org.springframework.boot.actuate.metrics.writer.CounterWriter;
+import org.springframework.boot.actuate.metrics.writer.Delta;
+import org.springframework.boot.actuate.metrics.writer.GaugeWriter;
 import org.springframework.boot.actuate.metrics.writer.MetricWriter;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.ObjectUtils;
@@ -35,7 +40,15 @@ import org.springframework.util.ReflectionUtils;
 
 /**
  * {@link Exporter} that "exports" by copying metric data from a source
- * {@link MetricReader} to a destination {@link MetricWriter}.
+ * {@link MetricReader} to a destination {@link MetricWriter}. Actually the output writer
+ * can be a {@link GaugeWriter}, in which case all metrics are simply output as their
+ * current value. If the output writer is also a {@link CounterWriter} then metrics whose
+ * names begin with "counter." are special: instead of writing them out as simple gauges
+ * the writer will increment the counter value. This involves the exporter storing the
+ * previous value of the counter so the delta can be computed. For best results with the
+ * counters, do not use the exporter concurrently in multiple threads (normally it will
+ * only be used periodically and sequentially, even if it is in a background thread, and
+ * this is fine).
  *
  * @author Dave Syer
  * @since 1.3.0
@@ -46,7 +59,11 @@ public class MetricCopyExporter extends AbstractMetricExporter {
 
 	private final MetricReader reader;
 
-	private final MetricWriter writer;
+	private final GaugeWriter writer;
+
+	private final CounterWriter counter;
+
+	private ConcurrentMap<String, Long> counts = new ConcurrentHashMap<String, Long>();
 
 	private String[] includes = new String[0];
 
@@ -57,7 +74,7 @@ public class MetricCopyExporter extends AbstractMetricExporter {
 	 * @param reader the metric reader
 	 * @param writer the metric writer
 	 */
-	public MetricCopyExporter(MetricReader reader, MetricWriter writer) {
+	public MetricCopyExporter(MetricReader reader, GaugeWriter writer) {
 		this(reader, writer, "");
 	}
 
@@ -67,10 +84,16 @@ public class MetricCopyExporter extends AbstractMetricExporter {
 	 * @param writer the metric writer
 	 * @param prefix the name prefix
 	 */
-	public MetricCopyExporter(MetricReader reader, MetricWriter writer, String prefix) {
+	public MetricCopyExporter(MetricReader reader, GaugeWriter writer, String prefix) {
 		super(prefix);
 		this.reader = reader;
 		this.writer = writer;
+		if (writer instanceof CounterWriter) {
+			this.counter = (CounterWriter) writer;
+		}
+		else {
+			this.counter = null;
+		}
 	}
 
 	/**
@@ -104,8 +127,25 @@ public class MetricCopyExporter extends AbstractMetricExporter {
 	@Override
 	protected void write(String group, Collection<Metric<?>> values) {
 		for (Metric<?> value : values) {
-			this.writer.set(value);
+			if (value.getName().startsWith("counter.") && this.counter != null) {
+				this.counter.increment(calculateDelta(value));
+			}
+			else {
+				this.writer.set(value);
+			}
+		}
+	}
+
+	private Delta<?> calculateDelta(Metric<?> value) {
+		long delta = value.getValue().longValue();
+		Long old = this.counts.replace(value.getName(), delta);
+		if (old != null) {
+			delta = delta - old;
+		}
+		else {
+			this.counts.putIfAbsent(value.getName(), delta);
 		}
+		return new Delta<Long>(value.getName(), delta, value.getTimestamp());
 	}
 
 	@Override
@@ -113,7 +153,7 @@ public class MetricCopyExporter extends AbstractMetricExporter {
 		flush(this.writer);
 	}
 
-	private void flush(MetricWriter writer) {
+	private void flush(GaugeWriter writer) {
 		if (writer instanceof CompositeMetricWriter) {
 			for (MetricWriter child : (CompositeMetricWriter) writer) {
 				flush(child);

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/opentsdb/OpenTsdbMetricWriter.java

@@ -26,7 +26,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.boot.actuate.metrics.Metric;
-import org.springframework.boot.actuate.metrics.writer.Delta;
+import org.springframework.boot.actuate.metrics.writer.GaugeWriter;
 import org.springframework.boot.actuate.metrics.writer.MetricWriter;
 import org.springframework.http.HttpEntity;
 import org.springframework.http.HttpHeaders;
@@ -48,7 +48,7 @@ import org.springframework.web.client.RestTemplate;
  * @author Thomas Badie
  * @since 1.3.0
  */
-public class OpenTsdbMetricWriter implements MetricWriter {
+public class OpenTsdbMetricWriter implements GaugeWriter {
 
 	private static final Log logger = LogFactory.getLog(OpenTsdbMetricWriter.class);
 
@@ -99,11 +99,6 @@ public class OpenTsdbMetricWriter implements MetricWriter {
 		this.namingStrategy = namingStrategy;
 	}
 
-	@Override
-	public void increment(Delta<?> delta) {
-		throw new UnsupportedOperationException("Counters not supported via increment");
-	}
-
 	@Override
 	public void set(Metric<?> value) {
 		OpenTsdbData data = new OpenTsdbData(this.namingStrategy.getName(value.getName()),
@@ -147,9 +142,4 @@ public class OpenTsdbMetricWriter implements MetricWriter {
 		}
 	}
 
-	@Override
-	public void reset(String metricName) {
-		set(new Metric<Long>(metricName, 0L));
-	}
-
 }

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/statsd/StatsdMetricWriter.java

@@ -82,15 +82,18 @@ public class StatsdMetricWriter implements MetricWriter, Closeable {
 			this.client.recordExecutionTime(name, value.getValue().longValue());
 		}
 		else {
-			this.client.gauge(name, value.getValue().longValue());
+			if (name.contains("counter.")) {
+				this.client.count(name, value.getValue().longValue());
+			}
+			else {
+				this.client.gauge(name, value.getValue().doubleValue());
+			}
 		}
 	}
 
 	@Override
 	public void reset(String name) {
-		if (name.contains("counter.")) {
-			this.client.gauge(name, 0L);
-		}
+		// Not implemented
 	}
 
 	@Override

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/writer/CounterWriter.java

+/*
+ * Copyright 2012-2015 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;
+
+/**
+ * Simple writer for counters (metrics that increment).
+ *
+ * @author Dave Syer
+ */
+public interface CounterWriter {
+
+	/**
+	 * Increment the value of a metric (or decrement if the delta is negative). The name
+	 * of the delta is the name of the metric to increment.
+	 *
+	 * @param delta the amount to increment by
+	 */
+	void increment(Delta<?> delta);
+
+	/**
+	 * Reset the value of a metric, usually to zero value. Implementations can discard the
+	 * old values if desired, but may choose not to. This operation is optional (some
+	 * implementations may not be able to fulfil the contract, in which case they should
+	 * simply do nothing).
+	 *
+	 * @param metricName the name to reset
+	 */
+	void reset(String metricName);
+
+}

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/writer/GaugeWriter.java

+/*
+ * Copyright 2012-2015 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 org.springframework.boot.actuate.metrics.Metric;
+
+/**
+ * Writer for gauge values (simple metric with a number value).
+ *
+ * @author Dave Syer
+ */
+public interface GaugeWriter {
+
+	/**
+	 * Set the value of a metric.
+	 * @param value the value
+	 */
+	void set(Metric<?> value);
+
+}

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/metrics/writer/MetricWriter.java

@@ -23,26 +23,6 @@ import org.springframework.boot.actuate.metrics.Metric;
  *
  * @author Dave Syer
  */
-public interface MetricWriter {
-
-	/**
-	 * Increment the value of a metric (or decrement if the delta is negative). The name
-	 * of the delta is the name of the metric to increment.
-	 * @param delta the amount to increment by
-	 */
-	void increment(Delta<?> delta);
-
-	/**
-	 * Set the value of a metric.
-	 * @param value the value
-	 */
-	void set(Metric<?> value);
-
-	/**
-	 * Reset the value of a metric, usually to zero value. Implementations can discard the
-	 * old values if desired, but may choose not to.
-	 * @param metricName the name to reset
-	 */
-	void reset(String metricName);
+public interface MetricWriter extends GaugeWriter, CounterWriter {
 
 }

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/export/MetricCopyExporterTests.java

@@ -22,6 +22,8 @@ import org.junit.Test;
 
 import org.springframework.boot.actuate.metrics.Metric;
 import org.springframework.boot.actuate.metrics.repository.InMemoryMetricRepository;
+import org.springframework.boot.actuate.metrics.writer.Delta;
+import org.springframework.boot.actuate.metrics.writer.GaugeWriter;
 
 import static org.junit.Assert.assertEquals;
 
@@ -46,6 +48,29 @@ public class MetricCopyExporterTests {
 		assertEquals(1, this.writer.count());
 	}
 
+	@Test
+	public void counter() {
+		this.reader.increment(new Delta<Number>("counter.foo", 2));
+		this.exporter.export();
+		assertEquals(1, this.writer.count());
+		this.reader.increment(new Delta<Number>("counter.foo", 3));
+		this.exporter.export();
+		this.exporter.flush();
+		assertEquals(5L, this.writer.findOne("counter.foo").getValue());
+	}
+
+	@Test
+	public void counterWithGaugeWriter() {
+		SimpleGaugeWriter writer = new SimpleGaugeWriter();
+		MetricCopyExporter exporter = new MetricCopyExporter(this.reader, writer);
+		this.reader.increment(new Delta<Number>("counter.foo", 2));
+		exporter.export();
+		this.reader.increment(new Delta<Number>("counter.foo", 3));
+		exporter.export();
+		exporter.flush();
+		assertEquals(5L, writer.getValue().getValue());
+	}
+
 	@Test
 	public void exportIncludes() {
 		this.exporter.setIncludes("*");
@@ -90,4 +115,19 @@ public class MetricCopyExporterTests {
 		assertEquals(1, this.writer.count());
 	}
 
+	private static class SimpleGaugeWriter implements GaugeWriter {
+
+		private Metric<?> value;
+
+		@Override
+		public void set(Metric<?> value) {
+			this.value = value;
+		}
+
+		public Metric<?> getValue() {
+			return this.value;
+		}
+
+	}
+
 }

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/metrics/statsd/StatsdMetricWriterTests.java

@@ -71,7 +71,7 @@ public class StatsdMetricWriterTests {
 		this.writer.set(new Metric<Double>("gauge.foo", 3.7));
 		this.server.waitForMessage();
 		// Doubles are truncated
-		assertEquals("me.gauge.foo:3|g", this.server.messagesReceived().get(0));
+		assertEquals("me.gauge.foo:3.7|g", this.server.messagesReceived().get(0));
 	}
 
 	@Test