Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(replace-param): Fine-grained replacement parameter for Start Recording Mutator #1587

Merged
merged 29 commits into from
Aug 16, 2023
+893 −45
Merged

feat(replace-param): Fine-grained replacement parameter for Start Recording Mutator #1587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
65e9663
first commit to RecordingTargetHelper
aali309 Jul 12, 2023
849aa54
spotless run
aali309 Jul 12, 2023
b8a4e7a
not sure why im getting 'always' instead of 'never'
aali309 Jul 13, 2023
1990a76
resolved issues
aali309 Jul 13, 2023
c21450c
resolved issues: restart paramter reverted
aali309 Jul 19, 2023
6991c4f
spotless run
aali309 Jul 19, 2023
0c8e639
resolved issue: initialization of replace variable
aali309 Jul 19, 2023
84c87b8
resolved issue: return to primitive boolean
aali309 Jul 20, 2023
8a03848
handle when recording exists and running
aali309 Jul 25, 2023
1735ff3
spotless run
aali309 Jul 25, 2023
e314a9a
updated types.graphqls to include replace field
aali309 Jul 27, 2023
cdf4206
updated RecordingTargetHelperTest class
aali309 Jul 31, 2023
aa4f7c9
resolved all conversations
aali309 Aug 1, 2023
819fb04
removed the deprecated restart parameter
aali309 Aug 2, 2023
10eb62e
updated replace type on graphqls
aali309 Aug 2, 2023
75ac74d
resolved issues: added utility method
aali309 Aug 2, 2023
ea17d64
resolved issues:
aali309 Aug 2, 2023
f73b15b
fixed typo AlWAYS:ALWAYS
aali309 Aug 3, 2023
b66ef8a
rebase
aali309 Aug 4, 2023
a99d196
Revert "fixed typo AlWAYS:ALWAYS"
aali309 Aug 4, 2023
007803d
added itest on GraphQLIT
aali309 Aug 9, 2023
3d95439
added helper methods in GraphQLIT
aali309 Aug 10, 2023
18db76c
added helper methods in GraphQLIT
aali309 Aug 10, 2023
59f3a17
run spotless:apply
aali309 Aug 10, 2023
10bac6c
Need Help: individual test passes but collectively fails: has to do w…
aali309 Aug 11, 2023
f9023fc
Need Help2: individual test passes but collectively fails: has to do …
aali309 Aug 11, 2023
f7c7223
Thuans to view
aali309 Aug 11, 2023
9fa14d1
completed
aali309 Aug 14, 2023
e0bc84a
fixed final corrections
aali309 Aug 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingsPostHandler.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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