From ae30dce4e24ce5e0467d2a3219627cbefef1f0ae Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon (cshannon)" Date: Thu, 3 Mar 2022 08:10:58 -0500 Subject: [PATCH] AMQ-8520: Log4j2 test fixes Also fixes Log4jConfigView --- .../activemq/broker/TransportConnection.java | 2 +- .../activemq/broker/jmx/Log4JConfigView.java | 96 ++++++++++--------- .../activemq/broker/jmx/Log4JConfigTest.java | 6 +- .../activemq/security/SecurityJMXTest.java | 7 +- .../activemq/store/SharedFileLockerTest.java | 2 +- .../kahadb/KahaDBPersistenceAdapterTest.java | 8 +- .../TcpTransportCloseSocketNoWarnTest.java | 5 +- ...pTransportInactiveDuringHandshakeTest.java | 3 +- 8 files changed, 73 insertions(+), 56 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 index f239df1dba..7533a714ed 100644 --- 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 class TransportConnection implements Connection, Task, CommandVisitor { 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 index b58d72ec25..cb601f7daf 100644 --- 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.MalformedURLException; 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 class Log4JConfigView implements Log4JConfigViewMBean { 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 class Log4JConfigView implements Log4JConfigViewMBean { 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 class Log4JConfigView implements Log4JConfigViewMBean { // 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 class Log4JConfigView implements Log4JConfigViewMBean { 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 list = new ArrayList(); - Enumeration loggers = (Enumeration)getCurrentLoggers.invoke(null, (Object[])null); + Map loggers = (Map)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 class Log4JConfigView implements Log4JConfigViewMBean { 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 class Log4JConfigView implements Log4JConfigViewMBean { 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); + 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); - } + // 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 @@ public class Log4JConfigView implements Log4JConfigViewMBean { } 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 index a5255a6486..1b37882454 100644 --- 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 class Log4JConfigTest extends EmbeddedBrokerTestSupport { level = log4jConfigView.getLogLevel(BROKER_LOGGER); assertNotNull(level); assertEquals("INFO", level); + + List 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 index 5bf176a755..756c50412a 100644 --- 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.activemq.util.DefaultTestAppender; 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 class SecurityJMXTest extends TestCase { final AtomicBoolean gotExpected = new AtomicBoolean(false); final AtomicReference stackTrace = new AtomicReference(); - 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 class SecurityJMXTest extends TestCase { } }; + 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 index 2ae8b5c1cd..0a2d1cb9bb 100644 --- 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 class SharedFileLockerTest { 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/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBPersistenceAdapterTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBPersistenceAdapterTest.java index 3e2b634093..c45ff80c82 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBPersistenceAdapterTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/store/kahadb/KahaDBPersistenceAdapterTest.java @@ -28,7 +28,11 @@ import org.apache.logging.log4j.core.LogEvent; 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 class KahaDBPersistenceAdapterTest extends PersistenceAdapterTestSupport 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 class KahaDBPersistenceAdapterTest extends PersistenceAdapterTestSupport 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/activemq-unit-tests/src/test/java/org/apache/activemq/transport/tcp/TcpTransportCloseSocketNoWarnTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/tcp/TcpTransportCloseSocketNoWarnTest.java index 3be00bd4d7..d552995623 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/transport/tcp/TcpTransportCloseSocketNoWarnTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/tcp/TcpTransportCloseSocketNoWarnTest.java @@ -20,12 +20,14 @@ import org.apache.activemq.broker.BrokerService; 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 class TcpTransportCloseSocketNoWarnTest { @Before public void before() throws Exception { + gotExceptionInLog.set(false); brokerService = new BrokerService(); brokerService.setPersistent(false); brokerService.setUseJmx(false); @@ -90,7 +93,7 @@ public class TcpTransportCloseSocketNoWarnTest { 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/activemq-unit-tests/src/test/java/org/apache/activemq/transport/tcp/TcpTransportInactiveDuringHandshakeTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/tcp/TcpTransportInactiveDuringHandshakeTest.java index 5c88fefa3a..959dd9ca41 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/transport/tcp/TcpTransportInactiveDuringHandshakeTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/transport/tcp/TcpTransportInactiveDuringHandshakeTest.java @@ -80,7 +80,8 @@ public class TcpTransportInactiveDuringHandshakeTest { 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(); } }