Skip to content

Commit

Permalink
feat(rules): implement "archive" rules (#648)
Browse files Browse the repository at this point in the history 
* fix(rule): validate eventSpecifier

* feat(rules): implement "archive" rules

* fix(rules): don't enforce name uniqueness on archiver rules

* test(rules): add unit tests for archiver rule handling

* fix(rules): do not assume default rule periodic archive settings

* fix(rules): disallow configurations on archiver rules

* fix(rules): archive rules have optional names

* fix(rules): remove unnecessary check

* fix(rules): only enforce toDisk=true if maxAge/maxSize set

* chore(rules): apply spotless formatting

* fix(rules): avoid NPE during deserialization

* chore(rules): apply spotless formatting
  • Loading branch information
@andrewazores
andrewazores authored Aug 19, 2021
1 parent 7e37830 commit e46209f
Show file tree
Hide file tree
Showing 8 changed files with 528 additions and 69 deletions.
15 changes: 11 additions & 4 deletions HTTP_API.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1398,8 +1398,9 @@ The handler-specific descriptions below describe how each handler populates the
attributes `"name"`, `"matchExpression"`, and `"eventSpecifier"` must be
provided.
`"name"`: the name of this rule definition. This must be unique. This name
will also be used to generate the name of the associated recordings.
`"name"`: the name of this rule definition. This must be unique, except in
the case of "archiver rules" (see `eventSpecifier` below). This name will
also be used to generate the name of the associated recordings.
`"matchExpression"`: a string expression used to determine which target JVMs
this rule will apply to. The expression has a variable named `target` in
Expand All @@ -1412,9 +1413,15 @@ The handler-specific descriptions below describe how each handler populates the
The simple expression `true` may also be used to create a rule which applies
to any and all discovered targets.
`"eventSpecifier"`: a string of the form `template=Foo,type=TYPE`. This
`"eventSpecifier"`: a string of the form `template=Foo,type=TYPE`, which
defines the event template that will be used for creating new recordings in
matching targets.
matching targets; or, the special string `"archive"`, which signifies that
this rule should cause all matching targets to have their current (at the
time of rule creation) JFR data copied to the Cryostat archives as a
one-time operation. When using `"archive"`, it is invalid to provide
`archivalPeriodSeconds`, `preservedArchives`, `maxSizeBytes`, or
`maxAgeSeconds`. Such "archiver rules" are only processed once and are not
persisted, so the `name` and `description` become optional.
The following attributes are optional:
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/io/cryostat/rules/MatchExpressionValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ public class MatchExpressionValidator {

String validate(Rule rule) throws MatchExpressionValidationException {
try {
CompilationUnitTree cut = parser.parse(rule.getName(), rule.getMatchExpression(), null);
String name = rule.getName();
if (name == null) {
name = "";
}
CompilationUnitTree cut = parser.parse(name, rule.getMatchExpression(), null);
if (cut == null) {
throw new IllegalMatchExpressionException();
}
Expand Down
92 changes: 69 additions & 23 deletions src/main/java/io/cryostat/rules/Rule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@

import java.util.function.Function;

import io.cryostat.recordings.RecordingTargetHelper;

import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import io.vertx.core.MultiMap;
import org.apache.commons.lang3.StringUtils;
Expand All @@ -50,6 +53,8 @@ public class Rule {
private static final MatchExpressionValidator MATCH_EXPRESSION_VALIDATOR =
new MatchExpressionValidator();

static final String ARCHIVE_EVENT = "archive";

private final String name;
private final String description;
private final String matchExpression;
Expand All @@ -60,10 +65,14 @@ public class Rule {
private final int maxSizeBytes;

Rule(Builder builder) throws MatchExpressionValidationException {
this.name = sanitizeRuleName(requireNonBlank(builder.name, Attribute.NAME));
this.eventSpecifier = builder.eventSpecifier;
if (isArchiver()) {
this.name = builder.name;
} else {
this.name = sanitizeRuleName(requireNonBlank(builder.name, Attribute.NAME));
}
this.description = builder.description == null ? "" : builder.description;
this.matchExpression = builder.matchExpression;
this.eventSpecifier = builder.eventSpecifier;
this.archivalPeriodSeconds = builder.archivalPeriodSeconds;
this.preservedArchives = builder.preservedArchives;
this.maxAgeSeconds =
Expand Down Expand Up @@ -93,6 +102,10 @@ public String getEventSpecifier() {
return this.eventSpecifier;
}

public boolean isArchiver() {
return ARCHIVE_EVENT.equals(getEventSpecifier());
}

public int getArchivalPeriodSeconds() {
return this.archivalPeriodSeconds;
}
Expand All @@ -114,10 +127,6 @@ public static String sanitizeRuleName(String name) {
return name.replaceAll("\\s", "_");
}

static String validateMatchExpression(Rule rule) throws MatchExpressionValidationException {
return MATCH_EXPRESSION_VALIDATOR.validate(rule);
}

private static String requireNonBlank(String s, Attribute attr) {
if (StringUtils.isBlank(s)) {
throw new IllegalArgumentException(
Expand All @@ -126,6 +135,28 @@ private static String requireNonBlank(String s, Attribute attr) {
return s;
}

public void validate() throws IllegalArgumentException, MatchExpressionValidationException {
requireNonBlank(this.matchExpression, Attribute.MATCH_EXPRESSION);
validateEventSpecifier(requireNonBlank(this.eventSpecifier, Attribute.EVENT_SPECIFIER));
validateMatchExpression(this);

if (isArchiver()) {
requireNonPositive(this.archivalPeriodSeconds, Attribute.ARCHIVAL_PERIOD_SECONDS);
requireNonPositive(this.preservedArchives, Attribute.PRESERVED_ARCHIVES);
requireNonPositive(this.maxSizeBytes, Attribute.MAX_SIZE_BYTES);
requireNonPositive(this.maxAgeSeconds, Attribute.MAX_AGE_SECONDS);
} else {
requireNonBlank(this.name, Attribute.NAME);
requireNonNegative(this.archivalPeriodSeconds, Attribute.ARCHIVAL_PERIOD_SECONDS);
requireNonNegative(this.preservedArchives, Attribute.PRESERVED_ARCHIVES);
}
}

private static String validateMatchExpression(Rule rule)
throws MatchExpressionValidationException {
return MATCH_EXPRESSION_VALIDATOR.validate(rule);
}

private static int requireNonNegative(int i, Attribute attr) {
if (i < 0) {
throw new IllegalArgumentException(
Expand All @@ -134,13 +165,22 @@ private static int requireNonNegative(int i, Attribute attr) {
return i;
}

public void validate() throws IllegalArgumentException, MatchExpressionValidationException {
requireNonBlank(this.name, Attribute.NAME);
requireNonBlank(this.matchExpression, Attribute.MATCH_EXPRESSION);
requireNonBlank(this.eventSpecifier, Attribute.EVENT_SPECIFIER);
requireNonNegative(this.archivalPeriodSeconds, Attribute.ARCHIVAL_PERIOD_SECONDS);
requireNonNegative(this.preservedArchives, Attribute.PRESERVED_ARCHIVES);
validateMatchExpression(this);
private static int requireNonPositive(int i, Attribute attr) {
if (i > 0) {
throw new IllegalArgumentException(
String.format("\"%s\" cannot be positive, was \"%d\"", attr, i));
}
return i;
}

private static String validateEventSpecifier(String eventSpecifier)
throws IllegalArgumentException {
if (eventSpecifier.equals(ARCHIVE_EVENT)) {
return eventSpecifier;
}
// throws if cannot be parsed
RecordingTargetHelper.parseEventSpecifierToTemplate(eventSpecifier);
return eventSpecifier;
}

@Override
Expand All @@ -154,12 +194,12 @@ public int hashCode() {
}

public static class Builder {
private String name;
private String description;
private String matchExpression;
private String eventSpecifier;
private int archivalPeriodSeconds = 30;
private int preservedArchives = 1;
private String name = "";
private String description = "";
private String matchExpression = "";
private String eventSpecifier = "";
private int archivalPeriodSeconds = 0;
private int preservedArchives = 0;
private int maxAgeSeconds = -1;
private int maxSizeBytes = -1;

Expand Down Expand Up @@ -231,13 +271,11 @@ public static Builder from(MultiMap formAttributes) {
public static Builder from(JsonObject jsonObj) throws IllegalArgumentException {
Rule.Builder builder =
new Rule.Builder()
.name(jsonObj.get(Rule.Attribute.NAME.getSerialKey()).getAsString())
.name(getAsNullableString(jsonObj, Rule.Attribute.NAME))
.matchExpression(
jsonObj.get(Rule.Attribute.MATCH_EXPRESSION.getSerialKey())
.getAsString())
.description(
jsonObj.get(Rule.Attribute.DESCRIPTION.getSerialKey())
.getAsString())
.description(getAsNullableString(jsonObj, Rule.Attribute.DESCRIPTION))
.eventSpecifier(
jsonObj.get(Rule.Attribute.EVENT_SPECIFIER.getSerialKey())
.getAsString());
Expand All @@ -249,6 +287,14 @@ public static Builder from(JsonObject jsonObj) throws IllegalArgumentException {
return builder;
}

private static String getAsNullableString(JsonObject jsonObj, Rule.Attribute attr) {
JsonElement el = jsonObj.get(attr.getSerialKey());
if (el == null) {
return null;
}
return el.getAsString();
}

private Builder setOptionalInt(Rule.Attribute key, MultiMap formAttributes)
throws IllegalArgumentException {

Expand Down
80 changes: 54 additions & 26 deletions src/main/java/io/cryostat/rules/RuleProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.function.Consumer;

import org.openjdk.jmc.flightrecorder.configuration.recording.RecordingOptionsBuilder;
import org.openjdk.jmc.rjmx.services.jfr.IRecordingDescriptor;

import io.cryostat.configuration.CredentialsManager;
import io.cryostat.core.log.Logger;
Expand Down Expand Up @@ -159,29 +160,38 @@ private void activate(Rule rule, ServiceRef serviceRef) {

Credentials credentials =
credentialsManager.getCredentials(serviceRef.getServiceUri().toString());
try {
startRuleRecording(new ConnectionDescriptor(serviceRef, credentials), rule);
} catch (Exception e) {
logger.error(e);
}

logger.trace("Rule activation successful");
if (rule.getPreservedArchives() <= 0 || rule.getArchivalPeriodSeconds() <= 0) {
return;

if (rule.isArchiver()) {
try {
archiveRuleRecording(new ConnectionDescriptor(serviceRef, credentials), rule);
} catch (Exception e) {
logger.error(e);
}
} else {
try {
startRuleRecording(new ConnectionDescriptor(serviceRef, credentials), rule);
} catch (Exception e) {
logger.error(e);
}

if (rule.getPreservedArchives() <= 0 || rule.getArchivalPeriodSeconds() <= 0) {
return;
}
tasks.put(
Pair.of(serviceRef, rule),
scheduler.scheduleAtFixedRate(
periodicArchiverFactory.create(
serviceRef,
credentialsManager,
rule,
recordingArchiveHelper,
this::archivalFailureHandler,
base32),
rule.getArchivalPeriodSeconds(),
rule.getArchivalPeriodSeconds(),
TimeUnit.SECONDS));
}
tasks.put(
Pair.of(serviceRef, rule),
scheduler.scheduleAtFixedRate(
periodicArchiverFactory.create(
serviceRef,
credentialsManager,
rule,
recordingArchiveHelper,
this::archivalFailureHandler,
base32),
rule.getArchivalPeriodSeconds(),
rule.getArchivalPeriodSeconds(),
TimeUnit.SECONDS));
}

private void deactivate(Rule rule, ServiceRef serviceRef) {
Expand Down Expand Up @@ -217,6 +227,25 @@ private Void archivalFailureHandler(Pair<ServiceRef, Rule> id) {
return null;
}

private void archiveRuleRecording(ConnectionDescriptor connectionDescriptor, Rule rule)
throws Exception {
targetConnectionManager.executeConnectedTask(
connectionDescriptor,
connection -> {
IRecordingDescriptor descriptor =
connection.getService().getSnapshotRecording();
try {
recordingArchiveHelper
.saveRecording(connectionDescriptor, descriptor.getName())
.get();
} finally {
connection.getService().close(descriptor);
}

return null;
});
}

private void startRuleRecording(ConnectionDescriptor connectionDescriptor, Rule rule)
throws Exception {

Expand All @@ -226,16 +255,15 @@ private void startRuleRecording(ConnectionDescriptor connectionDescriptor, Rule
RecordingOptionsBuilder builder =
recordingOptionsBuilderFactory
.create(connection.getService())
.name(rule.getRecordingName())
.toDisk(true);
.name(rule.getRecordingName());
if (rule.getMaxAgeSeconds() > 0) {
builder = builder.maxAge(rule.getMaxAgeSeconds());
builder = builder.maxAge(rule.getMaxAgeSeconds()).toDisk(true);
}
if (rule.getMaxSizeBytes() > 0) {
builder = builder.maxSize(rule.getMaxSizeBytes());
builder = builder.maxSize(rule.getMaxSizeBytes()).toDisk(true);
}
Pair<String, TemplateType> template =
recordingTargetHelper.parseEventSpecifierToTemplate(
RecordingTargetHelper.parseEventSpecifierToTemplate(
rule.getEventSpecifier());
recordingTargetHelper.startRecording(
true,
Expand Down
28 changes: 15 additions & 13 deletions src/main/java/io/cryostat/rules/RuleRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,20 +94,22 @@ public void loadRules() throws IOException {
}

public Rule addRule(Rule rule) throws IOException {
if (hasRuleByName(rule.getName())) {
throw new RuleException(
String.format(
"Rule with name \"%s\" already exists; refusing to overwrite",
rule.getName()));
if (!rule.isArchiver()) {
if (hasRuleByName(rule.getName())) {
throw new RuleException(
String.format(
"Rule with name \"%s\" already exists; refusing to overwrite",
rule.getName()));
}
Path destination = rulesDir.resolve(rule.getName() + ".json");
this.fs.writeString(
destination,
gson.toJson(rule),
StandardOpenOption.WRITE,
StandardOpenOption.CREATE,
StandardOpenOption.TRUNCATE_EXISTING);
loadRules();
}
Path destination = rulesDir.resolve(rule.getName() + ".json");
this.fs.writeString(
destination,
gson.toJson(rule),
StandardOpenOption.WRITE,
StandardOpenOption.CREATE,
StandardOpenOption.TRUNCATE_EXISTING);
loadRules();
emit(RuleEvent.ADDED, rule);
return rule;
}
Expand Down
Loading

0 comments on commit e46209f

Please sign in to comment.