Simplify deploy metrics

6 files changed, 49 insertions, 190 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
index 2de8fa6457a..cd50382b1bf 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
@@ -22,7 +22,6 @@ import com.yahoo.vespa.hosted.controller.auditlog.AuditLogger;
 import com.yahoo.vespa.hosted.controller.config.ControllerConfig;
 import com.yahoo.vespa.hosted.controller.deployment.JobController;
 import com.yahoo.vespa.hosted.controller.dns.NameServiceForwarder;
-import com.yahoo.vespa.hosted.controller.metric.ConfigServerMetrics;
 import com.yahoo.vespa.hosted.controller.notification.NotificationsDb;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.controller.persistence.JobControlFlags;
@@ -73,7 +72,6 @@ public class Controller extends AbstractComponent {
     private final Clock clock;
     private final ZoneRegistry zoneRegistry;
     private final ServiceRegistry serviceRegistry;
-    private final ConfigServerMetrics metrics;
     private final AuditLogger auditLogger;
     private final FlagSource flagSource;
     private final NameServiceForwarder nameServiceForwarder;
@@ -111,7 +109,6 @@ public class Controller extends AbstractComponent {
         this.mavenRepository = Objects.requireNonNull(mavenRepository, "MavenRepository cannot be null");
         this.metric = Objects.requireNonNull(metric, "Metric cannot be null");
 
-        metrics = new ConfigServerMetrics(serviceRegistry.configServer());
         nameServiceForwarder = new NameServiceForwarder(curator);
         jobController = new JobController(this);
         applicationController = new ApplicationController(this, curator, accessControl, clock, flagSource, serviceRegistry.billingController());
@@ -268,10 +265,6 @@ public class Controller extends AbstractComponent {
         return HostName.from(hostnameSupplier.get());
     }
 
-    public ConfigServerMetrics metrics() {
-        return metrics;
-    }
-
     public SystemName system() {
         return zoneRegistry.system();
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java
index c8416578932..cb431158344 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainer.java
@@ -5,15 +5,21 @@ import com.yahoo.config.provision.SystemName;
 import com.yahoo.vespa.hosted.controller.ApplicationController;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.Instance;
+import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics;
+import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics;
 import com.yahoo.yolean.Exceptions;
 
 import java.time.Duration;
+import java.time.Instant;
+import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.ForkJoinPool;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -52,18 +58,13 @@ public class DeploymentMetricsMaintainer extends ControllerMaintainer {
                         attempts.incrementAndGet();
                         try {
                             if (deployment.version().getMajor() < 7) continue;
-                            var collectedMetrics = controller().metrics().getDeploymentMetrics(instance.id(), deployment.zone());
-                            var now = controller().clock().instant();
+                            List<ClusterMetrics> clusterMetrics = controller().serviceRegistry().configServer()
+                                    .getDeploymentMetrics(new DeploymentId(instance.id(), deployment.zone()));
+                            Instant now = controller().clock().instant();
                             applications.lockApplicationIfPresent(application.id(), locked -> {
                                 Deployment existingDeployment = locked.get().require(instance.name()).deployments().get(deployment.zone());
                                 if (existingDeployment == null) return; // Deployment removed since we started collecting metrics
-                                DeploymentMetrics newMetrics = existingDeployment.metrics()
-                                                                                 .withQueriesPerSecond(collectedMetrics.queriesPerSecond())
-                                                                                 .withWritesPerSecond(collectedMetrics.writesPerSecond())
-                                                                                 .withDocumentCount(collectedMetrics.documentCount())
-                                                                                 .withQueryLatencyMillis(collectedMetrics.queryLatencyMillis())
-                                                                                 .withWriteLatencyMillis(collectedMetrics.writeLatencyMillis())
-                                                                                 .at(now);
+                                DeploymentMetrics newMetrics = updateDeploymentMetrics(existingDeployment.metrics(), clusterMetrics).at(now);
                                 applications.store(locked.with(instance.name(),
                                                                lockedInstance -> lockedInstance.with(existingDeployment.zone(), newMetrics)
                                                                                                .recordActivityAt(now, existingDeployment.zone())));
@@ -93,4 +94,26 @@ public class DeploymentMetricsMaintainer extends ControllerMaintainer {
         return lastException.get() == null;
     }
 
+    static DeploymentMetrics updateDeploymentMetrics(DeploymentMetrics current, List<ClusterMetrics> metrics) {
+        return current
+                .withQueriesPerSecond(metrics.stream().flatMap(m -> m.queriesPerSecond().stream()).mapToDouble(Double::doubleValue).sum())
+                .withWritesPerSecond(metrics.stream().flatMap(m -> m.feedPerSecond().stream()).mapToDouble(Double::doubleValue).sum())
+                .withDocumentCount(metrics.stream().flatMap(m -> m.documentCount().stream()).mapToLong(Double::longValue).sum())
+                .withQueryLatencyMillis(weightedAverageLatency(metrics, ClusterMetrics::queriesPerSecond, ClusterMetrics::queryLatency))
+                .withWriteLatencyMillis(weightedAverageLatency(metrics, ClusterMetrics::feedPerSecond, ClusterMetrics::feedLatency));
+    }
+
+    private static double weightedAverageLatency(List<ClusterMetrics> metrics,
+                                                 Function<ClusterMetrics, Optional<Double>> rateExtractor,
+                                                 Function<ClusterMetrics, Optional<Double>> latencyExtractor) {
+        double rateSum = metrics.stream().flatMap(m -> rateExtractor.apply(m).stream()).mapToDouble(Double::longValue).sum();
+        if (rateSum == 0) return 0.0;
+
+        double weightedLatency = metrics.stream()
+                .flatMap(m -> latencyExtractor.apply(m).flatMap(l -> rateExtractor.apply(m).map(r -> l * r)).stream())
+                .mapToDouble(Double::doubleValue)
+                .sum();
+
+        return weightedLatency / rateSum;
+    }
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/ConfigServerMetrics.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/ConfigServerMetrics.java
deleted file mode 100644
index 91679e4d3d8..00000000000
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/ConfigServerMetrics.java
+++ /dev/null
@@ -1,59 +0,0 @@
-// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.controller.metric;
-
-import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.zone.ZoneId;
-import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics;
-import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
-import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServer;
-
-import java.util.List;
-import java.util.Optional;
-import java.util.function.Function;
-
-/**
- * Retrieves metrics from the configuration server.
- *
- * @author ogronnesby
- */
-public class ConfigServerMetrics {
-
-    private final ConfigServer configServer;
-
-    public ConfigServerMetrics(ConfigServer configServer) {
-        this.configServer = configServer;
-    }
-
-    public DeploymentMetrics getDeploymentMetrics(ApplicationId application, ZoneId zone) {
-        var deploymentId = new DeploymentId(application, zone);
-        var metrics = configServer.getDeploymentMetrics(deploymentId);
-
-        // The field names here come from the MetricsResponse class.
-        return new DeploymentMetrics(
-                metrics.stream().flatMap(m -> m.queriesPerSecond().stream()).mapToDouble(Double::doubleValue).sum(),
-                metrics.stream().flatMap(m -> m.feedPerSecond().stream()).mapToDouble(Double::doubleValue).sum(),
-                metrics.stream().flatMap(m -> m.documentCount().stream()).mapToLong(Double::longValue).sum(),
-                weightedAverageLatency(metrics, ClusterMetrics::queriesPerSecond, ClusterMetrics::queryLatency),
-                weightedAverageLatency(metrics, ClusterMetrics::feedPerSecond, ClusterMetrics::feedLatency)
-        );
-    }
-
-    private double weightedAverageLatency(List<ClusterMetrics> metrics,
-                                          Function<ClusterMetrics, Optional<Double>> rateExtractor,
-                                          Function<ClusterMetrics, Optional<Double>> latencyExtractor)
-    {
-        var rateSum = metrics.stream().flatMap(m -> rateExtractor.apply(m).stream()).mapToDouble(Double::longValue).sum();
-        if (rateSum == 0) {
-            return 0.0;
-        }
-
-        var weightedLatency = metrics.stream()
-                .flatMap(m -> {
-                    return latencyExtractor.apply(m).flatMap(l -> rateExtractor.apply(m).map(r -> l * r)).stream();
-                })
-                .mapToDouble(Double::doubleValue)
-                .sum();
-
-        return weightedLatency / rateSum;
-    }
-}
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/DeploymentMetrics.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/DeploymentMetrics.java
deleted file mode 100644
index 33a3ce957ed..00000000000
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/metric/DeploymentMetrics.java
+++ /dev/null
@@ -1,47 +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.hosted.controller.metric;
-
-/**
- * Metrics for a single deployment of an application.
- *
- * @author bratseth
- */
-public class DeploymentMetrics {
-
-    private final double queriesPerSecond;
-    private final double writesPerSecond;
-    private final long documentCount;
-    private final double queryLatencyMillis;
-    private final double writeLatencyMillis;
-
-    public DeploymentMetrics(double queriesPerSecond, double writesPerSecond,
-                             long documentCount,
-                             double queryLatencyMillis, double writeLatencyMillis) {
-        this.queriesPerSecond = queriesPerSecond;
-        this.writesPerSecond = writesPerSecond;
-        this.documentCount = documentCount;
-        this.queryLatencyMillis = queryLatencyMillis;
-        this.writeLatencyMillis = writeLatencyMillis;
-    }
-
-    public double queriesPerSecond() {
-        return queriesPerSecond;
-    }
-
-    public double writesPerSecond() {
-        return writesPerSecond;
-    }
-
-    public long documentCount() {
-        return documentCount;
-    }
-
-    public double queryLatencyMillis() {
-        return queryLatencyMillis;
-    }
-
-    public double writeLatencyMillis() {
-        return writeLatencyMillis;
-    }
-
-}
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java
index 84f4f3d9b7c..59fb5b596f1 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/DeploymentMetricsMaintainerTest.java
@@ -11,12 +11,14 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
+import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
 import org.junit.Test;
 
 import java.time.Duration;
 import java.time.Instant;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.function.Supplier;
 
@@ -113,6 +115,21 @@ public class DeploymentMetricsMaintainerTest {
         assertEquals(5, deployment.get().activity().lastWritesPerSecond().getAsDouble(), Double.MIN_VALUE);
     }
 
+    @Test
+    public void cluster_metric_aggregation_test() {
+        List<ClusterMetrics> clusterMetrics = List.of(
+                new ClusterMetrics("niceCluster", "container", Map.of("queriesPerSecond", 23.0, "queryLatency", 1337.0)),
+                new ClusterMetrics("alsoNiceCluster", "container", Map.of("queriesPerSecond", 11.0, "queryLatency", 12.0)));
+
+        DeploymentMetrics deploymentMetrics = DeploymentMetricsMaintainer.updateDeploymentMetrics(DeploymentMetrics.none, clusterMetrics);
+
+        assertEquals(23.0 + 11.0, deploymentMetrics.queriesPerSecond(), 0.001);
+        assertEquals(908.323, deploymentMetrics.queryLatencyMillis(), 0.001);
+        assertEquals(0, deploymentMetrics.documentCount(), 0.001);
+        assertEquals(0.0, deploymentMetrics.writeLatencyMillis(), 0.001);
+        assertEquals(0.0, deploymentMetrics.writesPerSecond(), 0.001);
+    }
+
     private void setMetrics(ApplicationId application, Map<String, Double> metrics) {
         var clusterMetrics = new ClusterMetrics("default", "container", metrics);
         tester.controllerTester().serviceRegistry().configServerMock().setMetrics(new DeploymentId(application, ZoneId.from("dev", "us-east-1")), clusterMetrics);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/metric/ConfigServerMetricsTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/metric/ConfigServerMetricsTest.java
deleted file mode 100644
index 787f2e93dcb..00000000000
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/metric/ConfigServerMetricsTest.java
+++ /dev/null
@@ -1,68 +0,0 @@
-// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.controller.metric;
-
-import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.SystemName;
-import com.yahoo.config.provision.zone.ZoneId;
-import com.yahoo.vespa.hosted.controller.api.application.v4.model.ClusterMetrics;
-import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
-import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock;
-import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock;
-import org.junit.Before;
-import org.junit.Test;
-
-import java.util.List;
-import java.util.Map;
-
-import static org.junit.Assert.assertEquals;
-
-/**
- * @author olaa
- */
-public class ConfigServerMetricsTest {
-
-    private final ApplicationId applicationId = new ApplicationId.Builder()
-                .tenant("foo")
-                .applicationName("bar")
-                .instanceName("default")
-                .build();
-
-    private final ZoneId zoneId = ZoneId.from("prod", "us-west-1");
-
-    private ConfigServerMock configServer;
-    private ConfigServerMetrics service;
-
-    @Before
-    public void before() {
-        configServer = new ConfigServerMock(new ZoneRegistryMock(SystemName.main));
-        service = new ConfigServerMetrics(configServer);
-    }
-
-    @Test
-    public void test_returning_metrics() {
-        //
-        // Wire up the test
-        //
-        var deploymentId = new DeploymentId(applicationId, zoneId);
-
-        var clusterMetrics1 = new ClusterMetrics("niceCluster", "container", Map.of("queriesPerSecond", 23.0, "queryLatency", 1337.0));
-
-        var clusterMetrics2 = new ClusterMetrics("alsoNiceCluster", "container", Map.of("queriesPerSecond", 11.0, "queryLatency", 12.0));
-
-        var response = List.of(clusterMetrics1, clusterMetrics2);
-
-        configServer.setMetrics(deploymentId, response);
-
-        //
-        // Now we can actually test stuff :(
-        //
-        var deploymentMetrics = service.getDeploymentMetrics(applicationId, zoneId);
-
-        assertEquals(23.0 + 11.0, deploymentMetrics.queriesPerSecond(), 0.001);
-        assertEquals(908.323, deploymentMetrics.queryLatencyMillis(), 0.001);
-        assertEquals(0, deploymentMetrics.documentCount());
-        assertEquals(0.0, deploymentMetrics.writeLatencyMillis(), 0.001);
-        assertEquals(0.0, deploymentMetrics.writesPerSecond(), 0.001);
-    }
-
-}