[log4j] migrate to log4j2 (#258)

* [log4j] migrate to log4j2 * [travis] fix log4j config file directive * [log4j2] support STDERR and STDOUT as ways to refer to log location too
DataDog · Jun 24, 2020 · 776565c · 776565c
1 parent f346e6a
commit 776565c
Show file tree

Hide file tree

Showing 10 changed files with 136 additions and 51 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -13,22 +13,22 @@ matrix:
     - jdk: oraclejdk8
       env: 
        - DESC="Oracle JDK8 linting"
-       - CMD="mvn verify -B -Dhttps.protocols=TLSv1.2 -DskipTests -Dlog4j.configuration=log4j.travis.properties"
+       - CMD="mvn verify -B -Dhttps.protocols=TLSv1.2 -DskipTests -Dlog4j.configuration=log4j2.travis.properties"
 
     - jdk: oraclejdk8
       env: 
        - DESC="Oracle JDK8 testing"
-       - CMD="mvn test -B -Dhttps.protocols=TLSv1.2 -Dcheckstyle.skip=true -Dlog4j.configuration=log4j.travis.properties"
+       - CMD="mvn test -B -Dhttps.protocols=TLSv1.2 -Dcheckstyle.skip=true -Dlog4j.configurationFile=log4j2.travis.properties"
 
     - jdk: oraclejdk7
       env: 
        - DESC="Oracle JDK7 testing"
-       - CMD="mvn test -B -Dhttps.protocols=TLSv1.2 -Dcheckstyle.skip=true -Dlog4j.configuration=log4j.travis.properties"
+       - CMD="mvn test -B -Dhttps.protocols=TLSv1.2 -Dcheckstyle.skip=true -Dlog4j.configurationFile=log4j2.travis.properties"
 
     - jdk: openjdk7
       env: 
        - DESC="OpenJDK7 testing"
-       - CMD="mvn test -B -Dhttps.protocols=TLSv1.2 -Dcheckstyle.skip=true -Dlog4j.configuration=log4j.travis.properties"
+       - CMD="mvn test -B -Dhttps.protocols=TLSv1.2 -Dcheckstyle.skip=true -Dlog4j.configurationFile=log4j2.travis.properties"
 
 script: echo "Running $DESC..." && (eval $CMD) 
 
diff --git a/log4j.travis.properties b/log4j.travis.properties
diff --git a/log4j2.travis.properties b/log4j2.travis.properties
@@ -0,0 +1,9 @@
+rootLogger.level = info
+rootLogger.appenderRef.stdout.ref = CONSOLE
+
+appender.console.type = Console
+appender.console.name = CONSOLE
+appender.console.layout.type = PatternLayout
+appender.console.layout.pattern = %p\t%d{ISO8601}\t%r\t%c\t[%t]\t%m%n
+appender.console.filter.threshold.type = ThresholdFilter
+appender.console.filter.threshold.level = info
diff --git a/pom.xml b/pom.xml
@@ -41,7 +41,8 @@
         <guava.version>17.0</guava.version>
         <java-dogstatsd-client.version>2.1.0</java-dogstatsd-client.version>
         <jcommander.version>1.35</jcommander.version>
-        <log4j.version>1.2.17</log4j.version>
+        <log4j.version>2.12.1</log4j.version>
+        <slf4j.version>1.7.26</slf4j.version>
         <jackson.version>2.10.0</jackson.version>
         <snakeyaml.version>1.13</snakeyaml.version>
         <!-- Note: using project checkstyle with AOSP style
@@ -121,8 +122,19 @@
             <version>${jackson.version}</version>
         </dependency>
         <dependency>
-            <groupId>log4j</groupId>
-            <artifactId>log4j</artifactId>
+            <groupId>org.projectlombok</groupId>
+            <artifactId>lombok</artifactId>
+            <version>1.18.8</version>
+            <scope>provided</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>slf4j-api</artifactId>
+            <version>${slf4j.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.logging.log4j</groupId>
+            <artifactId>log4j-core</artifactId>
             <version>${log4j.version}</version>
             <exclusions>
                 <exclusion>
@@ -139,6 +151,11 @@
                 </exclusion>
             </exclusions>
         </dependency>
+        <dependency>
+            <groupId>org.apache.logging.log4j</groupId>
+            <artifactId>log4j-slf4j-impl</artifactId>
+            <version>${log4j.version}</version>
+        </dependency>
         <dependency>
             <groupId>org.yaml</groupId>
             <artifactId>snakeyaml</artifactId>

diff --git a/src/main/java/org/datadog/jmxfetch/App.java b/src/main/java/org/datadog/jmxfetch/App.java
@@ -6,9 +6,8 @@
 import com.beust.jcommander.ParameterException;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import org.apache.commons.io.IOUtils;
-import org.apache.log4j.Appender;
-import org.apache.log4j.Level;
-import org.apache.log4j.Logger;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
 
 import org.datadog.jmxfetch.reporter.Reporter;
 import org.datadog.jmxfetch.tasks.TaskMethod;

diff --git a/src/main/java/org/datadog/jmxfetch/util/CustomLogger.java b/src/main/java/org/datadog/jmxfetch/util/CustomLogger.java
@@ -3,47 +3,110 @@
 import com.google.common.collect.HashMultiset;
 import com.google.common.collect.Multiset;
 
-import org.apache.log4j.ConsoleAppender;
-import org.apache.log4j.Level;
-import org.apache.log4j.Logger;
-import org.apache.log4j.PatternLayout;
-import org.apache.log4j.RollingFileAppender;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.Filter.Result;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.appender.ConsoleAppender;
+import org.apache.logging.log4j.core.appender.RollingFileAppender;
+import org.apache.logging.log4j.core.appender.rolling.DefaultRolloverStrategy;
+import org.apache.logging.log4j.core.appender.rolling.SizeBasedTriggeringPolicy;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.Configurator;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+import org.apache.logging.log4j.core.filter.ThresholdFilter;
+import org.apache.logging.log4j.core.layout.PatternLayout;
 
 public class CustomLogger {
     private static final Logger LOGGER = Logger.getLogger(CustomLogger.class.getName());
     private static final Multiset<String> messageCount = HashMultiset.create();
     private static final String LOGGER_LAYOUT = "%d | %-5p| %c{1} | %m%n";
+    // log4j2 uses SYSTEM_OUT and SYSTEM_ERR - support both
+    private static final String SYSTEM_OUT_ALT = "STDOUT";
+    private static final String SYSTEM_ERR_ALT = "STDERR";
 
     /** Sets up the custom logger to the specified level and location. */
     public static void setup(Level level, String logLocation) {
+        final LoggerContext ctx = (LoggerContext) LogManager.getContext(false);
+        final Configuration config = ctx.getConfiguration();
+        String target = "CONSOLE";
+
         if (logLocation != null
-                && !ConsoleAppender.SYSTEM_ERR.equals(logLocation)
-                && !ConsoleAppender.SYSTEM_OUT.equals(logLocation)) {
-            RollingFileAppender fa = new RollingFileAppender();
-            fa.setName("FileLogger");
-            fa.setFile(logLocation);
-            fa.setMaxFileSize("5MB");
-            fa.setMaxBackupIndex(1);
-            fa.setLayout(new PatternLayout(LOGGER_LAYOUT));
-            fa.setThreshold(level);
-            fa.setAppend(true);
-            fa.activateOptions();
-            Logger.getRootLogger().addAppender(fa);
-            LOGGER.info("File Handler set");
+                && !ConsoleAppender.Target.SYSTEM_ERR.toString().equals(logLocation)
+                && !SYSTEM_ERR_ALT.equals(logLocation)
+                && !ConsoleAppender.Target.SYSTEM_OUT.toString().equals(logLocation)
+                && !SYSTEM_OUT_ALT.equals(logLocation)) {
+
+            target = "FileLogger";
+
+            PatternLayout layout = PatternLayout.newBuilder()
+                .withConfiguration(config)
+                .withPattern(LOGGER_LAYOUT)
+                .build();
+
+            RollingFileAppender fa = RollingFileAppender.newBuilder()
+                .setConfiguration(config)
+                .withName(target)
+                .withLayout(layout)
+                .withFileName(logLocation)
+                .withFilePattern(logLocation + ".%d")
+                .withPolicy(SizeBasedTriggeringPolicy.createPolicy("5MB"))
+                .withStrategy(DefaultRolloverStrategy.newBuilder().withMax("1").build())
+                .build();
+
+            fa.start();
+            config.addAppender(fa);
+            ctx.getRootLogger().addAppender(config.getAppender(fa.getName()));
+
+            log.info("File Handler set");
         } else {
-            ConsoleAppender consoleAppender = new ConsoleAppender(new PatternLayout(LOGGER_LAYOUT));
-            if (logLocation != null) {
-                consoleAppender.setTarget(logLocation);
+
+            if (logLocation != null
+                    && (ConsoleAppender.Target.SYSTEM_ERR.toString().equals(logLocation)
+                        || SYSTEM_ERR_ALT.equals(logLocation))) {
+
+                ConsoleAppender console = (ConsoleAppender)config.getAppender("CONSOLE");
+                console.stop();
+                config.getRootLogger().removeAppender("CONSOLE");
+                ctx.updateLoggers();
+
+                PatternLayout layout = PatternLayout.newBuilder()
+                    .withConfiguration(config)
+                    .withPattern(LOGGER_LAYOUT)
+                    .build();
+
+                ConsoleAppender ca = ConsoleAppender.newBuilder()
+                    .setConfiguration(config)
+                    .withName(logLocation)
+                    .setTarget(ConsoleAppender.Target.SYSTEM_ERR)
+                    .withLayout(layout)
+                    .build();
+
+                ca.start();
+                config.addAppender(ca);
+                ctx.getRootLogger().addAppender(config.getAppender(ca.getName()));
             }
-            consoleAppender.setThreshold(level);
-            Logger.getRootLogger().addAppender(consoleAppender);
         }
+
+        LoggerConfig loggerConfig = config.getLoggerConfig(LogManager.ROOT_LOGGER_NAME);
+        loggerConfig.setLevel(level);
+        ctx.updateLoggers();
     }
 
     /** Laconic logging for reduced verbosity. */
     public static void laconic(Logger logger, Level level, String message, int max) {
         if (messageCount.count(message) <= max) {
-            logger.log(level, message);
+            if (level.isInRange(Level.ERROR, Level.ALL)) {
+                logger.error(message);
+            } else if (level == Level.WARN) {
+                logger.warn(message);
+            } else if (level == Level.INFO) {
+                logger.info(message);
+            } else if (level == Level.DEBUG) {
+                logger.debug(message);
+            }
+
             messageCount.add(message);
         }
     }

diff --git a/src/main/resources/log4j.properties b/src/main/resources/log4j.properties
diff --git a/src/main/resources/log4j2.properties b/src/main/resources/log4j2.properties
@@ -0,0 +1,10 @@
+rootLogger.level = all
+rootLogger.appenderRef.stdout.ref = CONSOLE
+
+appender.console.type = Console
+appender.console.name = CONSOLE
+appender.console.target = SYSTEM_OUT
+appender.console.layout.type = PatternLayout
+appender.console.layout.pattern = %d | %-5p | %c{1} | %m%n
+appender.console.filter.threshold.type = ThresholdFilter
+appender.console.filter.threshold.level = all
diff --git a/src/test/java/org/datadog/jmxfetch/TestCommon.java b/src/test/java/org/datadog/jmxfetch/TestCommon.java
@@ -24,7 +24,7 @@
 import javax.management.MalformedObjectNameException;
 import javax.management.NotCompliantMBeanException;
 import javax.management.ObjectName;
-import org.apache.log4j.Level;
+import org.apache.logging.log4j.Level;
 import org.datadog.jmxfetch.reporter.ConsoleReporter;
 import org.datadog.jmxfetch.reporter.Reporter;
 import org.datadog.jmxfetch.util.CustomLogger;

diff --git a/src/test/java/org/datadog/jmxfetch/TestInstance.java b/src/test/java/org/datadog/jmxfetch/TestInstance.java
@@ -13,11 +13,12 @@
 import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.List;
-import org.apache.log4j.Logger;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.LogManager;
 import org.junit.Test;
 
 public class TestInstance extends TestCommon {
-    private static final Logger LOGGER = Logger.getLogger("Test Instance");
+    private static final Logger log = LogManager.getLogger("Test Instance");
 
     @Test
     public void testMinCollectionInterval() throws Exception {