diff --git a/spring-cloud-netflix-spectator/src/main/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricServices.java b/spring-cloud-netflix-spectator/src/main/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricServices.java index fc25e853..55162f75 100644 --- a/spring-cloud-netflix-spectator/src/main/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricServices.java +++ b/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 implements Gauge { - NumericGauge(Id id, Number val) { - super(registry.clock(), id, val); - } - - @Override - public Iterable 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; } } diff --git a/spring-cloud-netflix-spectator/src/test/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricServicesTests.java b/spring-cloud-netflix-spectator/src/test/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricServicesTests.java index 17ee9f2e..0624ff88 100644 --- a/spring-cloud-netflix-spectator/src/test/java/org/springframework/cloud/netflix/metrics/spectator/SpectatorMetricServicesTests.java +++ b/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 it = registry.get(registry.createId(name)).measure().iterator(); + assertTrue(it.hasNext()); + assertEquals(expected, it.next().value(), 1e-3); + assertTrue(!it.hasNext()); + } }