Don't use MetricsProxyContainer for port number. Synchronize methods. Refactoring

8 files changed, 351 insertions, 359 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
index db2c737a8f3..aa399640708 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
@@ -44,7 +44,7 @@ import com.yahoo.vespa.config.server.http.LogRetriever;
 import com.yahoo.vespa.config.server.http.SimpleHttpFetcher;
 import com.yahoo.vespa.config.server.http.v2.MetricsResponse;
 import com.yahoo.vespa.config.server.http.v2.PrepareResult;
-import com.yahoo.vespa.config.server.metrics.ClusterMetricsRetriever;
+import com.yahoo.vespa.config.server.metrics.ApplicationMetricsRetriever;
 import com.yahoo.vespa.config.server.provision.HostProvisionerProvider;
 import com.yahoo.vespa.config.server.session.LocalSession;
 import com.yahoo.vespa.config.server.session.LocalSessionRepo;
@@ -644,8 +644,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
 
     public MetricsResponse getMetrics(ApplicationId applicationId) {
         Application application = getApplication(applicationId);
-        ClusterMetricsRetriever clusterMetricsRetriever = new ClusterMetricsRetriever();
-        return clusterMetricsRetriever.getMetrics(application);
+        ApplicationMetricsRetriever applicationMetricsRetriever = new ApplicationMetricsRetriever();
+        return applicationMetricsRetriever.getMetrics(application);
     }
 
     // ---------------- Misc operations ----------------------------------------------------------------
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ApplicationMetricsRetriever.java b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ApplicationMetricsRetriever.java
new file mode 100644
index 00000000000..c6daaaae2f5
--- /dev/null
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ApplicationMetricsRetriever.java
@@ -0,0 +1,55 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.config.server.metrics;
+
+import com.yahoo.config.model.api.HostInfo;
+import com.yahoo.config.model.api.ServiceInfo;
+import com.yahoo.vespa.config.server.application.Application;
+import com.yahoo.vespa.config.server.http.v2.MetricsResponse;
+
+import java.net.URI;
+import java.util.Collection;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+
+/**
+ * Finds all hosts we want to fetch metrics for, generates the appropriate URIs
+ * and returns the generated MetricsResponse.
+ *
+ * @author olaa
+ */
+public class ApplicationMetricsRetriever {
+
+    private final ClusterMetricsRetriever metricsRetriever;
+
+    public ApplicationMetricsRetriever() {
+        this(new ClusterMetricsRetriever());
+    }
+
+    public ApplicationMetricsRetriever(ClusterMetricsRetriever metricsRetriever) {
+        this.metricsRetriever = metricsRetriever;
+    }
+
+    public MetricsResponse getMetrics(Application application) {
+        var hosts = getHostsOfApplication(application);
+        var clusterMetrics = metricsRetriever.requestMetricsGroupedByCluster(hosts);
+        return new MetricsResponse(200, application.getId(), clusterMetrics);
+    }
+
+    private static Collection<URI> getHostsOfApplication(Application application) {
+        return application.getModel().getHosts().stream()
+                .filter(host -> host.getServices().stream().noneMatch(isLogserver()))
+                .map(HostInfo::getHostname)
+                .map(ApplicationMetricsRetriever::createMetricsProxyURI)
+                .collect(Collectors.toList());
+
+    }
+
+    private static Predicate<ServiceInfo> isLogserver() {
+        return serviceInfo -> serviceInfo.getServiceType().equalsIgnoreCase("logserver");
+    }
+
+    private static URI createMetricsProxyURI(String hostname) {
+        return URI.create("http://" + hostname + ":19092/metrics/v1/values?consumer=Vespa");
+    }
+
+}
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetriever.java b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetriever.java
index 1fb45c65df9..913aed447c7 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetriever.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetriever.java
@@ -1,56 +1,130 @@
 // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server.metrics;
 
-import com.yahoo.config.model.api.HostInfo;
-import com.yahoo.config.model.api.ServiceInfo;
-import com.yahoo.vespa.config.server.application.Application;
-import com.yahoo.vespa.config.server.http.v2.MetricsResponse;
-import com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainer;
+import com.yahoo.slime.ArrayTraverser;
+import com.yahoo.slime.Inspector;
+import com.yahoo.slime.Slime;
+import com.yahoo.vespa.config.SlimeUtils;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.client.HttpClientBuilder;
 
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
 import java.net.URI;
 import java.util.Collection;
-import java.util.function.Predicate;
-import java.util.stream.Collectors;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.TimeUnit;
+import java.util.logging.Logger;
+
 
 /**
- * Finds all hosts we want to fetch metrics for, generates the appropriate URIs
- * and returns the metrics grouped by cluster.
+ * Client for reaching out to nodes in an application instance and get their
+ * metrics.
  *
  * @author olaa
+ * @author ogronnesby
  */
 public class ClusterMetricsRetriever {
 
-    private final MetricsRetriever metricsRetriever;
+    private static final Logger log = Logger.getLogger(ClusterMetricsRetriever.class.getName());
 
-    public ClusterMetricsRetriever() {
-        this(new MetricsRetriever());
-    }
+    private static final String VESPA_CONTAINER = "vespa.container";
+    private static final String VESPA_QRSERVER = "vespa.qrserver";
+    private static final String VESPA_DISTRIBUTOR = "vespa.distributor";
+    private static final List<String> WANTED_METRIC_SERVICES = List.of(VESPA_CONTAINER, VESPA_QRSERVER, VESPA_DISTRIBUTOR);
 
-    public ClusterMetricsRetriever(MetricsRetriever metricsRetriever) {
-        this.metricsRetriever = metricsRetriever;
-    }
+    /**
+     * Call the metrics API on each host and aggregate the metrics
+     * into a single value, grouped by cluster.
+     */
+    public Map<ClusterInfo, MetricsAggregator> requestMetricsGroupedByCluster(Collection<URI> hosts) {
+        Map<ClusterInfo, MetricsAggregator> clusterMetricsMap = new ConcurrentHashMap<>();
+
+        Runnable retrieveMetricsJob = () ->
+                hosts.parallelStream().forEach(host ->
+                    getHostMetrics(host, clusterMetricsMap)
+                );
+
+        ForkJoinPool threadPool = new ForkJoinPool(5);
+        threadPool.submit(retrieveMetricsJob);
+        threadPool.shutdown();
 
-    public MetricsResponse getMetrics(Application application) {
-        var hosts = getHostsOfApplication(application);
-        var clusterMetrics = metricsRetriever.requestMetricsGroupedByCluster(hosts);
-        return new MetricsResponse(200, application.getId(), clusterMetrics);
+        try {
+            threadPool.awaitTermination(1, TimeUnit.MINUTES);
+        } catch (InterruptedException e) {
+            throw new RuntimeException(e);
+        }
+        return clusterMetricsMap;
     }
 
-    private static Collection<URI> getHostsOfApplication(Application application) {
-        return application.getModel().getHosts().stream()
-                .filter(host -> host.getServices().stream().noneMatch(isLogserver()))
-                .map(HostInfo::getHostname)
-                .map(ClusterMetricsRetriever::createMetricsProxyURI)
-                .collect(Collectors.toList());
+    private static void getHostMetrics(URI hostURI, Map<ClusterInfo, MetricsAggregator> clusterMetricsMap) {
+            Slime responseBody = doMetricsRequest(hostURI);
+            var parseError = responseBody.get().field("error_message");
 
+            if (parseError.valid()) {
+                log.info("Failed to retrieve metrics from " + hostURI + ": " + parseError.asString());
+            }
+
+            Inspector services = responseBody.get().field("services");
+            services.traverse((ArrayTraverser) (i, servicesInspector) ->
+                parseService(servicesInspector, clusterMetricsMap)
+            );
     }
 
-    private static Predicate<ServiceInfo> isLogserver() {
-        return serviceInfo -> serviceInfo.getServiceType().equalsIgnoreCase("logserver");
+    private static Slime doMetricsRequest(URI hostURI) {
+        HttpGet get = new HttpGet(hostURI);
+        try {
+            HttpClient httpClient = HttpClientBuilder.create().build();
+            HttpResponse response = httpClient.execute(get);
+            InputStream is = response.getEntity().getContent();
+            Slime slime = SlimeUtils.jsonToSlime(is.readAllBytes());
+            is.close();
+            return slime;
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
     }
 
-    private static URI createMetricsProxyURI(String hostname) {
-        return URI.create("http://" + hostname + ":" + MetricsProxyContainer.BASEPORT + "/metrics/v1/values?consumer=Vespa");
+    private static void parseService(Inspector service, Map<ClusterInfo, MetricsAggregator> clusterMetricsMap) {
+        String serviceName = service.field("name").asString();
+        service.field("metrics").traverse((ArrayTraverser) (i, metric) ->
+                addMetricsToAggeregator(serviceName, metric, clusterMetricsMap)
+        );
     }
 
+    private static void addMetricsToAggeregator(String serviceName, Inspector metric, Map<ClusterInfo, MetricsAggregator> clusterMetricsMap) {
+        if (!WANTED_METRIC_SERVICES.contains(serviceName)) return;
+        Inspector values = metric.field("values");
+        ClusterInfo clusterInfo = getClusterInfoFromDimensions(metric.field("dimensions"));
+        MetricsAggregator metricsAggregator = clusterMetricsMap.computeIfAbsent(clusterInfo, c -> new MetricsAggregator());
+
+        switch (serviceName) {
+            case "vespa.container":
+                metricsAggregator.addContainerLatency(
+                        values.field("query_latency.sum").asDouble(),
+                        values.field("query_latency.count").asDouble());
+                metricsAggregator.addFeedLatency(
+                        values.field("feed_latency.sum").asDouble(),
+                        values.field("feed_latency.count").asDouble());
+                break;
+            case "vespa.qrserver":
+                metricsAggregator.addQrLatency(
+                        values.field("query_latency.sum").asDouble(),
+                        values.field("query_latency.count").asDouble());
+                break;
+            case "vespa.distributor":
+                metricsAggregator.addDocumentCount(values.field("vds.distributor.docsstored.average").asDouble());
+                break;
+        }
+    }
+
+    private static ClusterInfo getClusterInfoFromDimensions(Inspector dimensions) {
+        return new ClusterInfo(dimensions.field("clusterid").asString(), dimensions.field("clustertype").asString());
+    }
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsAggregator.java b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsAggregator.java
index 9934c074b15..9ecec471b07 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsAggregator.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsAggregator.java
@@ -1,7 +1,6 @@
 // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server.metrics;
 
-import java.time.Instant;
 import java.util.Optional;
 
 /**
@@ -15,22 +14,22 @@ public class MetricsAggregator {
     private LatencyMetrics container;
     private Double documentCount;
 
-    public MetricsAggregator addFeedLatency(double sum, double count) {
+    public synchronized MetricsAggregator addFeedLatency(double sum, double count) {
         this.feed = combineLatency(this.feed, sum, count);
         return this;
     }
 
-    public MetricsAggregator addQrLatency(double sum, double count) {
+    public synchronized MetricsAggregator addQrLatency(double sum, double count) {
         this.qr = combineLatency(this.qr, sum, count);
         return this;
     }
 
-    public MetricsAggregator addContainerLatency(double sum, double count) {
+    public synchronized MetricsAggregator addContainerLatency(double sum, double count) {
         this.container = combineLatency(this.container, sum, count);
         return this;
     }
 
-    public MetricsAggregator addDocumentCount(double count) {
+    public synchronized MetricsAggregator addDocumentCount(double count) {
         this.documentCount = (this.documentCount == null ? 0.0 : this.documentCount) + count;
         return this;
     }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsRetriever.java b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsRetriever.java
deleted file mode 100644
index b58afc727d6..00000000000
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsRetriever.java
+++ /dev/null
@@ -1,133 +0,0 @@
-// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.config.server.metrics;
-
-import com.yahoo.slime.ArrayTraverser;
-import com.yahoo.slime.Inspector;
-import com.yahoo.slime.Slime;
-import com.yahoo.vespa.config.SlimeUtils;
-import org.apache.http.HttpResponse;
-import org.apache.http.client.HttpClient;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.impl.client.HttpClientBuilder;
-
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.UncheckedIOException;
-import java.net.URI;
-import java.time.Instant;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
-import java.util.concurrent.ForkJoinPool;
-import java.util.concurrent.TimeUnit;
-import java.util.logging.Logger;
-
-
-/**
- * Client for reaching out to nodes in an application instance and get their
- * metrics.
- *
- * @author olaa
- * @author ogronnesby
- */
-public class MetricsRetriever {
-    private static final Logger log = Logger.getLogger(MetricsRetriever.class.getName());
-
-    private static final String VESPA_CONTAINER = "vespa.container";
-    private static final String VESPA_QRSERVER = "vespa.qrserver";
-    private static final String VESPA_DISTRIBUTOR = "vespa.distributor";
-    private static final List<String> WANTED_METRIC_SERVICES = List.of(VESPA_CONTAINER, VESPA_QRSERVER, VESPA_DISTRIBUTOR);
-
-    /**
-     * Call the metrics API on each host and aggregate the metrics
-     * into a single value, grouped by cluster.
-     */
-    public Map<ClusterInfo, MetricsAggregator> requestMetricsGroupedByCluster(Collection<URI> hosts) {
-        Map<ClusterInfo, MetricsAggregator> clusterMetricsMap = new ConcurrentHashMap<>();
-
-        Runnable retrieveMetricsJob = () ->
-                hosts.parallelStream().forEach(host ->
-                    getHostMetrics(host, clusterMetricsMap)
-                );
-
-        ForkJoinPool threadPool = new ForkJoinPool(5);
-        threadPool.submit(retrieveMetricsJob);
-        threadPool.shutdown();
-
-        try {
-            threadPool.awaitTermination(1, TimeUnit.MINUTES);
-        } catch (InterruptedException e) {
-            throw new RuntimeException(e);
-        }
-        return clusterMetricsMap;
-    }
-
-    private static void getHostMetrics(URI hostURI, Map<ClusterInfo, MetricsAggregator> clusterMetricsMap) {
-            Slime responseBody = doMetricsRequest(hostURI);
-            var parseError = responseBody.get().field("error_message");
-
-            if (parseError.valid()) {
-                log.info("Failed to retrieve metrics from " + hostURI + ": " + parseError.asString());
-            }
-
-            Inspector services = responseBody.get().field("services");
-            services.traverse((ArrayTraverser) (i, servicesInspector) ->
-                parseService(servicesInspector, clusterMetricsMap)
-            );
-    }
-
-    private static Slime doMetricsRequest(URI hostURI) {
-        HttpGet get = new HttpGet(hostURI);
-        try {
-            HttpClient httpClient = HttpClientBuilder.create().build();
-            HttpResponse response = httpClient.execute(get);
-            InputStream is = response.getEntity().getContent();
-            Slime slime = SlimeUtils.jsonToSlime(is.readAllBytes());
-            is.close();
-            return slime;
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
-        }
-    }
-
-    private static void parseService(Inspector service, Map<ClusterInfo, MetricsAggregator> clusterMetricsMap) {
-        String serviceName = service.field("name").asString();
-        service.field("metrics").traverse((ArrayTraverser) (i, metric) ->
-                addMetricsToAggeregator(serviceName, metric, clusterMetricsMap)
-        );
-    }
-
-    private static void addMetricsToAggeregator(String serviceName, Inspector metric, Map<ClusterInfo, MetricsAggregator> clusterMetricsMap) {
-        if (!WANTED_METRIC_SERVICES.contains(serviceName)) return;
-        Inspector values = metric.field("values");
-        ClusterInfo clusterInfo = getClusterInfoFromDimensions(metric.field("dimensions"));
-        MetricsAggregator metricsAggregator = clusterMetricsMap.computeIfAbsent(clusterInfo, c -> new MetricsAggregator());
-
-        switch (serviceName) {
-            case "vespa.container":
-                metricsAggregator.addContainerLatency(
-                        values.field("query_latency.sum").asDouble(),
-                        values.field("query_latency.count").asDouble());
-                metricsAggregator.addFeedLatency(
-                        values.field("feed_latency.sum").asDouble(),
-                        values.field("feed_latency.count").asDouble());
-                break;
-            case "vespa.qrserver":
-                metricsAggregator.addQrLatency(
-                        values.field("query_latency.sum").asDouble(),
-                        values.field("query_latency.count").asDouble());
-                break;
-            case "vespa.distributor":
-                metricsAggregator.addDocumentCount(values.field("vds.distributor.docsstored.average").asDouble());
-                break;
-        }
-    }
-
-    private static ClusterInfo getClusterInfoFromDimensions(Inspector dimensions) {
-        return new ClusterInfo(dimensions.field("clusterid").asString(), dimensions.field("clustertype").asString());
-    }
-}
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ApplicationMetricsRetrieverTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ApplicationMetricsRetrieverTest.java
new file mode 100644
index 00000000000..dd86eb4fd0d
--- /dev/null
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ApplicationMetricsRetrieverTest.java
@@ -0,0 +1,117 @@
+package com.yahoo.vespa.config.server.metrics;
+
+import com.yahoo.config.FileReference;
+import com.yahoo.config.model.api.FileDistribution;
+import com.yahoo.config.model.api.HostInfo;
+import com.yahoo.config.model.api.Model;
+import com.yahoo.config.model.api.ServiceInfo;
+import com.yahoo.config.provision.AllocatedHosts;
+import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.vespa.config.ConfigKey;
+import com.yahoo.vespa.config.ConfigPayload;
+import com.yahoo.vespa.config.buildergen.ConfigDefinition;
+import com.yahoo.vespa.config.server.application.Application;
+import org.junit.Test;
+
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.junit.Assert.*;
+
+/**
+ * @author olaa
+ */
+public class ApplicationMetricsRetrieverTest {
+
+    @Test
+    public void getMetrics()  {
+        MockModel mockModel = new MockModel(mockHosts());
+        MockMetricsRetriever mockMetricsRetriever = new MockMetricsRetriever();
+        Application application = new Application(mockModel, null, 0, false,
+        null, null, ApplicationId.fromSerializedForm("tenant:app:instance"));
+
+        ApplicationMetricsRetriever clusterMetricsRetriever = new ApplicationMetricsRetriever(mockMetricsRetriever);
+        clusterMetricsRetriever.getMetrics(application);
+
+        assertEquals(2, mockMetricsRetriever.hosts.size()); // Verify that logserver was ignored
+    }
+
+    private Collection<HostInfo> mockHosts() {
+
+        HostInfo hostInfo1 = new HostInfo("host1",
+                List.of(new ServiceInfo("content", "searchnode", null, null, "", "host1"))
+        );
+        HostInfo hostInfo2 = new HostInfo("host2",
+                List.of(new ServiceInfo("default", "container", null, null, "", "host2"))
+        );
+        HostInfo hostInfo3 = new HostInfo("host3",
+                List.of(new ServiceInfo("default", "logserver",  null, null, "", "host3"))
+        );
+
+        return List.of(hostInfo1, hostInfo2, hostInfo3);
+    }
+
+    class MockMetricsRetriever extends ClusterMetricsRetriever {
+
+        Collection<URI> hosts = new ArrayList<>();
+
+        @Override
+        public Map<ClusterInfo, MetricsAggregator> requestMetricsGroupedByCluster(Collection<URI> hosts) {
+            this.hosts = hosts;
+
+            return Map.of(
+                    new ClusterInfo("content_cluster_id", "content"),
+                    new MetricsAggregator().addDocumentCount(1000),
+                    new ClusterInfo("container_cluster_id", "container"),
+                    new MetricsAggregator().addContainerLatency(123, 5)
+            );
+        }
+    }
+
+    class MockModel implements Model {
+
+        Collection<HostInfo> hosts;
+
+        MockModel(Collection<HostInfo> hosts) {
+            this.hosts = hosts;
+        }
+
+        @Override
+        public ConfigPayload getConfig(ConfigKey<?> configKey, ConfigDefinition targetDef) {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public Set<ConfigKey<?>> allConfigsProduced() {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public Collection<HostInfo> getHosts() {
+            return hosts;
+        }
+
+        @Override
+        public Set<String> allConfigIds() {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public void distributeFiles(FileDistribution fileDistribution) {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public Set<FileReference> fileReferences() { return new HashSet<>(); }
+
+        @Override
+        public AllocatedHosts allocatedHosts() {
+            throw new UnsupportedOperationException();
+        }
+    }
+}
\ No newline at end of file
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetrieverTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetrieverTest.java
index 4d9a20ef6d8..da676663d3b 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetrieverTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetrieverTest.java
@@ -1,117 +1,100 @@
 package com.yahoo.vespa.config.server.metrics;
 
-import com.yahoo.config.FileReference;
-import com.yahoo.config.model.api.FileDistribution;
-import com.yahoo.config.model.api.HostInfo;
-import com.yahoo.config.model.api.Model;
-import com.yahoo.config.model.api.ServiceInfo;
-import com.yahoo.config.provision.AllocatedHosts;
-import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.vespa.config.ConfigKey;
-import com.yahoo.vespa.config.ConfigPayload;
-import com.yahoo.vespa.config.buildergen.ConfigDefinition;
-import com.yahoo.vespa.config.server.application.Application;
+import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import junit.framework.AssertionFailedError;
+import org.junit.Rule;
 import org.junit.Test;
 
+import java.io.IOException;
 import java.net.URI;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashSet;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
-
+import java.util.Optional;
+import java.util.function.BiConsumer;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options;
 import static org.junit.Assert.*;
 
+
 /**
  * @author olaa
  */
 public class ClusterMetricsRetrieverTest {
 
+    @Rule
+    public final WireMockRule wireMock = new WireMockRule(options().port(8080), true);
+
     @Test
-    public void getMetrics()  {
-        MockModel mockModel = new MockModel(mockHosts());
-        MockMetricsRetriever mockMetricsRetriever = new MockMetricsRetriever();
-        Application application = new Application(mockModel, null, 0, false,
-        null, null, ApplicationId.fromSerializedForm("tenant:app:instance"));
+    public void testMetricAggregation() throws IOException {
+        List<URI> hosts = List.of(URI.create("http://localhost:8080/1"), URI.create("http://localhost:8080/2"), URI.create("http://localhost:8080/3"));
 
-        ClusterMetricsRetriever clusterMetricsRetriever = new ClusterMetricsRetriever(mockMetricsRetriever);
-        clusterMetricsRetriever.getMetrics(application);
+        stubFor(get(urlEqualTo("/1"))
+                .willReturn(aResponse()
+                        .withStatus(200)
+                        .withBody(contentMetrics())));
 
-        assertEquals(2, mockMetricsRetriever.hosts.size()); // Verify that logserver was ignored
-    }
+        stubFor(get(urlEqualTo("/2"))
+                .willReturn(aResponse()
+                        .withStatus(200)
+                        .withBody(contentMetrics())));
 
-    private Collection<HostInfo> mockHosts() {
+        stubFor(get(urlEqualTo("/3"))
+                .willReturn(aResponse()
+                        .withStatus(200)
+                        .withBody(containerMetrics())));
 
-        HostInfo hostInfo1 = new HostInfo("host1",
-                List.of(new ServiceInfo("content", "searchnode", null, null, "", "host1"))
-        );
-        HostInfo hostInfo2 = new HostInfo("host2",
-                List.of(new ServiceInfo("default", "container", null, null, "", "host2"))
-        );
-        HostInfo hostInfo3 = new HostInfo("host3",
-                List.of(new ServiceInfo("default", "logserver",  null, null, "", "host3"))
-        );
-
-        return List.of(hostInfo1, hostInfo2, hostInfo3);
-    }
+        ClusterInfo expectedContentCluster = new ClusterInfo("content_cluster_id", "content");
+        ClusterInfo expectedContainerCluster = new ClusterInfo("container_cluster_id", "container");
 
-    class MockMetricsRetriever extends MetricsRetriever {
+        Map<ClusterInfo, MetricsAggregator> aggregatorMap = new ClusterMetricsRetriever().requestMetricsGroupedByCluster(hosts);
 
-        Collection<URI> hosts = new ArrayList<>();
+        compareAggregators(
+                new MetricsAggregator().addDocumentCount(6000.0),
+                aggregatorMap.get(expectedContentCluster)
+        );
 
-        @Override
-        public Map<ClusterInfo, MetricsAggregator> requestMetricsGroupedByCluster(Collection<URI> hosts) {
-            this.hosts = hosts;
+        compareAggregators(
+                new MetricsAggregator()
+                        .addContainerLatency(3000, 43)
+                        .addContainerLatency(2000, 0)
+                        .addQrLatency(3000, 43)
+                        .addFeedLatency(3000, 43),
+                aggregatorMap.get(expectedContainerCluster)
 
-            return Map.of(
-                    new ClusterInfo("content_cluster_id", "content"),
-                    new MetricsAggregator().addDocumentCount(1000),
-                    new ClusterInfo("container_cluster_id", "container"),
-                    new MetricsAggregator().addContainerLatency(123, 5)
-            );
-        }
+        );
+        wireMock.stop();
     }
 
-    class MockModel implements Model {
-
-        Collection<HostInfo> hosts;
-
-        MockModel(Collection<HostInfo> hosts) {
-            this.hosts = hosts;
-        }
-
-        @Override
-        public ConfigPayload getConfig(ConfigKey<?> configKey, ConfigDefinition targetDef) {
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public Set<ConfigKey<?>> allConfigsProduced() {
-            throw new UnsupportedOperationException();
-        }
+    private String containerMetrics() throws IOException {
+        return Files.readString(Path.of("src/test/resources/metrics/container_metrics"));
+    }
 
-        @Override
-        public Collection<HostInfo> getHosts() {
-            return hosts;
-        }
+    private String contentMetrics() throws IOException {
+        return Files.readString(Path.of("src/test/resources/metrics/content_metrics"));
+    }
 
-        @Override
-        public Set<String> allConfigIds() {
-            throw new UnsupportedOperationException();
-        }
+    // Same tolerance value as used internally in MetricsAggregator.isZero
+    private static final double metricsTolerance = 0.001;
 
-        @Override
-        public void distributeFiles(FileDistribution fileDistribution) {
-            throw new UnsupportedOperationException();
-        }
+    private void compareAggregators(MetricsAggregator expected, MetricsAggregator actual) {
+        BiConsumer<Double, Double> assertDoubles = (a, b) -> assertEquals(a.doubleValue(), b.doubleValue(), metricsTolerance);
 
-        @Override
-        public Set<FileReference> fileReferences() { return new HashSet<>(); }
+        compareOptionals(expected.aggregateDocumentCount(), actual.aggregateDocumentCount(), assertDoubles);
+        compareOptionals(expected.aggregateQueryRate(), actual.aggregateQueryRate(), assertDoubles);
+        compareOptionals(expected.aggregateFeedRate(), actual.aggregateFeedRate(), assertDoubles);
+        compareOptionals(expected.aggregateQueryLatency(), actual.aggregateQueryLatency(), assertDoubles);
+        compareOptionals(expected.aggregateFeedLatency(), actual.aggregateFeedLatency(), assertDoubles);
+    }
 
-        @Override
-        public AllocatedHosts allocatedHosts() {
-            throw new UnsupportedOperationException();
-        }
+    @SuppressWarnings("OptionalUsedAsFieldOrParameterType")
+    private static <T> void compareOptionals(Optional<T> a, Optional<T> b, BiConsumer<T, T> comparer) {
+        if (a.isPresent() != b.isPresent()) throw new AssertionFailedError("Both optionals are not present: " + a + ", " + b);
+        a.ifPresent(x -> b.ifPresent(y -> comparer.accept(x, y)));
     }
 }
\ No newline at end of file
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/MetricsRetrieverTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/MetricsRetrieverTest.java
deleted file mode 100644
index 9187b90e894..00000000000
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/MetricsRetrieverTest.java
+++ /dev/null
@@ -1,103 +0,0 @@
-package com.yahoo.vespa.config.server.metrics;
-
-import com.github.tomakehurst.wiremock.junit.WireMockRule;
-import junit.framework.AssertionFailedError;
-import org.junit.Rule;
-import org.junit.Test;
-
-import java.io.IOException;
-import java.net.URI;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.time.Instant;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.function.BiConsumer;
-
-import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
-import static com.github.tomakehurst.wiremock.client.WireMock.get;
-import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
-import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
-import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options;
-import static org.junit.Assert.*;
-
-
-/**
- * @author olaa
- */
-public class MetricsRetrieverTest {
-
-    @Rule
-    public final WireMockRule wireMock = new WireMockRule(options().port(8080), true);
-
-    @Test
-    public void testMetricAggregation() throws IOException {
-
-        List<URI> hosts = List.of(URI.create("http://localhost:8080/1"), URI.create("http://localhost:8080/2"), URI.create("http://localhost:8080/3"));
-
-        stubFor(get(urlEqualTo("/1"))
-                .willReturn(aResponse()
-                        .withStatus(200)
-                        .withBody(contentMetrics())));
-
-        stubFor(get(urlEqualTo("/2"))
-                .willReturn(aResponse()
-                        .withStatus(200)
-                        .withBody(contentMetrics())));
-
-        stubFor(get(urlEqualTo("/3"))
-                .willReturn(aResponse()
-                        .withStatus(200)
-                        .withBody(containerMetrics())));
-
-        ClusterInfo expectedContentCluster = new ClusterInfo("content_cluster_id", "content");
-        ClusterInfo expectedContainerCluster = new ClusterInfo("container_cluster_id", "container");
-
-        Map<ClusterInfo, MetricsAggregator> aggregatorMap = new MetricsRetriever().requestMetricsGroupedByCluster(hosts);
-
-        compareAggregators(
-                new MetricsAggregator().addDocumentCount(6000.0),
-                aggregatorMap.get(expectedContentCluster)
-        );
-
-        compareAggregators(
-                new MetricsAggregator()
-                        .addContainerLatency(3000, 43)
-                        .addContainerLatency(2000, 0)
-                        .addQrLatency(3000, 43)
-                        .addFeedLatency(3000, 43),
-                aggregatorMap.get(expectedContainerCluster)
-
-        );
-
-        wireMock.stop();
-    }
-
-    private String containerMetrics() throws IOException {
-        return Files.readString(Path.of("src/test/resources/metrics/container_metrics"));
-    }
-
-    private String contentMetrics() throws IOException {
-        return Files.readString(Path.of("src/test/resources/metrics/content_metrics"));
-    }
-
-    // Same tolerance value as used internally in MetricsAggregator.isZero
-    private static final double metricsTolerance = 0.001;
-
-    private void compareAggregators(MetricsAggregator expected, MetricsAggregator actual) {
-        BiConsumer<Double, Double> assertDoubles = (a, b) -> assertEquals(a.doubleValue(), b.doubleValue(), metricsTolerance);
-
-        compareOptionals(expected.aggregateDocumentCount(), actual.aggregateDocumentCount(), assertDoubles);
-        compareOptionals(expected.aggregateQueryRate(), actual.aggregateQueryRate(), assertDoubles);
-        compareOptionals(expected.aggregateFeedRate(), actual.aggregateFeedRate(), assertDoubles);
-        compareOptionals(expected.aggregateQueryLatency(), actual.aggregateQueryLatency(), assertDoubles);
-        compareOptionals(expected.aggregateFeedLatency(), actual.aggregateFeedLatency(), assertDoubles);
-    }
-
-    @SuppressWarnings("OptionalUsedAsFieldOrParameterType")
-    private static <T> void compareOptionals(Optional<T> a, Optional<T> b, BiConsumer<T, T> comparer) {
-        if (a.isPresent() != b.isPresent()) throw new AssertionFailedError("Both optionals are not present: " + a + ", " + b);
-        a.ifPresent(x -> b.ifPresent(y -> comparer.accept(x, y)));
-    }
-}
\ No newline at end of file