Remove metrics, use provided CoredumpHandler

3 files changed, 23 insertions, 57 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java
index 236415d1bcd..21c222d7e60 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java
@@ -9,9 +9,6 @@ import com.yahoo.config.provision.NodeType;
 import com.yahoo.io.IOUtils;
 import com.yahoo.system.ProcessExecuter;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
-import com.yahoo.vespa.hosted.dockerapi.metrics.CounterWrapper;
-import com.yahoo.vespa.hosted.dockerapi.metrics.Dimensions;
-import com.yahoo.vespa.hosted.dockerapi.metrics.GaugeWrapper;
 import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking;
@@ -53,38 +50,33 @@ public class StorageMaintainer {
     private static final ContainerName NODE_ADMIN = new ContainerName("node-admin");
     private static final ObjectMapper objectMapper = new ObjectMapper();
 
-    private final GaugeWrapper numberOfCoredumpsOnHost;
-    private final CounterWrapper numberOfNodeAdminMaintenanceFails;
     private final DockerOperations dockerOperations;
     private final ProcessExecuter processExecuter;
     private final Environment environment;
-    private final Optional<CoredumpHandler> coredumpHandler;
+    private final CoredumpHandler coredumpHandler;
     private final Clock clock;
 
     private final Map<ContainerName, MaintenanceThrottler> maintenanceThrottlerByContainerName = new ConcurrentHashMap<>();
 
     public StorageMaintainer(DockerOperations dockerOperations, ProcessExecuter processExecuter,
-                             MetricReceiverWrapper metricReceiver, Environment environment, Clock clock) {
-        this(dockerOperations, processExecuter, metricReceiver, environment, null, clock);
+                             MetricReceiverWrapper metricReceiver, Environment environment,
+                             CoredumpHandler coredumpHandler, Clock clock) {
+        this(dockerOperations, processExecuter, environment, coredumpHandler, clock);
     }
 
     public StorageMaintainer(DockerOperations dockerOperations, ProcessExecuter processExecuter,
-                             MetricReceiverWrapper metricReceiver, Environment environment,
+                             Environment environment, CoredumpHandler coredumpHandler) {
+        this(dockerOperations, processExecuter, environment, coredumpHandler, Clock.systemUTC());
+    }
+
+    public StorageMaintainer(DockerOperations dockerOperations, ProcessExecuter processExecuter,
+                             Environment environment,
                              CoredumpHandler coredumpHandler, Clock clock) {
         this.dockerOperations = dockerOperations;
         this.processExecuter = processExecuter;
         this.environment = environment;
-        this.coredumpHandler = Optional.ofNullable(coredumpHandler);
+        this.coredumpHandler = coredumpHandler;
         this.clock = clock;
-
-        Dimensions dimensions = new Dimensions.Builder()
-                .add("role", SecretAgentCheckConfig.nodeTypeToRole(environment.getNodeType()))
-                .build();
-        numberOfNodeAdminMaintenanceFails = metricReceiver.declareCounter(MetricReceiverWrapper.APPLICATION_DOCKER, dimensions, "nodes.maintenance.fails");
-        numberOfCoredumpsOnHost = metricReceiver.declareGauge(MetricReceiverWrapper.APPLICATION_DOCKER, dimensions, "nodes.coredumps");
-
-        metricReceiver.declareCounter(MetricReceiverWrapper.APPLICATION_DOCKER, dimensions, "nodes.running_on_host")
-                .add(environment.isRunningOnHost() ? 1 : 0);
     }
 
     public void writeMetricsConfig(ContainerName containerName, NodeSpec node) {
@@ -296,13 +288,6 @@ public class StorageMaintainer {
      * Checks if container has any new coredumps, reports and archives them if so
      */
     public void handleCoreDumpsForContainer(ContainerName containerName, NodeSpec node) {
-        // Sample number of coredumps on the host
-        try (Stream<Path> files = Files.list(environment.pathInNodeAdminToDoneCoredumps())) {
-            numberOfCoredumpsOnHost.sample(files.count());
-        } catch (IOException e) {
-            // Ignore for now - this is either test or a misconfiguration
-        }
-
         MaintainerExecutor maintainerExecutor = new MaintainerExecutor();
         addHandleCoredumpsCommand(maintainerExecutor, containerName, node);
         maintainerExecutor.execute();
@@ -316,21 +301,10 @@ public class StorageMaintainer {
         final Path coredumpsPath = environment.pathInNodeAdminFromPathInNode(
                 containerName, environment.pathInNodeUnderVespaHome("var/crash"));
         final Map<String, Object> nodeAttributes = getCoredumpNodeAttributes(node);
-        if (coredumpHandler.isPresent()) {
-            try {
-                coredumpHandler.get().processAll(coredumpsPath, nodeAttributes);
-            } catch (IOException e) {
-                throw new UncheckedIOException("Failed to process coredumps", e);
-            }
-        } else {
-            // Core dump handling is disabled.
-            if (!environment.getCoredumpFeedEndpoint().isPresent()) return;
-
-            maintainerExecutor.addJob("handle-core-dumps")
-                    .withArgument("coredumpsPath", coredumpsPath)
-                    .withArgument("doneCoredumpsPath", environment.pathInNodeAdminToDoneCoredumps())
-                    .withArgument("attributes", nodeAttributes)
-                    .withArgument("feedEndpoint", environment.getCoredumpFeedEndpoint().get());
+        try {
+            coredumpHandler.processAll(coredumpsPath, nodeAttributes);
+        } catch (IOException e) {
+            throw new UncheckedIOException("Failed to process coredumps", e);
         }
     }
 
@@ -452,7 +426,6 @@ public class StorageMaintainer {
             Pair<Integer, String> result = processExecuter.exec(command);
 
             if (result.getFirst() != 0) {
-                numberOfNodeAdminMaintenanceFails.add();
                 throw new RuntimeException(
                         String.format("Maintainer failed to execute command: %s, Exit code: %d, Stdout/stderr: %s",
                                 Arrays.toString(command), result.getFirst(), result.getSecond()));
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java
index 6b7d545c286..3bd380a73a2 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/StorageMaintainerMock.java
@@ -1,18 +1,19 @@
 // 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.node.admin.integrationTests;
 
-import com.yahoo.metrics.simple.MetricReceiver;
 import com.yahoo.system.ProcessExecuter;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
-import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.maintenance.StorageMaintainer;
 import com.yahoo.vespa.hosted.node.admin.component.Environment;
+import com.yahoo.vespa.hosted.node.admin.maintenance.coredump.CoredumpHandler;
 
 import java.time.Clock;
 import java.util.Optional;
 
+import static org.mockito.Mockito.mock;
+
 /**
  * @author freva
  */
@@ -20,7 +21,7 @@ public class StorageMaintainerMock extends StorageMaintainer {
     private final CallOrderVerifier callOrderVerifier;
 
     public StorageMaintainerMock(DockerOperations dockerOperations, ProcessExecuter processExecuter, Environment environment, CallOrderVerifier callOrderVerifier, Clock clock) {
-        super(dockerOperations, processExecuter, new MetricReceiverWrapper(MetricReceiver.nullImplementation), environment, clock);
+        super(dockerOperations, processExecuter, environment, mock(CoredumpHandler.class), clock);
         this.callOrderVerifier = callOrderVerifier;
     }
 
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
index d9cce7f80a0..a248b457e95 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
@@ -3,17 +3,16 @@ package com.yahoo.vespa.hosted.node.admin.maintenance;
 
 import com.yahoo.collections.Pair;
 import com.yahoo.config.provision.NodeType;
-import com.yahoo.metrics.simple.MetricReceiver;
 import com.yahoo.system.ProcessExecuter;
 import com.yahoo.test.ManualClock;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
-import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerNetworking;
 import com.yahoo.vespa.hosted.node.admin.docker.DockerOperations;
 import com.yahoo.vespa.hosted.node.admin.component.Environment;
 import com.yahoo.vespa.hosted.node.admin.component.PathResolver;
+import com.yahoo.vespa.hosted.node.admin.maintenance.coredump.CoredumpHandler;
 import com.yahoo.vespa.hosted.provision.Node;
 import org.junit.Rule;
 import org.junit.Test;
@@ -50,7 +49,7 @@ public class StorageMaintainerTest {
     private final DockerOperations docker = mock(DockerOperations.class);
     private final ProcessExecuter processExecuter = mock(ProcessExecuter.class);
     private final StorageMaintainer storageMaintainer = new StorageMaintainer(docker, processExecuter,
-            new MetricReceiverWrapper(MetricReceiver.nullImplementation), environment, clock);
+            environment, mock(CoredumpHandler.class), clock);
 
     @Rule
     public TemporaryFolder folder = new TemporaryFolder();
@@ -98,18 +97,11 @@ public class StorageMaintainerTest {
         storageMaintainer.removeOldFilesFromNode(containerName);
         verifyProcessExecuterCalled(2);
 
-        // Coredump handling is unthrottled
-        storageMaintainer.handleCoreDumpsForContainer(containerName, node);
-        verifyProcessExecuterCalled(3);
-
-        storageMaintainer.handleCoreDumpsForContainer(containerName, node);
-        verifyProcessExecuterCalled(4);
-
         // cleanupNodeStorage is unthrottled and it should reset previous times
         storageMaintainer.cleanupNodeStorage(containerName, node);
-        verifyProcessExecuterCalled(5);
+        verifyProcessExecuterCalled(3);
         storageMaintainer.cleanupNodeStorage(containerName, node);
-        verifyProcessExecuterCalled(6);
+        verifyProcessExecuterCalled(4);
     }
 
     @Test