Merge pull request #255 from brharrington/max-poll

Min/Max gauges should call poll for getValue
Netflix · May 28, 2014 · 16c6b8a · 16c6b8a
2 parents 3128289 + 377a8ca
commit 16c6b8a
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 8 deletions.
diff --git a/servo-core/src/main/java/com/netflix/servo/monitor/BasicTimer.java b/servo-core/src/main/java/com/netflix/servo/monitor/BasicTimer.java
@@ -170,12 +170,12 @@ public Long getCount() {
 
     /** Get the min value since the last reset. */
     public Double getMin() {
-        return min.getValue(0) * timeUnitNanosFactor;
+        return min.getCurrentValue(0) * timeUnitNanosFactor;
     }
 
     /** Get the max value since the last reset. */
     public Double getMax() {
-        return max.getValue(0) * timeUnitNanosFactor;
+        return max.getCurrentValue(0) * timeUnitNanosFactor;
     }
 
     /** {@inheritDoc} */

diff --git a/servo-core/src/main/java/com/netflix/servo/monitor/MaxGauge.java b/servo-core/src/main/java/com/netflix/servo/monitor/MaxGauge.java
@@ -67,6 +67,13 @@ public void update(long v) {
     /** {@inheritDoc} */
     @Override
     public Long getValue(int nth) {
+        return max.poll(nth).getValue();
+    }
+
+    /**
+     * Returns the current max value since the last reset.
+     */
+    public long getCurrentValue(int nth) {
         return max.getCurrent(nth).get();
     }
 

diff --git a/servo-core/src/main/java/com/netflix/servo/monitor/MinGauge.java b/servo-core/src/main/java/com/netflix/servo/monitor/MinGauge.java
@@ -71,7 +71,15 @@ public void update(long v) {
      */
     @Override
     public Long getValue(int pollerIdx) {
-        long v = min.getCurrent(pollerIdx).get();
+        long v = min.poll(pollerIdx).getValue();
+        return (v == Long.MAX_VALUE) ? 0L : v;
+    }
+
+    /**
+     * Returns the current min value since the last reset.
+     */
+    public long getCurrentValue(int nth) {
+        long v = min.getCurrent(nth).get();
         return (v == Long.MAX_VALUE) ? 0L : v;
     }
 

diff --git a/servo-core/src/test/java/com/netflix/servo/monitor/BucketTimerTest.java b/servo-core/src/test/java/com/netflix/servo/monitor/BucketTimerTest.java
@@ -18,9 +18,11 @@
 
 import static org.testng.Assert.*;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import org.testng.annotations.Test;
 
 public class BucketTimerTest extends AbstractMonitorTest<BucketTimer> {
@@ -110,11 +112,12 @@ public void testRecord() throws Exception {
     }
 
     private void assertMonitors(List<Monitor<?>> monitors, Map<String, Number> expectedValues) {
+        Set<String> exclude = ImmutableSet.of("count", "min", "max");
         String[] namespaces = new String[] { "statistic", "servo.bucket"};
         for (Monitor<?> monitor : monitors) {
             for (String namespace : namespaces) {
                 final String tag = monitor.getConfig().getTags().getValue(namespace);
-                if (tag != null && !tag.equals("count")) {
+                if (tag != null && !exclude.contains(tag)) {
                     final Number actual = (Number) monitor.getValue();
                     final Number expected = expectedValues.get(tag);
                     assertEquals(actual, expected, namespace + "." + tag);

diff --git a/servo-core/src/test/java/com/netflix/servo/monitor/MaxGaugeTest.java b/servo-core/src/test/java/com/netflix/servo/monitor/MaxGaugeTest.java
@@ -15,25 +15,48 @@
  */
 package com.netflix.servo.monitor;
 
-import com.netflix.servo.util.ClockWithOffset;
+import com.netflix.servo.util.ManualClock;
 import org.testng.annotations.Test;
 
 import static org.testng.Assert.assertEquals;
 
 public class MaxGaugeTest extends AbstractMonitorTest<MaxGauge> {
+
+    private ManualClock clock = new ManualClock(0L);
+
     @Override
     public MaxGauge newInstance(String name) {
-        return new MaxGauge(MonitorConfig.builder(name).build(), ClockWithOffset.INSTANCE);
+        return new MaxGauge(MonitorConfig.builder(name).build(), clock);
     }
 
     @Test
     public void testUpdate() throws Exception {
+        clock.set(0L);
         MaxGauge maxGauge = newInstance("max1");
         maxGauge.update(42L);
+        assertEquals(maxGauge.getValue().longValue(), 0L);
+        clock.set(60000L);
         assertEquals(maxGauge.getValue().longValue(), 42L);
+    }
+
+    @Test
+    public void testUpdate2() throws Exception {
+        clock.set(0L);
+        MaxGauge maxGauge = newInstance("max1");
+        maxGauge.update(42L);
         maxGauge.update(420L);
+        clock.set(60000L);
         assertEquals(maxGauge.getValue().longValue(), 420L);
+    }
+
+    @Test
+    public void testUpdate3() throws Exception {
+        clock.set(0L);
+        MaxGauge maxGauge = newInstance("max1");
+        maxGauge.update(42L);
+        maxGauge.update(420L);
         maxGauge.update(1L);
+        clock.set(60000L);
         assertEquals(maxGauge.getValue().longValue(), 420L);
     }
 }
diff --git a/servo-core/src/test/java/com/netflix/servo/monitor/MinGaugeTest.java b/servo-core/src/test/java/com/netflix/servo/monitor/MinGaugeTest.java
@@ -15,26 +15,48 @@
  */
 package com.netflix.servo.monitor;
 
-import com.netflix.servo.util.ClockWithOffset;
+import com.netflix.servo.util.ManualClock;
 import org.testng.annotations.Test;
 
 import static org.testng.Assert.assertEquals;
 
 public class MinGaugeTest extends AbstractMonitorTest<MinGauge> {
+
+    private ManualClock clock = new ManualClock(0L);
+
     @Override
     public MinGauge newInstance(String name) {
         MonitorConfig config = MonitorConfig.builder(name).build();
-        return new MinGauge(config, ClockWithOffset.INSTANCE);
+        return new MinGauge(config, clock);
     }
 
     @Test
     public void testUpdate() throws Exception {
+        clock.set(0L);
         MinGauge minGauge = newInstance("min1");
         minGauge.update(42L);
+        clock.set(60000L);
         assertEquals(minGauge.getValue().longValue(), 42L);
+    }
+
+    @Test
+    public void testUpdate2() throws Exception {
+        clock.set(0L);
+        MinGauge minGauge = newInstance("min1");
+        minGauge.update(42L);
         minGauge.update(420L);
+        clock.set(60000L);
         assertEquals(minGauge.getValue().longValue(), 42L);
+    }
+
+    @Test
+    public void testUpdate3() throws Exception {
+        clock.set(0L);
+        MinGauge minGauge = newInstance("min1");
+        minGauge.update(42L);
+        minGauge.update(420L);
         minGauge.update(1L);
+        clock.set(60000L);
         assertEquals(minGauge.getValue().longValue(), 1L);
     }
 }