Skip to content

Commit

Permalink
Move mandatory JVM options out of jvm.options and into JvmOptionsPa…
Browse files Browse the repository at this point in the history 
…rser

Certain values for the JVM are mandatory for Logstash to function correctly. Rather than
leave them in config/jvm.options where they can be updated, and can cause upgrade issues
for users when we add new mandatory options, move them into code where they cannot be
changed
  • Loading branch information
@robbavey
robbavey committed Mar 2, 2022
1 parent 5266987 commit 3ded1e8
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 92 deletions.
16 changes: 1 addition & 15 deletions config/jvm.options
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@
-Djruby.compile.invokedynamic=true
# Force Compilation
-Djruby.jit.threshold=0
# Make sure joni regexp interruptability is enabled
-Djruby.regexp.interruptible=true

## heap dumps

Expand All @@ -73,16 +71,4 @@
-Djava.security.egd=file:/dev/urandom

# Copy the logging context from parent threads to children
-Dlog4j2.isThreadContextMapInheritable=true

11-:--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED
11-:--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED
11-:--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED
11-:--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED
11-:--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED

11-:--add-opens=java.base/java.security=ALL-UNNAMED
11-:--add-opens=java.base/java.io=ALL-UNNAMED
11-:--add-opens=java.base/java.nio.channels=ALL-UNNAMED
11-:--add-opens=java.base/sun.nio.ch=ALL-UNNAMED
11-:--add-opens=java.management/sun.management=ALL-UNNAMED
-Dlog4j2.isThreadContextMapInheritable=true
178 changes: 102 additions & 76 deletions logstash-core/src/main/java/org/logstash/launchers/JvmOptionsParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,38 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;


/**
* Parse jvm.options file applying version conditional logic. Heavily inspired by same functionality in Elasticsearch.
* */
public class JvmOptionsParser {

private static final String[] MANDATORY_JVM_OPTIONS = new String[]{
"-Djruby.regexp.interruptible=true",
"16-:--add-exports=jdk.compiler/com.sun.tools.javac.api==ALL-UNNAMED",
"16-:--add-exports=jdk.compiler/com.sun.tools.javac.file==ALL-UNNAMED",
"16-:--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
"16-:--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
"16-:--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
"11-:--add-opens=java.base/java.security=ALL-UNNAMED",
"11-:--add-opens=java.base/java.io=ALL-UNNAMED",
"11-:--add-opens=java.base/java.nio.channels=ALL-UNNAMED",
"11-:--add-opens=java.base/sun.nio.ch=ALL-UNNAMED",
"11-:--add-opens=java.management/sun.management=ALL-UNNAMED"
};

static class JvmOptionsFileParserException extends Exception {

private static final long serialVersionUID = 2446165130736962758L;
Expand Down Expand Up @@ -71,8 +88,7 @@ public static void main(final String[] args) throws InterruptedException, IOExce
);
}
bailOnOldJava();
final String lsJavaOpts = System.getenv("LS_JAVA_OPTS");
handleJvmOptions(args, lsJavaOpts);
handleJvmOptions(args, System.getenv("LS_JAVA_OPTS"));
}

static void bailOnOldJava(){
Expand All @@ -93,7 +109,7 @@ static void handleJvmOptions(String[] args, String lsJavaOpts) {
final String jvmOpts = args.length == 2 ? args[1] : null;
try {
Optional<Path> jvmOptions = parser.lookupJvmOptionsFile(jvmOpts);
parser.parseAndInjectEnvironment(jvmOptions, lsJavaOpts);
parser.handleJvmOptions(jvmOptions, lsJavaOpts);
} catch (JvmOptionsFileParserException pex) {
System.err.printf(Locale.ROOT,
"encountered [%d] error%s parsing [%s]",
Expand Down Expand Up @@ -128,20 +144,33 @@ private Optional<Path> lookupJvmOptionsFile(String jvmOpts) {
.findFirst();
}

private void parseAndInjectEnvironment(Optional<Path> jvmOptionsFile, String lsJavaOpts) throws IOException, JvmOptionsFileParserException {
final List<String> jvmOptionsContent = new ArrayList<>(parseJvmOptions(jvmOptionsFile));
private void handleJvmOptions(Optional<Path> jvmOptionsFile, String lsJavaOpts) throws IOException, JvmOptionsFileParserException {
int javaMajorVersion = javaMajorVersion();

// Add JVM Options from config/jvm.options
final Set<String> jvmOptionsContent = new LinkedHashSet<>(getJvmOptionsFromFile(jvmOptionsFile, javaMajorVersion));

// Add JVM Options from LS_JAVA_OPTS
if (lsJavaOpts != null && !lsJavaOpts.isEmpty()) {
if (isDebugEnabled()) {
System.err.println("Appending jvm options from environment LS_JAVA_OPTS");
}
jvmOptionsContent.add(lsJavaOpts);
}
// Set mandatory JVM options
jvmOptionsContent.addAll(getMandatoryJvmOptions(javaMajorVersion));

System.out.println(String.join(" ", jvmOptionsContent));
}

private List<String> parseJvmOptions(Optional<Path> jvmOptionsFile) throws IOException, JvmOptionsFileParserException {
static List<String> getMandatoryJvmOptions(int javaMajorVersion){
return Arrays.stream(MANDATORY_JVM_OPTIONS)
.map(option -> jvmOptionFromLine(javaMajorVersion, option))
.flatMap(Optional::stream)
.collect(Collectors.toList());
}

private List<String> getJvmOptionsFromFile(final Optional<Path> jvmOptionsFile, final int javaMajorVersion) throws IOException, JvmOptionsFileParserException {
if (!jvmOptionsFile.isPresent()) {
System.err.println("Warning: no jvm.options file found.");
return Collections.emptyList();
Expand All @@ -155,13 +184,11 @@ private List<String> parseJvmOptions(Optional<Path> jvmOptionsFile) throws IOExc
if (isDebugEnabled()) {
System.err.format("Processing jvm.options file at `%s`\n", optionsFilePath);
}
final int majorJavaVersion = javaMajorVersion();

try (InputStream is = Files.newInputStream(optionsFilePath);
Reader reader = new InputStreamReader(is, StandardCharsets.UTF_8);
BufferedReader br = new BufferedReader(reader)
) {
final ParseResult parseResults = parse(majorJavaVersion, br);
final ParseResult parseResults = parse(javaMajorVersion, br);
if (parseResults.hasErrors()) {
throw new JvmOptionsFileParserException(optionsFilePath, parseResults.getInvalidLines());
}
Expand Down Expand Up @@ -209,7 +236,36 @@ public List<String> getJvmOptions() {
private static final Pattern OPTION_DEFINITION = Pattern.compile("((?<start>\\d+)(?<range>-)?(?<end>\\d+)?:)?(?<option>-.*)$");

/**
* Parse the line-delimited JVM options from the specified buffered reader for the specified Java major version.
*
* If the version syntax specified on a line matches the specified JVM options, the JVM option callback will be invoked with the JVM
* option. If the line does not match the specified syntax for the JVM options, the invalid line callback will be invoked with the
* contents of the entire line.
*
* @param javaMajorVersion the Java major version to match JVM options against
* @param br the buffered reader to read line-delimited JVM options from
* @return the admitted options lines respecting the javaMajorVersion and the error lines
* @throws IOException if an I/O exception occurs reading from the buffered reader
*/
static ParseResult parse(final int javaMajorVersion, final BufferedReader br) throws IOException {
final ParseResult result = new ParseResult();
int lineNumber = 0;
while (true) {
final String line = br.readLine();
lineNumber++;
if (line == null) {
break;
}
try{
jvmOptionFromLine(javaMajorVersion, line).ifPresent(result::appendOption);
} catch (IllegalArgumentException e){
result.appendError(lineNumber, line);
};
}
return result;
}

/**
* Parse the line-delimited JVM options from the specified string for the specified Java major version.
* Valid JVM options are:
* <ul>
* <li>
Expand Down Expand Up @@ -256,82 +312,52 @@ public List<String> getJvmOptions() {
* {@code 9-10:-Xlog:age*=trace,gc*,safepoint:file=logs/gc.log:utctime,pid,tags:filecount=32,filesize=64m}
* </li>
* </ul>
*
* If the version syntax specified on a line matches the specified JVM options, the JVM option callback will be invoked with the JVM
* option. If the line does not match the specified syntax for the JVM options, the invalid line callback will be invoked with the
* contents of the entire line.
*
* @param javaMajorVersion the Java major version to match JVM options against
* @param br the buffered reader to read line-delimited JVM options from
* @return the admitted options lines respecting the javaMajorVersion and the error lines
* @throws IOException if an I/O exception occurs reading from the buffered reader
* @param javaMajorVersion
* @param line
* @return Returns an Optional containing a string if the line contains a valid option for the specified Java
* version and empty otherwise
*/
static ParseResult parse(final int javaMajorVersion, final BufferedReader br) throws IOException {
final ParseResult result = new ParseResult();
int lineNumber = 0;
while (true) {
final String line = br.readLine();
lineNumber++;
if (line == null) {
break;
}
if (line.startsWith("#")) {
// lines beginning with "#" are treated as comments
continue;
}
if (line.matches("\\s*")) {
// skip blank lines
continue;
}
final Matcher matcher = OPTION_DEFINITION.matcher(line);
if (matcher.matches()) {
final String start = matcher.group("start");
final String end = matcher.group("end");
if (start == null) {
// no range present, unconditionally apply the JVM option
result.appendOption(line);
private static Optional<String> jvmOptionFromLine(final int javaMajorVersion, final String line){
if (line.startsWith("#") || line.matches("\\s*")) {
// Skip comments and blank lines
return Optional.empty();
}
final Matcher matcher = OPTION_DEFINITION.matcher(line);
if (matcher.matches()) {
final String start = matcher.group("start");
final String end = matcher.group("end");
if (start == null) {
// no range present, unconditionally apply the JVM option
return Optional.of(line);
} else {
final int lower = Integer.parseInt(start);
final int upper;
if (matcher.group("range") == null) {
// no range is present, apply the JVM option to the specified major version only
upper = lower;
} else if (end == null) {
// a range of the form \\d+- is present, apply the JVM option to all major versions larger than the specified one
upper = Integer.MAX_VALUE;
} else {
final int lower;
try {
lower = Integer.parseInt(start);
} catch (final NumberFormatException e) {
result.appendError(lineNumber, line);
continue;
}
final int upper;
if (matcher.group("range") == null) {
// no range is present, apply the JVM option to the specified major version only
upper = lower;
} else if (end == null) {
// a range of the form \\d+- is present, apply the JVM option to all major versions larger than the specified one
upper = Integer.MAX_VALUE;
} else {
// a range of the form \\d+-\\d+ is present, apply the JVM option to the specified range of major versions
try {
upper = Integer.parseInt(end);
} catch (final NumberFormatException e) {
result.appendError(lineNumber, line);
continue;
}
if (upper < lower) {
result.appendError(lineNumber, line);
continue;
}
}
if (lower <= javaMajorVersion && javaMajorVersion <= upper) {
result.appendOption(matcher.group("option"));
upper = Integer.parseInt(end);
if (upper < lower) {
throw new IllegalArgumentException("Upper bound must be greater than lower bound");
}
}
} else {
result.appendError(lineNumber, line);
if (lower <= javaMajorVersion && javaMajorVersion <= upper) {
return Optional.of(matcher.group("option"));
}
}
} else {
throw new IllegalArgumentException("Illegal JVM Option");
}
return result;
return Optional.empty();
}

private static final Pattern JAVA_VERSION = Pattern.compile("^(?:1\\.)?(?<javaMajorVersion>\\d+)(?:\\.\\d+)?$");

private int javaMajorVersion() {
public static int javaMajorVersion() {
final String specVersion = System.getProperty("java.specification.version");
final Matcher specVersionMatcher = JAVA_VERSION.matcher(specVersion);
if (!specVersionMatcher.matches()) {
Expand Down
23 changes: 22 additions & 1 deletion logstash-core/src/test/java/org/logstash/launchers/JvmOptionsParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public void test_LS_JAVA_OPTS_isUsedWhenNoJvmOptionsIsAvailable() throws IOExcep

// Verify
final String output = outputStreamCaptor.toString();
assertEquals("Output MUST contains the options present in LS_JAVA_OPTS", "-Xblabla" + System.lineSeparator(), output);
assertTrue("Output MUST contains the options present in LS_JAVA_OPTS", output.contains("-Xblabla"));
}

@SuppressWarnings({ "unchecked" })
Expand Down Expand Up @@ -89,6 +89,27 @@ public void testParseOptionVersionRange() throws IOException {
assertTrue("No option match outside the range [10-11]", res.getJvmOptions().isEmpty());
}

@Test
public void testMandatoryJvmOptionApplicableJvmPresent() throws IOException{
assertTrue("Contains add-exports value for Java 17",
JvmOptionsParser.getMandatoryJvmOptions(17).contains("--add-exports=jdk.compiler/com.sun.tools.javac.api==ALL-UNNAMED"));
}

@Test
public void testMandatoryJvmOptionNonApplicableJvmNotPresent() throws IOException{
assertFalse("Does not contains add-exports value for Java 11",
JvmOptionsParser.getMandatoryJvmOptions(11).contains("--add-exports=jdk.compiler/com.sun.tools.javac.api==ALL-UNNAMED"));
}

@Test
public void testAlwaysMandatoryJvmPresent() {
assertTrue("Contains regexp interruptible for Java 11",
JvmOptionsParser.getMandatoryJvmOptions(11).contains("-Djruby.regexp.interruptible=true"));
assertTrue("Contains regexp interruptible for Java 17",
JvmOptionsParser.getMandatoryJvmOptions(17).contains("-Djruby.regexp.interruptible=true"));

}

@Test
public void testErrorLinesAreReportedCorrectly() throws IOException {
final String jvmOptionsContent = "10-11:-XX:+UseConcMarkSweepGC" + System.lineSeparator() +
Expand Down

0 comments on commit 3ded1e8

Please sign in to comment.