diff options
author | Martin Polden <mpolden@mpolden.no> | 2019-07-03 11:38:41 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-07-03 11:38:41 +0200 |
commit | f2a8e61f0cb05e4694573b65b668184d1cd6b25f (patch) | |
tree | c99ca3686d741148a61e54f76bf3356c8af55a0e | |
parent | 347e4be990263ae8c4c2020bb10de3265ff92b68 (diff) | |
parent | 5407c03b494d907d16946966cc06179f160871d0 (diff) |
Merge pull request #9945 from vespa-engine/mpolden/remove-chef-metric
Stop reporting Chef convergence metric
8 files changed, 18 insertions, 238 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/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/test-config.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/test-config.json deleted file mode 100644 index fef3cf6a372..00000000000 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/test-config.json +++ /dev/null @@ -1,21 +0,0 @@ -{ - "application": "tenant1:application1:default", - "zone": "dev.us-east-1", - "system": "main", - "endpoints": { - "dev.us-east-1": [ - "http://old-endpoint.vespa.yahooapis.com:4080" - ], - "prod.us-central-1": [ - "http://old-endpoint.vespa.yahooapis.com:4080" - ] - }, - "zoneEndpoints": { - "dev.us-east-1": { - "default": "http://old-endpoint.vespa.yahooapis.com:4080" - }, - "prod.us-central-1": { - "default": "http://old-endpoint.vespa.yahooapis.com:4080" - } - } -} 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 - } - } - ] -} diff --git a/controller-server/src/test/resources/job-grandparent.json b/controller-server/src/test/resources/job-grandparent.json deleted file mode 100644 index 63602bed146..00000000000 --- a/controller-server/src/test/resources/job-grandparent.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "duration": 720000, - "causes": [] -} diff --git a/controller-server/src/test/resources/job-parent.json b/controller-server/src/test/resources/job-parent.json deleted file mode 100644 index 88d50de394f..00000000000 --- a/controller-server/src/test/resources/job-parent.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "duration": 1200000, - "causes": [ - { - "upstreamBuild": 231, - "upstreamProject": "3-v3-job-grandparent" - } - ] -} |