Merge pull request #9907 from vespa-engine/freva/report-0-instead-of-nan

Node-Admin: Do not report NaN metric MERGEOK

5 files changed, 111 insertions, 20 deletions

diff --git a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/DimensionMetrics.java b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/DimensionMetrics.java
index ef59c4b17d6..46a0f9b9b10 100644
--- a/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/DimensionMetrics.java
+++ b/docker-api/src/main/java/com/yahoo/vespa/hosted/dockerapi/metrics/DimensionMetrics.java
@@ -9,6 +9,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.TreeMap;
+import java.util.stream.Collectors;
 
 /**
  * @author freva
@@ -24,7 +25,9 @@ public class DimensionMetrics {
     DimensionMetrics(String application, Dimensions dimensions, Map<String, Number> metrics) {
         this.application = Objects.requireNonNull(application);
         this.dimensions = Objects.requireNonNull(dimensions);
-        this.metrics = Objects.requireNonNull(metrics);
+        this.metrics = metrics.entrySet().stream()
+                .filter(DimensionMetrics::metricIsFinite)
+                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
     }
 
     public String toSecretAgentReport() throws JsonProcessingException {
@@ -65,6 +68,10 @@ public class DimensionMetrics {
         return Objects.hash(application, dimensions, metrics);
     }
 
+    private static boolean metricIsFinite(Map.Entry<String, Number> metric) {
+        return ! (metric.getValue() instanceof Double) || Double.isFinite((double) metric.getValue());
+    }
+
     public static class Builder {
         private final String application;
         private final Dimensions dimensions;
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
index 44bcae3e838..90eda96d445 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
@@ -638,12 +638,16 @@ public class NodeAgentImpl implements NodeAgent {
         private long deltaThrottledPeriods;
 
         private void updateCpuDeltas(ContainerStats.CpuStats cpuStats) {
-            deltaSystemUsage = totalSystemUsage == 0 ? 0 : (cpuStats.getSystemCpuUsage() - totalSystemUsage);
-            deltaContainerUsage = cpuStats.getTotalUsage() - totalContainerUsage;
-            deltaContainerKernelUsage = cpuStats.getUsageInKernelMode() - containerKernelUsage;
-            deltaThrottledTime = cpuStats.getThrottledTime() - throttledTime;
-            deltaThrottlingActivePeriods = cpuStats.getThrottlingActivePeriods() - throttlingActivePeriods;
-            deltaThrottledPeriods = cpuStats.getThrottledPeriods() - throttledPeriods;
+            // Do not calculate delta during the first tick - that will result in a metric value that is
+            // average since container start
+            if (totalSystemUsage != 0) {
+                deltaSystemUsage = cpuStats.getSystemCpuUsage() - totalSystemUsage;
+                deltaContainerUsage = cpuStats.getTotalUsage() - totalContainerUsage;
+                deltaContainerKernelUsage = cpuStats.getUsageInKernelMode() - containerKernelUsage;
+                deltaThrottledTime = cpuStats.getThrottledTime() - throttledTime;
+                deltaThrottlingActivePeriods = cpuStats.getThrottlingActivePeriods() - throttlingActivePeriods;
+                deltaThrottledPeriods = cpuStats.getThrottledPeriods() - throttledPeriods;
+            }
 
             totalSystemUsage = cpuStats.getSystemCpuUsage();
             totalContainerUsage = cpuStats.getTotalUsage();
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
index b4db8ff40d5..46af7e7bafd 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
@@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.node.admin.nodeagent;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.NodeType;
-import com.yahoo.io.IOUtils;
 import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.hosted.dockerapi.Container;
@@ -29,13 +28,14 @@ import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException;
 import org.junit.Test;
 import org.mockito.InOrder;
 
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
-import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.List;
 import java.util.Optional;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
+import static com.yahoo.yolean.Exceptions.uncheck;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
@@ -640,8 +640,7 @@ public class NodeAgentImplTest {
     @Test
     @SuppressWarnings("unchecked")
     public void testGetRelevantMetrics() throws Exception {
-        ClassLoader classLoader = getClass().getClassLoader();
-        String json = IOUtils.readAll(classLoader.getResourceAsStream("docker.stats.json"), StandardCharsets.UTF_8);
+        String json = Files.readString(Paths.get("src/test/resources/docker.stats.json"));
         ContainerStats stats2 = ContainerStats.fromJson(json);
         ContainerStats stats1 = ContainerStats.fromJson(json.replace("\"cpu_stats\"", "\"cpu_stats2\"").replace("\"precpu_stats\"", "\"cpu_stats\""));
 
@@ -667,15 +666,14 @@ public class NodeAgentImplTest {
         when(dockerOperations.getContainerStats(eq(context)))
                 .thenReturn(Optional.of(stats1))
                 .thenReturn(Optional.of(stats2));
-        
-        nodeAgent.updateContainerNodeMetrics(); // Update metrics once to init and lastCpuMetric
 
-        Path pathToExpectedMetrics = Paths.get(classLoader.getResource("expected.container.system.metrics.txt").getPath());
-        String expectedMetrics = new String(Files.readAllBytes(pathToExpectedMetrics))
-                .replaceAll("\\s", "")
-                .replaceAll("\\n", "");
+        List<String> expectedMetrics = Stream.of(0, 1)
+                .map(i -> Paths.get("src/test/resources/expected.container.system.metrics." + i + ".txt"))
+                .map(path -> uncheck(() -> Files.readString(path)))
+                .map(content -> content.replaceAll("\\s", "").replaceAll("\\n", ""))
+                .collect(Collectors.toList());
+        int[] counter = {0};
 
-        String[] expectedCommand = {"vespa-rpc-invoke",  "-t", "2",  "tcp/localhost:19095",  "setExtraMetrics", expectedMetrics};
         doAnswer(invocation -> {
             NodeAgentContext calledContainerName = (NodeAgentContext) invocation.getArguments()[0];
             long calledTimeout = (long) invocation.getArguments()[1];
@@ -687,11 +685,15 @@ public class NodeAgentImplTest {
 
             assertEquals(context, calledContainerName);
             assertEquals(5L, calledTimeout);
-            assertArrayEquals(expectedCommand, calledCommand);
+            String[] expectedCommand = {"vespa-rpc-invoke", "-t", "2", "tcp/localhost:19095",
+                    "setExtraMetrics", expectedMetrics.get(counter[0])};
+            assertArrayEquals("Ivocation #" + counter[0], expectedCommand, calledCommand);
+            counter[0]++;
             return null;
         }).when(dockerOperations).executeCommandInContainerAsRoot(any(), any(), any());
 
         nodeAgent.updateContainerNodeMetrics();
+        nodeAgent.updateContainerNodeMetrics();
     }
 
     @Test
diff --git a/node-admin/src/test/resources/expected.container.system.metrics.0.txt b/node-admin/src/test/resources/expected.container.system.metrics.0.txt
new file mode 100644
index 00000000000..ea6036ce2ea
--- /dev/null
+++ b/node-admin/src/test/resources/expected.container.system.metrics.0.txt
@@ -0,0 +1,78 @@
+s:
+{
+    "application": "vespa.node",
+    "dimensions": {
+        "host": "host1.test.yahoo.com",
+        "orchestratorState":"ALLOWED_TO_BE_DOWN",
+        "parentHostname": "parent.host.name.yahoo.com",
+        "role": "tenants",
+        "state": "active"
+    },
+    "metrics": {
+        "cpu.vcpus": 2.0,
+        "disk.limit": 250000000000,
+        "disk.used": 39625000000,
+        "disk.util": 15.85,
+        "mem.limit": 4294967296,
+        "mem.used": 1073741824,
+        "mem.util": 25.0,
+        "mem_total.used": 1752707072,
+        "mem_total.util": 40.808
+    },
+    "routing": {
+        "yamas": {
+            "namespaces": ["Vespa"]
+        }
+    },
+    "timestamp": 0
+}
+{
+    "application": "vespa.node",
+    "dimensions": {
+        "host": "host1.test.yahoo.com",
+        "interface": "eth0",
+        "orchestratorState":"ALLOWED_TO_BE_DOWN",
+        "parentHostname": "parent.host.name.yahoo.com",
+        "role": "tenants",
+        "state": "active"
+    },
+    "metrics": {
+        "net.in.bytes": 19499270,
+        "net.in.dropped": 4,
+        "net.in.errors": 55,
+        "net.out.bytes": 20303455,
+        "net.out.dropped": 13,
+        "net.out.errors": 3
+    },
+    "routing": {
+        "yamas": {
+            "namespaces": ["Vespa"]
+        }
+    },
+    "timestamp": 0
+}
+{
+    "application": "vespa.node",
+    "dimensions": {
+        "host": "host1.test.yahoo.com",
+        "interface": "eth1",
+        "orchestratorState":"ALLOWED_TO_BE_DOWN",
+        "parentHostname": "parent.host.name.yahoo.com",
+        "role": "tenants",
+        "state": "active"
+    },
+    "metrics": {
+        "net.in.bytes": 3245766,
+        "net.in.dropped": 0,
+        "net.in.errors": 0,
+        "net.out.bytes": 54246745,
+        "net.out.dropped": 0,
+        "net.out.errors": 0
+    },
+    "routing": {
+        "yamas": {
+            "namespaces": ["Vespa"]
+        }
+    },
+    "timestamp": 0
+}
\ No newline at end of file
diff --git a/node-admin/src/test/resources/expected.container.system.metrics.txt b/node-admin/src/test/resources/expected.container.system.metrics.1.txt
index 54d4d36c7d0..54d4d36c7d0 100644
--- a/node-admin/src/test/resources/expected.container.system.metrics.txt
+++ b/node-admin/src/test/resources/expected.container.system.metrics.1.txt