Switched to Instant/Duration for time, test case cleanup

2 files changed, 29 insertions, 30 deletions

diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetrics.java b/container-disc/src/main/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetrics.java
index c208da6a07d..317cd3134af 100644
--- a/container-disc/src/main/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetrics.java
+++ b/container-disc/src/main/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetrics.java
@@ -6,6 +6,7 @@ import java.lang.management.GarbageCollectorMXBean;
 import java.lang.management.ManagementFactory;
 import java.time.Clock;
 import java.time.Duration;
+import java.time.Instant;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -15,21 +16,22 @@ import java.util.Map;
  * @author ollivir
  */
 public class GarbageCollectionMetrics {
-    private static final String GC_COUNT = "jdisc.gc.count";
-    private static final String GC_TIME = "jdisc.gc.ms";
+    private static final String GC_PREFIX = "jdisc.gc.";
+    private static final String GC_COUNT = GC_PREFIX + ".count";
+    private static final String GC_TIME = GC_PREFIX + ".ms";
     private static final String DIMENSION_KEY = "gcName";
 
-    public static final long REPORTING_INTERVAL = Duration.ofSeconds(62).toMillis();
+    public static final Duration REPORTING_INTERVAL = Duration.ofSeconds(62);
 
     static class GcStats {
-        private final long when;
+        private final Instant when;
         private final long count;
-        private final long ms;
+        private final Duration totalRuntime;
 
-        private GcStats(long when, long count, long ms) {
+        private GcStats(Instant when, long count, Duration totalRuntime) {
             this.when = when;
             this.count = count;
-            this.ms = ms;
+            this.totalRuntime = totalRuntime;
         }
     }
 
@@ -40,26 +42,26 @@ public class GarbageCollectionMetrics {
     public GarbageCollectionMetrics(Clock clock) {
         this.clock = clock;
         this.gcStatistics = new HashMap<>();
-        collectGcStatistics(clock.millis());
+        collectGcStatistics(clock.instant());
     }
 
-    private void collectGcStatistics(long now) {
+    private void collectGcStatistics(Instant now) {
         for (GarbageCollectorMXBean gcBean : ManagementFactory.getGarbageCollectorMXBeans()) {
             String gcName = gcBean.getName().replace(" ", "");
-            GcStats stats = new GcStats(now, gcBean.getCollectionCount(), gcBean.getCollectionTime());
+            GcStats stats = new GcStats(now, gcBean.getCollectionCount(), Duration.ofMillis(gcBean.getCollectionTime()));
 
             LinkedList<GcStats> window = gcStatistics.computeIfAbsent(gcName, anyName -> new LinkedList<>());
             window.addLast(stats);
         }
     }
 
-    private void cleanStatistics(long now) {
-        long oldestToKeep = now - REPORTING_INTERVAL;
+    private void cleanStatistics(Instant now) {
+        Instant oldestToKeep = now.minus(REPORTING_INTERVAL);
 
         for(Iterator<Map.Entry<String, LinkedList<GcStats>>> it = gcStatistics.entrySet().iterator(); it.hasNext(); ) {
             Map.Entry<String, LinkedList<GcStats>> entry = it.next();
             LinkedList<GcStats> history = entry.getValue();
-            while(history.isEmpty() == false && history.getFirst().when < oldestToKeep) {
+            while(history.isEmpty() == false && oldestToKeep.isAfter(history.getFirst().when)) {
                 history.removeFirst();
             }
             if(history.isEmpty()) {
@@ -69,7 +71,7 @@ public class GarbageCollectionMetrics {
     }
 
     public void emitMetrics(Metric metric) {
-        long now = clock.millis();
+        Instant now = clock.instant();
 
         collectGcStatistics(now);
         cleanStatistics(now);
@@ -82,7 +84,7 @@ public class GarbageCollectionMetrics {
             Metric.Context gcContext = metric.createContext(contextData);
 
             metric.set(GC_COUNT, latest.count - reference.count, gcContext);
-            metric.set(GC_TIME, latest.ms - reference.ms, gcContext);
+            metric.set(GC_TIME, latest.totalRuntime.minus(reference.totalRuntime).toMillis(), gcContext);
         }
     }
 
diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetricsTest.java b/container-disc/src/test/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetricsTest.java
index 76ad430f4b9..61d8763b852 100644
--- a/container-disc/src/test/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetricsTest.java
+++ b/container-disc/src/test/java/com/yahoo/container/jdisc/metric/GarbageCollectionMetricsTest.java
@@ -1,55 +1,52 @@
 package com.yahoo.container.jdisc.metric;
 
 import com.yahoo.jdisc.Metric;
+import com.yahoo.test.ManualClock;
 import org.junit.Test;
 
 import java.lang.management.ManagementFactory;
-import java.time.Clock;
+import java.time.Duration;
 import java.util.LinkedList;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
-import static org.mockito.Mockito.atLeast;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 
 /**
  * @author ollivir
  */
 public class GarbageCollectionMetricsTest {
     @Test
-    public void gc_metrics_reported_at_intervals() {
-        Clock clock = mock(Clock.class);
+    public void gc_metrics_are_collected_in_a_sliding_window() {
+        ManualClock clock = new ManualClock();
         Metric metric = mock(Metric.class);
-        int base = ManagementFactory.getGarbageCollectorMXBeans().size();
+        int garbageCollectors = ManagementFactory.getGarbageCollectorMXBeans().size();
 
-        long iv = GarbageCollectionMetrics.REPORTING_INTERVAL;
-        when(clock.millis()).thenReturn(10L);
+        Duration interval = GarbageCollectionMetrics.REPORTING_INTERVAL;
         GarbageCollectionMetrics garbageCollectionMetrics = new GarbageCollectionMetrics(clock);
-        assertThat(garbageCollectionMetrics.getGcStatistics().keySet().size(), is(base));
+        assertThat(garbageCollectionMetrics.getGcStatistics().keySet().size(), is(garbageCollectors));
 
-        when(clock.millis()).thenReturn(iv);
+        clock.advance(interval.minus(Duration.ofMillis(10)));
         garbageCollectionMetrics.emitMetrics(metric);
         assertWindowLengths(garbageCollectionMetrics, 2);
 
-        when(clock.millis()).thenReturn(10L + iv);
+        clock.advance(Duration.ofMillis(10));
         garbageCollectionMetrics.emitMetrics(metric);
         assertWindowLengths(garbageCollectionMetrics, 3);
 
-        when(clock.millis()).thenReturn(20L + iv);
+        clock.advance(Duration.ofMillis(10));
         garbageCollectionMetrics.emitMetrics(metric);
         assertWindowLengths(garbageCollectionMetrics, 3);
 
-        when(clock.millis()).thenReturn(20L + iv + iv);
+        clock.advance(interval);
         garbageCollectionMetrics.emitMetrics(metric);
         assertWindowLengths(garbageCollectionMetrics, 2);
 
-        verify(metric, times(base * 4 * 2)).set(anyString(), any(), any());
+        verify(metric, times(garbageCollectors * 4 * 2)).set(anyString(), any(), any());
     }
 
     private static void assertWindowLengths(GarbageCollectionMetrics gcm, int count) {