Skip to content

Commit

Permalink
Bump org.apache.logging.log4j:log4j-core from 2.17.1 to 2.20.0 (opens…
Browse files Browse the repository at this point in the history
…earch-project#8307)

- Use log4j's PluginProcessor annotation processor for plugin discovery
- Remove usage of deprecated explicit plugin discovery
- Pass through Configuration in OpenSearchJsonLayout to work around https://issues.apache.org/jira/browse/LOG4J2-3562

Signed-off-by: Thomas <tsfarr@amazon.com>
Signed-off-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Co-authored-by: Daniel (dB.) Doubrovkine <dblock@amazon.com>
Co-authored-by: Andrew Ross <andrross@amazon.com>
  • Loading branch information
3 people authored and raghuvanshraj committed Jul 12, 2023
1 parent 28949e1 commit 18cb245
Show file tree
Hide file tree
Showing 31 changed files with 57 additions and 52 deletions.
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ out/
benchmarks/src/main/generated/*
benchmarks/bin/*
benchmarks/build-eclipse-default/*
server/bin/*
server/build-eclipse-default/*
test/framework/build-eclipse-default/*

# eclipse files
.project
Expand Down Expand Up @@ -61,4 +64,4 @@ testfixtures_shared/
.ci/jobs/

# build files generated
doc-tools/missing-doclet/bin/
doc-tools/missing-doclet/bin/
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Bump `com.google.jimfs:jimfs` from 1.2 to 1.3.0 (#8577, #8571)
- Bump `com.networknt:json-schema-validator` from 1.0.85 to 1.0.86 ([#8573](https://github.com/opensearch-project/OpenSearch/pull/8573))
- Bump `com.google.cloud:google-cloud-core-http` from 2.17.0 to 2.21.0 ([#8586](https://github.com/opensearch-project/OpenSearch/pull/8586))
- Bump `org.apache.logging.log4j:log4j-core` from 2.17.1 to 2.20.0 ([#8307](https://github.com/opensearch-project/OpenSearch/pull/8307))

### Changed
- Replace jboss-annotations-api_1.2_spec with jakarta.annotation-api ([#7836](https://github.com/opensearch-project/OpenSearch/pull/7836))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ repositories {
mavenCentral()
}
dependencies {
implementation 'org.apache.logging.log4j:log4j-core:2.20.0'
implementation "org.apache.logging.log4j:log4j-core:2.20.0"
}

["0.0.1", "0.0.2"].forEach { v ->
Expand Down
3 changes: 1 addition & 2 deletions buildSrc/version.properties
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ jackson_databind = 2.15.2
snakeyaml = 2.0
icu4j = 70.1
supercsv = 2.4.0
# Update to 2.17.2+ is breaking OpenSearchJsonLayout (see https://issues.apache.org/jira/browse/LOG4J2-3562)
log4j = 2.17.1
log4j = 2.20.0
slf4j = 1.7.36
asm = 9.5
jettison = 1.5.4
Expand Down
1 change: 0 additions & 1 deletion libs/core/licenses/log4j-api-2.17.1.jar.sha1

This file was deleted.

1 change: 1 addition & 0 deletions libs/core/licenses/log4j-api-2.20.0.jar.sha1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1fe6082e660daf07c689a89c94dc0f49c26b44bb

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
689151374756cb809cb029f2501015bdc7733179

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
689151374756cb809cb029f2501015bdc7733179

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
689151374756cb809cb029f2501015bdc7733179

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
689151374756cb809cb029f2501015bdc7733179

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
7ab4f082fd162f60afcaf2b8744a3d959feab3e8

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
689151374756cb809cb029f2501015bdc7733179
3 changes: 1 addition & 2 deletions qa/wildfly/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,10 @@ dependencies {
api "com.fasterxml.jackson.jakarta.rs:jackson-jakarta-rs-base:${versions.jackson}"
api "com.fasterxml.jackson.jakarta.rs:jackson-jakarta-rs-json-provider:${versions.jackson}"
api "com.github.fge:json-patch:1.9"
api "org.apache.logging.log4j:log4j-api:${versions.log4j}"
api "org.apache.logging.log4j:log4j-core:${versions.log4j}"
api(project(path: ':client:rest-high-level')) {
exclude module: 'jakarta.annotation-api'
}
testImplementation "org.apache.logging.log4j:log4j-slf4j-impl:${versions.log4j}"
testImplementation(project(':test:framework')) {
exclude module: 'jakarta.annotation-api'
}
Expand Down
20 changes: 0 additions & 20 deletions qa/wildfly/src/main/resources/log4j2.properties

This file was deleted.

5 changes: 5 additions & 0 deletions server/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ dependencies {
api "org.apache.logging.log4j:log4j-api:${versions.log4j}"
api "org.apache.logging.log4j:log4j-jul:${versions.log4j}"
api "org.apache.logging.log4j:log4j-core:${versions.log4j}", optional
annotationProcessor "org.apache.logging.log4j:log4j-core:${versions.log4j}"

// jna
api "net.java.dev.jna:jna:${versions.jna}"
Expand Down Expand Up @@ -175,6 +176,10 @@ tasks.withType(JavaCompile).configureEach {
options.compilerArgs -= '-Xlint:unchecked'
}

compileJava {
options.compilerArgs += ['-processor', 'org.apache.logging.log4j.core.config.plugins.processor.PluginProcessor']
}

tasks.named("internalClusterTest").configure {
// TODO: these run faster with C2 only because they run for so, so long
jvmArgs -= '-XX:TieredStopAtLevel=1'
Expand Down
1 change: 0 additions & 1 deletion server/licenses/log4j-api-2.17.1.jar.sha1

This file was deleted.

1 change: 1 addition & 0 deletions server/licenses/log4j-api-2.20.0.jar.sha1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1fe6082e660daf07c689a89c94dc0f49c26b44bb
1 change: 0 additions & 1 deletion server/licenses/log4j-core-2.17.1.jar.sha1

This file was deleted.

1 change: 1 addition & 0 deletions server/licenses/log4j-core-2.20.0.jar.sha1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
eb2a9a47b1396e00b5eee1264296729a70565cc0
1 change: 0 additions & 1 deletion server/licenses/log4j-jul-2.17.1.jar.sha1

This file was deleted.

1 change: 1 addition & 0 deletions server/licenses/log4j-jul-2.20.0.jar.sha1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
8170e6118eac1ab332046c179718a0f107f688e1
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import org.apache.logging.log4j.core.config.builder.api.ConfigurationBuilderFactory;
import org.apache.logging.log4j.core.config.builder.impl.BuiltConfiguration;
import org.apache.logging.log4j.core.config.composite.CompositeConfiguration;
import org.apache.logging.log4j.core.config.plugins.util.PluginManager;
import org.apache.logging.log4j.core.config.properties.PropertiesConfiguration;
import org.apache.logging.log4j.core.config.properties.PropertiesConfigurationFactory;
import org.apache.logging.log4j.status.StatusConsoleListener;
Expand Down Expand Up @@ -140,13 +139,6 @@ public static void configure(final Environment environment) throws IOException,
configure(environment.settings(), environment.configDir(), environment.logsDir());
}

/**
* Load logging plugins so we can have {@code node_name} in the pattern.
*/
public static void loadLog4jPlugins() {
PluginManager.addPackage(LogConfigurator.class.getPackage().getName());
}

/**
* Sets the node name. This is called before logging is configured if the
* node name is set in opensearch.yml. Otherwise it is called as soon
Expand All @@ -172,8 +164,6 @@ private static void configure(final Settings settings, final Path configsPath, f
Objects.requireNonNull(configsPath);
Objects.requireNonNull(logsPath);

loadLog4jPlugins();

setLogConfigurationSystemProperty(logsPath, settings);
// we initialize the status logger immediately otherwise Log4j will complain when we try to get the context
configureStatusLogger();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@

import org.apache.logging.log4j.core.Layout;
import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.config.Configuration;
import org.apache.logging.log4j.core.config.Node;
import org.apache.logging.log4j.core.config.plugins.Plugin;
import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
import org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory;
import org.apache.logging.log4j.core.config.plugins.PluginConfiguration;
import org.apache.logging.log4j.core.config.plugins.PluginFactory;
import org.apache.logging.log4j.core.layout.AbstractStringLayout;
import org.apache.logging.log4j.core.layout.ByteBufferDestination;
Expand Down Expand Up @@ -94,11 +96,18 @@ public class OpenSearchJsonLayout extends AbstractStringLayout {

private final PatternLayout patternLayout;

protected OpenSearchJsonLayout(String typeName, Charset charset, String[] opensearchMessageFields, int maxMessageLength) {
protected OpenSearchJsonLayout(
String typeName,
Charset charset,
String[] opensearchMessageFields,
int maxMessageLength,
Configuration configuration
) {
super(charset);
this.patternLayout = PatternLayout.newBuilder()
.withPattern(pattern(typeName, opensearchMessageFields, maxMessageLength))
.withAlwaysWriteExceptions(false)
.withConfiguration(configuration)
.build();
}

Expand Down Expand Up @@ -173,8 +182,14 @@ private String inQuotes(String s) {
}

@PluginFactory
public static OpenSearchJsonLayout createLayout(String type, Charset charset, String[] opensearchmessagefields, int maxMessageLength) {
return new OpenSearchJsonLayout(type, charset, opensearchmessagefields, maxMessageLength);
public static OpenSearchJsonLayout createLayout(
String type,
Charset charset,
String[] opensearchmessagefields,
int maxMessageLength,
Configuration configuration
) {
return new OpenSearchJsonLayout(type, charset, opensearchmessagefields, maxMessageLength, configuration);
}

PatternLayout getPatternLayout() {
Expand Down Expand Up @@ -202,6 +217,9 @@ public static class Builder<B extends OpenSearchJsonLayout.Builder<B>> extends A
@PluginAttribute(value = "maxmessagelength", defaultInt = 10000)
private int maxMessageLength;

@PluginConfiguration
private Configuration configuration;

public Builder() {
setCharset(StandardCharsets.UTF_8);
setMaxMessageLength(10000);
Expand All @@ -210,7 +228,7 @@ public Builder() {
@Override
public OpenSearchJsonLayout build() {
String[] split = Strings.isNullOrEmpty(opensearchMessageFields) ? new String[] {} : opensearchMessageFields.split(",");
return OpenSearchJsonLayout.createLayout(type, charset, split, maxMessageLength);
return OpenSearchJsonLayout.createLayout(type, charset, split, maxMessageLength, configuration);
}

public Charset getCharset() {
Expand Down Expand Up @@ -248,6 +266,15 @@ public B setMaxMessageLength(final int maxMessageLength) {
this.maxMessageLength = maxMessageLength;
return asBuilder();
}

public Configuration getConfiguration() {
return configuration;
}

public B setConfiguration(final Configuration configuration) {
this.configuration = configuration;
return asBuilder();
}
}

@PluginBuilderFactory
Expand Down
3 changes: 3 additions & 0 deletions test/framework/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,14 @@ dependencies {
api "org.mockito:mockito-core:${versions.mockito}"
api "net.bytebuddy:byte-buddy:${versions.bytebuddy}"
api "org.objenesis:objenesis:${versions.objenesis}"

annotationProcessor "org.apache.logging.log4j:log4j-core:${versions.log4j}"
}

compileJava.options.compilerArgs -= '-Xlint:cast'
compileJava.options.compilerArgs -= '-Xlint:rawtypes'
compileJava.options.compilerArgs -= '-Xlint:unchecked'
compileJava.options.compilerArgs += ['-processor', 'org.apache.logging.log4j.core.config.plugins.processor.PluginProcessor']
compileTestJava.options.compilerArgs -= '-Xlint:rawtypes'

// the main files are actually test files, so use the appropriate forbidden api sigs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@
import org.opensearch.common.logging.DeprecatedMessage;
import org.opensearch.common.logging.HeaderWarning;
import org.opensearch.common.logging.HeaderWarningAppender;
import org.opensearch.common.logging.LogConfigurator;
import org.opensearch.common.logging.Loggers;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
Expand Down Expand Up @@ -239,7 +238,6 @@ public void tearDown() throws Exception {
static {
TEST_WORKER_VM_ID = System.getProperty(TEST_WORKER_SYS_PROPERTY, DEFAULT_TEST_WORKER_ID);
setTestSysProps();
LogConfigurator.loadLog4jPlugins();

String leakLoggerName = "io.netty.util.ResourceLeakDetector";
Logger leakLogger = LogManager.getLogger(leakLoggerName);
Expand Down

0 comments on commit 18cb245

Please sign in to comment.