Skip to content

Commit

Permalink
fix(recordingOptions): scope customizers to each Target
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewazores committed Apr 15, 2024
1 parent 794db33 commit 31ed317
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 54 deletions.
2 changes: 1 addition & 1 deletion src/main/java/io/cryostat/recordings/RecordingHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ public ActiveRecording createSnapshot(Target target, JFRConnection connection)
String rename = String.format("%s-%d", desc.getName().toLowerCase(), desc.getId());

RecordingOptionsBuilder recordingOptionsBuilder =
recordingOptionsBuilderFactory.create(connection.getService());
recordingOptionsBuilderFactory.create(target);
recordingOptionsBuilder.name(rename);

connection.getService().updateRecordingOptions(desc, recordingOptionsBuilder.build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,27 @@

import org.openjdk.jmc.common.unit.QuantityConversionException;
import org.openjdk.jmc.flightrecorder.configuration.recording.RecordingOptionsBuilder;
import org.openjdk.jmc.rjmx.services.jfr.IFlightRecorderService;

public interface RecordingOptionsBuilderFactory {
RecordingOptionsBuilder create(IFlightRecorderService service)
throws QuantityConversionException;
import io.cryostat.targets.Target;
import io.cryostat.targets.TargetConnectionManager;

import io.smallrye.common.annotation.Blocking;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;

@ApplicationScoped
public class RecordingOptionsBuilderFactory {

@Inject RecordingOptionsCustomizerFactory customizerFactory;
@Inject TargetConnectionManager connectionManager;

@Blocking
public RecordingOptionsBuilder create(Target target) throws QuantityConversionException {
return connectionManager.executeConnectedTask(
target,
conn ->
customizerFactory
.create(target)
.apply(new RecordingOptionsBuilder(conn.getService())));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright The Cryostat Authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.cryostat.recordings;

import java.util.HashMap;

import io.cryostat.core.RecordingOptionsCustomizer;
import io.cryostat.targets.Target;
import io.cryostat.targets.Target.EventKind;
import io.cryostat.targets.Target.TargetDiscovery;

import io.quarkus.vertx.ConsumeEvent;
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;
import org.jboss.logging.Logger;

@ApplicationScoped
public class RecordingOptionsCustomizerFactory {

@Inject Logger logger;

private final HashMap<Target, RecordingOptionsCustomizer> customizers = new HashMap<>();

@ConsumeEvent(Target.TARGET_JVM_DISCOVERY)
void onMessage(TargetDiscovery event) {
if (EventKind.LOST.equals(event.kind())) {
customizers.remove(event.serviceRef());
}
}

public RecordingOptionsCustomizer create(Target target) {
return customizers.computeIfAbsent(
target, (t) -> new RecordingOptionsCustomizer(logger::debug));
}
}
45 changes: 22 additions & 23 deletions src/main/java/io/cryostat/recordings/Recordings.java
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public class Recordings {
@Inject TargetConnectionManager connectionManager;
@Inject EventBus bus;
@Inject RecordingOptionsBuilderFactory recordingOptionsBuilderFactory;
@Inject RecordingOptionsCustomizer recordingOptionsCustomizer;
@Inject RecordingOptionsCustomizerFactory recordingOptionsCustomizerFactory;
@Inject EventOptionsBuilder.Factory eventOptionsBuilderFactory;
@Inject Clock clock;
@Inject S3Client storage;
Expand Down Expand Up @@ -604,7 +604,7 @@ public Response createRecording(
connection -> {
RecordingOptionsBuilder optionsBuilder =
recordingOptionsBuilderFactory
.create(connection.getService())
.create(target)
.name(recordingName);
if (duration.isPresent()) {
optionsBuilder.duration(TimeUnit.SECONDS.toMillis(duration.get()));
Expand Down Expand Up @@ -884,8 +884,7 @@ public Map<String, Object> getRecordingOptions(@RestPath long id) throws Excepti
return connectionManager.executeConnectedTask(
target,
connection -> {
RecordingOptionsBuilder builder =
recordingOptionsBuilderFactory.create(connection.getService());
RecordingOptionsBuilder builder = recordingOptionsBuilderFactory.create(target);
return getRecordingOptions(connection.getService(), builder);
});
}
Expand All @@ -906,6 +905,9 @@ public Response patchRecordingOptionsV1(@RestPath URI connectUrl) {
@Blocking
@Path("/api/v3/targets/{id}/recordingOptions")
@RolesAllowed("read")
@SuppressFBWarnings(
value = "UC_USELESS_OBJECT",
justification = "SpotBugs thinks the options map is unused, but it is used")
public Map<String, Object> patchRecordingOptions(
@RestPath long id,
@RestForm String toDisk,
Expand All @@ -914,20 +916,20 @@ public Map<String, Object> patchRecordingOptions(
throws Exception {
final String unsetKeyword = "unset";

Map<String, String> form = new HashMap<>();
Map<String, String> options = new HashMap<>();
Pattern bool = Pattern.compile("true|false|" + unsetKeyword);
if (toDisk != null) {
Matcher m = bool.matcher(toDisk);
if (!m.matches()) {
throw new BadRequestException("Invalid options");
}
form.put("toDisk", toDisk);
options.put("toDisk", toDisk);
}
if (maxAge != null) {
if (!unsetKeyword.equals(maxAge)) {
try {
Long.parseLong(maxAge);
form.put("maxAge", maxAge);
options.put("maxAge", maxAge);
} catch (NumberFormatException e) {
throw new BadRequestException("Invalid options");
}
Expand All @@ -937,31 +939,28 @@ public Map<String, Object> patchRecordingOptions(
if (!unsetKeyword.equals(maxSize)) {
try {
Long.parseLong(maxSize);
form.put("maxSize", maxSize);
options.put("maxSize", maxSize);
} catch (NumberFormatException e) {
throw new BadRequestException("Invalid options");
}
}
}
form.entrySet()
.forEach(
e -> {
RecordingOptionsCustomizer.OptionKey optionKey =
RecordingOptionsCustomizer.OptionKey.fromOptionName(e.getKey())
.get();
if ("unset".equals(e.getValue())) {
recordingOptionsCustomizer.unset(optionKey);
} else {
recordingOptionsCustomizer.set(optionKey, e.getValue());
}
});

Target target = Target.find("id", id).singleResult();
for (var entry : options.entrySet()) {
RecordingOptionsCustomizer.OptionKey optionKey =
RecordingOptionsCustomizer.OptionKey.fromOptionName(entry.getKey()).get();
var recordingOptionsCustomizer = recordingOptionsCustomizerFactory.create(target);
if (unsetKeyword.equals(entry.getValue())) {
recordingOptionsCustomizer.unset(optionKey);
} else {
recordingOptionsCustomizer.set(optionKey, entry.getValue());
}
}

return connectionManager.executeConnectedTask(
target,
connection -> {
RecordingOptionsBuilder builder =
recordingOptionsBuilderFactory.create(connection.getService());
var builder = recordingOptionsBuilderFactory.create(target);
return getRecordingOptions(connection.getService(), builder);
});
}
Expand Down
20 changes: 0 additions & 20 deletions src/main/java/io/cryostat/recordings/RecordingsModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,37 +15,17 @@
*/
package io.cryostat.recordings;

import org.openjdk.jmc.flightrecorder.configuration.recording.RecordingOptionsBuilder;

import io.cryostat.core.EventOptionsBuilder;
import io.cryostat.core.RecordingOptionsCustomizer;

import jakarta.enterprise.context.ApplicationScoped;
import jakarta.enterprise.inject.Produces;
import jakarta.inject.Singleton;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@Singleton
public class RecordingsModule {

@Produces
@ApplicationScoped
public RecordingOptionsBuilderFactory provideRecordingOptionsBuilderFactory(
RecordingOptionsCustomizer customizer) {
return service -> customizer.apply(new RecordingOptionsBuilder(service));
}

@Produces
@ApplicationScoped
public EventOptionsBuilder.Factory provideEventOptionsBuilderFactory() {
return new EventOptionsBuilder.Factory();
}

@Produces
@ApplicationScoped
public RecordingOptionsCustomizer provideRecordingOptionsCustomizer() {
Logger log = LoggerFactory.getLogger(RecordingOptionsCustomizer.class);
return new RecordingOptionsCustomizer(log::debug);
}
}
9 changes: 3 additions & 6 deletions src/main/java/io/cryostat/rules/RuleService.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.openjdk.jmc.rjmx.ConnectionException;
import org.openjdk.jmc.rjmx.ServiceNotAvailableException;

import io.cryostat.core.net.JFRConnection;
import io.cryostat.core.templates.Template;
import io.cryostat.core.templates.TemplateType;
import io.cryostat.expressions.MatchExpressionEvaluator;
Expand Down Expand Up @@ -143,7 +142,7 @@ public void activate(Rule rule, Target target) throws Exception {
connectionManager.executeConnectedTask(
target,
connection -> {
var recordingOptions = createRecordingOptions(rule, connection);
var recordingOptions = createRecordingOptions(rule, target);

Pair<String, TemplateType> pair =
recordingHelper.parseEventSpecifier(rule.eventSpecifier);
Expand Down Expand Up @@ -173,15 +172,13 @@ public void activate(Rule rule, Target target) throws Exception {
}
}

private IConstrainedMap<String> createRecordingOptions(Rule rule, JFRConnection connection)
private IConstrainedMap<String> createRecordingOptions(Rule rule, Target target)
throws ConnectionException,
QuantityConversionException,
IOException,
ServiceNotAvailableException {
RecordingOptionsBuilder optionsBuilder =
recordingOptionsBuilderFactory
.create(connection.getService())
.name(rule.getRecordingName());
recordingOptionsBuilderFactory.create(target).name(rule.getRecordingName());
if (rule.maxAgeSeconds > 0) {
optionsBuilder.maxAge(rule.maxAgeSeconds);
}
Expand Down

0 comments on commit 31ed317

Please sign in to comment.