Stop reporting Chef convergence metric

5 files changed, 18 insertions, 204 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
index e840deb062c..f34c24c497a 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
@@ -5,15 +5,14 @@ import com.yahoo.component.AbstractComponent;
 import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.hosted.controller.Controller;
-import com.yahoo.vespa.hosted.controller.api.integration.organization.Billing;
-import com.yahoo.vespa.hosted.controller.api.integration.organization.ContactRetriever;
-import com.yahoo.vespa.hosted.controller.api.integration.resource.ResourceSnapshotConsumer;
-import com.yahoo.vespa.hosted.controller.authority.config.ApiAuthorityConfig;
-import com.yahoo.vespa.hosted.controller.api.integration.chef.Chef;
 import com.yahoo.vespa.hosted.controller.api.integration.dns.NameService;
 import com.yahoo.vespa.hosted.controller.api.integration.noderepository.NodeRepositoryClientInterface;
+import com.yahoo.vespa.hosted.controller.api.integration.organization.Billing;
+import com.yahoo.vespa.hosted.controller.api.integration.organization.ContactRetriever;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.DeploymentIssues;
 import com.yahoo.vespa.hosted.controller.api.integration.organization.OwnershipIssues;
+import com.yahoo.vespa.hosted.controller.api.integration.resource.ResourceSnapshotConsumer;
+import com.yahoo.vespa.hosted.controller.authority.config.ApiAuthorityConfig;
 import com.yahoo.vespa.hosted.controller.maintenance.config.MaintainerConfig;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.controller.restapi.cost.CostReportConsumer;
@@ -59,7 +58,7 @@ public class ControllerMaintenance extends AbstractComponent {
 
     @SuppressWarnings("unused") // instantiated by Dependency Injection
     public ControllerMaintenance(MaintainerConfig maintainerConfig, ApiAuthorityConfig apiAuthorityConfig, Controller controller, CuratorDb curator,
-                                 JobControl jobControl, Metric metric, Chef chefClient,
+                                 JobControl jobControl, Metric metric,
                                  DeploymentIssues deploymentIssues, OwnershipIssues ownershipIssues,
                                  NameService nameService, NodeRepositoryClientInterface nodeRepositoryClient,
                                  ContactRetriever contactRetriever,
@@ -71,7 +70,7 @@ public class ControllerMaintenance extends AbstractComponent {
         this.jobControl = jobControl;
         deploymentExpirer = new DeploymentExpirer(controller, maintenanceInterval, jobControl);
         deploymentIssueReporter = new DeploymentIssueReporter(controller, deploymentIssues, maintenanceInterval, jobControl);
-        metricsReporter = new MetricsReporter(controller, metric, chefClient, jobControl, controller.system());
+        metricsReporter = new MetricsReporter(controller, metric, jobControl);
         outstandingChangeDeployer = new OutstandingChangeDeployer(controller, Duration.ofMinutes(1), jobControl);
         versionStatusUpdater = new VersionStatusUpdater(controller, Duration.ofMinutes(1), jobControl);
         upgrader = new Upgrader(controller, maintenanceInterval, jobControl, curator);
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 5e3f21c6b98..c7b76696d84 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
@@ -3,14 +3,9 @@ package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.google.common.collect.ImmutableMap;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.config.provision.SystemName;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
-import com.yahoo.vespa.hosted.controller.api.integration.chef.AttributeMapping;
-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.api.integration.deployment.JobType;
 import com.yahoo.vespa.hosted.controller.application.ApplicationList;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
@@ -24,10 +19,8 @@ import java.time.Duration;
 import java.time.Instant;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.stream.Collectors;
 
 /**
@@ -36,7 +29,6 @@ import java.util.stream.Collectors;
  */
 public class MetricsReporter extends Maintainer {
 
-    public static final String CONVERGENCE_METRIC = "seconds.since.last.chef.convergence";
     public static final String DEPLOYMENT_FAIL_METRIC = "deployment.failurePercentage";
     public static final String DEPLOYMENT_AVERAGE_DURATION = "deployment.averageDuration";
     public static final String DEPLOYMENT_FAILING_UPGRADES = "deployment.failingUpgrades";
@@ -46,27 +38,16 @@ public class MetricsReporter extends Maintainer {
     public static final String NAME_SERVICE_REQUESTS_QUEUED = "dns.queuedRequests";
 
     private final Metric metric;
-    private final Chef chefClient;
     private final Clock clock;
-    private final SystemName system;
 
-    public MetricsReporter(Controller controller, Metric metric, Chef chefClient, JobControl jobControl,
-                           SystemName system) {
-        this(controller, metric, chefClient, Clock.systemUTC(), jobControl, system);
-    }
-
-    public MetricsReporter(Controller controller, Metric metric, Chef chefClient, Clock clock,
-                           JobControl jobControl, SystemName system) {
+    public MetricsReporter(Controller controller, Metric metric, JobControl jobControl) {
         super(controller, Duration.ofMinutes(1), jobControl); // use fixed rate for metrics
         this.metric = metric;
-        this.chefClient = chefClient;
-        this.clock = clock;
-        this.system = system;
+        this.clock = controller.clock();
     }
 
     @Override
     public void maintain() {
-        reportChefMetrics();
         reportDeploymentMetrics();
         reportRemainingRotations();
         reportQueuedNameServiceRequests();
@@ -79,49 +60,6 @@ public class MetricsReporter extends Maintainer {
         }
     }
 
-    private void reportChefMetrics() {
-        String query = "chef_environment:hosted*";
-        if (system == SystemName.cd) {
-            query += " AND hosted_system:" + system;
-        }
-        PartialNodeResult nodeResult = chefClient.partialSearchNodes(query,
-                List.of(
-                        AttributeMapping.simpleMapping("fqdn"),
-                        AttributeMapping.simpleMapping("ohai_time"),
-                        AttributeMapping.deepMapping("tenant", List.of("hosted", "owner", "tenant")),
-                        AttributeMapping.deepMapping("application", List.of("hosted", "owner", "application")),
-                        AttributeMapping.deepMapping("instance", List.of("hosted", "owner", "instance")),
-                        AttributeMapping.deepMapping("environment", List.of("hosted", "environment")),
-                        AttributeMapping.deepMapping("region", List.of("hosted", "region")),
-                        AttributeMapping.deepMapping("system", List.of("hosted", "system"))
-                ));
-
-        // The above search will return a correct list if the system is CD. However for main, it will
-        // return all nodes, since system==nil for main
-        keepNodesWithSystem(nodeResult, system);
-        
-        Instant instant = clock.instant();
-        for (PartialNode node : nodeResult.rows) {
-            String hostname = node.getFqdn();
-            long secondsSinceConverge = Duration.between(Instant.ofEpochSecond(node.getOhaiTime().longValue()), instant).getSeconds();
-            Map<String, String> dimensions = new HashMap<>();
-            dimensions.put("host", hostname);
-            dimensions.put("system", node.getValue("system").orElse("main"));
-            Optional<String> environment = node.getValue("environment");
-            Optional<String> region = node.getValue("region");
-
-            if (environment.isPresent() && region.isPresent()) {
-                dimensions.put("zone", String.format("%s.%s", environment.get(), region.get()));
-            }
-
-            node.getValue("tenant").ifPresent(tenant -> dimensions.put("tenantName", tenant));
-            Optional<String> application = node.getValue("application");
-            application.ifPresent(app -> dimensions.put("app", String.format("%s.%s", app, node.getValue("instance").orElse("default"))));
-            Metric.Context context = metric.createContext(dimensions);
-            metric.set(CONVERGENCE_METRIC, secondsSinceConverge, context);
-        }
-    }
-
     private void reportDeploymentMetrics() {
         ApplicationList applications = ApplicationList.from(controller().applications().asList())
                                                       .hasProductionDeployment();
@@ -210,10 +148,6 @@ public class MetricsReporter extends Maintainer {
                           .max(Integer::compareTo)
                           .orElse(0);
     }
-    
-    private static void keepNodesWithSystem(PartialNodeResult nodeResult, SystemName system) {
-        nodeResult.rows.removeIf(node -> !system.value().equals(node.getValue("system").orElse("main")));
-    }
 
     private static Map<String, String> dimensions(ApplicationId application) {
         return ImmutableMap.of(
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 58f35c0ac05..148be3f258e 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
@@ -1,38 +1,23 @@
 // 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.fasterxml.jackson.databind.DeserializationFeature;
-import com.fasterxml.jackson.databind.ObjectMapper;
 import com.yahoo.component.Version;
 import com.yahoo.config.provision.Environment;
-import com.yahoo.config.provision.SystemName;
-import com.yahoo.test.ManualClock;
+import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.ControllerTester;
 import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
-import com.yahoo.vespa.hosted.controller.api.integration.chef.ChefMock;
-import com.yahoo.vespa.hosted.controller.api.integration.chef.rest.PartialNodeResult;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.ApplicationVersion;
-import com.yahoo.config.provision.zone.ZoneId;
 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.deployment.InternalDeploymentTester;
 import com.yahoo.vespa.hosted.controller.integration.MetricsMock;
-import com.yahoo.vespa.hosted.controller.integration.MetricsMock.MapContext;
 import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb;
-import org.junit.Before;
 import org.junit.Test;
 
-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.util.Map;
 
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.component;
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.productionUsWest1;
@@ -40,39 +25,13 @@ import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobTy
 import static com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType.systemTest;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNull;
 
 /**
  * @author mortent
  */
 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() {
-        Clock clock = new ManualClock(Instant.ofEpochSecond(1475497913));
-        ControllerTester tester = new ControllerTester();
-        MetricsReporter metricsReporter = createReporter(clock, tester.controller(), metrics, SystemName.cd);
-        metricsReporter.maintain();
-        assertEquals(2, metrics.getMetrics().size());
-
-        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");
-        assertDimension(metricContext, "zone", "prod.cd-us-east-1");
-        assertEquals(727, metricEntry.getValue().get(MetricsReporter.CONVERGENCE_METRIC).longValue());
-    }
+    private final MetricsMock metrics = new MetricsMock();
 
     @Test
     public void test_deployment_fail_ratio() {
@@ -81,7 +40,7 @@ public class MetricsReporterTest {
                 .environment(Environment.prod)
                 .region("us-west-1")
                 .build();
-        MetricsReporter metricsReporter = createReporter(tester.controller(), metrics, SystemName.main);
+        MetricsReporter metricsReporter = createReporter(tester.controller());
 
         metricsReporter.maintain();
         assertEquals(0.0, metrics.getMetric(MetricsReporter.DEPLOYMENT_FAIL_METRIC));
@@ -108,14 +67,6 @@ public class MetricsReporterTest {
     }
 
     @Test
-    public void test_chef_metrics_omit_zone_when_unknown() {
-        ControllerTester tester = new ControllerTester();
-        String hostname = "fake-node2.test";
-        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()
@@ -123,7 +74,7 @@ public class MetricsReporterTest {
                 .region("us-west-1")
                 .build();
 
-        MetricsReporter reporter = createReporter(tester.controller(), metrics, SystemName.main);
+        MetricsReporter reporter = createReporter(tester.controller());
 
         Application app = tester.createApplication("app1", "tenant1", 1, 11L);
         tester.deployCompletely(app, applicationPackage);
@@ -165,7 +116,7 @@ public class MetricsReporterTest {
                 .region("us-west-1")
                 .build();
 
-        MetricsReporter reporter = createReporter(tester.controller(), metrics, SystemName.main);
+        MetricsReporter reporter = createReporter(tester.controller());
         Application app = tester.createApplication("app1", "tenant1", 1, 11L);
 
         // Initial deployment without failures
@@ -216,7 +167,7 @@ public class MetricsReporterTest {
                 .region("us-west-1")
                 .region("us-east-3")
                 .build();
-        MetricsReporter reporter = createReporter(tester.controller(), metrics, SystemName.main);
+        MetricsReporter reporter = createReporter(tester.controller());
         Application application = tester.createApplication("app1", "tenant1", 1, 11L);
         tester.configServer().generateWarnings(new DeploymentId(application.id(), ZoneId.from("prod", "us-west-1")), 3);
         tester.configServer().generateWarnings(new DeploymentId(application.id(), ZoneId.from("prod", "us-east-3")), 4);
@@ -231,7 +182,7 @@ public class MetricsReporterTest {
         ApplicationVersion version = tester.deployNewSubmission();
         assertEquals(1000, version.buildTime().get().toEpochMilli());
 
-        MetricsReporter reporter = createReporter(tester.tester().controller(), metrics, SystemName.main);
+        MetricsReporter reporter = createReporter(tester.tester().controller());
         reporter.maintain();
         assertEquals(tester.clock().instant().getEpochSecond() - 1,
                      getMetric(MetricsReporter.DEPLOYMENT_BUILD_AGE_SECONDS, tester.app()));
@@ -246,7 +197,7 @@ public class MetricsReporterTest {
                 .region("us-west-1")
                 .region("us-east-3")
                 .build();
-        MetricsReporter reporter = createReporter(tester.controller(), metrics, SystemName.main);
+        MetricsReporter reporter = createReporter(tester.controller());
         Application application = tester.createApplication("app1", "tenant1", 1, 11L);
         reporter.maintain();
         assertEquals("Queue is empty initially", 0, metrics.getMetric(MetricsReporter.NAME_SERVICE_REQUESTS_QUEUED).intValue());
@@ -279,43 +230,8 @@ public class MetricsReporterTest {
                       .orElseThrow(() -> new RuntimeException("Expected metric to exist for " + application.id()));
     }
 
-    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) {
-        ChefMock chef = new ChefMock();
-        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);
-        }
-        chef.addPartialResult(result.rows);
-        return new MetricsReporter(controller, metricsMock, chef, clock, new JobControl(new MockCuratorDb()), system);
-    }
-
-    private Map<MapContext, Map<String, Number>> getMetricsByHost(String hostname) {
-        return metrics.getMetrics((dimensions) -> hostname.equals(dimensions.get("host")));
-    }
-    
-    private MapContext getMetricContextByHost(Controller controller, String hostname) {
-        MetricsReporter metricsReporter = createReporter(controller, metrics, SystemName.main);
-        metricsReporter.maintain();
-
-        assertFalse(metrics.getMetrics().isEmpty());
-
-        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();
-    }
-
-    private static void assertDimension(MapContext metricContext, String dimensionName, String expectedValue) {
-        assertEquals(expectedValue, metricContext.getDimensions().get(dimensionName));
+    private MetricsReporter createReporter(Controller controller) {
+        return new MetricsReporter(controller, metrics, new JobControl(new MockCuratorDb()));
     }
 
     private static String appDimension(Application application) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java
index 53476a2e42f..797d2b9aa0e 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ControllerContainerTest.java
@@ -62,7 +62,6 @@ public class ControllerContainerTest {
                "  <component id='com.yahoo.vespa.flags.InMemoryFlagSource'/>\n" +
                "  <component id='com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb'/>\n" +
                "  <component id='com.yahoo.vespa.hosted.controller.athenz.mock.AthenzClientFactoryMock'/>\n" +
-               "  <component id='com.yahoo.vespa.hosted.controller.api.integration.chef.ChefMock'/>\n" +
                "  <component id='com.yahoo.vespa.hosted.controller.api.integration.dns.MemoryNameService'/>\n" +
                "  <component id='com.yahoo.vespa.hosted.controller.api.integration.entity.MemoryEntityService'/>\n" +
                "  <component id='com.yahoo.vespa.hosted.controller.api.integration.github.GitHubMock'/>\n" +
diff --git a/controller-server/src/test/resources/chef_output.json b/controller-server/src/test/resources/chef_output.json
deleted file mode 100644
index 257065f7b5b..00000000000
--- a/controller-server/src/test/resources/chef_output.json
+++ /dev/null
@@ -1,34 +0,0 @@
-{
-  "total": 1,
-  "start": 0,
-  "rows": [
-    {
-      "url": "https://chef-server.test/organizations/vespa/nodes/fake-node.test",
-      "data": {
-        "fqdn": "fake-node.test",
-        "ohai_time": 1475497186.68962,
-        "tenant": "ciintegrationtests",
-        "application": "restart",
-        "instance": "default",
-        "zone": "cd_cd-us-east-1_prod",
-        "system": "cd",
-        "environment": "prod",
-        "region": "cd-us-east-1"
-      }
-    },
-    {
-      "url": "https://chef-server.test/organizations/vespa/nodes/fake-node2.test",
-      "data": {
-        "fqdn": "fake-node2.test",
-        "ohai_time": 1475497186.68962,
-        "tenant": null,
-        "application": null,
-        "instance": null,
-        "zone": null,
-        "system": null,
-        "environment": null,
-        "region": null
-      }
-    }
-  ]
-}