Bump org.apache.logging.log4j:log4j-core from 2.17.1 to 2.20.0 (opens…

…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 <[email protected]> Signed-off-by: Daniel (dB.) Doubrovkine <[email protected]> Signed-off-by: Thomas Farr <[email protected]> Co-authored-by: Daniel (dB.) Doubrovkine <[email protected]> Co-authored-by: Andrew Ross <[email protected]>
raghuvanshraj · Jul 12, 2023 · 3a033fe · 3a033fe
1 parent 21ca9ab
commit 3a033fe
Show file tree

Hide file tree

Showing 31 changed files with 57 additions and 52 deletions.
diff --git a/.gitignore b/.gitignore
@@ -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
@@ -61,4 +64,4 @@ testfixtures_shared/
 .ci/jobs/
 
 # build files generated
-doc-tools/missing-doclet/bin/
+doc-tools/missing-doclet/bin/
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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))

diff --git a/buildSrc/src/testKit/thirdPartyAudit/sample_jars/build.gradle b/buildSrc/src/testKit/thirdPartyAudit/sample_jars/build.gradle
@@ -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 ->

diff --git a/buildSrc/version.properties b/buildSrc/version.properties
@@ -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

diff --git a/libs/core/licenses/log4j-api-2.17.1.jar.sha1 b/libs/core/licenses/log4j-api-2.17.1.jar.sha1
diff --git a/libs/core/licenses/log4j-api-2.20.0.jar.sha1 b/libs/core/licenses/log4j-api-2.20.0.jar.sha1
@@ -0,0 +1 @@
+1fe6082e660daf07c689a89c94dc0f49c26b44bb
diff --git a/plugins/discovery-azure-classic/licenses/log4j-1.2-api-2.17.1.jar.sha1 b/plugins/discovery-azure-classic/licenses/log4j-1.2-api-2.17.1.jar.sha1
diff --git a/plugins/discovery-azure-classic/licenses/log4j-1.2-api-2.20.0.jar.sha1 b/plugins/discovery-azure-classic/licenses/log4j-1.2-api-2.20.0.jar.sha1
@@ -0,0 +1 @@
+689151374756cb809cb029f2501015bdc7733179
diff --git a/plugins/discovery-ec2/licenses/log4j-1.2-api-2.17.1.jar.sha1 b/plugins/discovery-ec2/licenses/log4j-1.2-api-2.17.1.jar.sha1
diff --git a/plugins/discovery-ec2/licenses/log4j-1.2-api-2.20.0.jar.sha1 b/plugins/discovery-ec2/licenses/log4j-1.2-api-2.20.0.jar.sha1
@@ -0,0 +1 @@
+689151374756cb809cb029f2501015bdc7733179
diff --git a/plugins/discovery-gce/licenses/log4j-1.2-api-2.17.1.jar.sha1 b/plugins/discovery-gce/licenses/log4j-1.2-api-2.17.1.jar.sha1
diff --git a/plugins/discovery-gce/licenses/log4j-1.2-api-2.20.0.jar.sha1 b/plugins/discovery-gce/licenses/log4j-1.2-api-2.20.0.jar.sha1
@@ -0,0 +1 @@
+689151374756cb809cb029f2501015bdc7733179
diff --git a/plugins/repository-gcs/licenses/log4j-1.2-api-2.17.1.jar.sha1 b/plugins/repository-gcs/licenses/log4j-1.2-api-2.17.1.jar.sha1
diff --git a/plugins/repository-gcs/licenses/log4j-1.2-api-2.20.0.jar.sha1 b/plugins/repository-gcs/licenses/log4j-1.2-api-2.20.0.jar.sha1
@@ -0,0 +1 @@
+689151374756cb809cb029f2501015bdc7733179
diff --git a/plugins/repository-hdfs/licenses/log4j-slf4j-impl-2.17.1.jar.sha1 b/plugins/repository-hdfs/licenses/log4j-slf4j-impl-2.17.1.jar.sha1
diff --git a/plugins/repository-hdfs/licenses/log4j-slf4j-impl-2.20.0.jar.sha1 b/plugins/repository-hdfs/licenses/log4j-slf4j-impl-2.20.0.jar.sha1
@@ -0,0 +1 @@
+7ab4f082fd162f60afcaf2b8744a3d959feab3e8
diff --git a/plugins/repository-s3/licenses/log4j-1.2-api-2.17.1.jar.sha1 b/plugins/repository-s3/licenses/log4j-1.2-api-2.17.1.jar.sha1
diff --git a/plugins/repository-s3/licenses/log4j-1.2-api-2.20.0.jar.sha1 b/plugins/repository-s3/licenses/log4j-1.2-api-2.20.0.jar.sha1
@@ -0,0 +1 @@
+689151374756cb809cb029f2501015bdc7733179
diff --git a/qa/wildfly/build.gradle b/qa/wildfly/build.gradle
@@ -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'
   }

diff --git a/qa/wildfly/src/main/resources/log4j2.properties b/qa/wildfly/src/main/resources/log4j2.properties
diff --git a/server/build.gradle b/server/build.gradle
@@ -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}"
@@ -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'

diff --git a/server/licenses/log4j-api-2.17.1.jar.sha1 b/server/licenses/log4j-api-2.17.1.jar.sha1
diff --git a/server/licenses/log4j-api-2.20.0.jar.sha1 b/server/licenses/log4j-api-2.20.0.jar.sha1
@@ -0,0 +1 @@
+1fe6082e660daf07c689a89c94dc0f49c26b44bb
diff --git a/server/licenses/log4j-core-2.17.1.jar.sha1 b/server/licenses/log4j-core-2.17.1.jar.sha1
diff --git a/server/licenses/log4j-core-2.20.0.jar.sha1 b/server/licenses/log4j-core-2.20.0.jar.sha1
@@ -0,0 +1 @@
+eb2a9a47b1396e00b5eee1264296729a70565cc0
diff --git a/server/licenses/log4j-jul-2.17.1.jar.sha1 b/server/licenses/log4j-jul-2.17.1.jar.sha1
diff --git a/server/licenses/log4j-jul-2.20.0.jar.sha1 b/server/licenses/log4j-jul-2.20.0.jar.sha1
@@ -0,0 +1 @@
+8170e6118eac1ab332046c179718a0f107f688e1
diff --git a/server/src/main/java/org/opensearch/common/logging/LogConfigurator.java b/server/src/main/java/org/opensearch/common/logging/LogConfigurator.java
@@ -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;
@@ -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
@@ -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();

diff --git a/server/src/main/java/org/opensearch/common/logging/OpenSearchJsonLayout.java b/server/src/main/java/org/opensearch/common/logging/OpenSearchJsonLayout.java
@@ -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;
@@ -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();
     }
 
@@ -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() {
@@ -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);
@@ -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() {
@@ -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

diff --git a/test/framework/build.gradle b/test/framework/build.gradle
@@ -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

diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchTestCase.java
@@ -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;
@@ -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);