From 45a5e288d3943e4257f09b7e9b97a2f807ad7662 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 3 May 2024 17:32:04 -0400 Subject: [PATCH 1/2] fix(recording): fix NPE when starting a recording without specified template type --- .../java/io/cryostat/recordings/RecordingHelper.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/java/io/cryostat/recordings/RecordingHelper.java b/src/main/java/io/cryostat/recordings/RecordingHelper.java index a332d6797..cf9885eca 100644 --- a/src/main/java/io/cryostat/recordings/RecordingHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingHelper.java @@ -597,6 +597,16 @@ public Template getPreferredTemplate( return Optional.empty(); } }; + if (templateType == null) { + return custom.get() + .or(() -> remote.get()) + .orElseThrow( + () -> + new BadRequestException( + String.format( + "Invalid/unknown event template %s", + templateName))); + } switch (templateType) { case TARGET: return remote.get().orElseThrow(); From 897c3c5e3460e0bafd82067195a7840602d56371 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Fri, 3 May 2024 17:46:31 -0400 Subject: [PATCH 2/2] fix(graphql): avoid ConcurrentModificationException --- .../io/cryostat/graphql/ActiveRecordings.java | 229 +++++++++--------- 1 file changed, 112 insertions(+), 117 deletions(-) diff --git a/src/main/java/io/cryostat/graphql/ActiveRecordings.java b/src/main/java/io/cryostat/graphql/ActiveRecordings.java index 2960b6924..a9877ee20 100644 --- a/src/main/java/io/cryostat/graphql/ActiveRecordings.java +++ b/src/main/java/io/cryostat/graphql/ActiveRecordings.java @@ -44,7 +44,6 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.smallrye.common.annotation.Blocking; import io.smallrye.graphql.api.Nullable; -import io.smallrye.graphql.execution.ExecutionException; import jakarta.inject.Inject; import jakarta.transaction.Transactional; import jdk.jfr.RecordingState; @@ -72,39 +71,39 @@ public class ActiveRecordings { "Start a new Flight Recording on all Targets under the subtrees of the discovery nodes" + " matching the given filter") public List createRecording( - @NonNull DiscoveryNodeFilter nodes, @NonNull RecordingSettings recording) { - return DiscoveryNode.listAll().stream() - .filter(nodes) - .flatMap( - node -> - RootNode.recurseChildren(node, n -> n.target != null).stream() - .map(n -> n.target)) - .map( - target -> { - var template = - recordingHelper.getPreferredTemplate( - target, - recording.template, - TemplateType.valueOf(recording.templateType)); - try { - return recordingHelper - .startRecording( - target, - Optional.ofNullable(recording.replace) - .map(RecordingReplace::valueOf) - .orElse(RecordingReplace.STOPPED), - template, - recording.asOptions(), - Optional.ofNullable(recording.metadata) - .map(s -> s.labels) - .orElse(Map.of())) - .await() - .atMost(Duration.ofSeconds(10)); - } catch (QuantityConversionException qce) { - throw new ExecutionException(qce); - } - }) - .toList(); + @NonNull DiscoveryNodeFilter nodes, @NonNull RecordingSettings recording) + throws QuantityConversionException { + var list = + DiscoveryNode.listAll().stream() + .filter(nodes) + .flatMap( + node -> + RootNode.recurseChildren(node, n -> n.target != null) + .stream() + .map(n -> n.target)) + .toList(); + var recordings = new ArrayList(); + for (var t : list) { + var template = + recordingHelper.getPreferredTemplate( + t, recording.template, TemplateType.valueOf(recording.templateType)); + var r = + recordingHelper + .startRecording( + t, + Optional.ofNullable(recording.replace) + .map(RecordingReplace::valueOf) + .orElse(RecordingReplace.STOPPED), + template, + recording.asOptions(), + Optional.ofNullable(recording.metadata) + .map(s -> s.labels) + .orElse(Map.of())) + .await() + .atMost(Duration.ofSeconds(10)); + recordings.add(r); + } + return recordings; } @Blocking @@ -114,26 +113,29 @@ public List createRecording( "Archive an existing Flight Recording matching the given filter, on all Targets under" + " the subtrees of the discovery nodes matching the given filter") public List archiveRecording( - @NonNull DiscoveryNodeFilter nodes, @Nullable ActiveRecordingsFilter recordings) { - return DiscoveryNode.listAll().stream() - .filter(nodes) - .flatMap( - node -> - RootNode.recurseChildren(node, n -> n.target != null).stream() - .map(n -> n.target)) - .flatMap( - t -> - recordingHelper.listActiveRecordings(t).stream() - .filter(r -> recordings == null || recordings.test(r))) - .map( - recording -> { - try { - return recordingHelper.archiveRecording(recording, null, null); - } catch (Exception e) { - throw new ExecutionException(e); - } - }) - .toList(); + @NonNull DiscoveryNodeFilter nodes, @Nullable ActiveRecordingsFilter recordings) + throws Exception { + var list = + DiscoveryNode.listAll().stream() + .filter(nodes) + .flatMap( + node -> + RootNode.recurseChildren(node, n -> n.target != null) + .stream() + .map(n -> n.target)) + .flatMap( + t -> + recordingHelper.listActiveRecordings(t).stream() + .filter( + r -> + recordings == null + || recordings.test(r))) + .toList(); + var archives = new ArrayList(); + for (var r : list) { + archives.add(recordingHelper.archiveRecording(r, null, null)); + } + return archives; } @Blocking @@ -143,29 +145,28 @@ public List archiveRecording( "Stop an existing Flight Recording matching the given filter, on all Targets under" + " the subtrees of the discovery nodes matching the given filter") public List stopRecording( - @NonNull DiscoveryNodeFilter nodes, @Nullable ActiveRecordingsFilter recordings) { - return DiscoveryNode.listAll().stream() - .filter(nodes) - .flatMap( - node -> - RootNode.recurseChildren(node, n -> n.target != null).stream() - .map(n -> n.target)) - .flatMap( - t -> - recordingHelper.listActiveRecordings(t).stream() - .filter(r -> recordings == null || recordings.test(r))) - .map( - recording -> { - try { - return recordingHelper - .stopRecording(recording) - .await() - .atMost(Duration.ofSeconds(10)); - } catch (Exception e) { - throw new ExecutionException(e); - } - }) - .toList(); + @NonNull DiscoveryNodeFilter nodes, @Nullable ActiveRecordingsFilter recordings) + throws Exception { + var list = + DiscoveryNode.listAll().stream() + .filter(nodes) + .flatMap( + node -> + RootNode.recurseChildren(node, n -> n.target != null) + .stream() + .map(n -> n.target)) + .flatMap( + t -> + recordingHelper.listActiveRecordings(t).stream() + .filter( + r -> + recordings == null + || recordings.test(r))) + .toList(); + for (var r : list) { + recordingHelper.stopRecording(r).await().atMost(Duration.ofSeconds(10)); + } + return list; } @Blocking @@ -176,28 +177,26 @@ public List stopRecording( + " the subtrees of the discovery nodes matching the given filter") public List deleteRecording( @NonNull DiscoveryNodeFilter nodes, @Nullable ActiveRecordingsFilter recordings) { - return DiscoveryNode.listAll().stream() - .filter(nodes) - .flatMap( - node -> - RootNode.recurseChildren(node, n -> n.target != null).stream() - .map(n -> n.target)) - .flatMap( - t -> - recordingHelper.listActiveRecordings(t).stream() - .filter(r -> recordings == null || recordings.test(r))) - .map( - recording -> { - try { - return recordingHelper - .deleteRecording(recording) - .await() - .atMost(Duration.ofSeconds(10)); - } catch (Exception e) { - throw new ExecutionException(e); - } - }) - .toList(); + var list = + DiscoveryNode.listAll().stream() + .filter(nodes) + .flatMap( + node -> + RootNode.recurseChildren(node, n -> n.target != null) + .stream() + .map(n -> n.target)) + .flatMap( + t -> + recordingHelper.listActiveRecordings(t).stream() + .filter( + r -> + recordings == null + || recordings.test(r))) + .toList(); + for (var r : list) { + recordingHelper.deleteRecording(r).await().atMost(Duration.ofSeconds(10)); + } + return list; } @Blocking @@ -207,24 +206,20 @@ public List deleteRecording( "Create a Flight Recorder Snapshot on all Targets under" + " the subtrees of the discovery nodes matching the given filter") public List createSnapshot(@NonNull DiscoveryNodeFilter nodes) { - return DiscoveryNode.listAll().stream() - .filter(nodes) - .flatMap( - node -> - RootNode.recurseChildren(node, n -> n.target != null).stream() - .map(n -> n.target)) - .map( - target -> { - try { - return recordingHelper - .createSnapshot(target) - .await() - .atMost(Duration.ofSeconds(10)); - } catch (Exception e) { - throw new ExecutionException(e); - } - }) - .toList(); + var targets = + DiscoveryNode.listAll().stream() + .filter(nodes) + .flatMap( + node -> + RootNode.recurseChildren(node, n -> n.target != null) + .stream() + .map(n -> n.target)) + .toList(); + var snapshots = new ArrayList(); + for (var t : targets) { + snapshots.add(recordingHelper.createSnapshot(t).await().atMost(Duration.ofSeconds(10))); + } + return snapshots; } @Blocking