Merge pull request #800 from brharrington/spectator-issue-264

* spectator-issue-264:
  fix registration bug in SpectatorMetricServices

spring-cloud-netflix-spectator/src/main/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricServices.java

@@ -13,7 +13,6 @@
 
 package org.springframework.cloud.netflix.metrics.spectator;
 
-import java.util.Collections;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.TimeUnit;
@@ -22,10 +21,7 @@ import java.util.concurrent.atomic.AtomicLong;
 import org.springframework.boot.actuate.metrics.CounterService;
 import org.springframework.boot.actuate.metrics.GaugeService;
 
-import com.netflix.spectator.api.AbstractMeter;
-import com.netflix.spectator.api.Gauge;
 import com.netflix.spectator.api.Id;
-import com.netflix.spectator.api.Measurement;
 import com.netflix.spectator.api.Registry;
 import com.netflix.spectator.impl.AtomicDouble;
 
@@ -74,7 +70,6 @@ public class SpectatorMetricServices implements CounterService, GaugeService {
 			final Id id = registry.createId(name);
 			final AtomicLong gauge = getCounterStorage(id);
 			gauge.addAndGet(value);
-			registry.register(new NumericGauge(id, gauge));
 		}
 	}
 
@@ -87,48 +82,31 @@ public class SpectatorMetricServices implements CounterService, GaugeService {
 
 	@Override
 	public void submit(String name, double dValue) {
-		long value = ((Double) dValue).longValue();
 		if (name.startsWith("histogram.")) {
-			registry.distributionSummary(stripMetricName(name)).record(value);
+			registry.distributionSummary(stripMetricName(name)).record((long) dValue);
 		}
 		else if (name.startsWith("timer.")) {
-			registry.timer(stripMetricName(name)).record(value, TimeUnit.MILLISECONDS);
+			// Input is in milliseconds. Convert to nanos before casting to long to allow
+			// sub-millisecond durations to be recorded correctly.
+			long value = (long) (dValue * 1e6);
+			registry.timer(stripMetricName(name)).record(value, TimeUnit.NANOSECONDS);
 		}
 		else {
 			final Id id = registry.createId(name);
 			final AtomicDouble gauge = getGaugeStorage(id);
 			gauge.set(dValue);
-			registry.register(new NumericGauge(id, gauge));
 		}
 	}
 
 	private AtomicDouble getGaugeStorage(Id id) {
 		final AtomicDouble newGauge = new AtomicDouble(0);
 		final AtomicDouble existingGauge = gauges.putIfAbsent(id, newGauge);
-		return existingGauge == null ? newGauge : existingGauge;
+		return existingGauge == null ? registry.gauge(id, newGauge) : existingGauge;
 	}
 
 	private AtomicLong getCounterStorage(Id id) {
 		final AtomicLong newCounter = new AtomicLong(0);
 		final AtomicLong existingCounter = counters.putIfAbsent(id, newCounter);
-		return existingCounter == null ? newCounter : existingCounter;
-	}
-
-	private class NumericGauge extends AbstractMeter<Number> implements Gauge {
-		NumericGauge(Id id, Number val) {
-			super(registry.clock(), id, val);
-		}
-
-		@Override
-		public Iterable<Measurement> measure() {
-			return Collections.singleton(new Measurement(this.id, this.clock.wallTime(),
-					this.value()));
-		}
-
-		@SuppressWarnings("ConstantConditions")
-		@Override
-		public double value() {
-			return this.ref.get().doubleValue();
-		}
+		return existingCounter == null ? registry.gauge(id, newCounter) : existingCounter;
 	}
 }

spring-cloud-netflix-spectator/src/test/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricServicesTests.java

@@ -13,9 +13,16 @@
 
 package org.springframework.cloud.netflix.metrics.spectator;
 
+import com.netflix.spectator.api.DefaultRegistry;
+import com.netflix.spectator.api.Measurement;
+import com.netflix.spectator.api.Registry;
 import org.junit.Test;
 
+import java.util.Iterator;
+import java.util.concurrent.TimeUnit;
+
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.springframework.cloud.netflix.metrics.spectator.SpectatorMetricServices.stripMetricName;
 
 public class SpectatorMetricServicesTests {
@@ -36,4 +43,81 @@ public class SpectatorMetricServicesTests {
 	public void metricTypeNameEmbeddedInMiddleOfMetricNameIsNotRemoved() {
 		assertEquals("bar.timer.foo", stripMetricName("bar.timer.foo"));
 	}
+
+	@Test
+	public void meterPrefixShouldIncrementCounter() {
+		Registry registry = new DefaultRegistry();
+		SpectatorMetricServices ms = new SpectatorMetricServices(registry);
+		ms.increment("meter.test");
+		assertEquals(1, registry.counter("test").count());
+	}
+
+	@Test
+	public void otherPrefixShouldUpdateGauge() {
+		Registry registry = new DefaultRegistry();
+		SpectatorMetricServices ms = new SpectatorMetricServices(registry);
+
+		ms.increment("gauge.test");
+		assertGaugeEquals(registry, "gauge.test", 1.0);
+
+		ms.decrement("gauge.test");
+		assertGaugeEquals(registry, "gauge.test", 0.0);
+	}
+
+	@Test
+	public void histogramSubmit() {
+		Registry registry = new DefaultRegistry();
+		SpectatorMetricServices ms = new SpectatorMetricServices(registry);
+		ms.submit("histogram.test", 42.0);
+		assertEquals(1L, registry.distributionSummary("test").count());
+		assertEquals(42L, registry.distributionSummary("test").totalAmount());
+	}
+
+	@Test
+	public void timerSubmit() {
+		Registry registry = new DefaultRegistry();
+		SpectatorMetricServices ms = new SpectatorMetricServices(registry);
+		ms.submit("timer.test", 42.0);
+		assertEquals(1L, registry.timer("test").count());
+		assertEquals(TimeUnit.MILLISECONDS.toNanos(42L), registry.timer("test").totalTime());
+	}
+
+	@Test
+	public void timerMicrosSubmit() {
+		Registry registry = new DefaultRegistry();
+		SpectatorMetricServices ms = new SpectatorMetricServices(registry);
+		ms.submit("timer.test", 0.042);
+		assertEquals(1L, registry.timer("test").count());
+		assertEquals(TimeUnit.MICROSECONDS.toNanos(42L), registry.timer("test").totalTime());
+	}
+
+	@Test
+	public void gaugeSubmit() {
+		Registry registry = new DefaultRegistry();
+		SpectatorMetricServices ms = new SpectatorMetricServices(registry);
+		ms.submit("gauge.test", 42.0);
+		assertGaugeEquals(registry, "gauge.test", 42.0);
+
+		ms.submit("gauge.test", 1.0);
+		assertGaugeEquals(registry, "gauge.test", 1.0);
+	}
+
+	@Test
+	public void gaugeSubmitManyTimes() {
+		// Sanity check for memory leak reported in:
+		// https://github.com/Netflix/spectator/issues/264
+		Registry registry = new DefaultRegistry();
+		SpectatorMetricServices ms = new SpectatorMetricServices(registry);
+		for (int i = 0; i < 10000; ++i) {
+			ms.submit("gauge.test", 42.0);
+			assertGaugeEquals(registry, "gauge.test", 42.0);
+		}
+	}
+
+	private void assertGaugeEquals(Registry registry, String name, double expected) {
+		Iterator<Measurement> it = registry.get(registry.createId(name)).measure().iterator();
+		assertTrue(it.hasNext());
+		assertEquals(expected, it.next().value(), 1e-3);
+		assertTrue(!it.hasNext());
+	}
 }