Skip to content

Commit

Permalink
feat(recordings): fine-grained recording replacement parameter (#1587)
Browse files Browse the repository at this point in the history
  • Loading branch information
@aali309
aali309 authored Aug 16, 2023
1 parent 37148c1 commit 3fbb131
Show file tree
Hide file tree
Showing 9 changed files with 893 additions and 45 deletions.
12 changes: 7 additions & 5 deletions src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingsPostHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import io.cryostat.recordings.RecordingMetadataManager.Metadata;
import io.cryostat.recordings.RecordingOptionsBuilderFactory;
import io.cryostat.recordings.RecordingTargetHelper;
import io.cryostat.recordings.RecordingTargetHelper.ReplacementPolicy;

import com.google.gson.Gson;
import com.google.gson.JsonSyntaxException;
Expand Down Expand Up @@ -151,10 +152,11 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
recordingOptionsBuilderFactory
.create(connection.getService())
.name(recordingName);
boolean restart = false;
if (attrs.contains("restart")) {
restart = Boolean.parseBoolean(attrs.get("restart"));
}

String replace = attrs.get("replace");
ReplacementPolicy replacementPolicy =
ReplacementPolicy.fromString(replace);

if (attrs.contains("duration")) {
builder =
builder.duration(
Expand Down Expand Up @@ -199,7 +201,7 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
eventSpecifier);
IRecordingDescriptor descriptor =
recordingTargetHelper.startRecording(
restart,
replacementPolicy,
connectionDescriptor,
builder.build(),
template.getLeft(),
Expand Down
11 changes: 7 additions & 4 deletions src/main/java/io/cryostat/net/web/http/api/v2/graph/StartRecordingOnTargetMutator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import io.cryostat.recordings.RecordingMetadataManager.Metadata;
import io.cryostat.recordings.RecordingOptionsBuilderFactory;
import io.cryostat.recordings.RecordingTargetHelper;
import io.cryostat.recordings.RecordingTargetHelper.ReplacementPolicy;

import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;
Expand Down Expand Up @@ -110,13 +111,15 @@ public HyperlinkedSerializableRecordingDescriptor getAuthenticated(
return targetConnectionManager.executeConnectedTask(
cd,
conn -> {
boolean restart = false;
ReplacementPolicy replace = ReplacementPolicy.NEVER;
RecordingOptionsBuilder builder =
recordingOptionsBuilderFactory
.create(conn.getService())
.name((String) settings.get("name"));
if (settings.containsKey("restart")) {
restart = Boolean.TRUE.equals(settings.get("restart"));

if (settings.containsKey("replace")) {
String replaceValue = (String) settings.get("replace");
replace = ReplacementPolicy.fromString(replaceValue);
}
if (settings.containsKey("duration")) {
builder =
Expand Down Expand Up @@ -157,7 +160,7 @@ public HyperlinkedSerializableRecordingDescriptor getAuthenticated(
}
IRecordingDescriptor desc =
recordingTargetHelper.startRecording(
restart,
replace,
cd,
builder.build(),
(String) settings.get("template"),
Expand Down
53 changes: 47 additions & 6 deletions src/main/java/io/cryostat/recordings/RecordingTargetHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,25 @@ public List<IRecordingDescriptor> getRecordings(ConnectionDescriptor connectionD
connection -> connection.getService().getAvailableRecordings());
}

public enum ReplacementPolicy {
ALWAYS,
STOPPED,
NEVER;

public static ReplacementPolicy fromString(String value) {
if (value == null) {
return NEVER;
}
try {
return valueOf(value.toUpperCase());
} catch (IllegalArgumentException e) {
return NEVER;
}
}
}

public IRecordingDescriptor startRecording(
boolean restart,
ReplacementPolicy replace,
ConnectionDescriptor connectionDescriptor,
IConstrainedMap<String> recordingOptions,
String templateName,
Expand All @@ -124,6 +141,7 @@ public IRecordingDescriptor startRecording(
boolean archiveOnStop)
throws Exception {
String recordingName = (String) recordingOptions.get(RecordingOptionsBuilder.KEY_NAME);

return targetConnectionManager.executeConnectedTask(
connectionDescriptor,
connection -> {
Expand All @@ -132,13 +150,18 @@ public IRecordingDescriptor startRecording(
Optional<IRecordingDescriptor> previous =
getDescriptorByName(connection, recordingName);
if (previous.isPresent()) {
if (!restart) {
if (shouldRestartRecording(replace, previous.get())) {
if (isRecordingStopped(previous.get())) {
connection.getService().close(previous.get());
} else {
// If recording exists & running, close it before starting new one
connection.getService().close(previous.get());
}
} else {
throw new IllegalArgumentException(
String.format(
"Recording with name \"%s\" already exists",
recordingName));
} else {
connection.getService().close(previous.get());
}
}
IRecordingDescriptor desc =
Expand Down Expand Up @@ -175,15 +198,33 @@ public IRecordingDescriptor startRecording(
if (fixedDuration != null) {
Long delay =
Long.valueOf(fixedDuration.toString().replaceAll("[^0-9]", ""));

scheduleRecordingTasks(
recordingName, delay, connectionDescriptor, archiveOnStop);
}

return desc;
});
}

private boolean shouldRestartRecording(
ReplacementPolicy replace, IRecordingDescriptor descriptor) {
if (replace != null) {
switch (replace) {
case ALWAYS:
return true;
case STOPPED:
return descriptor.getState() == IRecordingDescriptor.RecordingState.STOPPED;
case NEVER:
return false;
}
}
// If neither restart nor replace is specified, default to never
return false;
}

private boolean isRecordingStopped(IRecordingDescriptor recording) {
return recording.getState() == IRecordingDescriptor.RecordingState.STOPPED;
}

/**
* The returned {@link InputStream}, if any, is only readable while the remote connection
* remains open. And so, {@link
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/io/cryostat/rules/RuleProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import io.cryostat.recordings.RecordingMetadataManager.Metadata;
import io.cryostat.recordings.RecordingOptionsBuilderFactory;
import io.cryostat.recordings.RecordingTargetHelper;
import io.cryostat.recordings.RecordingTargetHelper.ReplacementPolicy;
import io.cryostat.rules.RuleRegistry.RuleEvent;
import io.cryostat.util.events.Event;
import io.cryostat.util.events.EventListener;
Expand Down Expand Up @@ -383,7 +384,7 @@ private void startRuleRecording(ConnectionDescriptor connectionDescriptor, Rule
RecordingTargetHelper.parseEventSpecifierToTemplate(
rule.getEventSpecifier());
return recordingTargetHelper.startRecording(
true,
ReplacementPolicy.ALWAYS,
connectionDescriptor,
builder.build(),
template.getLeft(),
Expand Down
8 changes: 7 additions & 1 deletion src/main/resources/types.graphqls
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ scalar Long
scalar Int
scalar Float

enum ReplacementPolicy {
ALWAYS
STOPPED
NEVER
}

input EnvironmentNodeFilterInput {
id: Int
name: String
Expand Down Expand Up @@ -166,7 +172,7 @@ type RecordingMetadata {
}

input RecordingSettings {
restart: Boolean
replace: ReplacementPolicy
name: String!
template: String!
templateType: String!
Expand Down
36 changes: 22 additions & 14 deletions src/test/java/io/cryostat/net/web/http/api/v1/TargetRecordingsPostHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import io.cryostat.recordings.RecordingMetadataManager.Metadata;
import io.cryostat.recordings.RecordingOptionsBuilderFactory;
import io.cryostat.recordings.RecordingTargetHelper;
import io.cryostat.recordings.RecordingTargetHelper.ReplacementPolicy;

import com.google.gson.Gson;
import io.vertx.core.MultiMap;
Expand Down Expand Up @@ -172,6 +173,7 @@ void shouldStartRecording() throws Exception {
attrs.add("maxAge", "50");
attrs.add("maxSize", "64");
attrs.add("archiveOnStop", "false");

Mockito.when(ctx.response()).thenReturn(resp);
Mockito.when(
resp.putHeader(
Expand All @@ -181,7 +183,7 @@ void shouldStartRecording() throws Exception {
IRecordingDescriptor descriptor = createDescriptor("someRecording");
Mockito.when(
recordingTargetHelper.startRecording(
Mockito.anyBoolean(),
Mockito.any(),
Mockito.any(),
Mockito.any(),
Mockito.any(),
Expand All @@ -201,7 +203,8 @@ void shouldStartRecording() throws Exception {
Mockito.verify(recordingOptionsBuilder).maxAge(50L);
Mockito.verify(recordingOptionsBuilder).maxSize(64L);

ArgumentCaptor<Boolean> restartCaptor = ArgumentCaptor.forClass(Boolean.class);
ArgumentCaptor<ReplacementPolicy> replaceCaptor =
ArgumentCaptor.forClass(ReplacementPolicy.class);

ArgumentCaptor<ConnectionDescriptor> connectionDescriptorCaptor =
ArgumentCaptor.forClass(ConnectionDescriptor.class);
Expand All @@ -220,15 +223,16 @@ void shouldStartRecording() throws Exception {

Mockito.verify(recordingTargetHelper)
.startRecording(
restartCaptor.capture(),
replaceCaptor.capture(),
connectionDescriptorCaptor.capture(),
recordingOptionsCaptor.capture(),
templateNameCaptor.capture(),
templateTypeCaptor.capture(),
metadataCaptor.capture(),
archiveOnStopCaptor.capture());

MatcherAssert.assertThat(restartCaptor.getValue(), Matchers.equalTo(false));
MatcherAssert.assertThat(
replaceCaptor.getValue(), Matchers.equalTo(ReplacementPolicy.NEVER));

ConnectionDescriptor connectionDescriptor = connectionDescriptorCaptor.getValue();
MatcherAssert.assertThat(
Expand Down Expand Up @@ -285,7 +289,7 @@ void shouldRestartRecording() throws Exception {
IRecordingDescriptor descriptor = createDescriptor("someRecording");
Mockito.when(
recordingTargetHelper.startRecording(
Mockito.anyBoolean(),
Mockito.any(),
Mockito.any(),
Mockito.any(),
Mockito.any(),
Expand All @@ -307,15 +311,16 @@ void shouldRestartRecording() throws Exception {
resp.putHeader(
Mockito.any(CharSequence.class), Mockito.any(CharSequence.class)))
.thenReturn(resp);
attrs.add("restart", "true");
attrs.add("replace", "always");
attrs.add("recordingName", "someRecording");
attrs.add("events", "template=Foo");

handler.handle(ctx);

Mockito.verify(recordingOptionsBuilder).name("someRecording");

ArgumentCaptor<Boolean> restartCaptor = ArgumentCaptor.forClass(Boolean.class);
ArgumentCaptor<ReplacementPolicy> replaceCaptor =
ArgumentCaptor.forClass(ReplacementPolicy.class);

ArgumentCaptor<ConnectionDescriptor> connectionDescriptorCaptor =
ArgumentCaptor.forClass(ConnectionDescriptor.class);
Expand All @@ -334,15 +339,16 @@ void shouldRestartRecording() throws Exception {

Mockito.verify(recordingTargetHelper)
.startRecording(
restartCaptor.capture(),
replaceCaptor.capture(),
connectionDescriptorCaptor.capture(),
recordingOptionsCaptor.capture(),
templateNameCaptor.capture(),
templateTypeCaptor.capture(),
metadataCaptor.capture(),
archiveOnStopCaptor.capture());

MatcherAssert.assertThat(restartCaptor.getValue(), Matchers.equalTo(true));
MatcherAssert.assertThat(
replaceCaptor.getValue(), Matchers.equalTo(ReplacementPolicy.ALWAYS));

ConnectionDescriptor connectionDescriptor = connectionDescriptorCaptor.getValue();
MatcherAssert.assertThat(
Expand Down Expand Up @@ -389,7 +395,7 @@ void shouldHandleNameCollision() throws Exception {
Mockito.when(recordingOptionsBuilder.build()).thenReturn(recordingOptions);
Mockito.when(
recordingTargetHelper.startRecording(
Mockito.anyBoolean(),
Mockito.any(),
Mockito.any(),
Mockito.any(),
Mockito.any(),
Expand Down Expand Up @@ -552,7 +558,7 @@ void shouldStartRecordingAndArchiveOnStop() throws Exception {
IRecordingDescriptor descriptor = createDescriptor("someRecording");
Mockito.when(
recordingTargetHelper.startRecording(
Mockito.anyBoolean(),
Mockito.any(),
Mockito.any(),
Mockito.any(),
Mockito.any(),
Expand All @@ -572,7 +578,8 @@ void shouldStartRecordingAndArchiveOnStop() throws Exception {
Mockito.verify(recordingOptionsBuilder).maxAge(50L);
Mockito.verify(recordingOptionsBuilder).maxSize(64L);

ArgumentCaptor<Boolean> restartCaptor = ArgumentCaptor.forClass(Boolean.class);
ArgumentCaptor<ReplacementPolicy> replaceCaptor =
ArgumentCaptor.forClass(ReplacementPolicy.class);

ArgumentCaptor<ConnectionDescriptor> connectionDescriptorCaptor =
ArgumentCaptor.forClass(ConnectionDescriptor.class);
Expand All @@ -591,15 +598,16 @@ void shouldStartRecordingAndArchiveOnStop() throws Exception {

Mockito.verify(recordingTargetHelper)
.startRecording(
restartCaptor.capture(),
replaceCaptor.capture(),
connectionDescriptorCaptor.capture(),
recordingOptionsCaptor.capture(),
templateNameCaptor.capture(),
templateTypeCaptor.capture(),
metadataCaptor.capture(),
archiveOnStopCaptor.capture());

MatcherAssert.assertThat(restartCaptor.getValue(), Matchers.equalTo(false));
MatcherAssert.assertThat(
replaceCaptor.getValue(), Matchers.equalTo(ReplacementPolicy.NEVER));

ConnectionDescriptor connectionDescriptor = connectionDescriptorCaptor.getValue();
MatcherAssert.assertThat(
Expand Down
Loading

0 comments on commit 3fbb131

Please sign in to comment.