Test metrics fetch

7 files changed, 219 insertions, 29 deletions

diff --git a/node-repository/src/main/config/node-repository.xml b/node-repository/src/main/config/node-repository.xml
index 27f061d277c..186f052a274 100644
--- a/node-repository/src/main/config/node-repository.xml
+++ b/node-repository/src/main/config/node-repository.xml
@@ -1,7 +1,7 @@
 <!-- services.xml snippet for the node repository. Included in config server services.xml if the package is installed-->
 <!-- Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -->
 <component id="com.yahoo.vespa.hosted.provision.provisioning.InfraDeployerImpl" bundle="node-repository"/>
-<component id="com.yahoo.vespa.hosted.provision.autoscale.NodeMetricsHttpFetcher" bundle="node-repository"/>
+<component id="com.yahoo.vespa.hosted.provision.autoscale.NodeMetricsFetcher" bundle="node-repository"/>
 <component id="com.yahoo.vespa.hosted.provision.autoscale.NodeMetricsDb" bundle="node-repository"/>
 <component id="com.yahoo.vespa.hosted.provision.provisioning.NodeRepositoryProvisioner" bundle="node-repository" />
 <component id="NodeRepository" class="com.yahoo.vespa.hosted.provision.NodeRepository" bundle="node-repository"/>
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 b17ec8a9620..b6475e891b7 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
@@ -46,12 +46,13 @@ public class MetricsResponse {
     private void consumeNodeMetrics(String hostname, Inspector node) {
         long timestamp = node.field("timestamp").asLong();
         Map<String, Double> values = consumeMetrics(node.field("metrics"));
-        addMetricIfPresent(hostname, "cpu.util", timestamp, values);
+        for (Resource resource : Resource.values())
+            addMetricIfPresent(hostname, resource.metricName(), timestamp, values);
     }
 
     private void addMetricIfPresent(String hostname, String metricName, long timestamp, Map<String, Double> values) {
         if (values.containsKey(metricName))
-            metricValues.add(new NodeMetrics.MetricValue(hostname, metricName, timestamp, values.get("cpu.util").floatValue()));
+            metricValues.add(new NodeMetrics.MetricValue(hostname, metricName, timestamp, values.get(metricName).floatValue()));
     }
 
     private void consumeServiceMetrics(String hostname, Inspector node) {
@@ -62,8 +63,12 @@ public class MetricsResponse {
 
     private Map<String, Double> consumeMetrics(Inspector metrics) {
         Map<String, Double> values = new HashMap<>();
-        metrics.field("values").traverse((ObjectTraverser)(name, value) -> values.put(name, value.asDouble()));
+        metrics.traverse((ArrayTraverser) (__, item) -> consumeMetricsItem(item, values));
         return values;
     }
 
+    private void consumeMetricsItem(Inspector item, Map<String, Double> values) {
+        item.field("values").traverse((ObjectTraverser)(name, value) -> values.put(name, value.asDouble()));
+    }
+
 }
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 38a2194319a..54a8292eab6 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
@@ -38,6 +38,11 @@ public interface NodeMetrics {
         public long timestamp() { return timestamp; }
         public float value() { return value; }
 
+        @Override
+        public String toString() {
+            return "metric value " + name + ": " + value + " at " + timestamp + " for " + hostname;
+        }
+
     }
 
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsHttpFetcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsFetcher.java
index 5ac96a94a08..3feb3cbc83f 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsHttpFetcher.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsFetcher.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.provision.autoscale;
 
 import ai.vespa.util.http.VespaHttpClientBuilder;
+import com.google.inject.Inject;
 import com.yahoo.component.AbstractComponent;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.vespa.applicationmodel.HostName;
@@ -24,48 +25,43 @@ import java.util.logging.Logger;
  *
  * @author bratseth
  */
-public class NodeMetricsHttpFetcher extends AbstractComponent implements NodeMetrics {
+public class NodeMetricsFetcher extends AbstractComponent implements NodeMetrics {
 
-    private static final Logger log = Logger.getLogger(NodeMetricsHttpFetcher.class.getName());
+    private static final Logger log = Logger.getLogger(NodeMetricsFetcher.class.getName());
 
     private static final String apiPath = "/metrics/v2/values";
 
     private final NodeRepository nodeRepository;
     private final Orchestrator orchestrator;
-    private final CloseableHttpClient httpClient;
+    private final HttpClient httpClient;
 
-    public NodeMetricsHttpFetcher(NodeRepository nodeRepository, Orchestrator orchestrator) {
+    @Inject
+    public NodeMetricsFetcher(NodeRepository nodeRepository, Orchestrator orchestrator) {
+        this(nodeRepository, orchestrator, new ApacheHttpClient());
+    }
+
+    NodeMetricsFetcher(NodeRepository nodeRepository, Orchestrator orchestrator, HttpClient httpClient) {
         this.nodeRepository = nodeRepository;
         this.orchestrator = orchestrator;
-        httpClient = VespaHttpClientBuilder.createWithBasicConnectionManager().build();
+        this.httpClient = httpClient;
     }
 
     @Override
     public Collection<MetricValue> fetchMetrics(ApplicationId application) {
         Node metricsV2Container = nodeRepository.list()
+                                                .owner(application)
                                                 .state(Node.State.active)
                                                 .container()
                                                 .filter(node -> expectedUp(node))
                                                 .asList().get(0);
         String url = "https://" + metricsV2Container.hostname() + ":" + 4443 + apiPath + "?consumer=vespa-consumer-metrics";
-
-        try {
-            String response = httpClient.execute(new HttpGet(url), new BasicResponseHandler());
-            return new MetricsResponse(response).metrics();
-        }
-        catch (IOException e) {
-            throw new UncheckedIOException("Failed to get metrics on " + url, e);
-        }
+        String response = httpClient.get(url);
+        return new MetricsResponse(response).metrics();
     }
 
     @Override
     public void deconstruct() {
-        try {
-            httpClient.close();
-        }
-        catch (IOException e) {
-            log.log(Level.WARNING, "Exception deconstructing", e);
-        }
+        httpClient.close();
     }
 
     private boolean expectedUp(Node node) {
@@ -77,4 +73,40 @@ public class NodeMetricsHttpFetcher extends AbstractComponent implements NodeMet
         }
     }
 
+    /** The simplest possible http client interface */
+    public interface HttpClient {
+
+        String get(String url);
+        void close();
+
+    }
+
+    /** Implements the HttpClient interface by delegating to an Apache HTTP client */
+    public static class ApacheHttpClient implements HttpClient {
+
+        private final CloseableHttpClient httpClient = VespaHttpClientBuilder.createWithBasicConnectionManager().build();
+
+        @Override
+        public String get(String url) {
+            try {
+                return httpClient.execute(new HttpGet(url), new BasicResponseHandler());
+            }
+            catch (IOException e) {
+                throw new UncheckedIOException("Could not get " + url, e);
+            }
+        }
+
+        @Override
+        public void close() {
+            try {
+                httpClient.close();
+            }
+            catch (IOException e) {
+                log.log(Level.WARNING, "Exception deconstructing", e);
+            }
+        }
+
+
+    }
+
 }
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Resource.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Resource.java
index e4497f7d34a..1172856cd56 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Resource.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Resource.java
@@ -11,24 +11,24 @@ import com.yahoo.config.provision.NodeResources;
 public enum Resource {
 
     cpu {
-        String metric() { return "cpu.util"; }
+        String metricName() { return "cpu.util"; }
         double idealAverageLoad() { return 0.2; }
         double valueFrom(NodeResources resources) { return resources.vcpu(); }
     },
 
     memory {
-        String metric() { return "memory.util"; }
+        String metricName() { return "memory.util"; }
         double idealAverageLoad() { return 0.7; }
         double valueFrom(NodeResources resources) { return resources.memoryGb(); }
     },
 
     disk {
-        String metric() { return "disk.util"; }
+        String metricName() { return "disk.util"; }
         double idealAverageLoad() { return 0.7; }
         double valueFrom(NodeResources resources) { return resources.diskGb(); }
     };
 
-    abstract String metric();
+    abstract String metricName();
 
     /** The load we should have of this resource on average, when one node in the cluster is down */
     abstract double idealAverageLoad();
@@ -37,7 +37,7 @@ public enum Resource {
 
     public static Resource fromMetric(String metricName) {
         for (Resource resource : values())
-            if (resource.metric().equals(metricName)) return resource;
+            if (resource.metricName().equals(metricName)) return resource;
         throw new IllegalArgumentException("Metric '" + metricName + "' does not map to a resource");
     }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java
index fdcba7ba565..ed4c131a5a5 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java
@@ -134,7 +134,7 @@ class AutoscalingTester {
                     float effectiveValue = (r == resource ? value : (float) r.idealAverageLoad() * otherResourcesLoad)
                                            * oneExtraNodeFactor;
                     db.add(List.of(new NodeMetrics.MetricValue(node.hostname(),
-                                                               r.metric(),
+                                                               r.metricName(),
                                                                clock().instant().toEpochMilli(),
                                                                effectiveValue)));
                 }
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
new file mode 100644
index 00000000000..1964160192b
--- /dev/null
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsFetcherTest.java
@@ -0,0 +1,148 @@
+// Copyright 2019 Oath Inc. 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.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.Capacity;
+import com.yahoo.config.provision.NodeResources;
+import com.yahoo.vespa.hosted.provision.Node;
+import com.yahoo.vespa.hosted.provision.provisioning.ProvisioningTester;
+import com.yahoo.vespa.hosted.provision.testutils.OrchestratorMock;
+import com.yahoo.vespa.applicationmodel.HostName;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+
+public class NodeMetricsFetcherTest {
+
+    @Test
+    public void testMetricsFetch() {
+        NodeResources resources = new NodeResources(1, 10, 100, 1);
+        ProvisioningTester tester = new ProvisioningTester.Builder().build();
+        OrchestratorMock orchestrator = new OrchestratorMock();
+        MockHttpClient httpClient = new MockHttpClient();
+        NodeMetricsFetcher fetcher = new NodeMetricsFetcher(tester.nodeRepository(), orchestrator, httpClient);
+
+        tester.makeReadyNodes(4, resources); // Creates (in order) host-1.yahoo.com, host-2.yahoo.com, host-3.yahoo.com, host-4.yahoo.com
+        tester.deployZoneApp();
+
+        ApplicationId application1 = tester.makeApplicationId();
+        ApplicationId application2 = tester.makeApplicationId();
+        tester.deploy(application1, Capacity.fromCount(2, resources)); // host-1.yahoo.com, host-2.yahoo.com
+        tester.deploy(application2, Capacity.fromCount(2, resources)); // host-4.yahoo.com, host-3.yahoo.com
+
+        orchestrator.suspend(new HostName("host-4.yahoo.com"));
+
+        {
+            httpClient.cannedResponse = cannedResponseForApplication1;
+            List<NodeMetrics.MetricValue> values = new ArrayList<>(fetcher.fetchMetrics(application1));
+            assertEquals("https://host-1.yahoo.com:4443/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 memory.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());
+        }
+
+        {
+            httpClient.cannedResponse = cannedResponseForApplication2;
+            List<NodeMetrics.MetricValue> values = new ArrayList<>(fetcher.fetchMetrics(application2));
+            assertEquals("https://host-3.yahoo.com:4443/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 memory.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());
+        }
+    }
+
+    private static class MockHttpClient implements NodeMetricsFetcher.HttpClient {
+
+        List<String> requestsReceived = new ArrayList<>();
+
+        String cannedResponse = null;
+        @Override
+        public String get(String url) {
+            requestsReceived.add(url);
+            return cannedResponse;
+        }
+
+        @Override
+        public void close() { }
+
+    }
+
+    final String cannedResponseForApplication1 =
+            "{\n" +
+            "  \"nodes\": [\n" +
+            "    {\n" +
+            "      \"hostname\": \"host-1.yahoo.com\",\n" +
+            "      \"role\": \"role0\",\n" +
+            "      \"node\": {\n" +
+            "        \"timestamp\": 1234,\n" +
+            "        \"metrics\": [\n" +
+            "          {\n" +
+            "            \"values\": {\n" +
+            "              \"cpu.util\": 16.2,\n" +
+            "              \"memory.util\": 23.1,\n" +
+            "              \"disk.util\": 82\n" +
+            "            },\n" +
+            "            \"dimensions\": {\n" +
+            "              \"state\": \"active\"\n" +
+            "            }\n" +
+            "          }\n" +
+            "        ]\n" +
+            "      }\n" +
+            "    },\n" +
+            "    {\n" +
+            "      \"hostname\": \"host-2.yahoo.com\",\n" +
+            "      \"role\": \"role1\",\n" +
+            "      \"node\": {\n" +
+            "        \"timestamp\": 1200,\n" +
+            "        \"metrics\": [\n" +
+            "          {\n" +
+            "            \"values\": {\n" +
+            "              \"cpu.util\": 20,\n" +
+            "              \"disk.util\": 40\n" +
+            "            },\n" +
+            "            \"dimensions\": {\n" +
+            "              \"state\": \"active\"\n" +
+            "            }\n" +
+            "          }\n" +
+            "        ]\n" +
+            "      }\n" +
+            "    }\n" +
+            "  ]\n" +
+            "}\n";
+
+
+    final String cannedResponseForApplication2 =
+            "{\n" +
+            "  \"nodes\": [\n" +
+            "    {\n" +
+            "      \"hostname\": \"host-3.yahoo.com\",\n" +
+            "      \"role\": \"role0\",\n" +
+            "      \"node\": {\n" +
+            "        \"timestamp\": 1300,\n" +
+            "        \"metrics\": [\n" +
+            "          {\n" +
+            "            \"values\": {\n" +
+            "              \"cpu.util\": 10,\n" +
+            "              \"memory.util\": 15,\n" +
+            "              \"disk.util\": 20\n" +
+            "            },\n" +
+            "            \"dimensions\": {\n" +
+            "              \"state\": \"active\"\n" +
+            "            }\n" +
+            "          }\n" +
+            "        ]\n" +
+            "      }\n" +
+            "    }\n" +
+            "  ]\n" +
+            "}\n";
+
+}