Skip to content

Commit

Permalink
Remove event specifier string syntax (#486)
Browse files Browse the repository at this point in the history 
* Remove any reference to specifying events using event strings

* Update unit tests to conform with removal of event specifier string syntax

* Remove event specifier string syntax that was previously missed

* Update web-client to reflect corresponding -web PR changes for removing event specifier strings
  • Loading branch information
Hareet Dhillon authored Jun 24, 2021
1 parent ae3b9b4 commit 46ffa03
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 178 deletions.
22 changes: 8 additions & 14 deletions COMMANDS.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,23 +86,19 @@ formatted as a JSON response.

* #### `start`
###### usage
`start targetId foo jdk.SocketRead:enabled=true,jdk.PhysicalMemory:period=10ms`
`start targetId foo template=Foo`
###### synopsis
Starts a continuous recording in the target JVM with the given name
(`foo`), which will record events as configured in the events string.
(`foo`), which will record events as configured in the event template.

The targetID is a `hostname:port` or `service:rmi:jmx://` JMX Service URL
specifying the location of the remote target JVM to connect to.

The syntax of an individual event string is `eventID:option=value`.
The syntax of the overall events string is `event1,event2,event3`, for
N >= 1 events.
The event template (`template=Foo`) allows preset configurations of events
and options to be enabled.

The event string may also be provided in the form `template=Foo`. This
format allows preset configurations of events and options to be enabled.

The eventID is the fully qualified event name. For information about the
events and options available, see `list-event-types` or `search-events`.
For information about the events and options available, see `list-event-types`
or `search-events`.
###### see also
* [`list-event-types`](#list-event-types)
* [`search-events`](#search-events)
Expand All @@ -119,11 +115,11 @@ formatted as a JSON response.

* #### `dump`
###### usage
`dump targetId foo 30 jdk.SocketRead:enabled=true`
`dump targetId foo 30 template=Foo`
###### synopsis
Starts a recording in the specified target JVM with the given name (`foo`),
with a fixed duration of the given number of seconds (`30`), and recording
events as configured in the events string.
events as configured in the event template.
###### see also
[`start`](#start)

Expand Down Expand Up @@ -198,7 +194,6 @@ formatted as a JSON response.
###### synopsis
Searches for event types that can be produced by the specified target JVM
where the event name, category, label, etc. matches the given query (`foo`).
This is useful for preparing event options strings.
###### see also
* [`start`](#start)
* [`dump`](#dump)
Expand All @@ -210,7 +205,6 @@ formatted as a JSON response.
`list-event-types targetId`
###### synopsis
Lists event types that can be produced by the specified target JVM.
This is useful for preparing event options strings.
###### see also
* [`start`](#start)
* [`dump`](#dump)
Expand Down
7 changes: 2 additions & 5 deletions HTTP_API.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -851,10 +851,8 @@
`recordingName` - The name of the recording to create.
Should use percent-encoding.
`events` - The events configuration for the recording.
This can be a comma-seperated list of events, with each event having the
form `$EVENT_ID:$OPTION=$VALUE`; or it can be a template, using the form
`template=$TEMPLATE`.
`events` - The events configuration for the recording, as an
event template using the form `template=$TEMPLATE`.
**The request may include the following fields:**
Expand Down Expand Up @@ -1250,7 +1248,6 @@ The handler-specific descriptions below describe how each handler populates the
###### synopsis
Returns a list of event types that can be produced by a target JVM,
where the event name, category, label, etc. matches the given query.
This is useful for preparing event options strings.
###### request
`GET /api/v2/targets/:targetId/eventsSearch/:query`
Expand Down
92 changes: 34 additions & 58 deletions src/main/java/io/cryostat/commands/internal/AbstractRecordingCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ public abstract class AbstractRecordingCommand extends AbstractConnectedCommand

private static final Pattern TEMPLATE_PATTERN =
Pattern.compile("^template=([\\w]+)(?:,type=([\\w]+))?$");
private static final Pattern EVENTS_PATTERN =
Pattern.compile("([\\w\\.\\$]+):([\\w]+)=([\\w\\d\\.]+)");

protected final ClientWriter cw;
protected final EventOptionsBuilder.Factory eventOptionsBuilderFactory;
Expand All @@ -83,46 +81,42 @@ protected AbstractRecordingCommand(

protected IConstrainedMap<EventOptionID> enableEvents(JFRConnection connection, String events)
throws Exception {
if (TEMPLATE_PATTERN.matcher(events).matches()) {
Matcher m = TEMPLATE_PATTERN.matcher(events);
m.find();
String templateName = m.group(1);
String typeName = m.group(2);
if (ALL_EVENTS_TEMPLATE.getName().equals(templateName)) {
return enableAllEvents(connection);
}
if (typeName != null) {
return connection
.getTemplateService()
.getEvents(templateName, TemplateType.valueOf(typeName))
.orElseThrow(
() ->
new IllegalArgumentException(
String.format(
"No template \"%s\" found with type %s",
templateName, typeName)));
}
// if template type not specified, try to find a Custom template by that name. If none,
// fall back on finding a Target built-in template by the name. If not, throw an
// exception and bail out.
Matcher m = TEMPLATE_PATTERN.matcher(events);
m.find();
String templateName = m.group(1);
String typeName = m.group(2);
if (ALL_EVENTS_TEMPLATE.getName().equals(templateName)) {
return enableAllEvents(connection);
}
if (typeName != null) {
return connection
.getTemplateService()
.getEvents(templateName, TemplateType.CUSTOM)
.or(
() -> {
try {
return connection
.getTemplateService()
.getEvents(templateName, TemplateType.TARGET);
} catch (Exception e) {
cw.println(e);
return Optional.empty();
}
})
.orElseThrow(() -> new IllegalArgumentException(templateName));
.getEvents(templateName, TemplateType.valueOf(typeName))
.orElseThrow(
() ->
new IllegalArgumentException(
String.format(
"No template \"%s\" found with type %s",
templateName, typeName)));
}

return enableSelectedEvents(connection, events);
// if template type not specified, try to find a Custom template by that name. If none,
// fall back on finding a Target built-in template by the name. If not, throw an
// exception and bail out.
return connection
.getTemplateService()
.getEvents(templateName, TemplateType.CUSTOM)
.or(
() -> {
try {
return connection
.getTemplateService()
.getEvents(templateName, TemplateType.TARGET);
} catch (Exception e) {
cw.println(e);
return Optional.empty();
}
})
.orElseThrow(() -> new IllegalArgumentException(templateName));
}

protected IConstrainedMap<EventOptionID> enableAllEvents(JFRConnection connection)
Expand All @@ -136,25 +130,7 @@ protected IConstrainedMap<EventOptionID> enableAllEvents(JFRConnection connectio
return builder.build();
}

protected IConstrainedMap<EventOptionID> enableSelectedEvents(
JFRConnection connection, String events) throws Exception {
EventOptionsBuilder builder = eventOptionsBuilderFactory.create(connection);

Matcher matcher = EVENTS_PATTERN.matcher(events);
while (matcher.find()) {
String eventTypeId = matcher.group(1);
String option = matcher.group(2);
String value = matcher.group(3);

builder.addEvent(eventTypeId, option, value);
}

return builder.build();
}

protected boolean validateEvents(String events) {
// TODO better validation of entire events string (not just looking for one acceptable
// setting)
return TEMPLATE_PATTERN.matcher(events).matches() || EVENTS_PATTERN.matcher(events).find();
return TEMPLATE_PATTERN.matcher(events).matches();
}
}
88 changes: 33 additions & 55 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 @@ -89,8 +89,6 @@ public class TargetRecordingsPostHandler extends AbstractAuthenticatedRequestHan

private static final Pattern TEMPLATE_PATTERN =
Pattern.compile("^template=([\\w]+)(?:,type=([\\w]+))?$");
private static final Pattern EVENTS_PATTERN =
Pattern.compile("([\\w\\.\\$]+):([\\w]+)=([\\w\\d\\.]+)");

static final String PATH = "targets/:targetId/recordings";
private final TargetConnectionManager targetConnectionManager;
Expand Down Expand Up @@ -255,50 +253,46 @@ protected Optional<IRecordingDescriptor> getDescriptorByName(

protected IConstrainedMap<EventOptionID> enableEvents(JFRConnection connection, String events)
throws Exception {
if (TEMPLATE_PATTERN.matcher(events).matches()) {
Matcher m = TEMPLATE_PATTERN.matcher(events);
m.find();
String templateName = m.group(1);
String typeName = m.group(2);
if (ALL_EVENTS_TEMPLATE.getName().equals(templateName)) {
return enableAllEvents(connection);
}
if (typeName != null) {
return connection
.getTemplateService()
.getEvents(templateName, TemplateType.valueOf(typeName))
.orElseThrow(
() ->
new IllegalArgumentException(
String.format(
"No template \"%s\" found with type %s",
templateName, typeName)));
}
// if template type not specified, try to find a Custom template by that name. If none,
// fall back on finding a Target built-in template by the name. If not, throw an
// exception and bail out.
Matcher m = TEMPLATE_PATTERN.matcher(events);
m.find();
String templateName = m.group(1);
String typeName = m.group(2);
if (ALL_EVENTS_TEMPLATE.getName().equals(templateName)) {
return enableAllEvents(connection);
}
if (typeName != null) {
return connection
.getTemplateService()
.getEvents(templateName, TemplateType.CUSTOM)
.or(
() -> {
try {
return connection
.getTemplateService()
.getEvents(templateName, TemplateType.TARGET);
} catch (Exception e) {
return Optional.empty();
}
})
.getEvents(templateName, TemplateType.valueOf(typeName))
.orElseThrow(
() ->
new IllegalArgumentException(
String.format(
"Invalid/unknown event template %s",
templateName)));
"No template \"%s\" found with type %s",
templateName, typeName)));
}

return enableSelectedEvents(connection, events);
// if template type not specified, try to find a Custom template by that name. If none,
// fall back on finding a Target built-in template by the name. If not, throw an
// exception and bail out.
return connection
.getTemplateService()
.getEvents(templateName, TemplateType.CUSTOM)
.or(
() -> {
try {
return connection
.getTemplateService()
.getEvents(templateName, TemplateType.TARGET);
} catch (Exception e) {
return Optional.empty();
}
})
.orElseThrow(
() ->
new IllegalArgumentException(
String.format(
"Invalid/unknown event template %s",
templateName)));
}

protected IConstrainedMap<EventOptionID> enableAllEvents(JFRConnection connection)
Expand All @@ -311,20 +305,4 @@ protected IConstrainedMap<EventOptionID> enableAllEvents(JFRConnection connectio

return builder.build();
}

protected IConstrainedMap<EventOptionID> enableSelectedEvents(
JFRConnection connection, String events) throws Exception {
EventOptionsBuilder builder = eventOptionsBuilderFactory.create(connection);

Matcher matcher = EVENTS_PATTERN.matcher(events);
while (matcher.find()) {
String eventTypeId = matcher.group(1);
String option = matcher.group(2);
String value = matcher.group(3);

builder.addEvent(eventTypeId, option, value);
}

return builder.build();
}
}
28 changes: 3 additions & 25 deletions src/test/java/io/cryostat/commands/internal/AbstractRecordingCommandTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,46 +100,24 @@ void setup() {
"jdk:bar:baz",
"jdk.Event",
"Event",
"template=",
})
void shouldNotValidateInvalidEventString(String events) {
void shouldNotValidateInvalidEventTemplate(String events) {
assertFalse(command.validateEvents(events));
}

@ParameterizedTest
@ValueSource(
strings = {
"foo.Event:prop=val",
"foo.Event:prop=val,bar.Event:thing=1",
"foo.class$Inner:prop=val",
"template=ALL",
"template=Foo",
"template=Continuous,type=TARGET",
"template=Foo,type=CUSTOM",
})
void shouldValidateValidEventString(String events) {
void shouldValidateValidEventTemplate(String events) {
assertTrue(command.validateEvents(events));
}

@Test
void shouldBuildSelectedEventMap() throws Exception {
verifyNoInteractions(eventOptionsBuilderFactory);

EventOptionsBuilder builder = mock(EventOptionsBuilder.class);
when(eventOptionsBuilderFactory.create(Mockito.any())).thenReturn(builder);

command.enableEvents(
connection,
"foo.Bar$Inner:prop=some,bar.Baz$Inner2:key=val,jdk.CPULoad:enabled=true");

verify(builder).addEvent("foo.Bar$Inner", "prop", "some");
verify(builder).addEvent("bar.Baz$Inner2", "key", "val");
verify(builder).addEvent("jdk.CPULoad", "enabled", "true");
verify(builder).build();

verifyNoMoreInteractions(builder);
verifyNoMoreInteractions(eventOptionsBuilderFactory);
}

@Test
void shouldBuildTemplateEventMap() throws Exception {
TemplateService templateSvc = mock(TemplateService.class);
Expand Down
Loading

0 comments on commit 46ffa03

Please sign in to comment.