From 2f82e3847608b7c45a0ffb53285026f442fb2e01 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 15 Apr 2025 13:15:16 -0400 Subject: [PATCH 01/11] feat(rules): implement query parameter filtering, config default filter --- .../java/io/cryostat/ConfigProperties.java | 1 + src/main/java/io/cryostat/Producers.java | 8 ++++ .../LongRunningRequestGenerator.java | 17 +++++-- .../reports/MemoryCachingReportsService.java | 17 +++---- .../reports/ReportSidecarService.java | 3 +- .../java/io/cryostat/reports/Reports.java | 16 ++++--- .../io/cryostat/reports/ReportsService.java | 9 +--- .../cryostat/reports/ReportsServiceImpl.java | 44 ++++++++++++++----- .../reports/StorageCachingReportsService.java | 20 ++++----- src/main/resources/application.properties | 1 + 10 files changed, 86 insertions(+), 50 deletions(-) diff --git a/src/main/java/io/cryostat/ConfigProperties.java b/src/main/java/io/cryostat/ConfigProperties.java index f78921829..405b4efd9 100644 --- a/src/main/java/io/cryostat/ConfigProperties.java +++ b/src/main/java/io/cryostat/ConfigProperties.java @@ -32,6 +32,7 @@ public class ConfigProperties { public static final String CONNECTIONS_FAILED_TIMEOUT = "cryostat.connections.failed-timeout"; public static final String CONNECTIONS_UPLOAD_TIMEOUT = "cryostat.connections.upload-timeout"; + public static final String REPORTS_FILTER = "cryostat.services.reports.filter"; public static final String REPORTS_SIDECAR_URL = "quarkus.rest-client.reports.url"; public static final String REPORTS_USE_PRESIGNED_TRANSFER = "cryostat.services.reports.use-presigned-transfer"; diff --git a/src/main/java/io/cryostat/Producers.java b/src/main/java/io/cryostat/Producers.java index 4bef25407..e88264505 100644 --- a/src/main/java/io/cryostat/Producers.java +++ b/src/main/java/io/cryostat/Producers.java @@ -20,6 +20,7 @@ import java.util.concurrent.ForkJoinPool; import io.cryostat.core.reports.InterruptibleReportGenerator; +import io.cryostat.core.util.RuleFilterParser; import io.cryostat.libcryostat.sys.Clock; import io.cryostat.libcryostat.sys.FileSystem; import io.cryostat.recordings.LongRunningRequestGenerator; @@ -77,6 +78,13 @@ public static InterruptibleReportGenerator produceInterruptibleReportGenerator() singleThread ? Executors.newSingleThreadExecutor() : ForkJoinPool.commonPool()); } + @Produces + @ApplicationScoped + @DefaultBean + public static RuleFilterParser produceRuleFilterParser() { + return new RuleFilterParser(); + } + @Produces @RequestScoped @DefaultBean diff --git a/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java b/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java index 04d65cfdb..da63eaf54 100644 --- a/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java +++ b/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java @@ -22,6 +22,7 @@ import io.cryostat.ConfigProperties; import io.cryostat.core.reports.InterruptibleReportGenerator.AnalysisResult; +import io.cryostat.core.util.RuleFilterParser; import io.cryostat.recordings.ArchivedRecordings.ArchivedRecording; import io.cryostat.reports.AnalysisReportAggregator; import io.cryostat.reports.ReportsService; @@ -197,7 +198,7 @@ public Uni onMessage(GrafanaActiveUploadRequest request) { public Uni> onMessage(ActiveReportRequest request) { logger.trace("Job ID: " + request.id() + " submitted."); return reportsService - .reportFor(request.recording) + .reportFor(request.recording, request.filter) .onItem() .invoke( (report) -> { @@ -234,7 +235,7 @@ public Uni> onMessage(ActiveReportRequest request) { public Uni> onMessage(ArchivedReportRequest request) { logger.tracev("Job ID: {0} submitted.", request.id()); return reportsService - .reportFor(request.pair().getKey(), request.pair().getValue()) + .reportFor(request.pair().getKey(), request.pair().getValue(), request.filter) .onItem() .invoke( (report) -> { @@ -304,11 +305,15 @@ public record GrafanaActiveUploadRequest(String id, long remoteId, long targetId // of the record. It shouldn't be a problem and we do similar things // elswhere with other records. @SuppressFBWarnings(value = {"EI_EXPOSE_REP", "EI_EXPOSE_REP2"}) - public record ActiveReportRequest(String id, ActiveRecording recording) { + public record ActiveReportRequest(String id, ActiveRecording recording, String filter) { public ActiveReportRequest { Objects.requireNonNull(id); Objects.requireNonNull(recording); } + + public ActiveReportRequest(String id, ActiveRecording recording) { + this(id, recording, RuleFilterParser.ALL_WILDCARD_TOKEN); + } } @SuppressFBWarnings(value = {"EI_EXPOSE_REP", "EI_EXPOSE_REP2"}) @@ -321,11 +326,15 @@ public record ActiveReportCompletion( } } - public record ArchivedReportRequest(String id, Pair pair) { + public record ArchivedReportRequest(String id, Pair pair, String filter) { public ArchivedReportRequest { Objects.requireNonNull(id); Objects.requireNonNull(pair); } + + public ArchivedReportRequest(String id, Pair pair) { + this(id, pair, RuleFilterParser.ALL_WILDCARD_TOKEN); + } } @SuppressFBWarnings(value = {"EI_EXPOSE_REP", "EI_EXPOSE_REP2"}) diff --git a/src/main/java/io/cryostat/reports/MemoryCachingReportsService.java b/src/main/java/io/cryostat/reports/MemoryCachingReportsService.java index 0b2e1a8a2..8afa1efb1 100644 --- a/src/main/java/io/cryostat/reports/MemoryCachingReportsService.java +++ b/src/main/java/io/cryostat/reports/MemoryCachingReportsService.java @@ -16,12 +16,10 @@ package io.cryostat.reports; import java.util.Map; -import java.util.function.Predicate; - -import org.openjdk.jmc.flightrecorder.rules.IRule; import io.cryostat.ConfigProperties; import io.cryostat.core.reports.InterruptibleReportGenerator.AnalysisResult; +import io.cryostat.core.util.RuleFilterParser; import io.cryostat.recordings.ActiveRecording; import io.cryostat.recordings.RecordingHelper; @@ -67,11 +65,10 @@ class MemoryCachingReportsService implements ReportsService { @Inject Logger logger; @Override - public Uni> reportFor( - ActiveRecording recording, Predicate predicate) { + public Uni> reportFor(ActiveRecording recording, String filter) { if (!quarkusCache || !memoryCache) { logger.trace("cache disabled, delegating..."); - return delegate.reportFor(recording, predicate); + return delegate.reportFor(recording, filter); } String key = ReportsService.key(recording); logger.tracev("reportFor {0}", key); @@ -85,10 +82,10 @@ public Uni> reportFor( @Override public Uni> reportFor( - String jvmId, String filename, Predicate predicate) { + String jvmId, String filename, String filter) { if (!quarkusCache || !memoryCache) { logger.trace("cache disabled, delegating..."); - return delegate.reportFor(jvmId, filename, predicate); + return delegate.reportFor(jvmId, filename, filter); } String key = recordingHelper.archivedRecordingKey(jvmId, filename); logger.tracev("reportFor {0}", key); @@ -102,12 +99,12 @@ public Uni> reportFor( @Override public Uni> reportFor(ActiveRecording recording) { - return reportFor(recording, r -> true); + return reportFor(recording, RuleFilterParser.ALL_WILDCARD_TOKEN); } @Override public Uni> reportFor(String jvmId, String filename) { - return reportFor(jvmId, filename, r -> true); + return reportFor(jvmId, filename, RuleFilterParser.ALL_WILDCARD_TOKEN); } @Override diff --git a/src/main/java/io/cryostat/reports/ReportSidecarService.java b/src/main/java/io/cryostat/reports/ReportSidecarService.java index ee763a0e2..711c0d644 100644 --- a/src/main/java/io/cryostat/reports/ReportSidecarService.java +++ b/src/main/java/io/cryostat/reports/ReportSidecarService.java @@ -37,7 +37,8 @@ public interface ReportSidecarService { @POST @Consumes(MediaType.MULTIPART_FORM_DATA) Uni> generate( - @RestForm("file") @PartType(MediaType.APPLICATION_OCTET_STREAM) InputStream file); + @RestForm("file") @PartType(MediaType.APPLICATION_OCTET_STREAM) InputStream file, + @RestForm("filter") @PartType(MediaType.TEXT_PLAIN) String filter); @Path("/remote_report") @POST diff --git a/src/main/java/io/cryostat/reports/Reports.java b/src/main/java/io/cryostat/reports/Reports.java index b091f721f..fec3cd89b 100644 --- a/src/main/java/io/cryostat/reports/Reports.java +++ b/src/main/java/io/cryostat/reports/Reports.java @@ -90,8 +90,10 @@ void onStart(@Observes StartupEvent evt) { // Response isn't strongly typed which allows us to return either the Analysis result // or a job ID String along with setting different Status codes. // TODO: Is there a cleaner way to accomplish this? - public Response get(HttpServerResponse response, @RestPath String encodedKey) { - // TODO implement query parameter for evaluation predicate + public Response get( + HttpServerResponse response, + @RestPath String encodedKey, + @QueryParam("filter") @DefaultValue("") String filter) { var pair = helper.decodedKey(encodedKey); // Check if we have a cached result already for this report @@ -110,7 +112,7 @@ public Response get(HttpServerResponse response, @RestPath String encodedKey) { // and return the job ID with a location header. logger.trace("Cache miss. Creating archived reports request"); ArchivedReportRequest request = - new ArchivedReportRequest(UUID.randomUUID().toString(), pair); + new ArchivedReportRequest(UUID.randomUUID().toString(), pair, filter); response.bodyEndHandler( (e) -> bus.publish( @@ -188,7 +190,10 @@ public Uni>> getCached(@RestPath long t // or a job ID String along with setting different Status codes. // TODO: Is there a cleaner way to accomplish this? public Response getActive( - HttpServerResponse response, @RestPath long targetId, @RestPath long recordingId) + HttpServerResponse response, + @RestPath long targetId, + @RestPath long recordingId, + @QueryParam("filter") @DefaultValue("") String filter) throws Exception { var target = Target.getTargetById(targetId); var recording = target.getRecordingById(recordingId); @@ -209,13 +214,12 @@ public Response getActive( // and return the job ID with a location header. logger.trace("Cache miss. Creating active reports request"); ActiveReportRequest request = - new ActiveReportRequest(UUID.randomUUID().toString(), recording); + new ActiveReportRequest(UUID.randomUUID().toString(), recording, filter); response.bodyEndHandler( (e) -> bus.publish( LongRunningRequestGenerator.ACTIVE_REPORT_REQUEST_ADDRESS, request)); - // TODO implement query parameter for evaluation predicate return Response.ok(request.id(), MediaType.TEXT_PLAIN) .status(Response.Status.ACCEPTED) .location( diff --git a/src/main/java/io/cryostat/reports/ReportsService.java b/src/main/java/io/cryostat/reports/ReportsService.java index 4d982d45b..a78227c5d 100644 --- a/src/main/java/io/cryostat/reports/ReportsService.java +++ b/src/main/java/io/cryostat/reports/ReportsService.java @@ -16,9 +16,6 @@ package io.cryostat.reports; import java.util.Map; -import java.util.function.Predicate; - -import org.openjdk.jmc.flightrecorder.rules.IRule; import io.cryostat.core.reports.InterruptibleReportGenerator.AnalysisResult; import io.cryostat.recordings.ActiveRecording; @@ -26,13 +23,11 @@ import io.smallrye.mutiny.Uni; public interface ReportsService { - Uni> reportFor( - ActiveRecording recording, Predicate predicate); + Uni> reportFor(ActiveRecording recording, String filter); Uni> reportFor(ActiveRecording recording); - Uni> reportFor( - String jvmId, String filename, Predicate predicate); + Uni> reportFor(String jvmId, String filename, String filter); Uni> reportFor(String jvmId, String filename); diff --git a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java index b2acb4d23..1b59af91d 100644 --- a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java +++ b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java @@ -29,6 +29,7 @@ import io.cryostat.ConfigProperties; import io.cryostat.core.reports.InterruptibleReportGenerator; import io.cryostat.core.reports.InterruptibleReportGenerator.AnalysisResult; +import io.cryostat.core.util.RuleFilterParser; import io.cryostat.recordings.ActiveRecording; import io.cryostat.recordings.RecordingHelper; @@ -65,16 +66,19 @@ class ReportsServiceImpl implements ReportsService { @ConfigProperty(name = ConfigProperties.STORAGE_EXT_URL) Optional externalStorageUrl; + @ConfigProperty(name = ConfigProperties.REPORTS_FILTER) + String configFilter; + @Inject ObjectMapper mapper; @Inject RecordingHelper helper; @Inject InterruptibleReportGenerator reportGenerator; + @Inject RuleFilterParser ruleFilterParser; @Inject @RestClient ReportSidecarService sidecar; @Inject S3Presigner presigner; @Inject Logger logger; @Override - public Uni> reportFor( - ActiveRecording recording, Predicate predicate) { + public Uni> reportFor(ActiveRecording recording, String filter) { InputStream stream; try { stream = helper.getActiveInputStream(recording, uploadFailedTimeout); @@ -85,18 +89,18 @@ public Uni> reportFor( logger.tracev( "inprocess reportFor active recording {0} {1}", recording.target.jvmId, recording.remoteId); - return process(stream, predicate); + return process(stream, filter); } else { logger.tracev( "sidecar reportFor active recording {0} {1}", recording.target.jvmId, recording.remoteId); - return sidecar.generate(stream); + return fireRequest(stream, filter); } } @Override public Uni> reportFor( - String jvmId, String filename, Predicate predicate) { + String jvmId, String filename, String filter) { InputStream stream; try { stream = helper.getArchivedRecordingStream(jvmId, filename); @@ -105,31 +109,51 @@ public Uni> reportFor( } if (!useSidecar()) { logger.tracev("inprocess reportFor archived recording {0} {1}", jvmId, filename); - return process(stream, predicate); + return process(stream, filter); } else if (usePresignedSidecar()) { logger.tracev( "sidecar reportFor presigned archived recording {0} {1}", jvmId, filename); try { var uri = getPresignedPath(jvmId, filename); - return sidecar.generatePresigned(uri.getPath(), uri.getQuery(), null); + return sidecar.generatePresigned(uri.getPath(), uri.getQuery(), filter); } catch (URISyntaxException e) { logger.error(e); throw new InternalServerErrorException(e); } } else { logger.tracev("sidecar reportFor archived recording {0} {1}", jvmId, filename); - return sidecar.generate(stream); + return fireRequest(stream, filter); } } @Override public Uni> reportFor(ActiveRecording recording) { - return reportFor(recording, r -> true); + return reportFor(recording, RuleFilterParser.ALL_WILDCARD_TOKEN); } @Override public Uni> reportFor(String jvmId, String filename) { - return reportFor(jvmId, filename, r -> true); + return reportFor(jvmId, filename, RuleFilterParser.ALL_WILDCARD_TOKEN); + } + + private Uni> process(InputStream stream, String filter) { + return Uni.createFrom() + .future( + reportGenerator.generateEvalMapInterruptibly( + new BufferedInputStream(stream), + ruleFilterParser + .parse(configFilter) + .and(ruleFilterParser.parse(filter)))); + } + + private Uni> fireRequest(InputStream stream, String filter) { + return sidecar.generate(stream, String.format("%s,%s", configFilter, filter)); + } + + public static class ReportGenerationException extends RuntimeException { + public ReportGenerationException(Throwable cause) { + super(cause); + } } @Override diff --git a/src/main/java/io/cryostat/reports/StorageCachingReportsService.java b/src/main/java/io/cryostat/reports/StorageCachingReportsService.java index d68d82853..8276f656b 100644 --- a/src/main/java/io/cryostat/reports/StorageCachingReportsService.java +++ b/src/main/java/io/cryostat/reports/StorageCachingReportsService.java @@ -20,12 +20,10 @@ import java.time.Instant; import java.util.Map; import java.util.concurrent.CompletionException; -import java.util.function.Predicate; - -import org.openjdk.jmc.flightrecorder.rules.IRule; import io.cryostat.ConfigProperties; import io.cryostat.core.reports.InterruptibleReportGenerator.AnalysisResult; +import io.cryostat.core.util.RuleFilterParser; import io.cryostat.recordings.ActiveRecording; import io.cryostat.recordings.RecordingHelper; import io.cryostat.util.HttpMimeType; @@ -76,19 +74,18 @@ class StorageCachingReportsService implements ReportsService { @Inject Logger logger; @Override - public Uni> reportFor( - ActiveRecording recording, Predicate predicate) { + public Uni> reportFor(ActiveRecording recording, String filter) { String key = ReportsService.key(recording); logger.tracev("reportFor {0}", key); - return delegate.reportFor(recording, predicate); + return delegate.reportFor(recording, filter); } @Override public Uni> reportFor( - String jvmId, String filename, Predicate predicate) { + String jvmId, String filename, String filter) { if (!enabled) { logger.trace("cache disabled, delegating..."); - return delegate.reportFor(jvmId, filename, predicate); + return delegate.reportFor(jvmId, filename, filter); } var key = recordingHelper.archivedRecordingKey(jvmId, filename); logger.tracev("reportFor {0}", key); @@ -99,8 +96,7 @@ public Uni> reportFor( if (found) { return getStorage(key); } else { - return putStorage( - key, delegate.reportFor(jvmId, filename, predicate)); + return putStorage(key, delegate.reportFor(jvmId, filename, filter)); } }); } @@ -161,12 +157,12 @@ private Uni> getStorage(String key) { @Override public Uni> reportFor(ActiveRecording recording) { - return reportFor(recording, r -> true); + return reportFor(recording, RuleFilterParser.ALL_WILDCARD_TOKEN); } @Override public Uni> reportFor(String jvmId, String filename) { - return reportFor(jvmId, filename, r -> true); + return reportFor(jvmId, filename, RuleFilterParser.ALL_WILDCARD_TOKEN); } @Override diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 8b662bfd7..871dbf6bd 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -42,6 +42,7 @@ cryostat.services.reports.storage-cache.enabled=true cryostat.services.reports.storage-cache.name=archivedreports cryostat.services.reports.storage-cache.expiry-duration=24h cryostat.services.reports.use-presigned-transfer=true +cryostat.services.reports.filter=* cryostat.http.proxy.tls-enabled=false cryostat.http.proxy.host=${quarkus.http.host} From d0f385561d9fe0c104089415f1f43bd8ddc6d2a2 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 15 Apr 2025 13:27:21 -0400 Subject: [PATCH 02/11] fixup! feat(rules): implement query parameter filtering, config default filter --- schema/openapi.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/schema/openapi.yaml b/schema/openapi.yaml index e5c381664..0e600dd2c 100644 --- a/schema/openapi.yaml +++ b/schema/openapi.yaml @@ -1621,6 +1621,11 @@ paths: required: true schema: type: string + - in: query + name: filter + schema: + default: "" + type: string requestBody: content: application/json: @@ -2365,6 +2370,11 @@ paths: schema: format: int64 type: integer + - in: query + name: filter + schema: + default: "" + type: string requestBody: content: application/json: From 3eaa5768aa8777788b86be0e032e508acb7d4147 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 15 Apr 2025 14:36:52 -0400 Subject: [PATCH 03/11] graphql report result filtering --- schema/schema.graphql | 13 +++- .../java/io/cryostat/graphql/TargetNodes.java | 61 ++++++++++++++++++- 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/schema/schema.graphql b/schema/schema.graphql index 28f05aeaa..f6afe2863 100644 --- a/schema/schema.graphql +++ b/schema/schema.graphql @@ -246,7 +246,7 @@ type Target { mbeanMetrics: MBeanMetrics "Get the active and archived recordings belonging to this target" recordings: Recordings - report: Report + report(filter: ReportFilterInput): Report } type ThreadMetrics { @@ -341,3 +341,14 @@ input RecordingSettingsInput { templateType: String! toDisk: Boolean } + +input ReportFilterInput { + id: String + ids: [String] + notId: String + notIds: [String] + notTopic: String + notTopics: [String] + topic: String + topics: [String] +} diff --git a/src/main/java/io/cryostat/graphql/TargetNodes.java b/src/main/java/io/cryostat/graphql/TargetNodes.java index f13c2c7d1..0d90d27ee 100644 --- a/src/main/java/io/cryostat/graphql/TargetNodes.java +++ b/src/main/java/io/cryostat/graphql/TargetNodes.java @@ -20,7 +20,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Objects; +import java.util.function.Predicate; import java.util.stream.Collectors; import org.openjdk.jmc.flightrecorder.rules.Severity; @@ -107,7 +109,7 @@ public ArchivedRecordings archivedRecordings( return recordings; } - public Uni report(@Source Target target) { + public Uni report(@Source Target target, @Nullable ReportFilter filter) { var fTarget = Target.getTargetById(target.id); return reportAggregator .getEntry(fTarget.jvmId) @@ -116,7 +118,14 @@ public Uni report(@Source Target target) { e -> { var report = new Report(); report.lastUpdated = e.timestamp(); - report.data = e.report(); + report.data = + e.report().entrySet().stream() + .filter(r -> filter == null || filter.test(r)) + .collect( + Collectors.toMap( + Entry::getKey, + Entry + ::getValue)); report.aggregate = ReportAggregateInfo.from(report.data); return report; }) @@ -239,4 +248,52 @@ public static ReportAggregateInfo from(Map report) { .orElse(Severity.NA.getLimit())); } } + + @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") + public static class ReportFilter implements Predicate> { + public @Nullable String id; + public @Nullable List ids; + public @Nullable String notId; + public @Nullable List notIds; + public @Nullable String topic; + public @Nullable List topics; + public @Nullable String notTopic; + public @Nullable List notTopics; + + @Override + public boolean test(Map.Entry e) { + Predicate> matchesId = + r -> id == null || Objects.equals(id, r.getKey()); + Predicate> matchesIds = + r -> ids == null || ids.contains(r.getKey()); + + Predicate> matchesNotId = + r -> notId == null || !Objects.equals(notId, r.getKey()); + Predicate> matchesNotIds = + r -> notIds == null || !notIds.contains(r.getKey()); + + Predicate> matchesTopic = + r -> topic == null || Objects.equals(topic, r.getValue().getTopic()); + Predicate> matchesTopics = + r -> topics == null || topics.contains(r.getValue().getTopic()); + + Predicate> matchesNotTopic = + r -> notTopic == null || !Objects.equals(notTopic, r.getValue().getTopic()); + Predicate> matchesNotTopics = + r -> notTopics == null || !notTopics.contains(r.getValue().getTopic()); + + return List.of( + matchesId, + matchesIds, + matchesNotId, + matchesNotIds, + matchesTopic, + matchesTopics, + matchesNotTopic, + matchesNotTopics) + .stream() + .reduce(x -> true, Predicate::and) + .test(e); + } + } } From 4d59d3c282b2190a55e82db508ceaaf2b9040cf7 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 16 Apr 2025 14:07:11 -0400 Subject: [PATCH 04/11] add endpoint to retrieve available automated analysis report rules --- .../java/io/cryostat/reports/Reports.java | 30 +++++++++++++++++++ .../java/io/cryostat/reports/ReportsTest.java | 23 ++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/src/main/java/io/cryostat/reports/Reports.java b/src/main/java/io/cryostat/reports/Reports.java index fec3cd89b..7f99bfac3 100644 --- a/src/main/java/io/cryostat/reports/Reports.java +++ b/src/main/java/io/cryostat/reports/Reports.java @@ -19,7 +19,13 @@ import java.time.Instant; import java.util.Date; import java.util.Map; +import java.util.Objects; import java.util.UUID; +import java.util.stream.Stream; + +import org.openjdk.jmc.flightrecorder.rules.IRule; +import org.openjdk.jmc.flightrecorder.rules.RuleRegistry; +import org.openjdk.jmc.flightrecorder.rules.util.RulesToolkit.EventAvailability; import io.cryostat.ConfigProperties; import io.cryostat.StorageBuckets; @@ -50,6 +56,7 @@ import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.UriBuilder; +import org.apache.commons.lang3.StringUtils; import org.eclipse.microprofile.config.inject.ConfigProperty; import org.jboss.logging.Logger; import org.jboss.resteasy.reactive.RestPath; @@ -125,6 +132,15 @@ public Response get( .build(); } + @GET + @Path("/api/v4.1/reports_rules") + @RolesAllowed("read") + public Stream listReportRules() { + return RuleRegistry.getRules().stream() + .map(ReportRule::new) + .sorted((a, b) -> StringUtils.compare(a.id(), b.id())); + } + @POST @Blocking @Transactional @@ -230,4 +246,18 @@ public Response getActive( .build()) .build(); } + + private record ReportRule( + String id, String name, String topic, Map requiredEvents) { + ReportRule { + Objects.requireNonNull(id); + Objects.requireNonNull(name); + Objects.requireNonNull(topic); + Objects.requireNonNull(requiredEvents); + } + + ReportRule(IRule rule) { + this(rule.getId(), rule.getName(), rule.getTopic(), rule.getRequiredEvents()); + } + } } diff --git a/src/test/java/io/cryostat/reports/ReportsTest.java b/src/test/java/io/cryostat/reports/ReportsTest.java index 7bbf8f7f7..5a7963d37 100644 --- a/src/test/java/io/cryostat/reports/ReportsTest.java +++ b/src/test/java/io/cryostat/reports/ReportsTest.java @@ -32,6 +32,7 @@ import io.vertx.core.json.JsonObject; import jakarta.inject.Inject; import jakarta.websocket.DeploymentException; +import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; import org.junit.jupiter.api.Test; @@ -42,6 +43,28 @@ public class ReportsTest extends AbstractTransactionalTestBase { @Inject ObjectMapper mapper; + @Test + void testGetReportsRules() { + var json = + given().log() + .all() + .when() + .get("/api/v4.1/reports_rules") + .then() + .log() + .all() + .and() + .assertThat() + .statusCode(200) + .contentType(ContentType.JSON) + .and() + .extract() + .body() + .jsonPath(); + MatcherAssert.assertThat(json, Matchers.notNullValue()); + MatcherAssert.assertThat(json.get("$.size()"), Matchers.greaterThan(0)); + } + @Test void testGetBadArchiveSource() { given().log() From f7f05916d1449fae169db56f22ada7809a88fa41 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 16 Apr 2025 15:16:44 -0400 Subject: [PATCH 05/11] pass through parameter to delegate --- .../java/io/cryostat/reports/MemoryCachingReportsService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/cryostat/reports/MemoryCachingReportsService.java b/src/main/java/io/cryostat/reports/MemoryCachingReportsService.java index 8afa1efb1..daefe4af7 100644 --- a/src/main/java/io/cryostat/reports/MemoryCachingReportsService.java +++ b/src/main/java/io/cryostat/reports/MemoryCachingReportsService.java @@ -76,7 +76,7 @@ public Uni> reportFor(ActiveRecording recording, Str key, k -> { logger.tracev("reportFor {0} cache miss", k); - return delegate.reportFor(recording); + return delegate.reportFor(recording, filter); }); } @@ -93,7 +93,7 @@ public Uni> reportFor( key, k -> { logger.tracev("reportFor {0} cache miss", k); - return delegate.reportFor(jvmId, filename); + return delegate.reportFor(jvmId, filename, filter); }); } From 9f2c9623c77b7f14d8baaefdb57cf88387c1e7db Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Wed, 16 Apr 2025 15:32:07 -0400 Subject: [PATCH 06/11] correct request/config filter combining behaviour --- .../LongRunningRequestGenerator.java | 5 ++-- .../reports/MemoryCachingReportsService.java | 5 ++-- .../cryostat/reports/ReportsServiceImpl.java | 27 ++++++++++++++----- .../reports/StorageCachingReportsService.java | 5 ++-- src/main/resources/application.properties | 2 +- 5 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java b/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java index da63eaf54..9a154981a 100644 --- a/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java +++ b/src/main/java/io/cryostat/recordings/LongRunningRequestGenerator.java @@ -22,7 +22,6 @@ import io.cryostat.ConfigProperties; import io.cryostat.core.reports.InterruptibleReportGenerator.AnalysisResult; -import io.cryostat.core.util.RuleFilterParser; import io.cryostat.recordings.ArchivedRecordings.ArchivedRecording; import io.cryostat.reports.AnalysisReportAggregator; import io.cryostat.reports.ReportsService; @@ -312,7 +311,7 @@ public record ActiveReportRequest(String id, ActiveRecording recording, String f } public ActiveReportRequest(String id, ActiveRecording recording) { - this(id, recording, RuleFilterParser.ALL_WILDCARD_TOKEN); + this(id, recording, null); } } @@ -333,7 +332,7 @@ public record ArchivedReportRequest(String id, Pair pair, String } public ArchivedReportRequest(String id, Pair pair) { - this(id, pair, RuleFilterParser.ALL_WILDCARD_TOKEN); + this(id, pair, null); } } diff --git a/src/main/java/io/cryostat/reports/MemoryCachingReportsService.java b/src/main/java/io/cryostat/reports/MemoryCachingReportsService.java index daefe4af7..ffd9c3c02 100644 --- a/src/main/java/io/cryostat/reports/MemoryCachingReportsService.java +++ b/src/main/java/io/cryostat/reports/MemoryCachingReportsService.java @@ -19,7 +19,6 @@ import io.cryostat.ConfigProperties; import io.cryostat.core.reports.InterruptibleReportGenerator.AnalysisResult; -import io.cryostat.core.util.RuleFilterParser; import io.cryostat.recordings.ActiveRecording; import io.cryostat.recordings.RecordingHelper; @@ -99,12 +98,12 @@ public Uni> reportFor( @Override public Uni> reportFor(ActiveRecording recording) { - return reportFor(recording, RuleFilterParser.ALL_WILDCARD_TOKEN); + return reportFor(recording, null); } @Override public Uni> reportFor(String jvmId, String filename) { - return reportFor(jvmId, filename, RuleFilterParser.ALL_WILDCARD_TOKEN); + return reportFor(jvmId, filename, null); } @Override diff --git a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java index 1b59af91d..1271f0fe4 100644 --- a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java +++ b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java @@ -67,7 +67,7 @@ class ReportsServiceImpl implements ReportsService { Optional externalStorageUrl; @ConfigProperty(name = ConfigProperties.REPORTS_FILTER) - String configFilter; + Optional configFilter; @Inject ObjectMapper mapper; @Inject RecordingHelper helper; @@ -128,12 +128,27 @@ public Uni> reportFor( @Override public Uni> reportFor(ActiveRecording recording) { - return reportFor(recording, RuleFilterParser.ALL_WILDCARD_TOKEN); + return reportFor(recording, null); } @Override public Uni> reportFor(String jvmId, String filename) { - return reportFor(jvmId, filename, RuleFilterParser.ALL_WILDCARD_TOKEN); + return reportFor(jvmId, filename, null); + } + + private String getEffectiveFilter(String requested) { + // if the request has its own filter, combine it with the config filter by separating them + // with a comma + return Optional.ofNullable(requested) + .flatMap( + s -> + configFilter + .map(c -> String.format("%s,%s", c, s)) + .or(() -> Optional.of(s))) + // if there is no request filter, fall back to the config filter + .or(() -> configFilter) + // if there is no config filter either, then use the wildcard filter to process all + .orElse(RuleFilterParser.ALL_WILDCARD_TOKEN); } private Uni> process(InputStream stream, String filter) { @@ -141,13 +156,11 @@ private Uni> process(InputStream stream, String filt .future( reportGenerator.generateEvalMapInterruptibly( new BufferedInputStream(stream), - ruleFilterParser - .parse(configFilter) - .and(ruleFilterParser.parse(filter)))); + ruleFilterParser.parse(getEffectiveFilter(filter)))); } private Uni> fireRequest(InputStream stream, String filter) { - return sidecar.generate(stream, String.format("%s,%s", configFilter, filter)); + return sidecar.generate(stream, getEffectiveFilter(filter)); } public static class ReportGenerationException extends RuntimeException { diff --git a/src/main/java/io/cryostat/reports/StorageCachingReportsService.java b/src/main/java/io/cryostat/reports/StorageCachingReportsService.java index 8276f656b..46e07c2bc 100644 --- a/src/main/java/io/cryostat/reports/StorageCachingReportsService.java +++ b/src/main/java/io/cryostat/reports/StorageCachingReportsService.java @@ -23,7 +23,6 @@ import io.cryostat.ConfigProperties; import io.cryostat.core.reports.InterruptibleReportGenerator.AnalysisResult; -import io.cryostat.core.util.RuleFilterParser; import io.cryostat.recordings.ActiveRecording; import io.cryostat.recordings.RecordingHelper; import io.cryostat.util.HttpMimeType; @@ -157,12 +156,12 @@ private Uni> getStorage(String key) { @Override public Uni> reportFor(ActiveRecording recording) { - return reportFor(recording, RuleFilterParser.ALL_WILDCARD_TOKEN); + return reportFor(recording, null); } @Override public Uni> reportFor(String jvmId, String filename) { - return reportFor(jvmId, filename, RuleFilterParser.ALL_WILDCARD_TOKEN); + return reportFor(jvmId, filename, null); } @Override diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index 871dbf6bd..036ee0f7e 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -42,7 +42,7 @@ cryostat.services.reports.storage-cache.enabled=true cryostat.services.reports.storage-cache.name=archivedreports cryostat.services.reports.storage-cache.expiry-duration=24h cryostat.services.reports.use-presigned-transfer=true -cryostat.services.reports.filter=* +cryostat.services.reports.filter= cryostat.http.proxy.tls-enabled=false cryostat.http.proxy.host=${quarkus.http.host} From 57239f73e5d6bad4a459901cfeaad01f6d6de854 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 24 Apr 2025 13:33:48 -0400 Subject: [PATCH 07/11] remove rebase duplicate --- src/main/java/io/cryostat/reports/ReportsServiceImpl.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java index 1271f0fe4..a4aeb41a9 100644 --- a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java +++ b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java @@ -209,10 +209,4 @@ private URI getPresignedPath(String jvmId, String filename) throws URISyntaxExce .build(); return URI.create(presigner.presignGetObject(presignRequest).url().toString()).normalize(); } - - public static class ReportGenerationException extends RuntimeException { - public ReportGenerationException(Throwable cause) { - super(cause); - } - } } From 735f23071360f8d29e6905c4f961737c42424785 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 24 Apr 2025 13:35:50 -0400 Subject: [PATCH 08/11] schema --- schema/openapi.yaml | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/schema/openapi.yaml b/schema/openapi.yaml index 0e600dd2c..3c0bcc086 100644 --- a/schema/openapi.yaml +++ b/schema/openapi.yaml @@ -285,6 +285,14 @@ components: useRethrow: type: boolean type: object + EventAvailability: + enum: + - AVAILABLE + - ENABLED + - DISABLED + - NONE + - UNKNOWN + type: string Field: properties: contentType: @@ -479,6 +487,19 @@ components: - STOPPED - CLOSED type: string + ReportRule: + properties: + id: + type: string + name: + type: string + requiredEvents: + additionalProperties: + $ref: '#/components/schemas/EventAvailability' + type: object + topic: + type: string + type: object RequestData: properties: matchExpression: @@ -874,6 +895,25 @@ paths: - SecurityScheme: [] tags: - Analysis Report Aggregator + /api/v4.1/reports_rules: + get: + responses: + "200": + content: + application/json: + schema: + items: + $ref: '#/components/schemas/ReportRule' + type: array + description: OK + "401": + description: Not Authorized + "403": + description: Not Allowed + security: + - SecurityScheme: [] + tags: + - Reports /api/v4.1/targets/{targetId}/reports: get: parameters: From 039ac20b456ddaddb85ea0b5aab063be3ed426f1 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 24 Apr 2025 13:37:00 -0400 Subject: [PATCH 09/11] fixup! schema --- .../cryostat/reports/ReportsServiceImpl.java | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java index a4aeb41a9..aca6fcba5 100644 --- a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java +++ b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java @@ -22,9 +22,6 @@ import java.time.Duration; import java.util.Map; import java.util.Optional; -import java.util.function.Predicate; - -import org.openjdk.jmc.flightrecorder.rules.IRule; import io.cryostat.ConfigProperties; import io.cryostat.core.reports.InterruptibleReportGenerator; @@ -163,12 +160,6 @@ private Uni> fireRequest(InputStream stream, String return sidecar.generate(stream, getEffectiveFilter(filter)); } - public static class ReportGenerationException extends RuntimeException { - public ReportGenerationException(Throwable cause) { - super(cause); - } - } - @Override public boolean keyExists(ActiveRecording recording) { return false; @@ -187,14 +178,6 @@ private boolean usePresignedSidecar() { return useSidecar() && usePresignedTransfer; } - private Uni> process( - InputStream stream, Predicate predicate) { - return Uni.createFrom() - .future( - reportGenerator.generateEvalMapInterruptibly( - new BufferedInputStream(stream), predicate)); - } - private URI getPresignedPath(String jvmId, String filename) throws URISyntaxException { logger.infov("Handling presigned download request for {0}/{1}", jvmId, filename); GetObjectRequest getRequest = @@ -209,4 +192,10 @@ private URI getPresignedPath(String jvmId, String filename) throws URISyntaxExce .build(); return URI.create(presigner.presignGetObject(presignRequest).url().toString()).normalize(); } + + public static class ReportGenerationException extends RuntimeException { + public ReportGenerationException(Throwable cause) { + super(cause); + } + } } From 4cc5ceb6d0b4eb80a1d614bf69878c88933c2f20 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 24 Apr 2025 15:08:03 -0400 Subject: [PATCH 10/11] refactor stream handling and exception handling --- .../cryostat/reports/ReportsServiceImpl.java | 49 +++++++++++-------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java index aca6fcba5..72a5a41af 100644 --- a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java +++ b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java @@ -16,6 +16,7 @@ package io.cryostat.reports; import java.io.BufferedInputStream; +import java.io.IOException; import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; @@ -86,40 +87,38 @@ public Uni> reportFor(ActiveRecording recording, Str logger.tracev( "inprocess reportFor active recording {0} {1}", recording.target.jvmId, recording.remoteId); - return process(stream, filter); + return process(stream, filter).invoke(safeClose(stream)); } else { logger.tracev( "sidecar reportFor active recording {0} {1}", recording.target.jvmId, recording.remoteId); - return fireRequest(stream, filter); + return fireRequest(stream, filter).invoke(safeClose(stream)); } } @Override public Uni> reportFor( String jvmId, String filename, String filter) { - InputStream stream; try { - stream = helper.getArchivedRecordingStream(jvmId, filename); - } catch (Exception e) { - throw new ReportGenerationException(e); - } - if (!useSidecar()) { - logger.tracev("inprocess reportFor archived recording {0} {1}", jvmId, filename); - return process(stream, filter); - } else if (usePresignedSidecar()) { - logger.tracev( - "sidecar reportFor presigned archived recording {0} {1}", jvmId, filename); - try { + if (!useSidecar()) { + InputStream stream = helper.getArchivedRecordingStream(jvmId, filename); + logger.tracev("inprocess reportFor archived recording {0} {1}", jvmId, filename); + return process(stream, filter).invoke(safeClose(stream)); + } else if (usePresignedSidecar()) { + logger.tracev( + "sidecar reportFor presigned archived recording {0} {1}", jvmId, filename); var uri = getPresignedPath(jvmId, filename); return sidecar.generatePresigned(uri.getPath(), uri.getQuery(), filter); - } catch (URISyntaxException e) { - logger.error(e); - throw new InternalServerErrorException(e); + } else { + InputStream stream = helper.getArchivedRecordingStream(jvmId, filename); + logger.tracev("sidecar reportFor archived recording {0} {1}", jvmId, filename); + return fireRequest(stream, filter).invoke(safeClose(stream)); } - } else { - logger.tracev("sidecar reportFor archived recording {0} {1}", jvmId, filename); - return fireRequest(stream, filter); + } catch (URISyntaxException e) { + logger.error(e); + throw new InternalServerErrorException(e); + } catch (Exception e) { + throw new ReportGenerationException(e); } } @@ -193,6 +192,16 @@ private URI getPresignedPath(String jvmId, String filename) throws URISyntaxExce return URI.create(presigner.presignGetObject(presignRequest).url().toString()).normalize(); } + private Runnable safeClose(InputStream stream) { + return () -> { + try { + stream.close(); + } catch (IOException e) { + logger.warn(e); + } + }; + } + public static class ReportGenerationException extends RuntimeException { public ReportGenerationException(Throwable cause) { super(cause); From 4991550a3be9095a2f14b560098496979be87eb9 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Thu, 24 Apr 2025 15:44:36 -0400 Subject: [PATCH 11/11] better operation for cleanup --- src/main/java/io/cryostat/recordings/RecordingHelper.java | 2 +- src/main/java/io/cryostat/reports/ReportsServiceImpl.java | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/io/cryostat/recordings/RecordingHelper.java b/src/main/java/io/cryostat/recordings/RecordingHelper.java index 3e669a0d0..56d16516f 100644 --- a/src/main/java/io/cryostat/recordings/RecordingHelper.java +++ b/src/main/java/io/cryostat/recordings/RecordingHelper.java @@ -1303,7 +1303,7 @@ private Uni uploadToJFRDatasource(Path recordingPath) }); } - Optional getRecordingCopyPath( + private Optional getRecordingCopyPath( JFRConnection connection, Target target, String recordingName) throws Exception { return connection.getService().getAvailableRecordings().stream() .filter(recording -> recording.getName().equals(recordingName)) diff --git a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java index 72a5a41af..8b49008b4 100644 --- a/src/main/java/io/cryostat/reports/ReportsServiceImpl.java +++ b/src/main/java/io/cryostat/reports/ReportsServiceImpl.java @@ -87,12 +87,12 @@ public Uni> reportFor(ActiveRecording recording, Str logger.tracev( "inprocess reportFor active recording {0} {1}", recording.target.jvmId, recording.remoteId); - return process(stream, filter).invoke(safeClose(stream)); + return process(stream, filter).eventually(safeClose(stream)); } else { logger.tracev( "sidecar reportFor active recording {0} {1}", recording.target.jvmId, recording.remoteId); - return fireRequest(stream, filter).invoke(safeClose(stream)); + return fireRequest(stream, filter).eventually(safeClose(stream)); } } @@ -103,7 +103,7 @@ public Uni> reportFor( if (!useSidecar()) { InputStream stream = helper.getArchivedRecordingStream(jvmId, filename); logger.tracev("inprocess reportFor archived recording {0} {1}", jvmId, filename); - return process(stream, filter).invoke(safeClose(stream)); + return process(stream, filter).eventually(safeClose(stream)); } else if (usePresignedSidecar()) { logger.tracev( "sidecar reportFor presigned archived recording {0} {1}", jvmId, filename); @@ -112,7 +112,7 @@ public Uni> reportFor( } else { InputStream stream = helper.getArchivedRecordingStream(jvmId, filename); logger.tracev("sidecar reportFor archived recording {0} {1}", jvmId, filename); - return fireRequest(stream, filter).invoke(safeClose(stream)); + return fireRequest(stream, filter).eventually(safeClose(stream)); } } catch (URISyntaxException e) { logger.error(e);