From d360a51ce105f1568292b1a7ea07d2aca9156a07 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Wed, 16 Aug 2023 11:46:06 -0400 Subject: [PATCH 1/9] accept snapshot --- .../agent/remote/RecordingsContext.java | 86 +++++++++++++------ 1 file changed, 58 insertions(+), 28 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index fbd7b0ca..6c481417 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -62,8 +62,7 @@ class RecordingsContext implements RemoteContext { private static final String PATH = "/recordings/"; - private static final Pattern PATH_ID_PATTERN = - Pattern.compile("^" + PATH + "(\\d+)$", Pattern.MULTILINE); + private static final Pattern PATH_ID_PATTERN = Pattern.compile("^" + PATH + "(\\d+)$", Pattern.MULTILINE); private final Logger log = LoggerFactory.getLogger(getClass()); private final SmallRyeConfig config; @@ -106,7 +105,7 @@ public void handle(HttpExchange exchange) throws IOException { } break; case "POST": - handleStart(exchange); + handleStartRecordingOrSnapshot(exchange); break; case "PATCH": id = extractId(exchange); @@ -193,13 +192,26 @@ private void handleGetRecording(HttpExchange exchange, long id) { }); } - private void handleStart(HttpExchange exchange) throws IOException { + private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOException { try (InputStream body = exchange.getRequestBody()) { StartRecordingRequest req = mapper.readValue(body, StartRecordingRequest.class); if (!req.isValid()) { exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); return; } + if (req.requestSnapshot()) { + try { + SerializableRecordingDescriptor snapshot = startSnapshot(req, exchange); + exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); + try (OutputStream response = exchange.getResponseBody()) { + mapper.writeValue(response, snapshot); + } + return; + } catch (IOException e) { + log.error("Failed to start snapshot", e); + exchange.sendResponseHeaders(HttpStatus.SC_SERVICE_UNAVAILABLE, BODY_LENGTH_NONE); + } + } SerializableRecordingDescriptor recording = startRecording(req); exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { @@ -283,35 +295,31 @@ private List getRecordings() { private SerializableRecordingDescriptor startRecording(StartRecordingRequest req) throws QuantityConversionException, ServiceNotAvailableException, - FlightRecorderException, - org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, - InvalidEventTemplateException, InvalidXmlException, IOException { + FlightRecorderException, + org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, + InvalidEventTemplateException, InvalidXmlException, IOException { Runnable cleanup = () -> {}; try { - JFRConnection conn = - jfrConnectionToolkit.connect( - jfrConnectionToolkit.createServiceURL("localhost", 0)); + JFRConnection conn = jfrConnectionToolkit.connect( + jfrConnectionToolkit.createServiceURL("localhost", 0)); IConstrainedMap events; if (req.requestsCustomTemplate()) { - Template template = - localStorageTemplateService.addTemplate( - new ByteArrayInputStream( - req.template.getBytes(StandardCharsets.UTF_8))); + Template template = localStorageTemplateService.addTemplate( + new ByteArrayInputStream( + req.template.getBytes(StandardCharsets.UTF_8))); events = localStorageTemplateService.getEvents(template).orElseThrow(); - cleanup = - () -> { - try { - localStorageTemplateService.deleteTemplate(template); - } catch (InvalidEventTemplateException | IOException e) { - log.error("Failed to clean up template " + template.getName(), e); - } - }; + cleanup = () -> { + try { + localStorageTemplateService.deleteTemplate(template); + } catch (InvalidEventTemplateException | IOException e) { + log.error("Failed to clean up template " + template.getName(), e); + } + }; } else { - events = - new RemoteTemplateService(conn) - .getEvents(req.localTemplateName, TemplateType.TARGET).stream() - .findFirst() - .orElseThrow(); + events = new RemoteTemplateService(conn) + .getEvents(req.localTemplateName, TemplateType.TARGET).stream() + .findFirst() + .orElseThrow(); } IFlightRecorderService svc = conn.getService(); return new SerializableRecordingDescriptor( @@ -329,6 +337,21 @@ private SerializableRecordingDescriptor startRecording(StartRecordingRequest req } } + private SerializableRecordingDescriptor startSnapshot(StartRecordingRequest req, HttpExchange exchange) + throws IOException { + Runnable cleanup = () -> {}; + try { + Recording snapshot = FlightRecorder.getFlightRecorder().takeSnapshot(); + if (snapshot.getSize() == 0) { + log.warn("No active recordings"); + exchange.sendResponseHeaders(HttpStatus.SC_SERVICE_UNAVAILABLE, BODY_LENGTH_NONE); + } + return new SerializableRecordingDescriptor(snapshot); + } finally { + cleanup.run(); + } + } + static class StartRecordingRequest { public String name; @@ -346,12 +369,19 @@ boolean requestsBundledTemplate() { return !StringUtils.isBlank(localTemplateName); } + boolean requestSnapshot() { + boolean snapshotName = name.equals("snapshot"); + boolean snapshotTemplate = StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); + boolean snapshotFeatures = duration == 0 && maxSize == 0 && maxAge == 0; + return snapshotName && snapshotTemplate && snapshotFeatures; + } + boolean isValid() { boolean requestsCustomTemplate = requestsCustomTemplate(); boolean requestsBundledTemplate = requestsBundledTemplate(); boolean requestsEither = requestsCustomTemplate || requestsBundledTemplate; boolean requestsBoth = requestsCustomTemplate && requestsBundledTemplate; - return requestsEither && !requestsBoth; + return requestsEither && !requestsBoth && !requestSnapshot(); } } } From f80cb466ced29f328e0e16a2e3042ba6ddbe32cd Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Wed, 16 Aug 2023 13:33:52 -0400 Subject: [PATCH 2/9] update --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 6c481417..dcecc4b6 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -206,11 +206,11 @@ private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOExce try (OutputStream response = exchange.getResponseBody()) { mapper.writeValue(response, snapshot); } - return; } catch (IOException e) { log.error("Failed to start snapshot", e); exchange.sendResponseHeaders(HttpStatus.SC_SERVICE_UNAVAILABLE, BODY_LENGTH_NONE); } + return; } SerializableRecordingDescriptor recording = startRecording(req); exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); From 5e42c4cedea760245a69c39d6be222708cc7ec9f Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Thu, 24 Aug 2023 10:17:03 -0400 Subject: [PATCH 3/9] clean up --- .../agent/remote/RecordingsContext.java | 55 +++++++++++-------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index dcecc4b6..bb3324f9 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -62,7 +62,8 @@ class RecordingsContext implements RemoteContext { private static final String PATH = "/recordings/"; - private static final Pattern PATH_ID_PATTERN = Pattern.compile("^" + PATH + "(\\d+)$", Pattern.MULTILINE); + private static final Pattern PATH_ID_PATTERN = + Pattern.compile("^" + PATH + "(\\d+)$", Pattern.MULTILINE); private final Logger log = LoggerFactory.getLogger(getClass()); private final SmallRyeConfig config; @@ -208,7 +209,8 @@ private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOExce } } catch (IOException e) { log.error("Failed to start snapshot", e); - exchange.sendResponseHeaders(HttpStatus.SC_SERVICE_UNAVAILABLE, BODY_LENGTH_NONE); + exchange.sendResponseHeaders( + HttpStatus.SC_SERVICE_UNAVAILABLE, BODY_LENGTH_NONE); } return; } @@ -295,31 +297,35 @@ private List getRecordings() { private SerializableRecordingDescriptor startRecording(StartRecordingRequest req) throws QuantityConversionException, ServiceNotAvailableException, - FlightRecorderException, - org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, - InvalidEventTemplateException, InvalidXmlException, IOException { + FlightRecorderException, + org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, + InvalidEventTemplateException, InvalidXmlException, IOException { Runnable cleanup = () -> {}; try { - JFRConnection conn = jfrConnectionToolkit.connect( - jfrConnectionToolkit.createServiceURL("localhost", 0)); + JFRConnection conn = + jfrConnectionToolkit.connect( + jfrConnectionToolkit.createServiceURL("localhost", 0)); IConstrainedMap events; if (req.requestsCustomTemplate()) { - Template template = localStorageTemplateService.addTemplate( - new ByteArrayInputStream( - req.template.getBytes(StandardCharsets.UTF_8))); + Template template = + localStorageTemplateService.addTemplate( + new ByteArrayInputStream( + req.template.getBytes(StandardCharsets.UTF_8))); events = localStorageTemplateService.getEvents(template).orElseThrow(); - cleanup = () -> { - try { - localStorageTemplateService.deleteTemplate(template); - } catch (InvalidEventTemplateException | IOException e) { - log.error("Failed to clean up template " + template.getName(), e); - } - }; + cleanup = + () -> { + try { + localStorageTemplateService.deleteTemplate(template); + } catch (InvalidEventTemplateException | IOException e) { + log.error("Failed to clean up template " + template.getName(), e); + } + }; } else { - events = new RemoteTemplateService(conn) - .getEvents(req.localTemplateName, TemplateType.TARGET).stream() - .findFirst() - .orElseThrow(); + events = + new RemoteTemplateService(conn) + .getEvents(req.localTemplateName, TemplateType.TARGET).stream() + .findFirst() + .orElseThrow(); } IFlightRecorderService svc = conn.getService(); return new SerializableRecordingDescriptor( @@ -337,8 +343,8 @@ private SerializableRecordingDescriptor startRecording(StartRecordingRequest req } } - private SerializableRecordingDescriptor startSnapshot(StartRecordingRequest req, HttpExchange exchange) - throws IOException { + private SerializableRecordingDescriptor startSnapshot( + StartRecordingRequest req, HttpExchange exchange) throws IOException { Runnable cleanup = () -> {}; try { Recording snapshot = FlightRecorder.getFlightRecorder().takeSnapshot(); @@ -371,7 +377,8 @@ boolean requestsBundledTemplate() { boolean requestSnapshot() { boolean snapshotName = name.equals("snapshot"); - boolean snapshotTemplate = StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); + boolean snapshotTemplate = + StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); boolean snapshotFeatures = duration == 0 && maxSize == 0 && maxAge == 0; return snapshotName && snapshotTemplate && snapshotFeatures; } From 39fcfb657fb66f1350b557015df414ce130ebd73 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Thu, 24 Aug 2023 10:41:01 -0400 Subject: [PATCH 4/9] check recording is valid --- .../agent/remote/RecordingsContext.java | 69 ++++++++++++++++--- 1 file changed, 61 insertions(+), 8 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index bb3324f9..220229f9 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -21,6 +21,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.nio.charset.StandardCharsets; +import java.time.Duration; import java.util.List; import java.util.Set; import java.util.function.Consumer; @@ -31,6 +32,7 @@ import javax.inject.Inject; import org.openjdk.jmc.common.unit.IConstrainedMap; +import org.openjdk.jmc.common.unit.ITypedQuantity; import org.openjdk.jmc.common.unit.QuantityConversionException; import org.openjdk.jmc.common.unit.UnitLookup; import org.openjdk.jmc.flightrecorder.configuration.events.EventOptionID; @@ -50,6 +52,7 @@ import io.cryostat.core.templates.Template; import io.cryostat.core.templates.TemplateType; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.sun.net.httpserver.HttpExchange; import io.smallrye.config.SmallRyeConfig; @@ -110,8 +113,19 @@ public void handle(HttpExchange exchange) throws IOException { break; case "PATCH": id = extractId(exchange); - if (id >= 0) { - handleStop(exchange, id); + String request = requestStopOrUpdate(exchange); + if (request == "STOPPED") { + if (id >= 0) { + handleStop(exchange, id); + } else { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + } + } else if (request == "UPDATE") { + if (id >= 0) { + handleRecordingUpdate(exchange, id); + } else { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + } } else { exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } @@ -135,6 +149,14 @@ public void handle(HttpExchange exchange) throws IOException { } } + private static String requestStopOrUpdate(HttpExchange exchange) throws IOException { + try (InputStream body = exchange.getRequestBody()) { + ObjectMapper mapper = new ObjectMapper(); + JsonNode jsonMap = mapper.readTree(body); + return jsonMap.get("state").toString(); + } + } + private static long extractId(HttpExchange exchange) throws IOException { Matcher m = PATH_ID_PATTERN.matcher(exchange.getRequestURI().getPath()); if (!m.find()) { @@ -196,10 +218,6 @@ private void handleGetRecording(HttpExchange exchange, long id) { private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOException { try (InputStream body = exchange.getRequestBody()) { StartRecordingRequest req = mapper.readValue(body, StartRecordingRequest.class); - if (!req.isValid()) { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); - return; - } if (req.requestSnapshot()) { try { SerializableRecordingDescriptor snapshot = startSnapshot(req, exchange); @@ -214,6 +232,10 @@ private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOExce } return; } + if (!req.isValid()) { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + return; + } SerializableRecordingDescriptor recording = startRecording(req); exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); try (OutputStream response = exchange.getResponseBody()) { @@ -231,6 +253,37 @@ private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOExce } } + private void handleRecordingUpdate(HttpExchange exchange, long id) throws IOException { + try (InputStream body = exchange.getRequestBody()) { + ObjectMapper mapper = new ObjectMapper(); + JsonNode jsonMap = mapper.readTree(body); + invokeOnRecording( + exchange, + id, + r -> { + try { + if (jsonMap.has("name")) { + r.setName(jsonMap.get("name").toString()); + } + if (jsonMap.has("toDisk")) { + r.setToDisk(jsonMap.get("toDisk").asBoolean()); + } + if (jsonMap.has("duration")) { + r.setDuration(Duration.ofMillis(jsonMap.get("duration").asLong())); + } + if (jsonMap.has("maxSize")) { + r.setMaxSize(jsonMap.get("name").asLong()); + } + if (jsonMap.has("maxAge")) { + r.setMaxAge(Duration.ofMillis(jsonMap.get("name").asLong())); + } + } catch (IllegalStateException e) { + sendHeader(exchange, HttpStatus.SC_CONFLICT); + } + }); + } + } + private void handleStop(HttpExchange exchange, long id) throws IOException { invokeOnRecording( exchange, @@ -378,7 +431,7 @@ boolean requestsBundledTemplate() { boolean requestSnapshot() { boolean snapshotName = name.equals("snapshot"); boolean snapshotTemplate = - StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); + StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); boolean snapshotFeatures = duration == 0 && maxSize == 0 && maxAge == 0; return snapshotName && snapshotTemplate && snapshotFeatures; } @@ -388,7 +441,7 @@ boolean isValid() { boolean requestsBundledTemplate = requestsBundledTemplate(); boolean requestsEither = requestsCustomTemplate || requestsBundledTemplate; boolean requestsBoth = requestsCustomTemplate && requestsBundledTemplate; - return requestsEither && !requestsBoth && !requestSnapshot(); + return (requestsEither && !requestsBoth) || requestSnapshot(); } } } From 2cc7720b646d19318af68c6542bfab006a956103 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Thu, 31 Aug 2023 15:00:18 -0400 Subject: [PATCH 5/9] update or stop --- .../agent/remote/RecordingsContext.java | 138 ++++++++++-------- 1 file changed, 76 insertions(+), 62 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 220229f9..e171d338 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -20,10 +20,14 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.net.MalformedURLException; import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.Map.Entry; import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -32,13 +36,14 @@ import javax.inject.Inject; import org.openjdk.jmc.common.unit.IConstrainedMap; -import org.openjdk.jmc.common.unit.ITypedQuantity; import org.openjdk.jmc.common.unit.QuantityConversionException; import org.openjdk.jmc.common.unit.UnitLookup; import org.openjdk.jmc.flightrecorder.configuration.events.EventOptionID; import org.openjdk.jmc.flightrecorder.configuration.recording.RecordingOptionsBuilder; +import org.openjdk.jmc.rjmx.ConnectionException; import org.openjdk.jmc.rjmx.ServiceNotAvailableException; import org.openjdk.jmc.rjmx.services.jfr.IFlightRecorderService; +import org.openjdk.jmc.rjmx.services.jfr.IRecordingDescriptor; import io.cryostat.agent.StringUtils; import io.cryostat.core.FlightRecorderException; @@ -113,19 +118,8 @@ public void handle(HttpExchange exchange) throws IOException { break; case "PATCH": id = extractId(exchange); - String request = requestStopOrUpdate(exchange); - if (request == "STOPPED") { - if (id >= 0) { - handleStop(exchange, id); - } else { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); - } - } else if (request == "UPDATE") { - if (id >= 0) { - handleRecordingUpdate(exchange, id); - } else { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); - } + if (id >= 0) { + handleStopOrUpdate(exchange, id); } else { exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } @@ -149,14 +143,6 @@ public void handle(HttpExchange exchange) throws IOException { } } - private static String requestStopOrUpdate(HttpExchange exchange) throws IOException { - try (InputStream body = exchange.getRequestBody()) { - ObjectMapper mapper = new ObjectMapper(); - JsonNode jsonMap = mapper.readTree(body); - return jsonMap.get("state").toString(); - } - } - private static long extractId(HttpExchange exchange) throws IOException { Matcher m = PATH_ID_PATTERN.matcher(exchange.getRequestURI().getPath()); if (!m.find()) { @@ -253,53 +239,81 @@ private void handleStartRecordingOrSnapshot(HttpExchange exchange) throws IOExce } } - private void handleRecordingUpdate(HttpExchange exchange, long id) throws IOException { - try (InputStream body = exchange.getRequestBody()) { - ObjectMapper mapper = new ObjectMapper(); + private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOException { + try { + JFRConnection conn = + jfrConnectionToolkit.connect( + jfrConnectionToolkit.createServiceURL("localhost", 0)); + IFlightRecorderService svc = conn.getService(); + IRecordingDescriptor dsc = svc.getAvailableRecordings().stream().filter(r -> r.getId() == id).findFirst().get(); + RecordingOptionsBuilder builder = new RecordingOptionsBuilder(conn.getService()); + + InputStream body = exchange.getRequestBody(); JsonNode jsonMap = mapper.readTree(body); - invokeOnRecording( - exchange, - id, - r -> { - try { - if (jsonMap.has("name")) { - r.setName(jsonMap.get("name").toString()); - } - if (jsonMap.has("toDisk")) { - r.setToDisk(jsonMap.get("toDisk").asBoolean()); - } - if (jsonMap.has("duration")) { - r.setDuration(Duration.ofMillis(jsonMap.get("duration").asLong())); - } - if (jsonMap.has("maxSize")) { - r.setMaxSize(jsonMap.get("name").asLong()); - } - if (jsonMap.has("maxAge")) { - r.setMaxAge(Duration.ofMillis(jsonMap.get("name").asLong())); - } - } catch (IllegalStateException e) { - sendHeader(exchange, HttpStatus.SC_CONFLICT); + Iterator> fields = jsonMap.fields(); + + while (fields.hasNext()) { + Map.Entry field = fields.next(); + + switch(field.getKey()){ + case "state": + if ("STOPPED".equals(field.getValue().toString())) { + handleStop(exchange, id); + break; + } else if (!"UPDATE".equals(field.getValue().toString())) { + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } - }); + break; + case "name": + builder = builder.name(field.getValue().toString()); + break; + case "duration": + builder = builder.duration(field.getValue().asLong()); + break; + case "maxSize": + builder = builder.maxSize(field.getValue().asLong()); + break; + case "maxAge": + builder = builder.maxAge(field.getValue().asLong()); + break; + case "toDisk": + builder = builder.toDisk(field.getValue().asBoolean()); + break; + default: + log.warn("Unknown recording option {}", field.getKey()); + exchange.sendResponseHeaders( + HttpStatus.SC_METHOD_NOT_ALLOWED, BODY_LENGTH_NONE); + break; + } + } + svc.updateRecordingOptions(dsc, builder.build()); + exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); + } catch ( ServiceNotAvailableException + | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException + | QuantityConversionException e){ + log.error("Failed to update recording", e); + exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, BODY_LENGTH_NONE); + } finally { + exchange.close(); } } private void handleStop(HttpExchange exchange, long id) throws IOException { invokeOnRecording( - exchange, - id, - r -> { - try { - boolean stopped = r.stop(); - if (!stopped) { - sendHeader(exchange, HttpStatus.SC_BAD_REQUEST); - } else { - sendHeader(exchange, HttpStatus.SC_NO_CONTENT); - } - } catch (IllegalStateException e) { - sendHeader(exchange, HttpStatus.SC_CONFLICT); + exchange, + id, + r -> { + try { + boolean stopped = r.stop(); + if (!stopped) { + sendHeader(exchange, HttpStatus.SC_BAD_REQUEST); + } else { + sendHeader(exchange, HttpStatus.SC_NO_CONTENT); } - }); + } catch (IllegalStateException e) { + sendHeader(exchange, HttpStatus.SC_CONFLICT); + } + }); } private void handleDelete(HttpExchange exchange, long id) throws IOException { @@ -352,7 +366,7 @@ private SerializableRecordingDescriptor startRecording(StartRecordingRequest req throws QuantityConversionException, ServiceNotAvailableException, FlightRecorderException, org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, - InvalidEventTemplateException, InvalidXmlException, IOException { + InvalidEventTemplateException, InvalidXmlException, IOException, FlightRecorderException { Runnable cleanup = () -> {}; try { JFRConnection conn = From 103361d2a90f4d347fa3fa56b4b4f25a293e8881 Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Fri, 1 Sep 2023 12:16:34 -0400 Subject: [PATCH 6/9] cleanup --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index e171d338..0ada1829 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -20,9 +20,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.net.MalformedURLException; import java.nio.charset.StandardCharsets; -import java.time.Duration; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -40,7 +38,6 @@ import org.openjdk.jmc.common.unit.UnitLookup; import org.openjdk.jmc.flightrecorder.configuration.events.EventOptionID; import org.openjdk.jmc.flightrecorder.configuration.recording.RecordingOptionsBuilder; -import org.openjdk.jmc.rjmx.ConnectionException; import org.openjdk.jmc.rjmx.ServiceNotAvailableException; import org.openjdk.jmc.rjmx.services.jfr.IFlightRecorderService; import org.openjdk.jmc.rjmx.services.jfr.IRecordingDescriptor; From 1a8f8da8d342ddfa84113329a952b5de9cb4e7ac Mon Sep 17 00:00:00 2001 From: Ming Wang Date: Thu, 7 Sep 2023 11:02:23 -0400 Subject: [PATCH 7/9] updates --- .../agent/remote/RecordingsContext.java | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 0ada1829..b7278448 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -257,7 +257,7 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti if ("STOPPED".equals(field.getValue().toString())) { handleStop(exchange, id); break; - } else if (!"UPDATE".equals(field.getValue().toString())) { + } else { exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } break; @@ -265,16 +265,32 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti builder = builder.name(field.getValue().toString()); break; case "duration": - builder = builder.duration(field.getValue().asLong()); + if (field.getValue().canConvertToLong()) { + builder = builder.duration(field.getValue().asLong()); + break; + } + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); break; case "maxSize": - builder = builder.maxSize(field.getValue().asLong()); + if (field.getValue().canConvertToLong()) { + builder = builder.maxSize(field.getValue().asLong()); + break; + } + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); break; case "maxAge": - builder = builder.maxAge(field.getValue().asLong()); + if (field.getValue().canConvertToLong()) { + builder = builder.maxAge(field.getValue().asLong()); + break; + } + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); break; case "toDisk": - builder = builder.toDisk(field.getValue().asBoolean()); + if (field.getValue().isBoolean()) { + builder = builder.toDisk(field.getValue().asBoolean()); + break; + } + exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); break; default: log.warn("Unknown recording option {}", field.getKey()); @@ -284,7 +300,15 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti } } svc.updateRecordingOptions(dsc, builder.build()); - exchange.sendResponseHeaders(HttpStatus.SC_CREATED, BODY_LENGTH_UNKNOWN); + exchange.sendResponseHeaders(HttpStatus.SC_OK, BODY_LENGTH_UNKNOWN); + + try (OutputStream response = exchange.getResponseBody()) { + if (response == null) { + exchange.sendResponseHeaders(HttpStatus.SC_NO_CONTENT, BODY_LENGTH_NONE); + } else { + mapper.writeValue(response, dsc); + } + } } catch ( ServiceNotAvailableException | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException | QuantityConversionException e){ @@ -293,6 +317,7 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti } finally { exchange.close(); } + } private void handleStop(HttpExchange exchange, long id) throws IOException { From 95a9430af7dd917e91e31d6040b5fb47935c6b1a Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 12 Sep 2023 11:42:13 -0400 Subject: [PATCH 8/9] mvn spotless:apply --- .../agent/remote/RecordingsContext.java | 51 ++++++++++--------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index b7278448..0b6c0b32 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -24,8 +24,8 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.Map.Entry; +import java.util.Set; import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -242,9 +242,13 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti jfrConnectionToolkit.connect( jfrConnectionToolkit.createServiceURL("localhost", 0)); IFlightRecorderService svc = conn.getService(); - IRecordingDescriptor dsc = svc.getAvailableRecordings().stream().filter(r -> r.getId() == id).findFirst().get(); + IRecordingDescriptor dsc = + svc.getAvailableRecordings().stream() + .filter(r -> r.getId() == id) + .findFirst() + .get(); RecordingOptionsBuilder builder = new RecordingOptionsBuilder(conn.getService()); - + InputStream body = exchange.getRequestBody(); JsonNode jsonMap = mapper.readTree(body); Iterator> fields = jsonMap.fields(); @@ -252,13 +256,14 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti while (fields.hasNext()) { Map.Entry field = fields.next(); - switch(field.getKey()){ + switch (field.getKey()) { case "state": if ("STOPPED".equals(field.getValue().toString())) { handleStop(exchange, id); break; } else { - exchange.sendResponseHeaders(HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); + exchange.sendResponseHeaders( + HttpStatus.SC_BAD_REQUEST, BODY_LENGTH_NONE); } break; case "name": @@ -309,33 +314,32 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti mapper.writeValue(response, dsc); } } - } catch ( ServiceNotAvailableException + } catch (ServiceNotAvailableException | org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException - | QuantityConversionException e){ + | QuantityConversionException e) { log.error("Failed to update recording", e); exchange.sendResponseHeaders(HttpStatus.SC_INTERNAL_SERVER_ERROR, BODY_LENGTH_NONE); } finally { exchange.close(); } - } private void handleStop(HttpExchange exchange, long id) throws IOException { invokeOnRecording( - exchange, - id, - r -> { - try { - boolean stopped = r.stop(); - if (!stopped) { - sendHeader(exchange, HttpStatus.SC_BAD_REQUEST); - } else { - sendHeader(exchange, HttpStatus.SC_NO_CONTENT); + exchange, + id, + r -> { + try { + boolean stopped = r.stop(); + if (!stopped) { + sendHeader(exchange, HttpStatus.SC_BAD_REQUEST); + } else { + sendHeader(exchange, HttpStatus.SC_NO_CONTENT); + } + } catch (IllegalStateException e) { + sendHeader(exchange, HttpStatus.SC_CONFLICT); } - } catch (IllegalStateException e) { - sendHeader(exchange, HttpStatus.SC_CONFLICT); - } - }); + }); } private void handleDelete(HttpExchange exchange, long id) throws IOException { @@ -388,7 +392,8 @@ private SerializableRecordingDescriptor startRecording(StartRecordingRequest req throws QuantityConversionException, ServiceNotAvailableException, FlightRecorderException, org.openjdk.jmc.rjmx.services.jfr.FlightRecorderException, - InvalidEventTemplateException, InvalidXmlException, IOException, FlightRecorderException { + InvalidEventTemplateException, InvalidXmlException, IOException, + FlightRecorderException { Runnable cleanup = () -> {}; try { JFRConnection conn = @@ -467,7 +472,7 @@ boolean requestsBundledTemplate() { boolean requestSnapshot() { boolean snapshotName = name.equals("snapshot"); boolean snapshotTemplate = - StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); + StringUtils.isBlank(template) && StringUtils.isBlank(localTemplateName); boolean snapshotFeatures = duration == 0 && maxSize == 0 && maxAge == 0; return snapshotName && snapshotTemplate && snapshotFeatures; } From 8fe5372b31a972947b13001e68983101a04f01f3 Mon Sep 17 00:00:00 2001 From: Andrew Azores Date: Tue, 12 Sep 2023 12:03:23 -0400 Subject: [PATCH 9/9] use textValue() instead of toString() --- src/main/java/io/cryostat/agent/remote/RecordingsContext.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java index 0b6c0b32..bfb45037 100644 --- a/src/main/java/io/cryostat/agent/remote/RecordingsContext.java +++ b/src/main/java/io/cryostat/agent/remote/RecordingsContext.java @@ -258,7 +258,7 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti switch (field.getKey()) { case "state": - if ("STOPPED".equals(field.getValue().toString())) { + if ("STOPPED".equals(field.getValue().textValue())) { handleStop(exchange, id); break; } else { @@ -267,7 +267,7 @@ private void handleStopOrUpdate(HttpExchange exchange, long id) throws IOExcepti } break; case "name": - builder = builder.name(field.getValue().toString()); + builder = builder.name(field.getValue().textValue()); break; case "duration": if (field.getValue().canConvertToLong()) {