8357187: JFR: User-defined defaults should be respected when an incorrect setting is set

Reviewed-by: mgronlun
This commit is contained in:
Erik Gahlin 2025-05-19 13:38:38 +00:00
parent 92fd44992b
commit 265d630125
6 changed files with 81 additions and 20 deletions

View File

@ -301,7 +301,7 @@ public final class EventControl {
private static Control defineThreshold(PlatformEventType type) {
String def = type.getAnnotationValue(Threshold.class, ThresholdSetting.DEFAULT_VALUE);
type.add(PrivateAccess.getInstance().newSettingDescriptor(TYPE_THRESHOLD, Threshold.NAME, def, Collections.emptyList()));
return new Control(new ThresholdSetting(type), def);
return new Control(new ThresholdSetting(type, def), def);
}
private static Control defineStackTrace(PlatformEventType type) {
@ -313,13 +313,13 @@ public final class EventControl {
private static Control defineCutoff(PlatformEventType type) {
String def = type.getAnnotationValue(Cutoff.class, CutoffSetting.DEFAULT_VALUE);
type.add(PrivateAccess.getInstance().newSettingDescriptor(TYPE_CUTOFF, Cutoff.NAME, def, Collections.emptyList()));
return new Control(new CutoffSetting(type), def);
return new Control(new CutoffSetting(type, def), def);
}
private static Control defineThrottle(PlatformEventType type) {
String def = type.getAnnotationValue(Throttle.class, ThrottleSetting.DEFAULT_VALUE);
type.add(PrivateAccess.getInstance().newSettingDescriptor(TYPE_THROTTLE, Throttle.NAME, def, Collections.emptyList()));
return new Control(new ThrottleSetting(type), def);
return new Control(new ThrottleSetting(type, def), def);
}
private static Control defineLevel(PlatformEventType type) {
@ -332,7 +332,7 @@ public final class EventControl {
private static Control definePeriod(PlatformEventType type) {
String def = type.getAnnotationValue(Period.class, PeriodSetting.DEFAULT_VALUE);
type.add(PrivateAccess.getInstance().newSettingDescriptor(TYPE_PERIOD, PeriodSetting.NAME, def, Collections.emptyList()));
return new Control(new PeriodSetting(type), def);
return new Control(new PeriodSetting(type, def), def);
}
void disable() {

View File

@ -39,6 +39,7 @@ import jdk.jfr.Timespan;
import jdk.jfr.internal.PlatformEventType;
import jdk.jfr.internal.Type;
import jdk.jfr.internal.util.ValueParser;
import jdk.jfr.internal.util.Utils;
@MetadataDefinition
@Label("Cutoff")
@ -47,11 +48,14 @@ import jdk.jfr.internal.util.ValueParser;
@Timespan
public final class CutoffSetting extends SettingControl {
public static final String DEFAULT_VALUE = ValueParser.INFINITY;
private String value = DEFAULT_VALUE;
private final PlatformEventType eventType;
private final String defaultValue;
private String value;
public CutoffSetting(PlatformEventType eventType) {
this.eventType = Objects.requireNonNull(eventType);
public CutoffSetting(PlatformEventType eventType, String defaultValue) {
this.eventType = Objects.requireNonNull(eventType);
this.defaultValue = Utils.validTimespanInfinity(eventType, "Cutoff", defaultValue, DEFAULT_VALUE);
this.value = defaultValue;
}
@Override
@ -65,7 +69,7 @@ public final class CutoffSetting extends SettingControl {
max = nanos;
}
}
return Objects.requireNonNullElse(text, DEFAULT_VALUE);
return Objects.requireNonNullElse(text, defaultValue);
}
@Override

View File

@ -36,6 +36,8 @@ import jdk.jfr.SettingControl;
import jdk.jfr.internal.PlatformEventType;
import jdk.jfr.internal.Type;
import jdk.jfr.internal.util.ValueParser;
import jdk.jfr.internal.util.Utils;
import static jdk.jfr.internal.util.ValueParser.MISSING;
@MetadataDefinition
@ -44,17 +46,27 @@ import static jdk.jfr.internal.util.ValueParser.MISSING;
@Name(Type.SETTINGS_PREFIX + "Period")
public final class PeriodSetting extends SettingControl {
private static final long typeId = Type.getTypeId(PeriodSetting.class);
public static final String EVERY_CHUNK = "everyChunk";
public static final String BEGIN_CHUNK = "beginChunk";
public static final String END_CHUNK = "endChunk";
public static final String DEFAULT_VALUE = EVERY_CHUNK;
public static final String NAME = "period";
private final PlatformEventType eventType;
private String value = EVERY_CHUNK;
private final String defaultValue;
private String value;
public PeriodSetting(PlatformEventType eventType) {
public PeriodSetting(PlatformEventType eventType, String defaultValue) {
this.eventType = Objects.requireNonNull(eventType);
this.defaultValue = validPeriod(defaultValue);
this.value = defaultValue;
}
private String validPeriod(String userDefault) {
return switch (userDefault) {
case BEGIN_CHUNK -> BEGIN_CHUNK;
case END_CHUNK -> END_CHUNK;
default -> Utils.validTimespanInfinity(eventType, "Period", userDefault, DEFAULT_VALUE);
};
}
@Override
@ -95,7 +107,7 @@ public final class PeriodSetting extends SettingControl {
if (!beginChunk && endChunk) {
return END_CHUNK;
}
return DEFAULT_VALUE; // "everyChunk" is default
return defaultValue;
}
@Override

View File

@ -38,6 +38,7 @@ import jdk.jfr.SettingControl;
import jdk.jfr.Timespan;
import jdk.jfr.internal.PlatformEventType;
import jdk.jfr.internal.Type;
import jdk.jfr.internal.util.Utils;
import jdk.jfr.internal.util.ValueParser;
@MetadataDefinition
@ -48,11 +49,14 @@ import jdk.jfr.internal.util.ValueParser;
public final class ThresholdSetting extends SettingControl {
public static final String DEFAULT_VALUE = "0 ns";
private static final long typeId = Type.getTypeId(ThresholdSetting.class);
private String value = DEFAULT_VALUE;
private final PlatformEventType eventType;
private final String defaultValue;
private String value;
public ThresholdSetting(PlatformEventType eventType) {
public ThresholdSetting(PlatformEventType eventType, String defaultValue) {
this.eventType = Objects.requireNonNull(eventType);
this.defaultValue = Utils.validTimespanInfinity(eventType, "Threshold", defaultValue, DEFAULT_VALUE);
this.value = defaultValue;
}
@Override
@ -68,7 +72,7 @@ public final class ThresholdSetting extends SettingControl {
}
}
}
return Objects.requireNonNullElse(text, DEFAULT_VALUE);
return Objects.requireNonNullElse(text, defaultValue);
}
@Override

View File

@ -51,10 +51,24 @@ import jdk.jfr.internal.util.Utils;
public final class ThrottleSetting extends SettingControl {
public static final String DEFAULT_VALUE = Throttle.DEFAULT;
private final PlatformEventType eventType;
private String value = DEFAULT_VALUE;
private final String defaultValue;
private String value;
public ThrottleSetting(PlatformEventType eventType) {
this.eventType = Objects.requireNonNull(eventType);
public ThrottleSetting(PlatformEventType eventType, String defaultValue) {
this.eventType = Objects.requireNonNull(eventType);
this.defaultValue = validRate(defaultValue);
this.value = defaultValue;
}
private String validRate(String defaultValue) {
if (DEFAULT_VALUE.equals(defaultValue)) {
return DEFAULT_VALUE; // Fast path to avoid parsing
}
if (Rate.of(defaultValue) == null) {
Utils.warnInvalidAnnotation(eventType, "Throttle", defaultValue, DEFAULT_VALUE);
return DEFAULT_VALUE;
}
return defaultValue;
}
@Override
@ -70,8 +84,7 @@ public final class ThrottleSetting extends SettingControl {
}
}
}
// "off" is default
return Objects.requireNonNullElse(text, DEFAULT_VALUE);
return Objects.requireNonNullElse(text, defaultValue);
}
@Override

View File

@ -55,6 +55,7 @@ import jdk.jfr.internal.LogLevel;
import jdk.jfr.internal.LogTag;
import jdk.jfr.internal.Logger;
import jdk.jfr.internal.MirrorEvent;
import jdk.jfr.internal.PlatformEventType;
import jdk.jfr.internal.SecuritySupport;
import jdk.jfr.internal.Type;
import jdk.jfr.internal.management.HiddenWait;
@ -459,4 +460,31 @@ public final class Utils {
File file = subPath == null ? new File(path) : new File(path, subPath);
return file.toPath().toAbsolutePath();
}
public static String validTimespanInfinity(PlatformEventType type, String annotation, String userDefault, String systemDefault) {
if (systemDefault.equals(userDefault)) {
return systemDefault; // Fast path to avoid parsing
}
if (ValueParser.parseTimespanWithInfinity(userDefault, ValueParser.MISSING) != ValueParser.MISSING) {
return userDefault;
}
warnInvalidAnnotation(type, annotation, userDefault, systemDefault);
return systemDefault;
}
public static void warnInvalidAnnotation(PlatformEventType type, String annotation, String userDefault, String systemDefault) {
StringBuilder sb = new StringBuilder();
sb.append("Programming error. Event setting ");
sb.append("@").append(annotation).append("(\"").append(userDefault).append("\")");
sb.append(" is invalid on event ");
sb.append(type.getName());
sb.append(", using ");
sb.append("@").append(annotation).append("(\"").append(systemDefault).append("\")");
sb.append( " instead.");
if (type.isSystem()) {
throw new InternalError(sb.toString()); // Fail fast for JDK and JVM events
}
Logger.log(LogTag.JFR_SETTING, LogLevel.WARN, sb.toString());
}
}