diff --git a/README.md b/README.md index 3ce06896..e57d266a 100644 --- a/README.md +++ b/README.md @@ -31,9 +31,18 @@ JAVA_OPTIONS="-Dcom.sun.management.jmxremote.port=9091 -Dcom.sun.management.jmxr ``` This assumes that the agent JAR has been included in the application image within `/deployments/app/`. +## Harvester + +The various `cryostat.agent.harvester.*` properties may be used to configure `cryostat-agent` to start a new Flight +Recording using a given event template on Agent initialization, and to periodically collect this recorded data and push +it to the Agent's associated Cryostat server. The Agent will also attempt to push the tail end of this recording on JVM +shutdown so that the cause of an unexpected JVM shutdown might be captured for later analysis. + ## SMART TRIGGERS -`cryostat-agent` supports smart triggers that listen to the values of the MBean Counters and can start recordings based on a set of constraints specified by the user. +`cryostat-agent` supports smart triggers that listen to the values of the MBean Counters and can start recordings based +on a set of constraints specified by the user. + The general form of a smart trigger expression is as follows: ``` @@ -46,7 +55,8 @@ An example for listening to CPU Usage and starting a recording using the Profili [ProcessCpuLoad>0.2]~profile ``` -An example for watching for the Thread Count to exceed 20 for longer than 10 seconds and starting a recording using the Continuous template: +An example for watching for the Thread Count to exceed 20 for longer than 10 seconds and starting a recording using the +Continuous template: ``` [ThreadCount>20&&TargetDuration>duration("10s")]~Continuous @@ -64,7 +74,16 @@ Multiple smart trigger definitions may be specified and separated by commas, for [ProcessCpuLoad>0.2]~profile,[ThreadCount>30]~Continuous ``` -**NOTE**: Smart Triggers are evaluated on a polling basis. The poll period is configurable (see list below). This means that your conditions are subject to sampling biases. +**NOTE**: Smart Triggers are evaluated on a polling basis. The poll period is configurable (see list below). This means +that your conditions are subject to sampling biases. + +### Harvester Integration + +Any Flight Recordings created by Smart Trigger will also be tracked by the Harvester system. This data will be captured +in a JFR Snapshot and pushed to the server on the Harvester's usual schedule. By defining Smart Triggers and a +Harvester period without a Harvester template, you can achieve a setup where dynamically-started Flight Recordings +begin when trigger conditions are met, and their data is then periodically captured until the recording is manually +stopped or the host JVM shuts down. ## CONFIGURATION @@ -90,8 +109,8 @@ and how it advertises itself to a Cryostat server instance. Required properties - [ ] `cryostat.agent.registration.retry-ms` [`long`]: the duration in milliseconds between attempts to register with the Cryostat server. Default `5000`. - [ ] `cryostat.agent.exit.signals` [`[String]`]: a comma-separated list of signals that the agent should handle. When any of these signals is caught the agent initiates an orderly shutdown, deregistering from the Cryostat server and potentially uploading the latest harvested JFR data. Default `INT,TERM`. - [ ] `cryostat.agent.exit.deregistration.timeout-ms` [`long`]: the duration in milliseconds to wait for a response from the Cryostat server when attempting to deregister at shutdown time . Default `3000`. -- [ ] `cryostat.agent.harvester.period-ms` [`long`]: the length of time between JFR collections and pushes by the harvester. This also controls the maximum age of data stored in the buffer for the harvester's managed Flight Recording. Every `period-ms` the harvester will upload a JFR binary file to the `cryostat.agent.baseuri` archives. Default `-1`, which indicates no harvesting will be performed. -- [ ] `cryostat.agent.harvester.template` [`String`]: the name of the `.jfc` event template configuration to use for the harvester's managed Flight Recording. Default `default`, the continuous monitoring event template. +- [ ] `cryostat.agent.harvester.period-ms` [`long`]: the length of time between JFR collections and pushes by the harvester. This also controls the maximum age of data stored in the buffer for the harvester's managed Flight Recording. Every `period-ms` the harvester will upload a JFR binary file to the `cryostat.agent.baseuri` archives. Default `-1`, which indicates no scheduled harvest uploading will be performed. +- [ ] `cryostat.agent.harvester.template` [`String`]: the name of the `.jfc` event template configuration to use for the harvester's managed Flight Recording. Defaults to the empty string, so that no recording is started. - [ ] `cryostat.agent.harvester.max-files` [`String`]: the maximum number of pushed files that Cryostat will keep over the network from the agent. This is supplied to the harvester's push requests which instructs Cryostat to prune, in a FIFO manner, the oldest JFR files within the attached JVM target's storage, while the number of stored recordings is greater than this configuration's maximum file limit. Default `2147483647` (`Integer.MAX_VALUE`). - [ ] `cryostat.agent.harvester.upload.timeout-ms` [`long`]: the duration in milliseconds to wait for HTTP upload requests to the Cryostat server to complete and respond. Default `30000`. - [ ] `cryostat.agent.harvester.exit.max-age-ms` [`long`]: the JFR `maxage` setting, specified in milliseconds, to apply to recording data uploaded to the Cryostat server when the JVM this Agent instance is attached to exits. This ensures that tail-end data is captured between the last periodic push and the application exit. Exit uploads only occur when the application receives `SIGINT`/`SIGTERM` from the operating system or container platform. diff --git a/src/main/java/io/cryostat/agent/Agent.java b/src/main/java/io/cryostat/agent/Agent.java index 91d390d9..d8714b74 100644 --- a/src/main/java/io/cryostat/agent/Agent.java +++ b/src/main/java/io/cryostat/agent/Agent.java @@ -28,6 +28,7 @@ import javax.inject.Named; import javax.inject.Singleton; +import io.cryostat.agent.harvest.Harvester; import io.cryostat.agent.triggers.TriggerEvaluator; import dagger.Component; diff --git a/src/main/java/io/cryostat/agent/ConfigModule.java b/src/main/java/io/cryostat/agent/ConfigModule.java index 212d8b00..c0f0dac8 100644 --- a/src/main/java/io/cryostat/agent/ConfigModule.java +++ b/src/main/java/io/cryostat/agent/ConfigModule.java @@ -232,7 +232,7 @@ public static long provideCryostatAgentHarvesterPeriod(SmallRyeConfig config) { @Singleton @Named(CRYOSTAT_AGENT_HARVESTER_TEMPLATE) public static String provideCryostatAgentHarvesterTemplate(SmallRyeConfig config) { - return config.getValue(CRYOSTAT_AGENT_HARVESTER_TEMPLATE, String.class); + return config.getOptionalValue(CRYOSTAT_AGENT_HARVESTER_TEMPLATE, String.class).orElse(""); } @Provides diff --git a/src/main/java/io/cryostat/agent/CryostatClient.java b/src/main/java/io/cryostat/agent/CryostatClient.java index fa91957c..a6d87193 100644 --- a/src/main/java/io/cryostat/agent/CryostatClient.java +++ b/src/main/java/io/cryostat/agent/CryostatClient.java @@ -30,13 +30,16 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.function.Function; +import io.cryostat.agent.FlightRecorderHelper.TemplatedRecording; import io.cryostat.agent.WebServer.Credentials; +import io.cryostat.agent.harvest.Harvester; import io.cryostat.agent.model.DiscoveryNode; import io.cryostat.agent.model.PluginInfo; import io.cryostat.agent.model.RegistrationInfo; @@ -46,6 +49,8 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import jdk.jfr.Configuration; +import jdk.jfr.Recording; import org.apache.commons.io.FileUtils; import org.apache.commons.io.input.CountingInputStream; import org.apache.http.HttpHeaders; @@ -360,21 +365,39 @@ public CompletableFuture update( } public CompletableFuture upload( - Harvester.PushType pushType, String template, int maxFiles, Path recording) + Harvester.PushType pushType, + Optional opt, + int maxFiles, + Path recording) throws IOException { Instant start = Instant.now(); String timestamp = start.truncatedTo(ChronoUnit.SECONDS).toString().replaceAll("[-:]", ""); - String fileName = String.format("%s_%s_%s.jfr", appName, template, timestamp); + String template = + opt.map(TemplatedRecording::getConfiguration) + .map(Configuration::getName) + .map(String::toLowerCase) + .map(String::trim) + .orElse("unknown"); + String fileName = + String.format( + "%s_%s_%s.jfr", + appName + + opt.map(TemplatedRecording::getRecording) + .map(Recording::getName) + .map(n -> "-" + n) + .orElse(""), + template, + timestamp); Map labels = Map.of( "jvmId", jvmId, + "pushType", + pushType.name(), "template.name", template, "template.type", - "TARGET", - "pushType", - pushType.name()); + "TARGET"); HttpPost req = new HttpPost(baseUri.resolve("/api/beta/recordings/" + jvmId)); diff --git a/src/main/java/io/cryostat/agent/FlightRecorderHelper.java b/src/main/java/io/cryostat/agent/FlightRecorderHelper.java index b12a4566..9814bc9c 100644 --- a/src/main/java/io/cryostat/agent/FlightRecorderHelper.java +++ b/src/main/java/io/cryostat/agent/FlightRecorderHelper.java @@ -15,65 +15,78 @@ */ package io.cryostat.agent; -import java.lang.management.ManagementFactory; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import jdk.jfr.Configuration; import jdk.jfr.FlightRecorder; import jdk.jfr.Recording; -import jdk.management.jfr.ConfigurationInfo; -import jdk.management.jfr.FlightRecorderMXBean; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class FlightRecorderHelper { - private final FlightRecorderMXBean bean = - ManagementFactory.getPlatformMXBean(FlightRecorderMXBean.class); private final Logger log = LoggerFactory.getLogger(getClass()); - // FIXME this is repeated logic shared with Harvester startRecording - public void startRecording(String templateNameOrLabel) { - getTemplate(templateNameOrLabel) - .ifPresentOrElse( - c -> { - long recordingId = bean.newRecording(); - bean.setPredefinedConfiguration(recordingId, c.getName()); - String recoringName = - String.format("cryostat-smart-trigger-%d", recordingId); - bean.setRecordingOptions( - recordingId, Map.of("name", recoringName, "disk", "true")); - bean.startRecording(recordingId); - log.info( - "Started recording \"{}\" using template \"{}\"", - recoringName, - templateNameOrLabel); - }, - () -> - log.error( - "Cannot start recording with template named or labelled {}", - templateNameOrLabel)); + public Optional createRecording(String templateNameOrLabel) { + Optional opt = getTemplate(templateNameOrLabel); + if (opt.isEmpty()) { + log.error( + "Cannot start recording with template named or labelled {}", + templateNameOrLabel); + return Optional.empty(); + } + Configuration configuration = opt.get(); + Recording recording = new Recording(configuration.getSettings()); + recording.setToDisk(true); + return Optional.of(new TemplatedRecording(configuration, recording)); } - public Optional getTemplate(String nameOrLabel) { - return bean.getConfigurations().stream() + public Optional getTemplate(String nameOrLabel) { + Objects.requireNonNull(nameOrLabel); + return Configuration.getConfigurations().stream() .filter(c -> c.getName().equals(nameOrLabel) || c.getLabel().equals(nameOrLabel)) .findFirst(); } public boolean isValidTemplate(String nameOrLabel) { + Objects.requireNonNull(nameOrLabel); return getTemplate(nameOrLabel).isPresent(); } public List getRecordings() { + if (!FlightRecorder.isAvailable()) { + log.error("FlightRecorder is unavailable"); + return List.of(); + } return FlightRecorder.getFlightRecorder().getRecordings().stream() .map(RecordingInfo::new) .collect(Collectors.toList()); } + @SuppressFBWarnings(value = {"EI_EXPOSE_REP", "EI_EXPOSE_REP2"}) + public static class TemplatedRecording { + private final Configuration configuration; + private final Recording recording; + + public TemplatedRecording(Configuration configuration, Recording recording) { + this.configuration = configuration; + this.recording = recording; + } + + public Configuration getConfiguration() { + return configuration; + } + + public Recording getRecording() { + return recording; + } + } + @SuppressFBWarnings(value = "URF_UNREAD_FIELD") public static class RecordingInfo { diff --git a/src/main/java/io/cryostat/agent/MainModule.java b/src/main/java/io/cryostat/agent/MainModule.java index 097f1d0f..12218056 100644 --- a/src/main/java/io/cryostat/agent/MainModule.java +++ b/src/main/java/io/cryostat/agent/MainModule.java @@ -34,11 +34,10 @@ import javax.net.ssl.TrustManager; import javax.net.ssl.X509TrustManager; -import io.cryostat.agent.Harvester.RecordingSettings; +import io.cryostat.agent.harvest.HarvestModule; import io.cryostat.agent.remote.RemoteContext; import io.cryostat.agent.remote.RemoteModule; -import io.cryostat.agent.triggers.TriggerEvaluator; -import io.cryostat.agent.triggers.TriggerParser; +import io.cryostat.agent.triggers.TriggerModule; import io.cryostat.core.net.JFRConnection; import io.cryostat.core.net.JFRConnectionToolkit; import io.cryostat.core.sys.Environment; @@ -64,6 +63,8 @@ includes = { ConfigModule.class, RemoteModule.class, + HarvestModule.class, + TriggerModule.class, }) public abstract class MainModule { @@ -71,7 +72,6 @@ public abstract class MainModule { private static final int NUM_WORKER_THREADS = 3; private static final String JVM_ID = "JVM_ID"; private static final String TEMPLATES_PATH = "TEMPLATES_PATH"; - private static final String TRIGGER_SCHEDULER = "TRIGGER_SCHEDULER"; @Provides @Singleton @@ -236,73 +236,12 @@ public static Registration provideRegistration( registrationCheckMs); } - @Provides - @Singleton - public static Harvester provideHarvester( - ScheduledExecutorService workerPool, - @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_PERIOD_MS) long period, - @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_TEMPLATE) String template, - @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_MAX_FILES) int maxFiles, - @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_EXIT_MAX_AGE_MS) long exitMaxAge, - @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_EXIT_MAX_SIZE_B) long exitMaxSize, - @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_MAX_AGE_MS) long maxAge, - @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_MAX_SIZE_B) long maxSize, - CryostatClient client, - Registration registration) { - RecordingSettings exitSettings = new RecordingSettings(); - exitSettings.maxAge = exitMaxAge; - exitSettings.maxSize = exitMaxSize; - RecordingSettings periodicSettings = new RecordingSettings(); - periodicSettings.maxAge = maxAge > 0 ? maxAge : (long) (period * 1.5); - periodicSettings.maxSize = maxSize; - return new Harvester( - Executors.newSingleThreadScheduledExecutor( - r -> { - Thread t = new Thread(r); - t.setName("cryostat-agent-harvester"); - t.setDaemon(true); - return t; - }), - workerPool, - period, - template, - maxFiles, - exitSettings, - periodicSettings, - client, - registration); - } - - @Provides - @Singleton - @Named(TRIGGER_SCHEDULER) - public static ScheduledExecutorService provideTriggerScheduler() { - return Executors.newScheduledThreadPool(0); - } - @Provides @Singleton public static FlightRecorderHelper provideFlightRecorderHelper() { return new FlightRecorderHelper(); } - @Provides - @Singleton - public static TriggerParser provideTriggerParser(FlightRecorderHelper helper) { - return new TriggerParser(helper); - } - - @Provides - @Singleton - public static TriggerEvaluator provideTriggerEvaluatorFactory( - @Named(TRIGGER_SCHEDULER) ScheduledExecutorService scheduler, - TriggerParser parser, - FlightRecorderHelper helper, - @Named(ConfigModule.CRYOSTAT_AGENT_SMART_TRIGGER_EVALUATION_PERIOD_MS) - long evaluationPeriodMs) { - return new TriggerEvaluator(scheduler, parser, helper, evaluationPeriodMs); - } - @Provides @Singleton public static FileSystem provideFileSystem() { diff --git a/src/main/java/io/cryostat/agent/Registration.java b/src/main/java/io/cryostat/agent/Registration.java index 8d7ca43a..2f792c33 100644 --- a/src/main/java/io/cryostat/agent/Registration.java +++ b/src/main/java/io/cryostat/agent/Registration.java @@ -36,12 +36,13 @@ import io.cryostat.agent.model.DiscoveryNode; import io.cryostat.agent.model.PluginInfo; +import io.cryostat.agent.util.StringUtils; import org.apache.http.client.utils.URIBuilder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -class Registration { +public class Registration { private static final String NODE_TYPE = "JVM"; @@ -186,7 +187,7 @@ void start() { log.info("{} started", getClass().getName()); } - void addRegistrationListener(Consumer listener) { + public void addRegistrationListener(Consumer listener) { this.listeners.add(listener); } @@ -358,9 +359,9 @@ public void notify(RegistrationEvent.State state) { executor.submit(() -> this.listeners.forEach(listener -> listener.accept(evt))); } - static class RegistrationEvent { + public static class RegistrationEvent { - enum State { + public enum State { UNREGISTERED, REGISTERED, PUBLISHED, @@ -368,7 +369,7 @@ enum State { REFRESHED, } - final State state; + public final State state; RegistrationEvent(State state) { this.state = state; diff --git a/src/main/java/io/cryostat/agent/harvest/HarvestModule.java b/src/main/java/io/cryostat/agent/harvest/HarvestModule.java new file mode 100644 index 00000000..9234bdb2 --- /dev/null +++ b/src/main/java/io/cryostat/agent/harvest/HarvestModule.java @@ -0,0 +1,73 @@ +/* + * Copyright The Cryostat Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.cryostat.agent.harvest; + +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; + +import javax.inject.Named; +import javax.inject.Singleton; + +import io.cryostat.agent.ConfigModule; +import io.cryostat.agent.CryostatClient; +import io.cryostat.agent.FlightRecorderHelper; +import io.cryostat.agent.Registration; +import io.cryostat.agent.harvest.Harvester.RecordingSettings; + +import dagger.Module; +import dagger.Provides; + +@Module +public abstract class HarvestModule { + @Provides + @Singleton + public static Harvester provideHarvester( + ScheduledExecutorService workerPool, + @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_PERIOD_MS) long period, + @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_TEMPLATE) String template, + @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_MAX_FILES) int maxFiles, + @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_EXIT_MAX_AGE_MS) long exitMaxAge, + @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_EXIT_MAX_SIZE_B) long exitMaxSize, + @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_MAX_AGE_MS) long maxAge, + @Named(ConfigModule.CRYOSTAT_AGENT_HARVESTER_MAX_SIZE_B) long maxSize, + CryostatClient client, + FlightRecorderHelper flightRecorderHelper, + Registration registration) { + RecordingSettings exitSettings = new RecordingSettings(); + exitSettings.maxAge = exitMaxAge; + exitSettings.maxSize = exitMaxSize; + RecordingSettings periodicSettings = new RecordingSettings(); + periodicSettings.maxAge = maxAge > 0 ? maxAge : (long) (period * 1.5); + periodicSettings.maxSize = maxSize; + return new Harvester( + Executors.newSingleThreadScheduledExecutor( + r -> { + Thread t = new Thread(r); + t.setName("cryostat-agent-harvester"); + t.setDaemon(true); + return t; + }), + workerPool, + period, + template, + maxFiles, + exitSettings, + periodicSettings, + client, + flightRecorderHelper, + registration); + } +} diff --git a/src/main/java/io/cryostat/agent/Harvester.java b/src/main/java/io/cryostat/agent/harvest/Harvester.java similarity index 54% rename from src/main/java/io/cryostat/agent/Harvester.java rename to src/main/java/io/cryostat/agent/harvest/Harvester.java index 1a1a50f1..558a7635 100644 --- a/src/main/java/io/cryostat/agent/Harvester.java +++ b/src/main/java/io/cryostat/agent/harvest/Harvester.java @@ -13,25 +13,32 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package io.cryostat.agent; +package io.cryostat.agent.harvest; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; -import java.text.ParseException; import java.time.Duration; +import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicLong; import java.util.function.UnaryOperator; -import jdk.jfr.Configuration; +import io.cryostat.agent.CryostatClient; +import io.cryostat.agent.FlightRecorderHelper; +import io.cryostat.agent.FlightRecorderHelper.TemplatedRecording; +import io.cryostat.agent.Registration; +import io.cryostat.agent.util.StringUtils; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import jdk.jfr.FlightRecorder; import jdk.jfr.FlightRecorderListener; import jdk.jfr.Recording; @@ -39,7 +46,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -class Harvester implements FlightRecorderListener { +public class Harvester implements FlightRecorderListener { private final Logger log = LoggerFactory.getLogger(getClass()); @@ -51,13 +58,16 @@ class Harvester implements FlightRecorderListener { private final RecordingSettings exitSettings; private final RecordingSettings periodicSettings; private final CryostatClient client; - private final AtomicLong recordingId = new AtomicLong(-1L); - private volatile Path exitPath; + private final FlightRecorderHelper flightRecorderHelper; + private final Set recordings = ConcurrentHashMap.newKeySet(); + private Optional sownRecording = Optional.empty(); + private final Map exitPaths = new ConcurrentHashMap<>(); private FlightRecorder flightRecorder; private Future task; private boolean running; - Harvester( + @SuppressFBWarnings("EI_EXPOSE_REP2") + public Harvester( ScheduledExecutorService executor, ScheduledExecutorService workerPool, long period, @@ -66,6 +76,7 @@ class Harvester implements FlightRecorderListener { RecordingSettings exitSettings, RecordingSettings periodicSettings, CryostatClient client, + FlightRecorderHelper flightRecorderHelper, Registration registration) { this.executor = executor; this.workerPool = workerPool; @@ -75,6 +86,7 @@ class Harvester implements FlightRecorderListener { this.exitSettings = exitSettings; this.periodicSettings = periodicSettings; this.client = client; + this.flightRecorderHelper = flightRecorderHelper; registration.addRegistrationListener( evt -> { @@ -101,10 +113,7 @@ public void start() { if (running) { return; } - if (period <= 0) { - log.info("Harvester disabled, period {} < 0", period); - return; - } + this.running = true; if (StringUtils.isBlank(template)) { log.info("Template not specified"); } @@ -121,10 +130,6 @@ public void start() { try { FlightRecorder.addListener(this); this.flightRecorder = FlightRecorder.getFlightRecorder(); - log.info( - "JFR Harvester started using template \"{}\" with period {}", - template, - Duration.ofMillis(period)); if (exitSettings.maxAge > 0) { log.info( "On-stop uploads will contain approximately the most recent" @@ -158,7 +163,19 @@ public void start() { return; } startRecording(true); - running = true; + if (this.task != null) { + this.task.cancel(true); + } + if (period > 0) { + log.info("JFR Harvester started with period {}", Duration.ofMillis(period)); + this.task = + workerPool.scheduleAtFixedRate( + this::uploadOngoing, period, period, TimeUnit.MILLISECONDS); + } else { + log.info( + "JFR Harvester started, periodic uploads disabled (period {} < 0)", + period); + } }); } @@ -182,48 +199,66 @@ public void stop() { @Override public void recordingStateChanged(Recording recording) { log.info("{}({}) {}", recording.getName(), recording.getId(), recording.getState().name()); - if (this.recordingId.get() == recording.getId()) { - switch (recording.getState()) { - case NEW: - break; - case DELAYED: - break; - case RUNNING: - break; - case STOPPED: - recording.close(); // we should get notified for the CLOSED state next - break; - case CLOSED: - executor.submit( - () -> { - if (running) { + getTrackedRecordingById(recording.getId()) + .ifPresent( + tr -> { + boolean isSownRecording = + sownRecording + .map(TemplatedRecording::getRecording) + .map(Recording::getId) + .map(id -> id == recording.getId()) + .orElse(false); + switch (recording.getState()) { + case NEW: + break; + case DELAYED: + break; + case RUNNING: + break; + case STOPPED: try { - uploadDumpedFile().get(); - } catch (ExecutionException - | InterruptedException - | IOException e) { + tr.getRecording().dump(exitPaths.get(tr)); + uploadRecording(tr).get(); + } catch (IOException e) { + log.error("Failed to dump recording to file", e); + } catch (InterruptedException | ExecutionException e) { log.warn("Could not upload exit dump file", e); - } finally { - startRecording(false); } - } - }); - break; - default: - log.warn( - "Unknown state {} for recording with ID {}", - recording.getState(), - recording.getId()); - break; - } - } + if (isSownRecording) { + safeCloseCurrentRecording(); + } + // next + break; + case CLOSED: + executor.submit( + () -> { + try { + recordings.remove(tr); + Path exitPath = exitPaths.remove(tr); + Files.deleteIfExists(exitPath); + log.trace("Deleted temp file {}", exitPath); + } catch (IOException e) { + log.warn("Could not delete temp file", e); + } finally { + startRecording(false); + } + }); + break; + default: + log.warn( + "Unknown state {} for recording with ID {}", + recording.getState(), + recording.getId()); + break; + } + }); } - Future exitUpload() { + public Future exitUpload() { return CompletableFuture.supplyAsync( () -> { running = false; - if (flightRecorder == null || period <= 0) { + if (flightRecorder == null) { return null; } try { @@ -240,56 +275,63 @@ Future exitUpload() { executor); } + public void handleNewRecording(TemplatedRecording tr) { + try { + Recording recording = tr.getRecording(); + recording.setToDisk(true); + recording.setDumpOnExit(true); + recording = this.periodicSettings.apply(recording); + Path path = Files.createTempFile(null, null); + Files.write(path, new byte[0], StandardOpenOption.TRUNCATE_EXISTING); + recording.setDestination(path); + log.trace("{}({}) will dump to {}", recording.getName(), recording.getId(), path); + this.recordings.add(tr); + this.exitPaths.put(tr, path); + } catch (IOException ioe) { + log.error("Unable to handle recording", ioe); + tr.getRecording().close(); + } + } + private void startRecording(boolean restart) { executor.submit( () -> { - if (restart) { + if (StringUtils.isBlank(template)) { + return; + } else if (restart) { safeCloseCurrentRecording(); - } else if (getById(this.recordingId.get()).isPresent()) { + } else if (sownRecording.isPresent()) { return; } - Recording recording = null; - try { - Configuration config = Configuration.getConfiguration(template); - recording = new Recording(config); - recording.setName("cryostat-agent"); - recording.setToDisk(true); - recording.setMaxAge(Duration.ofMillis(period)); - recording.setDumpOnExit(true); - this.exitPath = Files.createTempFile(null, null); - Files.write(exitPath, new byte[0], StandardOpenOption.TRUNCATE_EXISTING); - recording.setDestination(this.exitPath); - recording.start(); - this.recordingId.set(recording.getId()); - startPeriodic(); - } catch (ParseException | IOException e) { - log.error("Unable to start recording", e); - if (recording != null) { - recording.close(); - } - } + flightRecorderHelper + .createRecording(template) + .ifPresent( + recording -> { + recording + .getRecording() + .setName("cryostat-agent-harvester"); + handleNewRecording(recording); + this.sownRecording = Optional.of(recording); + recording.getRecording().start(); + log.info( + "JFR Harvester started recording using template" + + " \"{}\"", + template); + }); }); } - private void startPeriodic() { - if (this.task != null) { - this.task.cancel(true); - } - this.task = - workerPool.scheduleAtFixedRate( - this::uploadOngoing, period, period, TimeUnit.MILLISECONDS); - } - private void safeCloseCurrentRecording() { - getById(recordingId.get()).ifPresent(Recording::close); + sownRecording.map(TemplatedRecording::getRecording).ifPresent(Recording::close); + sownRecording = Optional.empty(); } - private Optional getById(long id) { + private Optional getTrackedRecordingById(long id) { if (id < 0) { return Optional.empty(); } - for (Recording recording : this.flightRecorder.getRecordings()) { - if (id == recording.getId()) { + for (TemplatedRecording recording : this.recordings) { + if (id == recording.getRecording().getId()) { return Optional.of(recording); } } @@ -307,9 +349,20 @@ private Future uploadOngoing(PushType pushType, RecordingSettings settings new IllegalStateException("No source recording data")); } try { + Path exitPath = Files.createTempFile(null, null); Files.write(exitPath, new byte[0], StandardOpenOption.TRUNCATE_EXISTING); recording.dump(exitPath); - return client.upload(pushType, template, maxFiles, exitPath); + log.trace("Dumping {}({}) to {}", recording.getName(), recording.getId(), exitPath); + return client.upload(pushType, sownRecording, maxFiles, exitPath) + .thenRun( + () -> { + try { + Files.deleteIfExists(exitPath); + log.trace("Deleted temp file {}", exitPath); + } catch (IOException ioe) { + log.warn("Failed to clean up snapshot dump file", ioe); + } + }); } catch (IOException e) { return CompletableFuture.failedFuture(e); } finally { @@ -317,19 +370,20 @@ private Future uploadOngoing(PushType pushType, RecordingSettings settings } } - private Future uploadDumpedFile() throws IOException { - return client.upload(PushType.EMERGENCY, template, maxFiles, exitPath); + private Future uploadRecording(TemplatedRecording tr) throws IOException { + Path exitPath = exitPaths.get(tr); + return client.upload(PushType.EMERGENCY, Optional.of(tr), maxFiles, exitPath); } - enum PushType { + public enum PushType { SCHEDULED, ON_STOP, EMERGENCY, } - static class RecordingSettings implements UnaryOperator { - long maxSize; - long maxAge; + public static class RecordingSettings implements UnaryOperator { + public long maxSize; + public long maxAge; @Override public Recording apply(Recording r) { diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index cc4502cc..db3399f8 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -44,7 +44,7 @@ import org.openjdk.jmc.rjmx.services.jfr.IRecordingDescriptor; import org.openjdk.jmc.rjmx.services.jfr.IRecordingDescriptor.RecordingState; -import io.cryostat.agent.StringUtils; +import io.cryostat.agent.util.StringUtils; import io.cryostat.core.FlightRecorderException; import io.cryostat.core.net.JFRConnection; import io.cryostat.core.net.JFRConnectionToolkit; diff --git a/src/main/java/io/cryostat/agent/triggers/TriggerEvaluator.java b/src/main/java/io/cryostat/agent/triggers/TriggerEvaluator.java index c2aad7ba..bec064d9 100644 --- a/src/main/java/io/cryostat/agent/triggers/TriggerEvaluator.java +++ b/src/main/java/io/cryostat/agent/triggers/TriggerEvaluator.java @@ -27,6 +27,7 @@ import java.util.concurrent.TimeUnit; import io.cryostat.agent.FlightRecorderHelper; +import io.cryostat.agent.harvest.Harvester; import io.cryostat.agent.model.MBeanInfo; import io.cryostat.agent.triggers.SmartTrigger.TriggerState; @@ -44,6 +45,7 @@ public class TriggerEvaluator { private final ScheduledExecutorService scheduler; private final TriggerParser parser; private final FlightRecorderHelper flightRecorderHelper; + private final Harvester harvester; private final long evaluationPeriodMs; private final ConcurrentLinkedQueue triggers = new ConcurrentLinkedQueue<>(); private Future task; @@ -52,11 +54,13 @@ public class TriggerEvaluator { public TriggerEvaluator( ScheduledExecutorService scheduler, TriggerParser parser, - FlightRecorderHelper flightRecorderModule, + FlightRecorderHelper flightRecorderHelper, + Harvester harvester, long evaluationPeriodMs) { this.scheduler = scheduler; this.parser = parser; - this.flightRecorderHelper = flightRecorderModule; + this.flightRecorderHelper = flightRecorderHelper; + this.harvester = harvester; this.evaluationPeriodMs = evaluationPeriodMs; } @@ -109,8 +113,7 @@ private void evaluate() { // met once if (t.isSimple() && evaluateTriggerConstraint(t, t.getTargetDuration())) { log.trace("Trigger {} satisfied, starting recording...", t); - flightRecorderHelper.startRecording(t.getRecordingTemplateName()); - t.setState(TriggerState.COMPLETE); + startRecording(t); } else if (!t.isSimple()) { if (evaluateTriggerConstraint(t, Duration.ZERO)) { // Condition was met, set the state accordingly @@ -128,8 +131,7 @@ private void evaluate() { // Condition was met at last check but duration hasn't passed if (evaluateTriggerConstraint(t, Duration.ofMillis(difference))) { log.trace("Trigger {} satisfied, completing...", t); - t.setState(TriggerState.COMPLETE); - flightRecorderHelper.startRecording(t.getRecordingTemplateName()); + startRecording(t); } else if (evaluateTriggerConstraint(t, Duration.ZERO)) { log.trace("Trigger {} satisfied, waiting for duration...", t); } else { @@ -153,6 +155,25 @@ private void evaluate() { } } + private void startRecording(SmartTrigger t) { + flightRecorderHelper + .createRecording(t.getRecordingTemplateName()) + .ifPresent( + tr -> { + String recordingName = + String.format( + "cryostat-smart-trigger-%d", tr.getRecording().getId()); + tr.getRecording().setName(recordingName); + harvester.handleNewRecording(tr); + tr.getRecording().start(); + t.setState(TriggerState.COMPLETE); + log.info( + "Started recording \"{}\" using template \"{}\"", + recordingName, + t.getRecordingTemplateName()); + }); + } + private boolean evaluateTriggerConstraint(SmartTrigger trigger, Duration targetDuration) { try { Map scriptVars = new HashMap<>(new MBeanInfo().getSimplifiedMetrics()); diff --git a/src/main/java/io/cryostat/agent/triggers/TriggerModule.java b/src/main/java/io/cryostat/agent/triggers/TriggerModule.java new file mode 100644 index 00000000..94896a2e --- /dev/null +++ b/src/main/java/io/cryostat/agent/triggers/TriggerModule.java @@ -0,0 +1,60 @@ +/* + * Copyright The Cryostat Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.cryostat.agent.triggers; + +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; + +import javax.inject.Named; +import javax.inject.Singleton; + +import io.cryostat.agent.ConfigModule; +import io.cryostat.agent.FlightRecorderHelper; +import io.cryostat.agent.harvest.Harvester; + +import dagger.Module; +import dagger.Provides; + +@Module +public abstract class TriggerModule { + + private static final String TRIGGER_SCHEDULER = "TRIGGER_SCHEDULER"; + + @Provides + @Singleton + @Named(TRIGGER_SCHEDULER) + public static ScheduledExecutorService provideTriggerScheduler() { + return Executors.newScheduledThreadPool(0); + } + + @Provides + @Singleton + public static TriggerParser provideTriggerParser(FlightRecorderHelper helper) { + return new TriggerParser(helper); + } + + @Provides + @Singleton + public static TriggerEvaluator provideTriggerEvaluatorFactory( + @Named(TRIGGER_SCHEDULER) ScheduledExecutorService scheduler, + TriggerParser parser, + FlightRecorderHelper helper, + Harvester harvester, + @Named(ConfigModule.CRYOSTAT_AGENT_SMART_TRIGGER_EVALUATION_PERIOD_MS) + long evaluationPeriodMs) { + return new TriggerEvaluator(scheduler, parser, helper, harvester, evaluationPeriodMs); + } +} diff --git a/src/main/java/io/cryostat/agent/triggers/TriggerParser.java b/src/main/java/io/cryostat/agent/triggers/TriggerParser.java index 4e099fee..391a45e6 100644 --- a/src/main/java/io/cryostat/agent/triggers/TriggerParser.java +++ b/src/main/java/io/cryostat/agent/triggers/TriggerParser.java @@ -22,7 +22,7 @@ import java.util.regex.Pattern; import io.cryostat.agent.FlightRecorderHelper; -import io.cryostat.agent.StringUtils; +import io.cryostat.agent.util.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/src/main/java/io/cryostat/agent/StringUtils.java b/src/main/java/io/cryostat/agent/util/StringUtils.java similarity index 95% rename from src/main/java/io/cryostat/agent/StringUtils.java rename to src/main/java/io/cryostat/agent/util/StringUtils.java index 6d756df7..a8f76f8f 100644 --- a/src/main/java/io/cryostat/agent/StringUtils.java +++ b/src/main/java/io/cryostat/agent/util/StringUtils.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package io.cryostat.agent; +package io.cryostat.agent.util; public class StringUtils { private StringUtils() {} diff --git a/src/main/resources/META-INF/microprofile-config.properties b/src/main/resources/META-INF/microprofile-config.properties index 73b3a1be..e2b80184 100644 --- a/src/main/resources/META-INF/microprofile-config.properties +++ b/src/main/resources/META-INF/microprofile-config.properties @@ -20,7 +20,7 @@ cryostat.agent.registration.check-ms=60000 cryostat.agent.exit.deregistration.timeout-ms=3000 cryostat.agent.harvester.period-ms=-1 -cryostat.agent.harvester.template=default +cryostat.agent.harvester.template= cryostat.agent.harvester.max-files=2147483647 cryostat.agent.harvester.upload.timeout-ms=30000 cryostat.agent.harvester.exit.max-age-ms=0