Merge pull request #12578 from vespa-engine/bratseth/metrics-timestamps-in-seconds

Consume metrics timestamps in seconds not ms

7 files changed, 29 insertions, 32 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
index 0632d85906a..8ddb85e9505 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
@@ -67,7 +67,6 @@ public class Autoscaler {
         if (clusterNodes.stream().anyMatch(node -> node.status().wantToRetire() ||
                                                    node.allocation().get().membership().retired() ||
                                                    node.allocation().get().isRemovable())) {
-            log.fine("Autoscaling " + applicationId + " " + cluster + ": Cluster is in flux");
             return Optional.empty(); // Don't autoscale clusters that are in flux
         }
         AllocatableClusterResources currentAllocation = new AllocatableClusterResources(clusterNodes, resourcesCalculator);
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java
index 653b786ebe5..87551a5bd5f 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricsResponse.java
@@ -44,17 +44,17 @@ public class MetricsResponse {
     }
 
     private void consumeNodeMetrics(String hostname, Inspector node) {
-        long timestamp = node.field("timestamp").asLong();
+        long timestampSecond = node.field("timestamp").asLong();
         Map<String, Double> values = consumeMetrics(node.field("metrics"));
         for (Resource resource : Resource.values())
-            addMetricIfPresent(hostname, resource, timestamp, values);
+            addMetricIfPresent(hostname, resource, timestampSecond, values);
     }
 
-    private void addMetricIfPresent(String hostname, Resource resource, long timestamp, Map<String, Double> values) {
+    private void addMetricIfPresent(String hostname, Resource resource, long timestampSecond, Map<String, Double> values) {
         if (values.containsKey(resource.metricName()))
             metricValues.add(new NodeMetrics.MetricValue(hostname,
                                                          resource.metricName(),
-                                                         timestamp,
+                                                         timestampSecond,
                                                          values.get(resource.metricName())));
     }
 
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetrics.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetrics.java
index 8abc2327d88..b0d73833bc6 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetrics.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetrics.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.provision.autoscale;
 
 import com.yahoo.config.provision.ApplicationId;
 
+import java.time.Instant;
 import java.util.Collection;
 
 /**
@@ -23,24 +24,24 @@ public interface NodeMetrics {
 
         private final String hostname;
         private final String name;
-        private long timestamp;
+        private long timestampSecond;
         private final double value;
 
-        public MetricValue(String hostname, String name, long timestamp, double value) {
+        public MetricValue(String hostname, String name, long timestampSecond, double value) {
             this.hostname = hostname;
             this.name = name;
-            this.timestamp = timestamp;
+            this.timestampSecond = timestampSecond;
             this.value = value;
         }
 
         public String hostname() { return hostname; }
         public String name() { return name; }
-        public long timestamp() { return timestamp; }
+        public long timestampSecond() { return timestampSecond; }
         public double value() { return value; }
 
         @Override
         public String toString() {
-            return "metric value " + name + ": " + value + " at " + timestamp + " for " + hostname;
+            return "metric value " + name + ": " + value + " at " + Instant.ofEpochSecond(timestampSecond) + " for " + hostname;
         }
 
     }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDb.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDb.java
index aa20752d8a7..c86546a7790 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDb.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDb.java
@@ -11,6 +11,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -34,21 +35,19 @@ public class NodeMetricsDb {
 
     /** Add a measurement to this */
     public void add(Collection<NodeMetrics.MetricValue> metricValues) {
-        log.fine("Adding " + metricValues.size() + " metric values" +
-                 (metricValues.size() > 0 ? ". First: " + metricValues.iterator().next() : ""));
         synchronized (lock) {
             for (var value : metricValues) {
                 Resource resource =  Resource.fromMetric(value.name());
                 List<Measurement> measurements = db.computeIfAbsent(new MeasurementKey(value.hostname(), resource),
                                                                     (__) -> new ArrayList<>());
-                measurements.add(new Measurement(value.timestamp(), (float)resource.valueFromMetric(value.value())));
+                measurements.add(new Measurement(value.timestampSecond() * 1000,
+                                                 (float)resource.valueFromMetric(value.value())));
             }
         }
     }
 
     /** Must be called intermittently (as long as add is called) to gc old measurements */
     public void gc(Clock clock) {
-        int gcCount = 0;
         synchronized (lock) {
             // TODO: We may need to do something more complicated to avoid spending too much memory to
             // lower the measurement interval (see NodeRepositoryMaintenance)
@@ -58,16 +57,13 @@ public class NodeMetricsDb {
             long oldestTimestamp = clock.instant().minus(dbWindow).toEpochMilli();
             for (Iterator<List<Measurement>> i = db.values().iterator(); i.hasNext(); ) {
                 List<Measurement> measurements = i.next();
-                while (!measurements.isEmpty() && measurements.get(0).timestamp < oldestTimestamp) {
+                while (!measurements.isEmpty() && measurements.get(0).timestamp < oldestTimestamp)
                     measurements.remove(0);
-                    gcCount++;
-                }
 
                 if (measurements.isEmpty())
                     i.remove();
             }
         }
-        log.fine("Gc'ed " + gcCount + " metric values");
     }
 
     /** Returns a window within which we can ask for specific information from this db */
@@ -87,9 +83,6 @@ public class NodeMetricsDb {
 
         public int measurementCount() {
             synchronized (lock) {
-                List<MeasurementKey> matches = keys.stream().filter(key -> db.get(key) != null).collect(Collectors.toList());
-                List<MeasurementKey> nonMatches = keys.stream().filter(key -> db.get(key) == null).collect(Collectors.toList());
-                log.fine("Counting measurements after " + startTime + ". Matches: " + matches + ". Non-matches: " + nonMatches);
                 return (int) keys.stream()
                                  .flatMap(key -> db.getOrDefault(key, List.of()).stream())
                                  .filter(measurement -> measurement.timestamp >= startTime)
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
index f9dff774a82..5a2ba674d99 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
@@ -1,6 +1,7 @@
 // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.provision.autoscale;
 
+import com.google.common.collect.Sets;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.CloudName;
 import com.yahoo.config.provision.ClusterSpec;
@@ -10,11 +11,14 @@ import com.yahoo.config.provision.NodeResources;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
+import com.yahoo.io.IOUtils;
 import org.junit.Test;
 
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java
index 519235857f1..af87c008260 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java
@@ -18,15 +18,15 @@ public class NodeMetricsDbTest {
         NodeMetricsDb db = new NodeMetricsDb();
         List<NodeMetrics.MetricValue> values = new ArrayList<>();
         for (int i = 0; i < 40; i++) {
-            values.add(new NodeMetrics.MetricValue("host0", "cpu.util", clock.instant().toEpochMilli(), 0.9f));
+            values.add(new NodeMetrics.MetricValue("host0", "cpu.util", clock.instant().getEpochSecond(), 0.9f));
             clock.advance(Duration.ofHours(1));
         }
         db.add(values);
 
-        assertEquals(30, db.getWindow(clock.instant().minus(Duration.ofHours(30)), Resource.cpu,    List.of("host0")).measurementCount());
+        assertEquals(29, db.getWindow(clock.instant().minus(Duration.ofHours(30)), Resource.cpu,    List.of("host0")).measurementCount());
         assertEquals( 0, db.getWindow(clock.instant().minus(Duration.ofHours(30)), Resource.memory, List.of("host0")).measurementCount());
         db.gc(clock);
-        assertEquals(24, db.getWindow(clock.instant().minus(Duration.ofHours(30)), Resource.cpu,    List.of("host0")).measurementCount());
+        assertEquals(23, db.getWindow(clock.instant().minus(Duration.ofHours(30)), Resource.cpu,    List.of("host0")).measurementCount());
         assertEquals( 0, db.getWindow(clock.instant().minus(Duration.ofHours(30)), Resource.memory, List.of("host0")).measurementCount());
     }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsFetcherTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsFetcherTest.java
index 4f039582125..da4f74bf05b 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsFetcherTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsFetcherTest.java
@@ -40,11 +40,11 @@ public class NodeMetricsFetcherTest {
             assertEquals("http://host-1.yahoo.com:4080/metrics/v2/values?consumer=vespa-consumer-metrics",
                          httpClient.requestsReceived.get(0));
             assertEquals(5, values.size());
-            assertEquals("metric value cpu.util: 16.2 at 1234 for host-1.yahoo.com", values.get(0).toString());
-            assertEquals("metric value mem.util: 23.1 at 1234 for host-1.yahoo.com", values.get(1).toString());
-            assertEquals("metric value disk.util: 82.0 at 1234 for host-1.yahoo.com", values.get(2).toString());
-            assertEquals("metric value cpu.util: 20.0 at 1200 for host-2.yahoo.com", values.get(3).toString());
-            assertEquals("metric value disk.util: 40.0 at 1200 for host-2.yahoo.com", values.get(4).toString());
+            assertEquals("metric value cpu.util: 16.2 at 1970-01-01T00:20:34Z for host-1.yahoo.com", values.get(0).toString());
+            assertEquals("metric value mem.util: 23.1 at 1970-01-01T00:20:34Z for host-1.yahoo.com", values.get(1).toString());
+            assertEquals("metric value disk.util: 82.0 at 1970-01-01T00:20:34Z for host-1.yahoo.com", values.get(2).toString());
+            assertEquals("metric value cpu.util: 20.0 at 1970-01-01T00:20:00Z for host-2.yahoo.com", values.get(3).toString());
+            assertEquals("metric value disk.util: 40.0 at 1970-01-01T00:20:00Z for host-2.yahoo.com", values.get(4).toString());
         }
 
         {
@@ -53,9 +53,9 @@ public class NodeMetricsFetcherTest {
             assertEquals("http://host-3.yahoo.com:4080/metrics/v2/values?consumer=vespa-consumer-metrics",
                          httpClient.requestsReceived.get(1));
             assertEquals(3, values.size());
-            assertEquals("metric value cpu.util: 10.0 at 1300 for host-3.yahoo.com", values.get(0).toString());
-            assertEquals("metric value mem.util: 15.0 at 1300 for host-3.yahoo.com", values.get(1).toString());
-            assertEquals("metric value disk.util: 20.0 at 1300 for host-3.yahoo.com", values.get(2).toString());
+            assertEquals("metric value cpu.util: 10.0 at 1970-01-01T00:21:40Z for host-3.yahoo.com", values.get(0).toString());
+            assertEquals("metric value mem.util: 15.0 at 1970-01-01T00:21:40Z for host-3.yahoo.com", values.get(1).toString());
+            assertEquals("metric value disk.util: 20.0 at 1970-01-01T00:21:40Z for host-3.yahoo.com", values.get(2).toString());
         }
     }