AMQ-8520: Log4j2 test fixes

Also fixes Log4jConfigView
NikitaShupletsov · Mar 3, 2022 · ae30dce · ae30dce
1 parent 7ec5254
commit ae30dce
Show file tree

Hide file tree

Showing 8 changed files with 74 additions and 57 deletions.
diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnection.java b/activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnection.java
@@ -309,7 +309,7 @@ public void serviceException(Throwable e) {
                 if (SERVICELOG.isDebugEnabled()) {
                     SERVICELOG.debug("Async error occurred: {}", e.getMessage(), e);
                 } else {
-                    SERVICELOG.warn("Async error occurred", e);
+                    SERVICELOG.warn("Async error occurred", e.getMessage());
                 }
                 ConnectionError ce = new ConnectionError();
                 ce.setException(e);

diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/Log4JConfigView.java b/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/Log4JConfigView.java
@@ -22,10 +22,9 @@
 import java.net.URL;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Enumeration;
 import java.util.List;
 import java.util.Locale;
-
+import java.util.Map;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -41,12 +40,13 @@ public String getRootLogLevel() throws Exception {
             return null;
         }
 
+        Class<?> logManagerClass = getLogManagerClass(cl);
         Class<?> loggerClass = getLoggerClass(cl);
-        if (loggerClass == null) {
+        if (logManagerClass == null || loggerClass == null) {
             return null;
         }
 
-        Method getRootLogger = loggerClass.getMethod("getRootLogger", new Class[]{});
+        Method getRootLogger = logManagerClass.getMethod("getRootLogger", new Class[]{});
         Method getLevel = loggerClass.getMethod("getLevel", new Class[]{});
         Object rootLogger = getRootLogger.invoke(null, (Object[])null);
 
@@ -61,16 +61,15 @@ public void setRootLogLevel(String level) throws Exception {
             return;
         }
 
+        Class<?> configuratorClass = getConfiguratorClass(cl);
         Class<?> loggerClass = getLoggerClass(cl);
         Class<?> levelClass = getLevelClass(cl);
-        if (levelClass == null || loggerClass == null) {
+        if (configuratorClass == null || levelClass == null || loggerClass == null) {
             return;
         }
 
         String targetLevel = level.toUpperCase(Locale.US);
-        Method getRootLogger = loggerClass.getMethod("getRootLogger", new Class[]{});
-        Method setLevel = loggerClass.getMethod("setLevel", levelClass);
-        Object rootLogger = getRootLogger.invoke(null, (Object[])null);
+        Method setRootLevel = configuratorClass.getMethod("setRootLevel", levelClass);
         Method toLevel = levelClass.getMethod("toLevel", String.class);
         Object newLevel = toLevel.invoke(null, targetLevel);
 
@@ -80,7 +79,7 @@ public void setRootLogLevel(String level) throws Exception {
         // matched what the user asked for.
         if (newLevel != null && newLevel.toString().equals(targetLevel)) {
             LOG.debug("Set level {} for root logger.", level);
-            setLevel.invoke(rootLogger, newLevel);
+            setRootLevel.invoke(configuratorClass, newLevel);
         }
     }
 
@@ -95,18 +94,23 @@ public List<String> getLoggers() throws Exception {
 
         Class<?> logManagerClass = getLogManagerClass(cl);
         Class<?> loggerClass = getLoggerClass(cl);
-        if (logManagerClass == null || loggerClass == null) {
+        Class<?> loggerConfigClass = getLoggerConfigClass(cl);
+        if (logManagerClass == null || loggerClass == null || loggerConfigClass == null) {
             return Collections.emptyList();
         }
 
-        Method getCurrentLoggers = logManagerClass.getMethod("getCurrentLoggers", new Class[]{});
-        Method getName = loggerClass.getMethod("getName", new Class[]{});
+        Method getContext = logManagerClass.getMethod("getContext", boolean.class);
+        Object logContext = getContext.invoke(logManagerClass, false);
+        Method getConfiguration = logContext.getClass().getMethod("getConfiguration");
+        Object configuration = getConfiguration.invoke(logContext);
+        Method getLoggers = configuration.getClass().getMethod("getLoggers");
+
+        Method getName = loggerConfigClass.getMethod("getName");
 
         List<String> list = new ArrayList<String>();
-        Enumeration<?> loggers = (Enumeration<?>)getCurrentLoggers.invoke(null, (Object[])null);
+        Map<String, ?> loggers = (Map<String, ?>)getLoggers.invoke(configuration);
 
-        while (loggers.hasMoreElements()) {
-            Object logger = loggers.nextElement();
+        for (Object logger : loggers.values()) {
             if (logger != null) {
                 list.add((String) getName.invoke(logger, (Object[])null));
             }
@@ -126,12 +130,13 @@ public String getLogLevel(String loggerName) throws Exception {
             return null;
         }
 
+        Class<?> logManagerClass = getLogManagerClass(cl);
         Class<?> loggerClass = getLoggerClass(cl);
-        if (loggerClass == null) {
+        if (logManagerClass == null || loggerClass == null) {
             return null;
         }
 
-        Method getLogger = loggerClass.getMethod("getLogger", String.class);
+        Method getLogger = logManagerClass.getMethod("getLogger", String.class);
         String logLevel = null;
 
         if (loggerName != null && !loggerName.isEmpty()) {
@@ -172,29 +177,26 @@ public void setLogLevel(String loggerName, String level) throws Exception {
             return;
         }
 
+        Class<?> configuratorClass = getConfiguratorClass(cl);
         Class<?> loggerClass = getLoggerClass(cl);
         Class<?> levelClass = getLevelClass(cl);
-        if (loggerClass == null || levelClass == null) {
+        if (configuratorClass == null || loggerClass == null || levelClass == null) {
             return;
         }
 
         String targetLevel = level.toUpperCase(Locale.US);
-        Method getLogger = loggerClass.getMethod("getLogger", String.class);
-        Method setLevel = loggerClass.getMethod("setLevel", levelClass);
+        Method setLevel = configuratorClass.getMethod("setLevel", String.class, levelClass);
         Method toLevel = levelClass.getMethod("toLevel", String.class);
 
-        Object logger = getLogger.invoke(null, loggerName);
-        if (logger != null) {
-            Object newLevel = toLevel.invoke(null, targetLevel);
-
-            // Check that the level conversion worked and that we got a level
-            // that matches what was asked for.  A bad level name will result
-            // in the lowest level value and we don't want to change unless we
-            // matched what the user asked for.
-            if (newLevel != null && newLevel.toString().equals(targetLevel)) {
-                LOG.debug("Set level {} for logger: {}", level, loggerName);
-                setLevel.invoke(logger, newLevel);
-            }
+        Object newLevel = toLevel.invoke(null, targetLevel);
+
+        // Check that the level conversion worked and that we got a level
+        // that matches what was asked for.  A bad level name will result
+        // in the lowest level value and we don't want to change unless we
+        // matched what the user asked for.
+        if (newLevel != null && newLevel.toString().equals(targetLevel)) {
+            LOG.debug("Set level {} for logger: {}", level, loggerName);
+            setLevel.invoke(configuratorClass, loggerName, newLevel);
         }
     }
 
@@ -258,29 +260,31 @@ private static boolean isLog4JAvailable(ClassLoader cl) {
     }
 
     private static Class<?> getLogManagerClass(ClassLoader cl) {
-        Class<?> logManagerClass = null;
-        try {
-            logManagerClass = cl.loadClass("org.apache.log4j.LogManager");
-        } catch (ClassNotFoundException e) {
-        }
-        return logManagerClass;
+        return getClass(cl, "org.apache.logging.log4j.LogManager");
     }
 
     private static Class<?> getLoggerClass(ClassLoader cl) {
-        Class<?> loggerClass = null;
-        try {
-            loggerClass = cl.loadClass("org.apache.log4j.Logger");
-        } catch (ClassNotFoundException e) {
-        }
-        return loggerClass;
+        return getClass(cl, "org.apache.logging.log4j.Logger");
+    }
+
+    private static Class<?> getLoggerConfigClass(ClassLoader cl) {
+        return getClass(cl, "org.apache.logging.log4j.core.config.LoggerConfig");
     }
 
     private static Class<?> getLevelClass(ClassLoader cl) {
-        Class<?> levelClass = null;
+        return getClass(cl, "org.apache.logging.log4j.Level");
+    }
+
+    private static Class<?> getConfiguratorClass(ClassLoader cl) {
+        return getClass(cl, "org.apache.logging.log4j.core.config.Configurator");
+    }
+
+    private static Class<?> getClass(ClassLoader cl, String clazz) {
+        Class<?> configuratorClass = null;
         try {
-            levelClass = cl.loadClass("org.apache.log4j.Level");
+            configuratorClass = cl.loadClass(clazz);
         } catch (ClassNotFoundException e) {
         }
-        return levelClass;
+        return configuratorClass;
     }
 }
diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/Log4JConfigTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/Log4JConfigTest.java
@@ -17,19 +17,16 @@
 package org.apache.activemq.broker.jmx;
 
 import java.util.List;
-
 import javax.jms.ConnectionFactory;
 import javax.management.MBeanServer;
 import javax.management.MBeanServerInvocationHandler;
 import javax.management.MalformedObjectNameException;
 import javax.management.ObjectName;
-
 import org.apache.activemq.ActiveMQConnectionFactory;
 import org.apache.activemq.EmbeddedBrokerTestSupport;
 import org.apache.activemq.broker.BrokerService;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.core.Logger;
 import org.junit.Test;
 import org.slf4j.LoggerFactory;
 
@@ -136,6 +133,9 @@ public void testLog4JConfigViewSetLevel() throws Throwable {
         level = log4jConfigView.getLogLevel(BROKER_LOGGER);
         assertNotNull(level);
         assertEquals("INFO", level);
+
+        List<String> loggers = log4jConfigView.getLoggers();
+        assertEquals(2, loggers.size());
     }
 
     @Test

diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/security/SecurityJMXTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/security/SecurityJMXTest.java
@@ -48,6 +48,10 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.core.Appender;
 import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+import org.apache.logging.log4j.core.config.Property;
+import org.apache.logging.log4j.core.filter.AbstractFilter;
+import org.apache.logging.log4j.core.layout.MessageLayout;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -71,7 +75,7 @@ public void testDeniedViaStompNoStackTrace() throws Exception {
         final AtomicBoolean gotExpected = new AtomicBoolean(false);
         final AtomicReference<Object> stackTrace = new AtomicReference<Object>();
 
-        final Appender appender = new DefaultTestAppender() {
+        final Appender appender = new AbstractAppender("testAppender", new AbstractFilter() {}, new MessageLayout(), false, new Property[0]) {
             @Override
             public void append(LogEvent event) {
                 String message =  event.getMessage().getFormattedMessage();
@@ -82,6 +86,7 @@ public void append(LogEvent event) {
             }
         };
 
+        appender.start();
         org.apache.logging.log4j.core.Logger toVerify = (org.apache.logging.log4j.core.Logger)LogManager.getLogger(TransportConnection.class.getName() + ".Service");
         toVerify.addAppender(appender);
 

diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/store/SharedFileLockerTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/store/SharedFileLockerTest.java
@@ -193,7 +193,7 @@ public void verifyLockAcquireWaitsForLockDrop() throws Exception {
         final AtomicInteger logCounts = new AtomicInteger(0);
      // start new
         final var logger = org.apache.logging.log4j.core.Logger.class.cast(LogManager.getLogger(SharedFileLocker.class));
-        final var appender = new AbstractAppender("testAppender", new AbstractFilter() {}, new MessageLayout(), false, new Property[0]) {
+        final var appender = new AbstractAppender("testAppender2", new AbstractFilter() {}, new MessageLayout(), false, new Property[0]) {
             @Override
             public void append(LogEvent event) {
                 logCounts.incrementAndGet();

diff --git a/...it-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBPersistenceAdapterTest.java b/...it-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBPersistenceAdapterTest.java
@@ -28,7 +28,11 @@
 import java.io.File;
 import java.io.IOException;
 import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
 import org.apache.logging.log4j.core.config.Configurator;
+import org.apache.logging.log4j.core.config.Property;
+import org.apache.logging.log4j.core.filter.AbstractFilter;
+import org.apache.logging.log4j.core.layout.MessageLayout;
 
 /**
  * 
@@ -52,7 +56,7 @@ public void testNoReplayOnStop() throws Exception {
         final AtomicBoolean gotSomeReplay = new AtomicBoolean(Boolean.FALSE);
         final AtomicBoolean trappedLogMessages = new AtomicBoolean(Boolean.FALSE);
 
-        Appender appender = new DefaultTestAppender() {
+        final var appender = new AbstractAppender("testAppender", new AbstractFilter() {}, new MessageLayout(), false, new Property[0]) {
             @Override
             public void append(LogEvent event) {
                 trappedLogMessages.set(true);
@@ -66,7 +70,7 @@ public void append(LogEvent event) {
         appender.start();
 
         try {
-            Configurator.setLevel(MessageDatabase.class.getName(), Level.INFO);
+            Configurator.setLevel(MessageDatabase.class.getName(), Level.DEBUG);
             ((org.apache.logging.log4j.core.Logger)LogManager.getLogger(MessageDatabase.class)).addAppender(appender);
 
             brokerService = new BrokerService();

diff --git a/...ts/src/test/java/org/apache/activemq/transport/tcp/TcpTransportCloseSocketNoWarnTest.java b/...ts/src/test/java/org/apache/activemq/transport/tcp/TcpTransportCloseSocketNoWarnTest.java
@@ -20,12 +20,14 @@
 import org.apache.activemq.broker.TransportConnection;
 import org.apache.activemq.broker.TransportConnector;
 
+import org.apache.activemq.store.kahadb.MessageDatabase;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.core.Appender;
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.Logger;
 import org.apache.logging.log4j.core.appender.AbstractAppender;
+import org.apache.logging.log4j.core.config.Configurator;
 import org.apache.logging.log4j.core.config.Property;
 import org.apache.logging.log4j.core.filter.AbstractFilter;
 import org.apache.logging.log4j.core.layout.MessageLayout;
@@ -82,6 +84,7 @@ public void append(LogEvent event) {
 
     @Before
     public void before() throws Exception {
+        gotExceptionInLog.set(false);
         brokerService = new BrokerService();
         brokerService.setPersistent(false);
         brokerService.setUseJmx(false);
@@ -90,7 +93,7 @@ public void before() throws Exception {
         rootLogger.get().addAppender(appender, Level.DEBUG, new AbstractFilter() {});
         rootLogger.addAppender(appender);
 
-        org.apache.logging.log4j.core.Logger.class.cast(LogManager.getLogger(TransportConnection.class.getName() + ".Transport")).setLevel(Level.WARN);
+        Configurator.setLevel(TransportConnection.class.getName() + ".Transport", Level.WARN);
     }
 
     @After

diff --git a/.../test/java/org/apache/activemq/transport/tcp/TcpTransportInactiveDuringHandshakeTest.java b/.../test/java/org/apache/activemq/transport/tcp/TcpTransportInactiveDuringHandshakeTest.java
@@ -80,7 +80,8 @@ public void before() throws Exception {
         appender = new AbstractAppender("testAppender", new AbstractFilter() {}, new MessageLayout(), false, new Property[0]) {
             @Override
             public void append(LogEvent event) {
-                if (Level.WARN.equals(event.getLevel()) && event.getMessage().getFormattedMessage().contains("InactivityIOException")) {
+                if (Level.WARN.equals(event.getLevel()) &&
+                    event.getMessage().getFormattedMessage().contains("CONNECT frame not received with in connectionTimeout")) {
                     inactivityMonitorFired.countDown();
                 }
             }