Merge pull request #10508 from vespa-engine/olaa/reapply-parallelization

Reapply parallelization for configserver metric aggregation

11 files changed, 355 insertions, 200 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 0f5540b8a12..8e59b52777d 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,9 +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.ClusterInfo;
-import com.yahoo.vespa.config.server.metrics.MetricsAggregator;
-import com.yahoo.vespa.config.server.metrics.MetricsRetriever;
+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;
@@ -73,11 +71,8 @@ import java.time.Duration;
 import java.time.Instant;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.HashSet;
-import java.util.LinkedHashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.logging.Level;
@@ -87,7 +82,6 @@ import java.util.stream.Collectors;
 import static com.yahoo.config.model.api.container.ContainerServiceType.CLUSTERCONTROLLER_CONTAINER;
 import static com.yahoo.config.model.api.container.ContainerServiceType.CONTAINER;
 import static com.yahoo.config.model.api.container.ContainerServiceType.LOGSERVER_CONTAINER;
-import static com.yahoo.config.model.api.container.ContainerServiceType.METRICS_PROXY_CONTAINER;
 import static com.yahoo.vespa.config.server.tenant.TenantRepository.HOSTED_VESPA_TENANT;
 import static java.nio.file.Files.readAttributes;
 
@@ -657,16 +651,9 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
     // ---------------- Metrics ------------------------------------------------------------------------
 
     public MetricsResponse getMetrics(ApplicationId applicationId) {
-        var metricsRetriever = new MetricsRetriever();
-        var clusters = getClustersOfApplication(applicationId);
-        var clusterMetrics = new LinkedHashMap<ClusterInfo, MetricsAggregator>();
-
-        clusters.forEach(cluster -> {
-            var metrics = metricsRetriever.requestMetricsForCluster(cluster);
-            clusterMetrics.put(cluster, metrics);
-        });
-
-        return new MetricsResponse(200, applicationId, clusterMetrics);
+        Application application = getApplication(applicationId);
+        ApplicationMetricsRetriever applicationMetricsRetriever = new ApplicationMetricsRetriever();
+        return applicationMetricsRetriever.getMetrics(application);
     }
 
     // ---------------- Misc operations ----------------------------------------------------------------
@@ -800,35 +787,6 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return port;
     }
 
-    /** Finds the hosts of an application, grouped by cluster name */
-    private Collection<ClusterInfo> getClustersOfApplication(ApplicationId applicationId) {
-        Application application = getApplication(applicationId);
-        Map<String, ClusterInfo> clusters = new HashMap<>();
-        application.getModel().getHosts().stream()
-                .filter(host -> host.getServices().stream().noneMatch(serviceInfo -> serviceInfo.getServiceType().equalsIgnoreCase("logserver")))
-                .forEach(hostInfo -> {
-                            ServiceInfo metricsService = getServiceInfoByType(hostInfo, METRICS_PROXY_CONTAINER.serviceName);
-                            ServiceInfo clusterServiceInfo =  getServiceInfoByType(hostInfo, "container", "searchnode");
-                            ClusterInfo clusterInfo = createClusterInfo(clusterServiceInfo);
-                            URI host = URI.create("http://" + hostInfo.getHostname() + ":" + servicePort(metricsService) + "/metrics/v1/values?consumer=Vespa");
-                            clusters.computeIfAbsent(clusterInfo.getClusterId(), c -> clusterInfo).addHost(host);
-                        }
-                );
-        return clusters.values();
-
-    }
-
-    private ServiceInfo getServiceInfoByType(HostInfo hostInfo, String... types) {
-        List<String> type = List.of(types);
-        return hostInfo.getServices().stream().filter(serviceInfo -> type.contains(serviceInfo.getServiceType())).findFirst().orElseThrow();
-    }
-
-    private ClusterInfo createClusterInfo(ServiceInfo serviceInfo) {
-        String clusterName = serviceInfo.getServiceName();
-        ClusterInfo.ClusterType clusterType = serviceInfo.getServiceType().equals("searchnode") ? ClusterInfo.ClusterType.content : ClusterInfo.ClusterType.container;
-        return new ClusterInfo(clusterName, clusterType);
-    }
-
     /** Returns version to use when deploying application in given environment */
     static Version decideVersion(ApplicationId application, Environment environment, Version sessionVersion, boolean bootstrap) {
         if (     environment.isManuallyDeployed()
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/MetricsResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/MetricsResponse.java
index 0abc59f54b2..7cf9357217e 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/MetricsResponse.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/MetricsResponse.java
@@ -32,7 +32,7 @@ public class MetricsResponse extends HttpResponse {
         for (var entry : aggregatedMetrics.entrySet()) {
             Cursor cluster = clusters.addObject();
             cluster.setString("clusterId", entry.getKey().getClusterId());
-            cluster.setString("clusterType", entry.getKey().getClusterType().name());
+            cluster.setString("clusterType", entry.getKey().getClusterType());
 
             MetricsAggregator aggregator = entry.getValue();
             Cursor metrics = cluster.setObject("metrics");
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/ClusterInfo.java b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterInfo.java
index 7507b5c4c2c..86144c1ddc3 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterInfo.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterInfo.java
@@ -1,9 +1,8 @@
 // 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.net.URI;
-import java.util.ArrayList;
-import java.util.List;
+
+import java.util.Objects;
 
 /**
  * @author olaa
@@ -11,46 +10,31 @@ import java.util.List;
 public class ClusterInfo {
 
     private final String clusterId;
-    private final ClusterType clusterType;
-    private final List<URI> hostnames;
-
-    public ClusterInfo(String clusterId, ClusterType clusterType) {
-        this(clusterId, clusterType, new ArrayList<>());
-    }
+    private final String clusterType;
 
-    public ClusterInfo(String clusterId, ClusterType clusterType, List<URI> hostnames) {
+    public ClusterInfo(String clusterId, String clusterType) {
         this.clusterId = clusterId;
         this.clusterType = clusterType;
-        this.hostnames = hostnames;
     }
 
     public String getClusterId() {
         return clusterId;
     }
 
-    public ClusterType getClusterType() {
+    public String getClusterType() {
         return clusterType;
     }
 
-    public List<URI> getHostnames() {
-        return hostnames;
+    @Override
+    public int hashCode() {
+        return Objects.hash(clusterId, clusterType);
     }
 
-    public void addHost(URI host) {
-        hostnames.add(host);
+    @Override
+    public boolean equals(Object o) {
+        if (!(o instanceof ClusterInfo)) return false;
+        ClusterInfo other = (ClusterInfo) o;
+        return clusterId.equals(other.clusterId) && clusterType.equals(other.clusterType);
     }
 
-    public enum ClusterType {
-        content,
-        container;
-
-        public static boolean isValidType(String enumString) {
-            try {
-                valueOf(enumString);
-                return true;
-            } catch (IllegalArgumentException e) {
-                return false;
-            }
-        }
-    };
 }
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
new file mode 100644
index 00000000000..b51f3d01e1a
--- /dev/null
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetriever.java
@@ -0,0 +1,132 @@
+// 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.util.Collection;
+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;
+
+
+/**
+ * Client for reaching out to nodes in an application instance and get their
+ * metrics.
+ *
+ * @author olaa
+ * @author ogronnesby
+ */
+public class ClusterMetricsRetriever {
+
+    private static final Logger log = Logger.getLogger(ClusterMetricsRetriever.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) {
+            // Usually caused by applications being deleted during metric retrieval
+            log.warning("Was unable to fetch metrics from " + hostURI);
+            return new Slime();
+        }
+    }
+
+    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 8fa08275ad5..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;
 
 /**
@@ -14,33 +13,27 @@ public class MetricsAggregator {
     private LatencyMetrics qr;
     private LatencyMetrics container;
     private Double documentCount;
-    private Instant timestamp;
 
-    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;
     }
 
-    public MetricsAggregator setTimestamp(Instant timestamp) {
-        this.timestamp = timestamp;
-        return this;
-    }
-
     public Optional<Double> aggregateFeedLatency() {
         return Optional.ofNullable(feed).map(m -> m.latencySum / m.latencyCount).filter(num -> !num.isNaN());
 
@@ -68,10 +61,6 @@ public class MetricsAggregator {
         return Optional.ofNullable(documentCount);
     }
 
-    public Instant getTimestamp() {
-        return timestamp;
-    }
-
     private LatencyMetrics combineLatency(LatencyMetrics metricsOrNull, double sum, double count) {
         var metrics = Optional.ofNullable(metricsOrNull).orElseGet(LatencyMetrics::new);
         metrics.latencyCount += count;
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 4fbeae11758..00000000000
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsRetriever.java
+++ /dev/null
@@ -1,97 +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.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 final HttpClient httpClient = HttpClientBuilder.create().build();
-
-    /**
-     * Call the metrics API on each host in the cluster and aggregate the metrics
-     * into a single value.
-     */
-    public MetricsAggregator requestMetricsForCluster(ClusterInfo clusterInfo) {
-        var aggregator = new MetricsAggregator();
-        clusterInfo.getHostnames().forEach(host -> getHostMetrics(host, aggregator));
-        return aggregator;
-    }
-
-    private void getHostMetrics(URI hostURI, MetricsAggregator metrics) {
-            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, metrics);
-            });
-    }
-
-    private Slime doMetricsRequest(URI hostURI) {
-        HttpGet get = new HttpGet(hostURI);
-        try {
-            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 void parseService(Inspector service, MetricsAggregator metrics) {
-        String serviceName = service.field("name").asString();
-        Instant timestamp = Instant.ofEpochSecond(service.field("timestamp").asLong());
-        metrics.setTimestamp(timestamp);
-        service.field("metrics").traverse((ArrayTraverser) (i, m) -> {
-            Inspector values = m.field("values");
-            switch (serviceName) {
-                case "vespa.container":
-                    metrics.addContainerLatency(
-                            values.field("query_latency.sum").asDouble(),
-                            values.field("query_latency.count").asDouble());
-                    metrics.addFeedLatency(
-                            values.field("feed_latency.sum").asDouble(),
-                            values.field("feed_latency.count").asDouble());
-                    break;
-                case "vespa.qrserver":
-                    metrics.addQrLatency(
-                            values.field("query_latency.sum").asDouble(),
-                            values.field("query_latency.count").asDouble());
-                    break;
-                case "vespa.distributor":
-                    metrics.addDocumentCount(values.field("vds.distributor.docsstored.average").asDouble());
-                    break;
-            }
-        });
-
-    }
-
-}
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/MetricsRetrieverTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetrieverTest.java
index 1b878a432c9..da676663d3b 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/MetricsRetrieverTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetrieverTest.java
@@ -10,6 +10,7 @@ import java.net.URI;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.function.BiConsumer;
 
@@ -24,19 +25,14 @@ import static org.junit.Assert.*;
 /**
  * @author olaa
  */
-public class MetricsRetrieverTest {
+public class ClusterMetricsRetrieverTest {
 
     @Rule
     public final WireMockRule wireMock = new WireMockRule(options().port(8080), true);
 
     @Test
     public void testMetricAggregation() throws IOException {
-        var metricsRetriever = new MetricsRetriever();
-
-        var clusters = List.of(
-                new ClusterInfo("cluster1", ClusterInfo.ClusterType.content, List.of(URI.create("http://localhost:8080/1"), URI.create("http://localhost:8080/2"))),
-                new ClusterInfo("cluster2", ClusterInfo.ClusterType.container, List.of(URI.create("http://localhost:8080/3")))
-        );
+        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()
@@ -53,9 +49,14 @@ public class MetricsRetrieverTest {
                         .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 ClusterMetricsRetriever().requestMetricsGroupedByCluster(hosts);
+
         compareAggregators(
                 new MetricsAggregator().addDocumentCount(6000.0),
-                metricsRetriever.requestMetricsForCluster(clusters.get(0))
+                aggregatorMap.get(expectedContentCluster)
         );
 
         compareAggregators(
@@ -64,9 +65,9 @@ public class MetricsRetrieverTest {
                         .addContainerLatency(2000, 0)
                         .addQrLatency(3000, 43)
                         .addFeedLatency(3000, 43),
-                metricsRetriever.requestMetricsForCluster(clusters.get(1))
-        );
+                aggregatorMap.get(expectedContainerCluster)
 
+        );
         wireMock.stop();
     }
 
diff --git a/configserver/src/test/resources/metrics/container_metrics b/configserver/src/test/resources/metrics/container_metrics
index a6a5828934c..c2118fa39a5 100644
--- a/configserver/src/test/resources/metrics/container_metrics
+++ b/configserver/src/test/resources/metrics/container_metrics
@@ -11,6 +11,10 @@
                   "document.count": 300000,
                   "feed.rate": 23.0,
                   "write_latency.sum": 2000
+              },
+              "dimensions": {
+                  "clustertype": "container",
+                  "clusterid": "container_cluster_id"
               }
             },
             {
@@ -20,7 +24,11 @@
                   "feed_latency.count": 43.0,
                   "feed_latency.sum": 3000
 
-              }
+              },
+              "dimensions": {
+                  "clustertype": "container",
+                  "clusterid": "container_cluster_id"
+                }
             }
           ]
       },
@@ -33,6 +41,10 @@
            "values": {
                 "query_latency.count": 43.0,
                 "query_latency.sum": 3000
+            },
+            "dimensions": {
+              "clustertype": "container",
+              "clusterid": "container_cluster_id"
             }
           }
         ]
diff --git a/configserver/src/test/resources/metrics/content_metrics b/configserver/src/test/resources/metrics/content_metrics
index c881e574f8b..4a1deced181 100644
--- a/configserver/src/test/resources/metrics/content_metrics
+++ b/configserver/src/test/resources/metrics/content_metrics
@@ -8,7 +8,11 @@
              "values": {
                   "vds.distributor.docsstored.average": 3000
 
-              }
+              },
+              "dimensions": {
+                  "clustertype": "content",
+                  "clusterid": "content_cluster_id"
+                }
             }
           ]
       }