Stop using report-cores-via-cfg

5 files changed, 67 insertions, 233 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollector.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollector.java
index d99b45dbbbc..cfb2d240bfa 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollector.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollector.java
@@ -10,9 +10,7 @@ import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult;
 
 import java.nio.file.Path;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
@@ -32,9 +30,6 @@ public class CoreCollector {
     private static final Pattern FROM_PATH_PATTERN = Pattern.compile("^.* from '(?<path>.*?)'");
     static final String GDB_PATH_RHEL8 = "/opt/rh/gcc-toolset-11/root/bin/gdb";
 
-    static final Map<String, Object> JAVA_HEAP_DUMP_METADATA =
-            Map.of("bin_path", "java", "backtrace", List.of("Heap dump, no backtrace available"));
-
     private final ContainerOperations container;
 
     public CoreCollector(ContainerOperations container) {
@@ -100,36 +95,7 @@ public class CoreCollector {
         return List.of(result.getOutput().split("\n"));
     }
 
-    /**
-     * Collects metadata about a given core dump
-     * @param context context of the NodeAgent that owns the core dump
-     * @param coredumpPath path to core dump file inside the container
-     * @return map of relevant metadata about the core dump
-     */
-    Map<String, Object> collect(NodeAgentContext context, ContainerPath coredumpPath) {
-        if (JAVA_HEAP_DUMP_PATTERN.matcher(coredumpPath.getFileName().toString()).find())
-            return JAVA_HEAP_DUMP_METADATA;
-
-        Map<String, Object> data = new HashMap<>();
-        try {
-            String binPath = readBinPath(context, coredumpPath);
-
-            data.put("bin_path", binPath);
-            if (Path.of(binPath).getFileName().toString().equals("java")) {
-                data.put("backtrace_all_threads", readJstack(context, coredumpPath, binPath));
-            } else {
-                data.put("backtrace", readBacktrace(context, coredumpPath, binPath, false));
-                data.put("backtrace_all_threads", readBacktrace(context, coredumpPath, binPath, true));
-            }
-        } catch (ConvergenceException e) {
-            context.log(logger, Level.WARNING, "Failed to extract backtrace: " + e.getMessage());
-        } catch (RuntimeException e) {
-            context.log(logger, Level.WARNING, "Failed to extract backtrace", e);
-        }
-        return data;
-    }
-
-    CoreDumpMetadata collect2(NodeAgentContext context, ContainerPath coredumpPath) {
+    CoreDumpMetadata collect(NodeAgentContext context, ContainerPath coredumpPath) {
         var metadata = new CoreDumpMetadata();
 
         if (JAVA_HEAP_DUMP_PATTERN.matcher(coredumpPath.getFileName().toString()).find()) {
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java
index 4a8353d92ec..94f402d5332 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java
@@ -1,14 +1,10 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
 
-import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.yahoo.config.provision.DockerImage;
 import com.yahoo.security.KeyId;
 import com.yahoo.security.SecretSharedKey;
 import com.yahoo.security.SharedKeyGenerator;
-import com.yahoo.vespa.flags.BooleanFlag;
 import com.yahoo.vespa.flags.FetchVector;
 import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.flags.Flags;
@@ -26,19 +22,16 @@ import com.yahoo.vespa.hosted.node.admin.task.util.file.FileDeleter;
 import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder;
 import com.yahoo.vespa.hosted.node.admin.task.util.file.FileMover;
 import com.yahoo.vespa.hosted.node.admin.task.util.file.MakeDirectory;
-import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath;
 import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath;
 
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.UncheckedIOException;
-import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.time.Clock;
 import java.util.Comparator;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -61,64 +54,49 @@ import static com.yahoo.yolean.Exceptions.uncheck;
  */
 public class CoredumpHandler {
 
+    public static final String COREDUMP_FILENAME_PREFIX = "dump_";
+
+    private static final Logger logger = Logger.getLogger(CoredumpHandler.class.getName());
     private static final Pattern HS_ERR_PATTERN = Pattern.compile("hs_err_pid[0-9]+\\.log");
     private static final String PROCESSING_DIRECTORY_NAME = "processing";
-    private static final String METADATA_FILE_NAME = "metadata.json";
     private static final String METADATA2_FILE_NAME = "metadata2.json";
     private static final String COMPRESSED_EXTENSION = ".zst";
     private static final String ENCRYPTED_EXTENSION = ".enc";
-    public static final String COREDUMP_FILENAME_PREFIX = "dump_";
-
-    private final Logger logger = Logger.getLogger(CoredumpHandler.class.getName());
-    private final ObjectMapper objectMapper = new ObjectMapper();
 
     private final CoreCollector coreCollector;
     private final Cores cores;
-    private final CoredumpReporter coredumpReporter;
     private final String crashPatchInContainer;
     private final Path doneCoredumpsPath;
     private final Metrics metrics;
     private final Clock clock;
     private final Supplier<String> coredumpIdSupplier;
     private final SecretSharedKeySupplier secretSharedKeySupplier;
-    private final BooleanFlag reportCoresViaCfgFlag;
     private final StringFlag coreEncryptionPublicKeyIdFlag;
 
     /**
      * @param crashPathInContainer path inside the container where core dump are dumped
      * @param doneCoredumpsPath    path on host where processed core dumps are stored
      */
-    public CoredumpHandler(CoreCollector coreCollector, Cores cores, CoredumpReporter coredumpReporter,
-                           String crashPathInContainer, Path doneCoredumpsPath, Metrics metrics,
-                           FlagSource flagSource) {
-        this(coreCollector, cores, coredumpReporter, crashPathInContainer, doneCoredumpsPath,
-             metrics, Clock.systemUTC(), () -> UUID.randomUUID().toString(), (ctx) -> Optional.empty() /*TODO*/,
-             flagSource);
-    }
-
-    // TODO remove redundant constructor once internal callsite has been updated
-    public CoredumpHandler(CoreCollector coreCollector, Cores cores, CoredumpReporter coredumpReporter,
+    public CoredumpHandler(CoreCollector coreCollector, Cores cores,
                            String crashPathInContainer, Path doneCoredumpsPath, Metrics metrics,
                            SecretSharedKeySupplier secretSharedKeySupplier, FlagSource flagSource) {
-        this(coreCollector, cores, coredumpReporter, crashPathInContainer, doneCoredumpsPath,
+        this(coreCollector, cores, crashPathInContainer, doneCoredumpsPath,
                 metrics, Clock.systemUTC(), () -> UUID.randomUUID().toString(), secretSharedKeySupplier,
                 flagSource);
     }
 
-    CoredumpHandler(CoreCollector coreCollector, Cores cores, CoredumpReporter coredumpReporter,
+    CoredumpHandler(CoreCollector coreCollector, Cores cores,
                     String crashPathInContainer, Path doneCoredumpsPath, Metrics metrics,
                     Clock clock, Supplier<String> coredumpIdSupplier,
                     SecretSharedKeySupplier secretSharedKeySupplier, FlagSource flagSource) {
         this.coreCollector = coreCollector;
         this.cores = cores;
-        this.coredumpReporter = coredumpReporter;
         this.crashPatchInContainer = crashPathInContainer;
         this.doneCoredumpsPath = doneCoredumpsPath;
         this.metrics = metrics;
         this.clock = clock;
         this.coredumpIdSupplier = coredumpIdSupplier;
         this.secretSharedKeySupplier = secretSharedKeySupplier;
-        this.reportCoresViaCfgFlag = Flags.REPORT_CORES_VIA_CFG.bindTo(flagSource);
         this.coreEncryptionPublicKeyIdFlag = Flags.CORE_ENCRYPTION_PUBLIC_KEY_ID.bindTo(flagSource);
     }
 
@@ -143,13 +121,7 @@ public class CoredumpHandler {
 
         // Check if we have already started to process a core dump or we can enqueue a new core one
         getCoredumpToProcess(context, containerCrashPath, containerProcessingPath)
-                .ifPresent(path -> {
-                    if (reportCoresViaCfgFlag.with(FetchVector.Dimension.NODE_TYPE, context.nodeType().name()).value()) {
-                        processAndReportSingleCoreDump2(context, path, dockerImage);
-                    } else {
-                        processAndReportSingleCoredump(context, path, nodeAttributesSupplier);
-                    }
-                });
+                .ifPresent(path -> processAndReportSingleCoreDump(context, path, dockerImage));
     }
 
     /** @return path to directory inside processing directory that contains a core dump file to process */
@@ -212,65 +184,6 @@ public class CoredumpHandler {
         return coreEncryptionPublicKeyIdFlag.with(FetchVector.Dimension.NODE_TYPE, context.nodeType().name()).value();
     }
 
-    void processAndReportSingleCoredump(NodeAgentContext context, ContainerPath coredumpDirectory, Supplier<Map<String, Object>> nodeAttributesSupplier) {
-        try {
-            Optional<SecretSharedKey> sharedCoreKey = Optional.of(corePublicKeyFlagValue(context))
-                    .filter(k -> !k.isEmpty())
-                    .map(KeyId::ofString)
-                    .flatMap(secretSharedKeySupplier::create);
-            Optional<String> decryptionToken = sharedCoreKey.map(k -> k.sealedSharedKey().toTokenString());
-            String metadata = getMetadata(context, coredumpDirectory, nodeAttributesSupplier, decryptionToken);
-            String coredumpId = coredumpDirectory.getFileName().toString();
-            coredumpReporter.reportCoredump(coredumpId, metadata);
-            finishProcessing(context, coredumpDirectory, sharedCoreKey);
-            context.log(logger, "Successfully reported coredump " + coredumpId);
-        } catch (Exception e) {
-            throw new RuntimeException("Failed to process coredump " + coredumpDirectory, e);
-        }
-    }
-
-    /**
-     * @return coredump metadata from metadata.json if present, otherwise attempts to get metadata using
-     * {@link CoreCollector} and stores it to metadata.json
-     */
-    String getMetadata(NodeAgentContext context, ContainerPath coredumpDirectory, Supplier<Map<String, Object>> nodeAttributesSupplier, Optional<String> decryptionToken) throws IOException {
-        UnixPath metadataPath = new UnixPath(coredumpDirectory.resolve(METADATA_FILE_NAME));
-        if (!metadataPath.exists()) {
-            ContainerPath coredumpFile = findCoredumpFileInProcessingDirectory(coredumpDirectory);
-            Map<String, Object> metadata = new HashMap<>(coreCollector.collect(context, coredumpFile));
-            metadata.putAll(nodeAttributesSupplier.get());
-            metadata.put("coredump_path", doneCoredumpsPath
-                    .resolve(context.containerName().asString())
-                    .resolve(coredumpDirectory.getFileName().toString())
-                    .resolve(coredumpFile.getFileName().toString()).toString());
-            decryptionToken.ifPresent(token -> metadata.put("decryption_token", token));
-
-            String metadataFields = objectMapper.writeValueAsString(Map.of("fields", metadata));
-            metadataPath.writeUtf8File(metadataFields);
-            return metadataFields;
-        } else {
-            if (decryptionToken.isPresent()) {
-                // Since encryption keys are single-use and generated for each core dump processing invocation,
-                // we must ensure we store and report the token associated with the _latest_ (i.e. current)
-                // attempt at processing the core dump. Patch and rewrite the file with a new token, if present.
-                String metadataFields = metadataWithPatchedTokenValue(metadataPath, decryptionToken.get());
-                metadataPath.deleteIfExists();
-                metadataPath.writeUtf8File(metadataFields);
-                return metadataFields;
-            } else {
-                return metadataPath.readUtf8File();
-            }
-        }
-    }
-
-    private String metadataWithPatchedTokenValue(UnixPath metadataPath, String decryptionToken) throws JsonProcessingException {
-        var jsonRoot = objectMapper.readTree(metadataPath.readUtf8File());
-        if (jsonRoot.path("fields").isObject()) {
-            ((ObjectNode)jsonRoot.get("fields")).put("decryption_token", decryptionToken);
-        } // else: unit testing case without real metadata
-        return objectMapper.writeValueAsString(jsonRoot);
-    }
-
     static OutputStream maybeWrapWithEncryption(OutputStream wrappedStream, Optional<SecretSharedKey> sharedCoreKey) {
         return sharedCoreKey
                 .map(key -> SharedKeyGenerator.makeAesGcmEncryptionCipher(key).wrapOutputStream(wrappedStream))
@@ -372,8 +285,8 @@ public class CoredumpHandler {
         return clock.instant().minusSeconds(60).isAfter(fileAttributes.lastModifiedTime());
     }
 
-    void processAndReportSingleCoreDump2(NodeAgentContext context, ContainerPath coreDumpDirectory,
-                                         Optional<DockerImage> dockerImage) {
+    void processAndReportSingleCoreDump(NodeAgentContext context, ContainerPath coreDumpDirectory,
+                                        Optional<DockerImage> dockerImage) {
         CoreDumpMetadata metadata = gatherMetadata(context, coreDumpDirectory);
         dockerImage.ifPresent(metadata::setDockerImage);
         dockerImage.flatMap(DockerImage::tag).ifPresent(metadata::setVespaVersion);
@@ -390,20 +303,17 @@ public class CoredumpHandler {
         finishProcessing(context, coreDumpDirectory, sharedCoreKey);
     }
 
-    private CoreDumpMetadata gatherMetadata(NodeAgentContext context, ContainerPath coreDumpDirectory) {
+    CoreDumpMetadata gatherMetadata(NodeAgentContext context, ContainerPath coreDumpDirectory) {
         ContainerPath metadataPath = coreDumpDirectory.resolve(METADATA2_FILE_NAME);
         Optional<ReportCoreDumpRequest> request = ReportCoreDumpRequest.load(metadataPath);
         if (request.isPresent()) {
-            return request.map(requestInstance -> {
-                              var metadata = new CoreDumpMetadata();
-                              requestInstance.populateMetadata(metadata, FileSystems.getDefault());
-                              return metadata;
-                          })
-                          .get();
+            var metadata = new CoreDumpMetadata();
+            request.get().populateMetadata(metadata, doneCoredumpsPath.getFileSystem());
+            return metadata;
         }
 
         ContainerPath coreDumpFile = findCoredumpFileInProcessingDirectory(coreDumpDirectory);
-        CoreDumpMetadata metadata = coreCollector.collect2(context, coreDumpFile);
+        CoreDumpMetadata metadata = coreCollector.collect(context, coreDumpFile);
         metadata.setCpuMicrocodeVersion(getMicrocodeVersion())
                 .setKernelVersion(System.getProperty("os.version"))
                 .setCoreDumpPath(doneCoredumpsPath.resolve(context.containerName().asString())
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpReporter.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpReporter.java
deleted file mode 100644
index 0e2bb724f50..00000000000
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpReporter.java
+++ /dev/null
@@ -1,11 +0,0 @@
-// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
-
-/**
- * @author freva
- */
-public interface CoredumpReporter {
-
-    /** Report a coredump with a given ID and given metadata */
-    void reportCoredump(String id, String metadata);
-}
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java
index ea73fd4af6f..8f251bf72c5 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
 
+import com.yahoo.vespa.hosted.node.admin.configserver.cores.CoreDumpMetadata;
 import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl;
@@ -10,10 +11,8 @@ import com.yahoo.vespa.test.file.TestFileSystem;
 import org.junit.jupiter.api.Test;
 
 import java.util.List;
-import java.util.Map;
 
 import static com.yahoo.vespa.hosted.node.admin.maintenance.coredump.CoreCollector.GDB_PATH_RHEL8;
-import static com.yahoo.vespa.hosted.node.admin.maintenance.coredump.CoreCollector.JAVA_HEAP_DUMP_METADATA;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.fail;
 import static org.mockito.Mockito.mock;
@@ -23,7 +22,6 @@ import static org.mockito.Mockito.when;
  * @author freva
  */
 public class CoreCollectorTest {
-    private final String JDK_PATH = "/path/to/jdk/java";
     private final ContainerOperations docker = mock(ContainerOperations.class);
     private final CoreCollector coreCollector = new CoreCollector(docker);
     private final NodeAgentContext context = NodeAgentContextImpl.builder("container-123.domain.tld")
@@ -144,11 +142,10 @@ public class CoreCollectorTest {
                         "/usr/bin/program", "/tmp/core.1234"},
                 String.join("\n", GDB_BACKTRACE));
 
-        Map<String, Object> expectedData = Map.of(
-                "bin_path", TEST_BIN_PATH,
-                "backtrace", GDB_BACKTRACE,
-                "backtrace_all_threads", GDB_BACKTRACE);
-        assertEquals(expectedData, coreCollector.collect(context, TEST_CORE_PATH));
+        var expected = new CoreDumpMetadata().setBinPath(TEST_BIN_PATH)
+                                             .setBacktrace(GDB_BACKTRACE)
+                                             .setBacktraceAllThreads(GDB_BACKTRACE);
+        assertEquals(expected, coreCollector.collect(context, TEST_CORE_PATH));
     }
 
     @Test
@@ -159,8 +156,8 @@ public class CoreCollectorTest {
         mockExec(new String[]{GDB_PATH_RHEL8 + " -n -ex set print frame-arguments none -ex bt -batch /usr/bin/program /tmp/core.1234"},
                 "", "Failure");
 
-        Map<String, Object> expectedData = Map.of("bin_path", TEST_BIN_PATH);
-        assertEquals(expectedData, coreCollector.collect(context, TEST_CORE_PATH));
+        var expected = new CoreDumpMetadata().setBinPath(TEST_BIN_PATH);
+        assertEquals(expected, coreCollector.collect(context, TEST_CORE_PATH));
     }
 
     @Test
@@ -168,22 +165,25 @@ public class CoreCollectorTest {
         mockExec(new String[]{"file", TEST_CORE_PATH.pathInContainer()},
                 "dump.core.5954: ELF 64-bit LSB core file x86-64, version 1 (SYSV), too many program header sections (33172)");
 
+        String jdkPath = "/path/to/jdk/java";
         mockExec(new String[]{GDB_PATH_RHEL8, "-n", "-batch", "-core", "/tmp/core.1234"},
-                "Core was generated by `" + JDK_PATH + " -Dconfig.id=default/container.11 -XX:+Pre'.");
+                 "Core was generated by `" + jdkPath + " -Dconfig.id=default/container.11 -XX:+Pre'.");
 
         String jstack = "jstack11";
-        mockExec(new String[]{"jhsdb", "jstack", "--exe", JDK_PATH, "--core", "/tmp/core.1234"},
+        mockExec(new String[]{"jhsdb", "jstack", "--exe", jdkPath, "--core", "/tmp/core.1234"},
                 jstack);
 
-        Map<String, Object> expectedData = Map.of(
-                "bin_path", JDK_PATH,
-                "backtrace_all_threads", List.of(jstack));
-        assertEquals(expectedData, coreCollector.collect(context, TEST_CORE_PATH));
+        var expected = new CoreDumpMetadata().setBinPath(jdkPath)
+                                             .setBacktraceAllThreads(List.of(jstack));
+        assertEquals(expected, coreCollector.collect(context, TEST_CORE_PATH));
     }
 
     @Test
     void metadata_for_java_heap_dump() {
-        assertEquals(JAVA_HEAP_DUMP_METADATA, coreCollector.collect(context, context.paths().of("/dump_java_pid123.hprof")));
+        var expected = new CoreDumpMetadata().setBinPath("java")
+                                             .setBacktrace(List.of("Heap dump, no backtrace available"));
+
+        assertEquals(expected, coreCollector.collect(context, context.paths().of("/dump_java_pid123.hprof")));
     }
 
     private void mockExec(String[] cmd, String output) {
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java
index b096db2eaf4..1fd688558a0 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java
@@ -1,12 +1,14 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
 
+import com.yahoo.config.provision.DockerImage;
 import com.yahoo.security.KeyId;
 import com.yahoo.security.SealedSharedKey;
 import com.yahoo.security.SecretSharedKey;
 import com.yahoo.test.ManualClock;
 import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
+import com.yahoo.vespa.hosted.node.admin.configserver.cores.CoreDumpMetadata;
 import com.yahoo.vespa.hosted.node.admin.configserver.cores.Cores;
 import com.yahoo.vespa.hosted.node.admin.container.metrics.DimensionMetrics;
 import com.yahoo.vespa.hosted.node.admin.container.metrics.Metrics;
@@ -26,7 +28,6 @@ import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.attribute.FileTime;
 import java.time.Duration;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -36,9 +37,12 @@ import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static com.yahoo.yolean.Exceptions.uncheck;
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
@@ -57,7 +61,6 @@ public class CoredumpHandlerTest {
 
     private final CoreCollector coreCollector = mock(CoreCollector.class);
     private final Cores cores = mock(Cores.class);
-    private final CoredumpReporter coredumpReporter = mock(CoredumpReporter.class);
     private final Metrics metrics = new Metrics();
     private final ManualClock clock = new ManualClock();
     @SuppressWarnings("unchecked")
@@ -65,7 +68,7 @@ public class CoredumpHandlerTest {
     private final SecretSharedKeySupplier secretSharedKeySupplier = mock(SecretSharedKeySupplier.class);
     private final InMemoryFlagSource flagSource = new InMemoryFlagSource();
     private final CoredumpHandler coredumpHandler =
-            new CoredumpHandler(coreCollector, cores, coredumpReporter, containerCrashPath.pathInContainer(),
+            new CoredumpHandler(coreCollector, cores, containerCrashPath.pathInContainer(),
                                 doneCoredumpsPath, metrics, clock, coredumpIdSupplier, secretSharedKeySupplier,
                                 flagSource);
 
@@ -148,31 +151,14 @@ public class CoredumpHandlerTest {
         verify(coredumpIdSupplier, times(1)).get();
     }
 
-    private static String buildExpectedMetadataString(Optional<String> decryptionToken) {
-        return "{\"fields\":{" +
-               "\"hostname\":\"host123.yahoo.com\"," +
-               "\"kernel_version\":\"3.10.0-862.9.1.el7.x86_64\"," +
-               "\"backtrace\":[\"call 1\",\"function 2\",\"something something\"]," +
-               "\"vespa_version\":\"6.48.4\"," +
-               "\"bin_path\":\"/bin/bash\"," +
-               "\"coredump_path\":\"/home/docker/dumps/container-123/id-123/dump_core.456\"," +
-               "\"docker_image\":\"vespa/ci:6.48.4\"" +
-               decryptionToken.map(",\"decryption_token\":\"%s\""::formatted).orElse("") +
-               "}}";
-    }
-
-    void do_get_metadata_test(Optional<String> oldDecryptionToken, Optional<String> newDecryptionToken) throws IOException {
-        Map<String, Object> metadata = new HashMap<>();
-        metadata.put("bin_path", "/bin/bash");
-        metadata.put("backtrace", List.of("call 1", "function 2", "something something"));
-
-        Map<String, Object> attributes = Map.of(
-                "hostname", "host123.yahoo.com",
-                "vespa_version", "6.48.4",
-                "kernel_version", "3.10.0-862.9.1.el7.x86_64",
-                "docker_image", "vespa/ci:6.48.4");
-
-        String expectedMetadataStr = buildExpectedMetadataString(oldDecryptionToken);
+    @Test
+    void gather_metadata_test() throws IOException {
+        var metadata = new CoreDumpMetadata().setKernelVersion("3.10.0-862.9.1.el7.x86_64")
+                                             .setBacktrace(List.of("call 1", "function 2", "something something"))
+                                             .setVespaVersion("6.48.4")
+                                             .setBinPath("/bin/bash")
+                                             .setCoreDumpPath(context.paths().of("/home/docker/dumps/container-123/id-123/dump_core.456"))
+                                             .setDockerImage(DockerImage.fromString("example.com/vespa/ci:6.48.4"));
 
         ContainerPath coredumpDirectory = context.paths().of("/var/crash/id-123");
         Files.createDirectories(coredumpDirectory.pathOnHost());
@@ -180,40 +166,20 @@ public class CoredumpHandlerTest {
         when(coreCollector.collect(eq(context), eq(coredumpDirectory.resolve("dump_core.456"))))
                 .thenReturn(metadata);
 
-        assertEquals(expectedMetadataStr, coredumpHandler.getMetadata(context, coredumpDirectory, () -> attributes, oldDecryptionToken));
+        assertEquals(metadata, coredumpHandler.gatherMetadata(context, coredumpDirectory));
         verify(coreCollector, times(1)).collect(any(), any());
 
-        // Calling it again will read the previously generated metadata from disk and selectively
-        // patch in an updated decryption token value, if one is provided.
-        // This avoids having to re-run a potentially expensive collector step.
-        expectedMetadataStr = buildExpectedMetadataString(newDecryptionToken);
-        assertEquals(expectedMetadataStr, coredumpHandler.getMetadata(context, coredumpDirectory, () -> attributes, newDecryptionToken));
+        // On second invocation the test already exist, so times(1) is not incremented
+        assertEquals(metadata, coredumpHandler.gatherMetadata(context, coredumpDirectory));
+        doThrow(new IllegalStateException("Should not be invoked"))
+                .when(coreCollector).collect(any(), any());
         verify(coreCollector, times(1)).collect(any(), any());
     }
 
     @Test
-    void get_metadata_test_without_encryption() throws IOException {
-        do_get_metadata_test(Optional.empty(), Optional.empty()); // No token in metadata
-    }
-
-    @Test
-    void get_metadata_test_with_encryption() throws IOException {
-        when(secretSharedKeySupplier.create(any())).thenReturn(Optional.of(makeFixedSecretSharedKey()));
-        do_get_metadata_test(Optional.of("AVeryCoolToken"), Optional.of("AnEvenCoolerToken"));
-    }
-
-    @Test
-    void get_metadata_test_without_encryption_then_with_encryption() throws IOException {
-        // Edge where encryption was enabled between attempted processing runs.
-        // We don't bother with testing the opposite edge case (encryption -> no encryption), since
-        // in that case the core dump itself won't be encrypted so the token will be a no-op.
-        do_get_metadata_test(Optional.empty(), Optional.of("TheSwaggestToken"));
-    }
-
-    @Test
     void cant_get_metadata_if_no_core_file() {
         assertThrows(IllegalStateException.class, () -> {
-            coredumpHandler.getMetadata(context, context.paths().of("/fake/path"), Map::of, Optional.empty());
+            coredumpHandler.gatherMetadata(context, context.paths().of("/fake/path"));
         });
     }
 
@@ -228,31 +194,34 @@ public class CoredumpHandlerTest {
         });
     }
 
-    void do_process_single_coredump_test(String expectedCoreFileName) throws IOException {
+    void do_process_single_coredump_test(String expectedCoreFileName, boolean encrypt) throws IOException {
         ContainerPath coredumpDirectory = context.paths().of("/path/to/coredump/proccessing/id-123");
         Files.createDirectories(coredumpDirectory);
-        Files.write(coredumpDirectory.resolve("metadata.json"), "{\"test-metadata\":{}}".getBytes());
+        Files.write(coredumpDirectory.resolve("metadata2.json"), "{\"test-metadata\":{}}".getBytes());
         Files.createFile(coredumpDirectory.resolve("dump_bash.core.431"));
-        assertFolderContents(coredumpDirectory, "metadata.json", "dump_bash.core.431");
+        assertFolderContents(coredumpDirectory, "metadata2.json", "dump_bash.core.431");
+        CoreDumpMetadata expectedMetadata = new CoreDumpMetadata();
+        if (encrypt)
+            expectedMetadata.setDecryptionToken("XWTN195KaZecNCloqRICq9YwxwWJEKGqyMYcTgHtwaZfnXtzHzd7bPrPPR60VdxxzA");
 
-        coredumpHandler.processAndReportSingleCoredump(context, coredumpDirectory, Map::of);
+        coredumpHandler.processAndReportSingleCoreDump(context, coredumpDirectory, Optional.empty());
         verify(coreCollector, never()).collect(any(), any());
-        verify(coredumpReporter, times(1)).reportCoredump(eq("id-123"), eq("{\"test-metadata\":{}}"));
+        verify(cores, times(1)).report(eq(context.hostname()), eq("id-123"), eq(expectedMetadata));
         assertFalse(Files.exists(coredumpDirectory));
         assertFolderContents(doneCoredumpsPath.resolve("container-123"), "id-123");
-        assertFolderContents(doneCoredumpsPath.resolve("container-123").resolve("id-123"), "metadata.json", expectedCoreFileName);
+        assertFolderContents(doneCoredumpsPath.resolve("container-123").resolve("id-123"), "metadata2.json", expectedCoreFileName);
     }
 
     @Test
     void process_single_coredump_test_without_encryption() throws IOException {
-        do_process_single_coredump_test("dump_bash.core.431.zst");
+        do_process_single_coredump_test("dump_bash.core.431.zst", false);
     }
 
     @Test
     void process_single_coredump_test_with_encryption() throws IOException {
         flagSource.withStringFlag(Flags.CORE_ENCRYPTION_PUBLIC_KEY_ID.id(), "bar-key");
         when(secretSharedKeySupplier.create(KeyId.ofString("bar-key"))).thenReturn(Optional.of(makeFixedSecretSharedKey()));
-        do_process_single_coredump_test("dump_bash.core.431.zst.enc");
+        do_process_single_coredump_test("dump_bash.core.431.zst.enc", true);
     }
 
     // TODO fail closed instead of open
@@ -260,7 +229,7 @@ public class CoredumpHandlerTest {
     void encryption_disabled_when_no_public_key_set_in_feature_flag() throws IOException {
         flagSource.withStringFlag(Flags.CORE_ENCRYPTION_PUBLIC_KEY_ID.id(), ""); // empty -> not set
         verify(secretSharedKeySupplier, never()).create(any());
-        do_process_single_coredump_test("dump_bash.core.431.zst"); // No .enc suffix; not encrypted
+        do_process_single_coredump_test("dump_bash.core.431.zst", false); // No .enc suffix; not encrypted
     }
 
     // TODO fail closed instead of open
@@ -268,7 +237,7 @@ public class CoredumpHandlerTest {
     void encryption_disabled_when_no_key_returned_for_key_id_specified_by_feature_flag() throws IOException {
         flagSource.withStringFlag(Flags.CORE_ENCRYPTION_PUBLIC_KEY_ID.id(), "baz-key");
         when(secretSharedKeySupplier.create(KeyId.ofString("baz-key"))).thenReturn(Optional.empty());
-        do_process_single_coredump_test("dump_bash.core.431.zst"); // No .enc suffix; not encrypted
+        do_process_single_coredump_test("dump_bash.core.431.zst", false); // No .enc suffix; not encrypted
     }
 
     @Test
@@ -278,7 +247,7 @@ public class CoredumpHandlerTest {
         Files.createFile(containerCrashPath.resolve("dump-2"));
         Files.createFile(containerCrashPath.resolve("hs_err_pid2.log"));
         Files.createDirectory(processingPath);
-        Files.createFile(processingPath.resolve("metadata.json"));
+        Files.createFile(processingPath.resolve("metadata2.json"));
         Files.createFile(processingPath.resolve("dump-3"));
 
         new UnixPath(doneCoredumpsPath.resolve("container-123").resolve("dump-3-folder").resolve("dump-3"))