Merge pull request #5362 from vespa-engine/mpolden/deployment-duration-metric

Deployment duration metric

11 files changed, 193 insertions, 67 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
index 950790e26b6..51ec621ce22 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
@@ -67,10 +67,10 @@ public class LockedApplication extends Application {
         );
     }
 
-    public LockedApplication withJobTriggering(JobType type, Change change, Instant triggerTime,
+    public LockedApplication withJobTriggering(JobType type, Instant triggerTime,
                                                Version version, ApplicationVersion applicationVersion,
                                                String reason) {
-        return new LockedApplication(new Builder(this).with(deploymentJobs().withTriggering(type, change, version, applicationVersion, reason, triggerTime)));
+        return new LockedApplication(new Builder(this).with(deploymentJobs().withTriggering(type, version, applicationVersion, reason, triggerTime)));
     }
 
     public LockedApplication withNewDeployment(ZoneId zone, ApplicationVersion applicationVersion, Version version,
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java
index 69790dfc857..01a45d3567f 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java
@@ -67,7 +67,6 @@ public class DeploymentJobs {
     }
 
     public DeploymentJobs withTriggering(JobType jobType,
-                                         Change change,
                                          Version version,
                                          ApplicationVersion applicationVersion,
                                          String reason,
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
index ee448775cbf..d7199539c16 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
@@ -347,7 +347,6 @@ public class DeploymentTrigger {
                                reason));
         deploymentQueue.addJob(application.id(), jobType, first);
         return application.withJobTriggering(jobType,
-                                             application.change(),
                                              clock.instant(),
                                              application.deployVersionFor(jobType, controller),
                                              application.deployApplicationVersionFor(jobType, controller, false)
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java
index ab388ca9a9f..967db0dff99 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporter.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.maintenance;
 
+import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.hosted.controller.Application;
@@ -10,6 +11,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.chef.Chef;
 import com.yahoo.vespa.hosted.controller.api.integration.chef.rest.PartialNode;
 import com.yahoo.vespa.hosted.controller.api.integration.chef.rest.PartialNodeResult;
 import com.yahoo.vespa.hosted.controller.application.ApplicationList;
+import com.yahoo.vespa.hosted.controller.application.JobStatus;
 import com.yahoo.vespa.hosted.controller.rotation.RotationLock;
 
 import java.time.Clock;
@@ -21,6 +23,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.stream.Collectors;
 
 /**
  * @author mortent
@@ -30,6 +33,7 @@ public class MetricsReporter extends Maintainer {
 
     public static final String convergeMetric = "seconds.since.last.chef.convergence";
     public static final String deploymentFailMetric = "deployment.failurePercentage";
+    public static final String deploymentAverageDuration = "deployment.averageDuration";
     public static final String remainingRotations = "remaining_rotations";
 
     private final Metric metric;
@@ -112,6 +116,10 @@ public class MetricsReporter extends Maintainer {
 
     private void reportDeploymentMetrics() {
         metric.set(deploymentFailMetric, deploymentFailRatio() * 100, metric.createContext(Collections.emptyMap()));
+        for (Map.Entry<ApplicationId, Duration> entry : averageDeploymentDurations().entrySet()) {
+            metric.set(deploymentAverageDuration, entry.getValue().getSeconds(),
+                       metric.createContext(Collections.singletonMap("application", entry.getKey().toString())));
+        }
     }
     
     private double deploymentFailRatio() {
@@ -121,8 +129,37 @@ public class MetricsReporter extends Maintainer {
                                                         .asList();
         if (applications.isEmpty()) return 0;
 
-        return (double)applications.stream().filter(a -> a.deploymentJobs().hasFailures()).count() / 
-               (double)applications.size();
+        return (double) applications.stream().filter(a -> a.deploymentJobs().hasFailures()).count() /
+               (double) applications.size();
+    }
+
+    private Map<ApplicationId, Duration> averageDeploymentDurations() {
+        Instant now = clock.instant();
+        return ApplicationList.from(controller().applications().asList())
+                              .notPullRequest()
+                              .hasProductionDeployment()
+                              .asList()
+                              .stream()
+                              .collect(Collectors.toMap(Application::id,
+                                                        application -> averageDeploymentDuration(application, now)));
+    }
+
+    private Duration averageDeploymentDuration(Application application, Instant now) {
+        List<Duration> jobDurations = application.deploymentJobs().jobStatus().values().stream()
+                                                 .filter(status -> status.lastTriggered().isPresent())
+                                                 .map(status -> {
+                                                     Instant triggeredAt = status.lastTriggered().get().at();
+                                                     Instant runningUntil = status.lastCompleted()
+                                                                                  .map(JobStatus.JobRun::at)
+                                                                                  .filter(at -> at.isAfter(triggeredAt))
+                                                                                  .orElse(now);
+                                                     return Duration.between(triggeredAt, runningUntil);
+                                                 })
+                                                 .collect(Collectors.toList());
+        return jobDurations.stream()
+                           .reduce(Duration::plus)
+                           .map(totalDuration -> totalDuration.dividedBy(jobDurations.size()))
+                           .orElse(Duration.ZERO);
     }
     
     private void keepNodesWithSystem(PartialNodeResult nodeResult, SystemName system) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index a898c3eec68..bba82ce2980 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
@@ -10,19 +10,14 @@ import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.TenantName;
-import com.yahoo.vespa.athenz.api.AthenzDomain;
 import com.yahoo.vespa.athenz.api.NToken;
 import com.yahoo.vespa.config.SlimeUtils;
-import com.yahoo.vespa.hosted.controller.api.Tenant;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.EndpointStatus;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.ScrewdriverBuildJob;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
-import com.yahoo.vespa.hosted.controller.api.identifiers.Property;
-import com.yahoo.vespa.hosted.controller.api.identifiers.PropertyId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.ScrewdriverId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
-import com.yahoo.vespa.hosted.controller.api.identifiers.UserGroup;
 import com.yahoo.vespa.hosted.controller.api.integration.BuildService;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.Record;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.RecordName;
@@ -36,11 +31,10 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType;
 import com.yahoo.vespa.hosted.controller.application.JobStatus;
 import com.yahoo.vespa.hosted.controller.application.SourceRevision;
-import com.yahoo.vespa.hosted.controller.athenz.mock.AthenzDbMock;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
+import com.yahoo.vespa.hosted.controller.deployment.BuildJob;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentQueue;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
-import com.yahoo.vespa.hosted.controller.deployment.BuildJob;
 import com.yahoo.vespa.hosted.controller.persistence.ApplicationSerializer;
 import com.yahoo.vespa.hosted.controller.rotation.RotationId;
 import com.yahoo.vespa.hosted.controller.rotation.RotationLock;
@@ -179,7 +173,7 @@ public class ControllerTest {
                 .environment(Environment.prod)
                 .region("us-east-3")
                 .build();
-        tester.jobCompletion(component).application(app1).buildNumber(43).uploadArtifact(applicationPackage).submit();
+        tester.jobCompletion(component).application(app1).nextBuildNumber().uploadArtifact(applicationPackage).submit();
         try {
             tester.deploy(systemTest, app1, applicationPackage);
             fail("Expected exception due to unallowed production deployment removal");
@@ -201,7 +195,7 @@ public class ControllerTest {
                 .environment(Environment.prod)
                 .region("us-east-3")
                 .build();
-        tester.jobCompletion(component).application(app1).buildNumber(44).uploadArtifact(applicationPackage).submit();
+        tester.jobCompletion(component).application(app1).nextBuildNumber(2).uploadArtifact(applicationPackage).submit();
         tester.deployAndNotify(app1, applicationPackage, true, systemTest);
         assertNull("Zone was removed",
                    applications.require(app1.id()).deployments().get(productionCorpUsEast1.zone(SystemName.main).get()));
@@ -266,7 +260,7 @@ public class ControllerTest {
                 .region("us-west-1")
                 .region("us-east-3")
                 .build();
-        tester.jobCompletion(component).application(app1).buildNumber(43).uploadArtifact(applicationPackage).submit();
+        tester.jobCompletion(component).application(app1).nextBuildNumber().uploadArtifact(applicationPackage).submit();
         tester.deployAndNotify(app1, applicationPackage, true, systemTest);
         tester.deployAndNotify(app1, applicationPackage, true, stagingTest);
         tester.deployAndNotify(app1, applicationPackage, true, productionUsWest1);
@@ -458,7 +452,7 @@ public class ControllerTest {
         // out of capacity retry mechanism
         tester.clock().advance(Duration.ofMinutes(15));
         tester.jobCompletion(stagingTest).application(app1).error(JobError.outOfCapacity).submit(); // Clear the previous staging test
-        tester.jobCompletion(component).application(app1).buildNumber(43).uploadArtifact(applicationPackage).submit();
+        tester.jobCompletion(component).application(app1).nextBuildNumber().uploadArtifact(applicationPackage).submit();
         tester.deployAndNotify(app1, applicationPackage, true, false, systemTest);
         tester.deploy(stagingTest, app1, applicationPackage);
         assertEquals(1, deploymentQueue.takeJobsToRun().size());
@@ -467,10 +461,10 @@ public class ControllerTest {
 
         // app2 and app3: New change triggers system-test jobs
         // Provide a changed application package, too, or the deployment is a no-op.
-        tester.jobCompletion(component).application(app2).buildNumber(43).uploadArtifact(applicationPackage).submit();
+        tester.jobCompletion(component).application(app2).nextBuildNumber().uploadArtifact(applicationPackage).submit();
         tester.deployAndNotify(app2, applicationPackage2, true, systemTest);
 
-        tester.jobCompletion(component).application(app3).buildNumber(43).uploadArtifact(applicationPackage).submit();
+        tester.jobCompletion(component).application(app3).nextBuildNumber().uploadArtifact(applicationPackage).submit();
         tester.deployAndNotify(app3, applicationPackage2, true, systemTest);
 
         assertEquals(2, deploymentQueue.jobs().size());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MetricsMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MetricsMock.java
index 343a9d2ed6e..7f23a40435f 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MetricsMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/MetricsMock.java
@@ -8,8 +8,13 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
+import java.util.function.BiPredicate;
 import java.util.stream.Collectors;
 
+/**
+ * @author mortent
+ */
 public class MetricsMock implements Metric {
 
     private final Map<Context, Map<String, Number>> metrics = new HashMap<>();
@@ -31,8 +36,9 @@ public class MetricsMock implements Metric {
     }
 
     @Override
+    @SuppressWarnings("unchecked")
     public Context createContext(Map<String, ?> properties) {
-        Context ctx = new MapContext(properties);
+        Context ctx = new MapContext((Map<String, String>) properties);
         metrics.putIfAbsent(ctx, new HashMap<>());
         return ctx;
     }
@@ -48,16 +54,32 @@ public class MetricsMock implements Metric {
         return valuesForEmptyContext.get(name);
     }
 
-    public Map<MapContext, Map<String, Number>> getMetricsFilteredByHost(String hostname) {
-        return getMetrics().entrySet().stream()
-                .filter(entry -> ((MapContext)entry.getKey()).containsDimensionValue("host", hostname))
-                .collect(Collectors.toMap(entry -> (MapContext) entry.getKey(), Map.Entry::getValue));
+    /** Returns metric and context for any metric matching the given dimension predicate */
+    public Map<MapContext, Map<String, Number>> getMetrics(BiPredicate<String, String> dimension) {
+        return metrics.entrySet()
+                      .stream()
+                      .filter(context -> ((MapContext) context.getKey())
+                              .getDimensions().entrySet()
+                              .stream()
+                              .anyMatch(d -> dimension.test(d.getKey(), d.getValue())))
+                      .collect(Collectors.toMap(entry -> (MapContext) entry.getKey(), Map.Entry::getValue));
+    }
+
+    /** Returns metric filtered by dimension and name */
+    public Optional<Number> getMetric(BiPredicate<String, String> dimension, String name) {
+        Map<String, Number> metrics = getMetrics(dimension).entrySet()
+                                                           .stream()
+                                                           .map(Map.Entry::getValue)
+                                                           .findFirst()
+                                                           .orElseGet(Collections::emptyMap);
+        return Optional.ofNullable(metrics.get(name));
     }
 
     public static class MapContext implements Context {
-        final Map<String, ?> dimensions;
 
-        public MapContext(Map<String, ?> dimensions) {
+        private final Map<String, String> dimensions;
+
+        public MapContext(Map<String, String> dimensions) {
             this.dimensions = dimensions;
         }
 
@@ -71,13 +93,10 @@ public class MetricsMock implements Metric {
             return Objects.toString(dimensions).hashCode();
         }
 
-        public Map<String, ?> getDimensions() {
+        public Map<String, String> getDimensions() {
             return dimensions;
         }
 
-        public boolean containsDimensionValue(String dimension, Object value) {
-            return value.equals(dimensions.get(dimension));
-        }
     }
 }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java
index 0ea50221cfc..bb8840a7b1f 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/BuildJob.java
@@ -74,6 +74,14 @@ public class BuildJob {
         return this;
     }
 
+    public BuildJob nextBuildNumber(int increment) {
+        return buildNumber(buildNumber + increment);
+    }
+
+    public BuildJob nextBuildNumber() {
+        return nextBuildNumber(1);
+    }
+
     public BuildJob projectId(long projectId) {
         this.projectId = projectId;
         return this;
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
index c4b3bd82bfe..5aa0d794e65 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
@@ -305,7 +305,7 @@ public class DeploymentTriggerTest {
         ApplicationPackage changedApplication = applicationPackageBuilder.searchDefinition(searchDefinition).build();
         tester.jobCompletion(component)
               .application(app)
-              .buildNumber(43)
+              .nextBuildNumber()
               .sourceRevision(new SourceRevision("repository1", "master", "cafed00d"))
               .uploadArtifact(changedApplication)
               .submit();
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java
index 4d3f26d247e..93fdf3618a7 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/MetricsReporterTest.java
@@ -17,18 +17,23 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTester;
 import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb;
+import org.junit.Before;
 import org.junit.Test;
 import org.mockito.Mockito;
 
 import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.time.Clock;
+import java.time.Duration;
 import java.time.Instant;
 import java.time.ZoneId;
 import java.util.Map;
 
 import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.component;
+import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.productionUsWest1;
+import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.stagingTest;
 import static com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType.systemTest;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -43,18 +48,24 @@ import static org.mockito.Mockito.when;
 public class MetricsReporterTest {
 
     private static final Path testData = Paths.get("src/test/resources/");
+    private MetricsMock metrics;
+
+    @Before
+    public void before() {
+        metrics = new MetricsMock();
+    }
 
     @Test
-    public void test_chef_metrics() throws IOException {
+    public void test_chef_metrics() {
+        Clock clock = Clock.fixed(Instant.ofEpochSecond(1475497913), ZoneId.systemDefault());;
         ControllerTester tester = new ControllerTester();
-        MetricsMock metricsMock = new MetricsMock();
-        MetricsReporter metricsReporter = setupMetricsReporter(tester.controller(), metricsMock, SystemName.cd);
+        MetricsReporter metricsReporter = createReporter(clock, tester.controller(), metrics, SystemName.cd);
         metricsReporter.maintain();
-        assertEquals(2, metricsMock.getMetrics().size());
+        assertEquals(2, metrics.getMetrics().size());
 
-        Map<MapContext, Map<String, Number>> metrics = metricsMock.getMetricsFilteredByHost("fake-node.test");
-        assertEquals(1, metrics.size());
-        Map.Entry<MapContext, Map<String, Number>> metricEntry = metrics.entrySet().iterator().next();
+        Map<MapContext, Map<String, Number>> hostMetrics = getMetricsByHost("fake-node.test");
+        assertEquals(1, hostMetrics.size());
+        Map.Entry<MapContext, Map<String, Number>> metricEntry = hostMetrics.entrySet().iterator().next();
         MapContext metricContext = metricEntry.getKey();
         assertDimension(metricContext, "tenantName", "ciintegrationtests");
         assertDimension(metricContext, "app", "restart.default");
@@ -63,17 +74,16 @@ public class MetricsReporterTest {
     }
 
     @Test
-    public void test_deployment_metrics() throws IOException {
+    public void test_deployment_fail_ratio() {
         DeploymentTester tester = new DeploymentTester();
         ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
                 .environment(Environment.prod)
                 .region("us-west-1")
                 .build();
-        MetricsMock metricsMock = new MetricsMock();
-        MetricsReporter metricsReporter = setupMetricsReporter(tester.controller(), metricsMock, SystemName.cd);
+        MetricsReporter metricsReporter = createReporter(tester.controller(), metrics, SystemName.cd);
 
         metricsReporter.maintain();
-        assertEquals(0.0, metricsMock.getMetric(MetricsReporter.deploymentFailMetric));
+        assertEquals(0.0, metrics.getMetric(MetricsReporter.deploymentFailMetric));
 
         // Deploy all apps successfully
         Application app1 = tester.createApplication("app1", "tenant1", 1, 11L);
@@ -86,49 +96,109 @@ public class MetricsReporterTest {
         tester.deployCompletely(app4, applicationPackage);
 
         metricsReporter.maintain();
-        assertEquals(0.0, metricsMock.getMetric(MetricsReporter.deploymentFailMetric));
+        assertEquals(0.0, metrics.getMetric(MetricsReporter.deploymentFailMetric));
 
         // 1 app fails system-test
-        tester.jobCompletion(component).application(app4).submit();
+        tester.jobCompletion(component).application(app4).nextBuildNumber().uploadArtifact(applicationPackage).submit();
         tester.deployAndNotify(app4, applicationPackage, false, systemTest);
 
         metricsReporter.maintain();
-        assertEquals(25.0, metricsMock.getMetric(MetricsReporter.deploymentFailMetric));
+        assertEquals(25.0, metrics.getMetric(MetricsReporter.deploymentFailMetric));
     }
 
     @Test
-    public void it_omits_zone_when_unknown() throws IOException {
+    public void it_omits_zone_when_unknown() {
         ControllerTester tester = new ControllerTester();
         String hostname = "fake-node2.test";
-        MapContext metricContext = getMetricsForHost(tester.controller(), hostname);
+        MapContext metricContext = getMetricContextByHost(tester.controller(), hostname);
         assertNull(metricContext.getDimensions().get("zone"));
     }
 
+    @Test
+    public void test_deployment_average_duration() {
+        DeploymentTester tester = new DeploymentTester();
+        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
+                .environment(Environment.prod)
+                .region("us-west-1")
+                .build();
+
+        MetricsReporter reporter = createReporter(tester.controller(), metrics, SystemName.cd);
+
+        Application app = tester.createApplication("app1", "tenant1", 1, 11L);
+        tester.deployCompletely(app, applicationPackage);
+        reporter.maintain();
+        assertEquals(Duration.ZERO, getAverageDeploymentDuration(app)); // An exceptionally fast deployment :-)
+
+        // App spends 3 hours deploying
+        tester.jobCompletion(component).application(app).nextBuildNumber().uploadArtifact(applicationPackage).submit();
+        tester.clock().advance(Duration.ofHours(1));
+        tester.deployAndNotify(app, applicationPackage, true, systemTest);
+
+        tester.clock().advance(Duration.ofMinutes(30));
+        tester.deployAndNotify(app, applicationPackage, true, stagingTest);
+
+        tester.clock().advance(Duration.ofMinutes(90));
+        tester.deployAndNotify(app, applicationPackage, true, productionUsWest1);
+        reporter.maintain();
+
+        // Average time is 1 hour
+        assertEquals(Duration.ofHours(1), getAverageDeploymentDuration(app));
+
+        // Another deployment starts and stalls for 12 hours
+        tester.jobCompletion(component).application(app).nextBuildNumber(2).uploadArtifact(applicationPackage).submit();
+        tester.clock().advance(Duration.ofHours(12));
+        reporter.maintain();
+
+        assertEquals(Duration.ofHours(12) // hanging system-test
+                             .plus(Duration.ofMinutes(30)) // previous staging-test
+                             .plus(Duration.ofMinutes(90)) // previous production job
+                             .dividedBy(3), // Total number of orchestrated jobs
+                     getAverageDeploymentDuration(app));
+    }
+
+    private Duration getAverageDeploymentDuration(Application application) {
+        return metrics.getMetric((dimension, value) -> dimension.equals("application") &&
+                                                       value.equals(application.id().toString()),
+                                 MetricsReporter.deploymentAverageDuration)
+                      .map(seconds -> Duration.ofSeconds(seconds.longValue()))
+                      .orElseThrow(() -> new RuntimeException("Expected metric to exist for " + application.id()));
+    }
+
     private void assertDimension(MapContext metricContext, String dimensionName, String expectedValue) {
         assertEquals(expectedValue, metricContext.getDimensions().get(dimensionName));
     }
 
-    private MetricsReporter setupMetricsReporter(Controller controller, MetricsMock metricsMock, SystemName system) throws IOException {
+    private MetricsReporter createReporter(Controller controller, MetricsMock metricsMock, SystemName system) {
+        return createReporter(controller.clock(), controller, metricsMock, system);
+    }
+
+    private MetricsReporter createReporter(Clock clock, Controller controller, MetricsMock metricsMock,
+                                           SystemName system) {
         Chef client = Mockito.mock(Chef.class);
-        PartialNodeResult result = new ObjectMapper()
-                .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
-                .readValue(testData.resolve("chef_output.json").toFile(), PartialNodeResult.class);
+        PartialNodeResult result;
+        try {
+            result = new ObjectMapper()
+                    .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
+                    .readValue(testData.resolve("chef_output.json").toFile(), PartialNodeResult.class);
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
         when(client.partialSearchNodes(anyString(), anyListOf(AttributeMapping.class))).thenReturn(result);
 
-        Clock clock = Clock.fixed(Instant.ofEpochSecond(1475497913), ZoneId.systemDefault());
+        return new MetricsReporter(controller, metricsMock, client, clock, new JobControl(new MockCuratorDb()), system);
+    }
 
-        return new MetricsReporter(controller, metricsMock, client, clock,
-                                   new JobControl(new MockCuratorDb()), system);
+    private Map<MapContext, Map<String, Number>> getMetricsByHost(String hostname) {
+        return metrics.getMetrics((dimension, value) -> dimension.equals("host") && value.equals(hostname));
     }
     
-    private MapContext getMetricsForHost(Controller controller, String hostname) throws IOException {
-        MetricsMock metricsMock = new MetricsMock();
-        MetricsReporter metricsReporter = setupMetricsReporter(controller, metricsMock, SystemName.main);
+    private MapContext getMetricContextByHost(Controller controller, String hostname) {
+        MetricsReporter metricsReporter = createReporter(controller, metrics, SystemName.main);
         metricsReporter.maintain();
 
-        assertFalse(metricsMock.getMetrics().isEmpty());
+        assertFalse(metrics.getMetrics().isEmpty());
 
-        Map<MapContext, Map<String, Number>> metrics = metricsMock.getMetricsFilteredByHost(hostname);
+        Map<MapContext, Map<String, Number>> metrics = getMetricsByHost(hostname);
         assertEquals(1, metrics.size());
         Map.Entry<MapContext, Map<String, Number>> metricEntry = metrics.entrySet().iterator().next();
         return metricEntry.getKey();
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java
index 4ee9d50a3f7..12fb2b6c862 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java
@@ -49,7 +49,7 @@ public class OutstandingChangeDeployerTest {
         tester.jobCompletion(DeploymentJobs.JobType.component)
               .application(tester.application("app1"))
               .sourceRevision(new SourceRevision("repository1","master", "cafed00d"))
-              .buildNumber(43)
+              .nextBuildNumber()
               .uploadArtifact(applicationPackage)
               .submit();
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
index 09836738af0..4481c1201d8 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
@@ -532,12 +532,12 @@ public class UpgraderTest {
 
         // Multiple application changes are triggered and fail, but does not affect version confidence as upgrade has
         // completed successfully
-        tester.jobCompletion(component).application(default0).buildNumber(43).uploadArtifact(canaryPolicy).unsuccessful().submit();
-        tester.jobCompletion(component).application(default1).buildNumber(43).uploadArtifact(canaryPolicy).unsuccessful().submit();
-        tester.jobCompletion(component).application(default2).buildNumber(43).uploadArtifact(defaultPolicy).submit();
-        tester.jobCompletion(component).application(default3).buildNumber(43).uploadArtifact(defaultPolicy).submit();
-        tester.jobCompletion(component).application(default2).buildNumber(44).uploadArtifact(canaryPolicy).unsuccessful().submit();
-        tester.jobCompletion(component).application(default3).buildNumber(44).uploadArtifact(canaryPolicy).unsuccessful().submit();
+        tester.jobCompletion(component).application(default0).nextBuildNumber().uploadArtifact(canaryPolicy).unsuccessful().submit();
+        tester.jobCompletion(component).application(default1).nextBuildNumber().uploadArtifact(canaryPolicy).unsuccessful().submit();
+        tester.jobCompletion(component).application(default2).nextBuildNumber().uploadArtifact(defaultPolicy).submit();
+        tester.jobCompletion(component).application(default3).nextBuildNumber().uploadArtifact(defaultPolicy).submit();
+        tester.jobCompletion(component).application(default2).nextBuildNumber().uploadArtifact(canaryPolicy).unsuccessful().submit();
+        tester.jobCompletion(component).application(default3).nextBuildNumber(2).uploadArtifact(canaryPolicy).unsuccessful().submit();
         tester.updateVersionStatus(version);
         assertEquals(VespaVersion.Confidence.normal, tester.controller().versionStatus().systemVersion().get().confidence());
     }
@@ -869,7 +869,7 @@ public class UpgraderTest {
         tester.deployAndNotify(app, applicationPackage, false, productionUsWest1);
 
         // New application change
-        tester.jobCompletion(component).application(app).buildNumber(43).uploadArtifact(applicationPackage).submit();
+        tester.jobCompletion(component).application(app).nextBuildNumber().uploadArtifact(applicationPackage).submit();
         String applicationVersion = "1.0.43-commit1";
 
         // Application change recorded together with ongoing upgrade