Redesign ArtifactProducer interface

Return list of files produced including asset classification and compression hint.
Change produceArtifacts method to take a single context parameter.
Support aliases for one or more artifacts.

10 files changed, 354 insertions, 140 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/Artifact.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/Artifact.java
index 7a3c891d969..edab90afea7 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/Artifact.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/Artifact.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump;
 
 import java.nio.file.Path;
+import java.util.Optional;
 
 /**
  * An artifact file produced by a {@link ArtifactProducer}.
@@ -20,33 +21,40 @@ class Artifact {
     }
 
     private final Classification classification;
-    private final Path file;
+    private final Path fileInNode;
+    private final Path fileOnHost;
     private final boolean compressOnUpload;
 
     private Artifact(Builder builder) {
-        this.file = builder.file;
+        if (builder.fileOnHost == null && builder.fileInNode == null) {
+            throw new IllegalArgumentException("No file specified");
+        } else if (builder.fileOnHost != null && builder.fileInNode != null) {
+            throw new IllegalArgumentException("Only a single file can be specified");
+        }
+        this.fileInNode = builder.fileInNode;
+        this.fileOnHost = builder.fileOnHost;
         this.classification = builder.classification;
-        this.compressOnUpload = builder.compressOnUpload != null ? builder.compressOnUpload : false;
+        this.compressOnUpload = Boolean.TRUE.equals(builder.compressOnUpload);
     }
 
-    static Builder newBuilder(Classification classification, Path file) {
-        return new Builder(classification, file);
-    }
+    static Builder newBuilder() { return new Builder(); }
 
-    Classification classification() { return classification; }
-    Path file() { return file; }
+    Optional<Classification> classification() { return Optional.ofNullable(classification); }
+    Optional<Path> fileInNode() { return Optional.ofNullable(fileInNode); }
+    Optional<Path> fileOnHost() { return Optional.ofNullable(fileOnHost); }
     boolean compressOnUpload() { return compressOnUpload; }
 
     static class Builder {
         private Classification classification;
-        private Path file;
+        private Path fileInNode;
+        private Path fileOnHost;
         private Boolean compressOnUpload;
 
-        private Builder(Classification classification, Path file) {
-            this.classification = classification;
-            this.file = file;
-        }
+        private Builder() {}
 
+        Builder classification(Classification c) { this.classification = c; return this; }
+        Builder fileInNode(Path f) { this.fileInNode = f; return this; }
+        Builder fileOnHost(Path f) { this.fileOnHost = f; return this; }
         Builder compressOnUpload() { this.compressOnUpload = true; return this; }
         Artifact build() { return new Artifact(this); }
     }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducer.java
index 83e83f1f4d2..6afa44bcf58 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducer.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducer.java
@@ -1,10 +1,11 @@
 // 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.servicedump;
 
-import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
-import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath;
+import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult;
 
-import java.io.IOException;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.OptionalDouble;
 
 /**
  * Produces service dump artifacts.
@@ -13,10 +14,22 @@ import java.io.IOException;
  */
 interface ArtifactProducer {
 
-    String name();
+    String artifactName();
+    String description();
+    List<Artifact> produceArtifacts(Context ctx);
 
-    void produceArtifact(NodeAgentContext context, String configId, ServiceDumpReport.DumpOptions options,
-                         UnixPath resultDirectoryInNode) throws IOException;
+    interface Context {
+        String serviceId();
+        int servicePid();
+        CommandResult executeCommandInNode(List<String> command, boolean logOutput);
+        Path outputDirectoryInNode();
+        Options options();
+
+        interface Options {
+            OptionalDouble duration();
+            boolean callGraphRecording();
+        }
+    }
 
 
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducers.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducers.java
new file mode 100644
index 00000000000..615fb2b5fbd
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducers.java
@@ -0,0 +1,92 @@
+// 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.servicedump;
+
+import com.yahoo.yolean.concurrent.Sleeper;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+/**
+ * @author bjorncs
+ */
+class ArtifactProducers {
+
+    private final Map<String, ArtifactProducer> producers;
+    private final Map<String, List<ArtifactProducer>> aliases;
+
+    private ArtifactProducers(Set<ArtifactProducer> producers,
+                              Map<String, List<Class<? extends ArtifactProducer>>> aliases) {
+        var producerMap = producers.stream()
+                .collect(Collectors.toMap(ArtifactProducer::artifactName, Function.identity()));
+        Map<String, List<ArtifactProducer>> aliasMap = new HashMap<>();
+        aliases.forEach((alias, mapping) -> {
+            List<ArtifactProducer> concreteMapping = mapping.stream()
+                    .map(type -> producers.stream()
+                            .filter(p -> p.getClass().equals(type))
+                            .findAny()
+                            .orElseThrow(() -> new IllegalArgumentException("No producer of type " + type)))
+                    .collect(Collectors.toList());
+            if (producerMap.containsKey(alias)) {
+                throw new IllegalStateException("Alias name '" + alias + "' conflicts with producer");
+            }
+            aliasMap.put(alias, concreteMapping);
+        });
+        this.producers = producerMap;
+        this.aliases = aliasMap;
+    }
+
+    static ArtifactProducers createDefault(Sleeper sleeper) {
+        var producers = Set.of(
+                new JavaFlightRecorder(sleeper),
+                new PerfReporter());
+        return new ArtifactProducers(producers, Map.of());
+    }
+
+    static ArtifactProducers createCustom(Set<ArtifactProducer> producers,
+                                          Map<String, List<Class<? extends ArtifactProducer>>> aliases) {
+        return new ArtifactProducers(producers, aliases);
+    }
+
+    List<ArtifactProducer> resolve(List<String> requestedArtifacts) {
+        List<ArtifactProducer> resolved = new ArrayList<>();
+        for (String artifact : requestedArtifacts) {
+            if (aliases.containsKey(artifact)) {
+                aliases.get(artifact).stream()
+                        .filter(p -> !resolved.contains(p))
+                        .forEach(resolved::add);
+            } else if (producers.containsKey(artifact)) {
+                ArtifactProducer producer = producers.get(artifact);
+                if (!resolved.contains(producer)) {
+                    resolved.add(producer);
+                }
+            } else {
+                throw createInvalidArtifactException(artifact);
+            }
+        }
+        return resolved;
+    }
+
+    private IllegalArgumentException createInvalidArtifactException(String artifact) {
+        String producersString = producers.keySet().stream()
+                .map(a -> "'" + a + "'")
+                .collect(Collectors.joining(", ", "[", "]"));
+        String aliasesString = aliases.entrySet().stream()
+                .map(e -> String.format(
+                        "'%s': %s",
+                        e.getKey(),
+                        e.getValue().stream()
+                                .map(p -> "'" + p.artifactName() + "'")
+                                .collect(Collectors.joining(", ", "[", "]")))
+                )
+                .collect(Collectors.joining(", ", "[", "]"));
+        String msg = String.format(
+                "Invalid artifact type '%s'. Valid types are %s and valid aliases are %s",
+                artifact, producersString, aliasesString);
+        return new IllegalArgumentException(msg);
+    }
+}
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JavaFlightRecorder.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JavaFlightRecorder.java
index 4d2b9d10069..9f716b3e884 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JavaFlightRecorder.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JavaFlightRecorder.java
@@ -2,50 +2,48 @@
 package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump;
 
 import com.yahoo.yolean.concurrent.Sleeper;
-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.task.util.file.UnixPath;
 
-import java.io.IOException;
+import java.nio.file.Path;
 import java.time.Duration;
 import java.util.List;
 
+import static com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.Artifact.Classification.CONFIDENTIAL;
+
 /**
  * Creates a Java Flight Recorder dump.
  *
  * @author bjorncs
  */
-class JavaFlightRecorder extends AbstractProducer {
+class JavaFlightRecorder implements ArtifactProducer {
 
     private final Sleeper sleeper;
 
-    JavaFlightRecorder(ContainerOperations container, Sleeper sleeper) {
-        super(container);
-        this.sleeper = sleeper;
-    }
-
-    static String NAME = "jfr-recording";
+    JavaFlightRecorder(Sleeper sleeper) { this.sleeper = sleeper; }
 
-    @Override public String name() { return NAME; }
+    @Override public String artifactName() { return "jfr-recording"; }
+    @Override public String description() { return "Java Flight Recorder recording"; }
 
     @Override
-    public void produceArtifact(NodeAgentContext ctx, String configId, ServiceDumpReport.DumpOptions options,
-                                UnixPath resultDirectoryInNode) throws IOException {
-        int pid = findVespaServicePid(ctx, configId);
-        int seconds = (int) (duration(ctx, options, 30.0));
-        UnixPath outputFile = resultDirectoryInNode.resolve("recording.jfr");
-        List<String> startCommand = List.of("jcmd", Integer.toString(pid), "JFR.start", "name=host-admin",
+    public List<Artifact> produceArtifacts(Context ctx) {
+        int seconds = (int) (ctx.options().duration().orElse(30.0));
+        Path outputFile = ctx.outputDirectoryInNode().resolve("recording.jfr");
+        List<String> startCommand = List.of("jcmd", Integer.toString(ctx.servicePid()), "JFR.start", "name=host-admin",
                 "path-to-gc-roots=true", "settings=profile", "filename=" + outputFile, "duration=" + seconds + "s");
-        executeCommand(ctx, startCommand, true);
+        ctx.executeCommandInNode(startCommand, true);
         sleeper.sleep(Duration.ofSeconds(seconds).plusSeconds(1));
         int maxRetries = 10;
-        List<String> checkCommand = List.of("jcmd", Integer.toString(pid), "JFR.check", "name=host-admin");
+        List<String> checkCommand = List.of("jcmd", Integer.toString(ctx.servicePid()), "JFR.check", "name=host-admin");
         for (int i = 0; i < maxRetries; i++) {
-            boolean stillRunning = executeCommand(ctx, checkCommand, true).getOutputLines().stream()
+            boolean stillRunning = ctx.executeCommandInNode(checkCommand, true).getOutputLines().stream()
                     .anyMatch(l -> l.contains("name=host-admin") && l.contains("running"));
-            if (!stillRunning) return;
+            if (!stillRunning) {
+                Artifact a = Artifact.newBuilder()
+                        .classification(CONFIDENTIAL).fileInNode(outputFile).compressOnUpload().build();
+                return List.of(a);
+            }
             sleeper.sleep(Duration.ofSeconds(1));
         }
-        throw new IOException("Failed to wait for JFR dump to complete after " + maxRetries + " retries");
+        throw new RuntimeException("Failed to wait for JFR dump to complete after " + maxRetries + " retries");
     }
+
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReportProducer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReportProducer.java
deleted file mode 100644
index 1370f09acac..00000000000
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReportProducer.java
+++ /dev/null
@@ -1,40 +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.servicedump;
-
-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.task.util.file.UnixPath;
-
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-
-/**
- * @author bjorncs
- */
-class PerfReportProducer extends AbstractProducer {
-
-    public static String NAME = "perf-report";
-
-    PerfReportProducer(ContainerOperations container) { super(container); }
-
-    @Override public String name() { return NAME; }
-
-    @Override
-    public void produceArtifact(NodeAgentContext context, String configId, ServiceDumpReport.DumpOptions options,
-                                UnixPath resultDirectoryInNode) throws IOException {
-        int pid = findVespaServicePid(context, configId);
-        int duration = (int) duration(context, options, 30.0);
-        List<String> perfRecordCommand = new ArrayList<>(List.of("perf", "record"));
-        if (options != null && Boolean.TRUE.equals(options.callGraphRecording())) {
-            perfRecordCommand.add("-g");
-        }
-        String recordFile = resultDirectoryInNode.resolve("perf-record.bin").toString();
-        perfRecordCommand.addAll(
-                List.of("--output=" + recordFile,
-                        "--pid=" + pid, "sleep", Integer.toString(duration)));
-        executeCommand(context, perfRecordCommand, true);
-        String perfReportFile = resultDirectoryInNode.resolve("perf-report.txt").toString();
-        executeCommand(context, List.of("bash", "-c", "perf report --input=" + recordFile + " > " + perfReportFile), true);
-    }
-}
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReporter.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReporter.java
new file mode 100644
index 00000000000..07c8b709e04
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReporter.java
@@ -0,0 +1,39 @@
+// 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.servicedump;
+
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+
+import static com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.Artifact.Classification.CONFIDENTIAL;
+import static com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.Artifact.Classification.INTERNAL;
+
+/**
+ * @author bjorncs
+ */
+class PerfReporter implements ArtifactProducer {
+
+    PerfReporter() {}
+
+    @Override public String artifactName() { return "perf-report"; }
+    @Override public String description() { return "Perf recording and report"; }
+
+    @Override
+    public List<Artifact> produceArtifacts(Context ctx) {
+        int duration = (int)ctx.options().duration().orElse(30.0);
+        List<String> perfRecordCommand = new ArrayList<>(List.of("perf", "record"));
+        if (ctx.options().callGraphRecording()) {
+            perfRecordCommand.add("-g");
+        }
+        Path recordFile = ctx.outputDirectoryInNode().resolve("perf-record.bin");
+        perfRecordCommand.addAll(
+                List.of("--output=" + recordFile,
+                        "--pid=" + ctx.servicePid(), "sleep", Integer.toString(duration)));
+        ctx.executeCommandInNode(perfRecordCommand, true);
+        Path reportFile = ctx.outputDirectoryInNode().resolve("perf-report.txt");
+        ctx.executeCommandInNode(List.of("bash", "-c", "perf report --input=" + recordFile + " > " + reportFile), true);
+        return List.of(
+                Artifact.newBuilder().classification(CONFIDENTIAL).fileInNode(recordFile).compressOnUpload().build(),
+                Artifact.newBuilder().classification(INTERNAL).fileInNode(reportFile).build());
+    }
+}
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java
index 75d5d640009..3fcaf0922a6 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java
@@ -1,7 +1,6 @@
 // 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.servicedump;
 
-import com.yahoo.yolean.concurrent.Sleeper;
 import com.yahoo.text.Lowercase;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeAttributes;
 import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository;
@@ -9,18 +8,21 @@ import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
 import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations;
 import com.yahoo.vespa.hosted.node.admin.maintenance.sync.SyncClient;
 import com.yahoo.vespa.hosted.node.admin.maintenance.sync.SyncFileInfo;
+import com.yahoo.vespa.hosted.node.admin.maintenance.sync.SyncFileInfo.Compression;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
-import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder;
 import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath;
+import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult;
+import com.yahoo.yolean.concurrent.Sleeper;
 
 import java.net.URI;
+import java.nio.file.Path;
 import java.time.Clock;
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
-import java.util.function.Function;
+import java.util.Optional;
+import java.util.OptionalDouble;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
@@ -40,24 +42,20 @@ public class VespaServiceDumperImpl implements VespaServiceDumper {
     private final SyncClient syncClient;
     private final NodeRepository nodeRepository;
     private final Clock clock;
-    private final Map<String, ArtifactProducer> artifactProducers;
+    private final ArtifactProducers artifactProducers;
 
     public VespaServiceDumperImpl(ContainerOperations container, SyncClient syncClient, NodeRepository nodeRepository) {
-        this(container, syncClient, nodeRepository, Clock.systemUTC(), Sleeper.DEFAULT);
+        this(ArtifactProducers.createDefault(Sleeper.DEFAULT), container, syncClient, nodeRepository, Clock.systemUTC());
     }
 
     // For unit testing
-    VespaServiceDumperImpl(ContainerOperations container, SyncClient syncClient, NodeRepository nodeRepository,
-                           Clock clock, Sleeper sleeper) {
+    VespaServiceDumperImpl(ArtifactProducers producers, ContainerOperations container, SyncClient syncClient,
+                           NodeRepository nodeRepository, Clock clock) {
         this.container = container;
         this.syncClient = syncClient;
         this.nodeRepository = nodeRepository;
         this.clock = clock;
-        List<AbstractProducer> producers = List.of(
-                new PerfReportProducer(container),
-                new JavaFlightRecorder(container, sleeper));
-        this.artifactProducers = producers.stream()
-                .collect(Collectors.toMap(ArtifactProducer::name, Function.identity()));
+        this.artifactProducers = producers;
     }
 
     @Override
@@ -84,8 +82,8 @@ public class VespaServiceDumperImpl implements VespaServiceDumper {
             handleFailure(context, request, startedAt, "Request already expired");
             return;
         }
-        List<String> artifactTypes = request.artifacts();
-        if (artifactTypes == null || artifactTypes.isEmpty()) {
+        List<String> requestedArtifacts = request.artifacts();
+        if (requestedArtifacts == null || requestedArtifacts.isEmpty()) {
             handleFailure(context, request, startedAt, "No artifacts requested");
             return;
         }
@@ -103,30 +101,14 @@ public class VespaServiceDumperImpl implements VespaServiceDumper {
             context.log(log, Level.INFO, "Creating '" + directoryOnHost +"'.");
             directoryOnHost.createDirectory();
             directoryOnHost.setPermissions("rwxrwxrwx");
-            List<SyncFileInfo> files = new ArrayList<>();
             URI destination = serviceDumpDestination(nodeSpec, createDumpId(request));
-            for (String artifactType : artifactTypes) {
-                ArtifactProducer producer = artifactProducers.get(artifactType);
-                if (producer == null) {
-                    String supportedValues = String.join(",", artifactProducers.keySet());
-                    handleFailure(context, request, startedAt, "No artifact producer exists for '" + artifactType + "'. " +
-                            "Following values are allowed: " + supportedValues);
-                    return;
-                }
-                context.log(log, "Producing artifact of type '" + artifactType + "'");
-                UnixPath producerDirectoryOnHost = directoryOnHost.resolve(artifactType);
-                producerDirectoryOnHost.createDirectory();
-                producerDirectoryOnHost.setPermissions("rwxrwxrwx");
-                UnixPath producerDirectoryInNode = directoryInNode.resolve(artifactType);
-                producer.produceArtifact(context, configId, request.dumpOptions(), producerDirectoryInNode);
-                collectArtifactFilesToUpload(files, producerDirectoryOnHost, destination.resolve(artifactType + '/'), expiry);
-            }
-            context.log(log, Level.INFO, "Uploading files with destination " + destination + " and expiry " + expiry);
-            if (!syncClient.sync(context, files, Integer.MAX_VALUE)) {
-                handleFailure(context, request, startedAt, "Unable to upload all files");
-                return;
+            ProducerContext producerCtx = new ProducerContext(context, directoryInNode.toPath(), request);
+            List<Artifact> producedArtifacts = new ArrayList<>();
+            for (ArtifactProducer producer : artifactProducers.resolve(requestedArtifacts)) {
+                context.log(log, "Producing artifact of type '" + producer.artifactName() + "'");
+                producedArtifacts.addAll(producer.produceArtifacts(producerCtx));
             }
-            context.log(log, Level.INFO, "Upload complete");
+            uploadArtifacts(context, destination, producedArtifacts, expiry);
             storeReport(context, ServiceDumpReport.createSuccessReport(request, startedAt, clock.instant(), destination));
         } catch (Exception e) {
             handleFailure(context, request, startedAt, e);
@@ -138,10 +120,22 @@ public class VespaServiceDumperImpl implements VespaServiceDumper {
         }
     }
 
-    private void collectArtifactFilesToUpload(List<SyncFileInfo> files, UnixPath directoryOnHost, URI destination, Instant expiry) {
-        FileFinder.files(directoryOnHost.toPath()).stream()
-                .flatMap(file -> SyncFileInfo.forServiceDump(destination, file.path(), expiry).stream())
-                .forEach(files::add);
+    private void uploadArtifacts(NodeAgentContext ctx, URI destination,
+                                 List<Artifact> producedArtifacts, Instant expiry) {
+        List<SyncFileInfo> filesToUpload = producedArtifacts.stream()
+                .map(a -> {
+                    Compression compression = a.compressOnUpload() ? Compression.ZSTD : Compression.NONE;
+                    Path fileInNode = a.fileInNode().orElse(null);
+                    Path fileOnHost = fileInNode != null ? ctx.pathOnHostFromPathInNode(fileInNode) : a.fileOnHost().orElseThrow();
+                    return SyncFileInfo.forServiceDump(destination, fileOnHost, expiry, compression);
+                })
+                .collect(Collectors.toList());
+        ctx.log(log, Level.INFO,
+                "Uploading " + filesToUpload.size() + " file(s) with destination " + destination + " and expiry " + expiry);
+        if (!syncClient.sync(ctx, filesToUpload, Integer.MAX_VALUE)) {
+            throw new RuntimeException("Unable to upload all files");
+        }
+        ctx.log(log, Level.INFO, "Upload complete");
     }
 
     private static Instant expireAt(Instant startedAt, ServiceDumpReport request) {
@@ -180,5 +174,70 @@ public class VespaServiceDumperImpl implements VespaServiceDumper {
         return archiveUri.resolve(targetDirectory);
     }
 
+    private class ProducerContext implements ArtifactProducer.Context, ArtifactProducer.Context.Options {
+
+        final NodeAgentContext nodeAgentCtx;
+        final Path outputDirectoryInNode;
+        final ServiceDumpReport request;
+        volatile int pid = -1;
+
+        ProducerContext(NodeAgentContext nodeAgentCtx, Path outputDirectoryInNode, ServiceDumpReport request) {
+            this.nodeAgentCtx = nodeAgentCtx;
+            this.outputDirectoryInNode = outputDirectoryInNode;
+            this.request = request;
+        }
+
+        @Override public String serviceId() { return request.configId(); }
+
+        @Override
+        public int servicePid() {
+            if (pid == -1) {
+                Path findPidBinary = nodeAgentCtx.pathInNodeUnderVespaHome("libexec/vespa/find-pid");
+                CommandResult findPidResult = executeCommandInNode(List.of(findPidBinary.toString(), serviceId()), true);
+                this.pid = Integer.parseInt(findPidResult.getOutput());
+            }
+            return pid;
+        }
+
+        @Override
+        public CommandResult executeCommandInNode(List<String> command, boolean logOutput) {
+            CommandResult result = container.executeCommandInContainerAsRoot(nodeAgentCtx, command.toArray(new String[0]));
+            String cmdString = command.stream().map(s -> "'" + s + "'").collect(Collectors.joining(" ", "\"", "\""));
+            int exitCode = result.getExitCode();
+            String output = result.getOutput().trim();
+            String prefixedOutput = output.contains("\n")
+                    ? "\n" + output
+                    : (output.isEmpty() ? "<no output>" : output);
+            if (exitCode > 0) {
+                String errorMsg = logOutput
+                        ? String.format("Failed to execute %s (exited with code %d): %s", cmdString, exitCode, prefixedOutput)
+                        : String.format("Failed to execute %s (exited with code %d)", cmdString, exitCode);
+                throw new RuntimeException(errorMsg);
+            } else {
+                String logMsg = logOutput
+                        ? String.format("Executed command %s. Exited with code %d and output: %s", cmdString, exitCode, prefixedOutput)
+                        : String.format("Executed command %s. Exited with code %d.", cmdString, exitCode);
+                nodeAgentCtx.log(log, logMsg);
+            }
+            return result;
+        }
+
+        @Override public Path outputDirectoryInNode() { return outputDirectoryInNode; }
+        @Override public Options options() { return this; }
 
+        @Override
+        public OptionalDouble duration() {
+            Double duration = dumpOptions()
+                    .map(ServiceDumpReport.DumpOptions::duration)
+                    .orElse(null);
+            return duration != null ? OptionalDouble.of(duration) : OptionalDouble.empty();
+        }
+
+        @Override
+        public boolean callGraphRecording() {
+            return dumpOptions().map(ServiceDumpReport.DumpOptions::callGraphRecording).orElse(false);
+        }
+
+        Optional<ServiceDumpReport.DumpOptions> dumpOptions() { return Optional.ofNullable(request.dumpOptions()); }
+    }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfo.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfo.java
index 652d9ff0af0..55fc54c7b6d 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfo.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/sync/SyncFileInfo.java
@@ -5,7 +5,6 @@ import java.net.URI;
 import java.nio.file.Path;
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
-import java.util.List;
 import java.util.Optional;
 
 /**
@@ -67,13 +66,10 @@ public class SyncFileInfo {
                 logFile, uri.resolve(dir + logFile.getFileName() + compression.extension), compression, expiry));
     }
 
-    public static Optional<SyncFileInfo> forServiceDump(URI directory, Path file, Instant expiry) {
+    public static SyncFileInfo forServiceDump(URI destinationDir, Path file, Instant expiry, Compression compression) {
         String filename = file.getFileName().toString();
-        List<String> filesToCompress = List.of(".bin", ".hprof", ".jfr", ".log");
-        Compression compression = filesToCompress.stream().anyMatch(filename::endsWith) ? Compression.ZSTD : Compression.NONE;
-        if (filename.startsWith(".")) return Optional.empty();
-        URI location = directory.resolve(filename + compression.extension);
-        return Optional.of(new SyncFileInfo(file, location, compression, expiry));
+        URI location = destinationDir.resolve(filename + compression.extension);
+        return new SyncFileInfo(file, location, compression, expiry);
     }
 
     public enum Compression {
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducersTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducersTest.java
new file mode 100644
index 00000000000..d295eda2d0a
--- /dev/null
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducersTest.java
@@ -0,0 +1,25 @@
+package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump;// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+import com.yahoo.yolean.concurrent.Sleeper;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+/**
+ * @author bjorncs
+ */
+class ArtifactProducersTest {
+
+    @Test
+    void generates_exception_on_unknown_artifact() {
+        ArtifactProducers instance = ArtifactProducers.createDefault(Sleeper.NOOP);
+        IllegalArgumentException exception = assertThrows(
+                IllegalArgumentException.class, () -> instance.resolve(List.of("unknown-artifact")));
+        String expectedMsg = "Invalid artifact type 'unknown-artifact'. " +
+                "Valid types are ['perf-report', 'jfr-recording'] and valid aliases are []";
+        assertEquals(expectedMsg, exception.getMessage());
+    }
+
+}
\ No newline at end of file
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java
index 036e39cded2..c701799b273 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java
@@ -1,4 +1,5 @@
-package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump;// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// 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.servicedump;
 
 import com.yahoo.yolean.concurrent.Sleeper;
 import com.yahoo.test.ManualClock;
@@ -52,7 +53,7 @@ class VespaServiceDumperImplTest {
     void creates_valid_dump_id_from_dump_request() {
         long nowMillis = Instant.now().toEpochMilli();
         ServiceDumpReport request = new ServiceDumpReport(
-                nowMillis, null, null, null, null, "default/container.3", null, null, List.of(PerfReportProducer.NAME), null);
+                nowMillis, null, null, null, null, "default/container.3", null, null, List.of("perf-report"), null);
         String dumpId = VespaServiceDumperImpl.createDumpId(request);
         assertEquals("default-container-3-" + nowMillis, dumpId);
     }
@@ -68,9 +69,10 @@ class VespaServiceDumperImplTest {
         SyncClient syncClient = createSyncClientMock();
         NodeRepoMock nodeRepository = new NodeRepoMock();
         ManualClock clock = new ManualClock(Instant.ofEpochMilli(1600001000000L));
-        NodeSpec nodeSpec = createNodeSpecWithDumpRequest(nodeRepository, PerfReportProducer.NAME, new ServiceDumpReport.DumpOptions(true, 45.0));
+        NodeSpec nodeSpec = createNodeSpecWithDumpRequest(nodeRepository, "perf-report", new ServiceDumpReport.DumpOptions(true, 45.0));
 
-        VespaServiceDumper reporter = new VespaServiceDumperImpl(operations, syncClient, nodeRepository, clock, Sleeper.NOOP);
+        VespaServiceDumper reporter = new VespaServiceDumperImpl(
+                ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, clock);
         NodeAgentContextImpl context = new NodeAgentContextImpl.Builder(nodeSpec)
                 .fileSystem(fileSystem)
                 .build();
@@ -79,11 +81,22 @@ class VespaServiceDumperImplTest {
         verify(operations).executeCommandInContainerAsRoot(
                 context, "/opt/vespa/libexec/vespa/find-pid", "default/container.1");
         verify(operations).executeCommandInContainerAsRoot(
-                context, "perf", "record", "-g", "--output=/opt/vespa/tmp/vespa-service-dump/perf-report/perf-record.bin",
+                context, "perf", "record", "-g", "--output=/opt/vespa/tmp/vespa-service-dump/perf-record.bin",
                 "--pid=12345", "sleep", "45");
         verify(operations).executeCommandInContainerAsRoot(
-                context, "bash", "-c", "perf report --input=/opt/vespa/tmp/vespa-service-dump/perf-report/perf-record.bin" +
-                        " > /opt/vespa/tmp/vespa-service-dump/perf-report/perf-report.txt");
+                context, "bash", "-c", "perf report --input=/opt/vespa/tmp/vespa-service-dump/perf-record.bin" +
+                        " > /opt/vespa/tmp/vespa-service-dump/perf-report.txt");
+
+        String expectedJson = "{\"createdMillis\":1600000000000,\"startedAt\":1600001000000,\"completedAt\":1600001000000," +
+                "\"location\":\"s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/\"," +
+                "\"configId\":\"default/container.1\",\"artifacts\":[\"perf-report\"]," +
+                "\"dumpOptions\":{\"callGraphRecording\":true,\"duration\":45.0}}";
+        assertReportEquals(nodeRepository, expectedJson);
+
+        List<URI> expectedUris = List.of(
+                URI.create("s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/perf-record.bin.zst"),
+                URI.create("s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/perf-report.txt"));
+        assertSyncedFiles(context, syncClient, expectedUris);
     }
 
     @Test
@@ -98,9 +111,10 @@ class VespaServiceDumperImplTest {
         NodeRepoMock nodeRepository = new NodeRepoMock();
         ManualClock clock = new ManualClock(Instant.ofEpochMilli(1600001000000L));
         NodeSpec nodeSpec = createNodeSpecWithDumpRequest(
-                nodeRepository, JavaFlightRecorder.NAME, new ServiceDumpReport.DumpOptions(null, null));
+                nodeRepository, "jfr-recording", new ServiceDumpReport.DumpOptions(null, null));
 
-        VespaServiceDumper reporter = new VespaServiceDumperImpl(operations, syncClient, nodeRepository, clock, Sleeper.NOOP);
+        VespaServiceDumper reporter = new VespaServiceDumperImpl(
+                ArtifactProducers.createDefault(Sleeper.NOOP), operations, syncClient, nodeRepository, clock);
         NodeAgentContextImpl context = new NodeAgentContextImpl.Builder(nodeSpec)
                 .fileSystem(fileSystem)
                 .build();
@@ -110,8 +124,18 @@ class VespaServiceDumperImplTest {
                 context, "/opt/vespa/libexec/vespa/find-pid", "default/container.1");
         verify(operations).executeCommandInContainerAsRoot(
                 context, "jcmd", "12345", "JFR.start", "name=host-admin", "path-to-gc-roots=true", "settings=profile",
-                "filename=/opt/vespa/tmp/vespa-service-dump/jfr-recording/recording.jfr", "duration=30s");
+                "filename=/opt/vespa/tmp/vespa-service-dump/recording.jfr", "duration=30s");
         verify(operations).executeCommandInContainerAsRoot(context, "jcmd", "12345", "JFR.check", "name=host-admin");
+
+        String expectedJson = "{\"createdMillis\":1600000000000,\"startedAt\":1600001000000," +
+                "\"completedAt\":1600001000000," +
+                "\"location\":\"s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/\"," +
+                "\"configId\":\"default/container.1\",\"artifacts\":[\"jfr-recording\"],\"dumpOptions\":{}}";
+        assertReportEquals(nodeRepository, expectedJson);
+
+        List<URI> expectedUris = List.of(
+                URI.create("s3://uri-1/tenant1/service-dump/default-container-1-1600000000000/recording.jfr.zst"));
+        assertSyncedFiles(context, syncClient, expectedUris);
     }
 
     private static NodeSpec createNodeSpecWithDumpRequest(NodeRepoMock repository, String artifactName, ServiceDumpReport.DumpOptions options) {