This is an automated email from the ASF dual-hosted git repository. ckozak pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 839dfb856cf46682062bd7f8746c0157a355aae4 Author: Carter Kozak <[email protected]> AuthorDate: Tue Aug 3 10:06:45 2021 -0400 LOG4J2-3083 Fix slf4j calling class lookup using both accessors `LoggerFactory.getLogger(name/class)` as well as `LoggerFactory.getILoggerFactory().getLogger(name)`. --- .../apache/logging/log4j/util/StackLocator.java | 18 +++++ .../logging/log4j/util/StackLocatorUtil.java | 13 ++++ .../apache/logging/slf4j/Log4jLoggerFactory.java | 12 ++- .../logging/other/pkg/LoggerContextAnchorTest.java | 91 ++++++++++++++++++++++ .../apache/logging/slf4j/Log4jLoggerFactory.java | 19 +++-- .../logging/other/pkg/LoggerContextAnchorTest.java | 91 ++++++++++++++++++++++ src/changes/changes.xml | 4 + 7 files changed, 238 insertions(+), 10 deletions(-) diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java index 86477de..3fb936a 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java @@ -19,6 +19,7 @@ package org.apache.logging.log4j.util; import java.util.List; import java.util.Stack; import java.util.stream.Collectors; +import java.util.function.Predicate; /** * <em>Consider this class private.</em> Determines the caller's class. @@ -39,6 +40,23 @@ public final class StackLocator { } @PerformanceSensitive + public Class<?> getCallerClass(final Class<?> sentinelClass, final Predicate<Class<?>> callerPredicate) { + if (sentinelClass == null) { + throw new IllegalArgumentException("sentinelClass cannot be null"); + } + if (callerPredicate == null) { + throw new IllegalArgumentException("callerPredicate cannot be null"); + } + return walker.walk(s -> s + .map(StackWalker.StackFrame::getDeclaringClass) + // Skip until the sentinel class is found + .dropWhile(clazz -> !sentinelClass.equals(clazz)) + // Skip until the predicate evaluates to true, also ignoring recurrences of the sentinel + .dropWhile(clazz -> sentinelClass.equals(clazz) || !callerPredicate.test(clazz)) + .findFirst().orElse(null)); + } + + @PerformanceSensitive public Class<?> getCallerClass(final String fqcn) { return getCallerClass(fqcn, ""); } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocatorUtil.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocatorUtil.java index 7fe2088..482ee33 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocatorUtil.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocatorUtil.java @@ -20,6 +20,7 @@ import org.apache.logging.log4j.status.StatusLogger; import java.util.NoSuchElementException; import java.util.Stack; +import java.util.function.Predicate; /** * <em>Consider this class private.</em> Provides various methods to determine the caller class. <h3>Background</h3> @@ -78,6 +79,18 @@ public final class StackLocatorUtil { return stackLocator.getCallerClass(fqcn, pkg, skipDepth); } + /** + * Search for a calling class. + * + * @param sentinelClass Sentinel class at which to begin searching + * @param callerPredicate Predicate checked after the sentinelClass is found + * @return the first matching class after <code>sentinelClass</code> is found. + */ + @PerformanceSensitive + public static Class<?> getCallerClass(final Class<?> sentinelClass, final Predicate<Class<?>> callerPredicate) { + return stackLocator.getCallerClass(sentinelClass, callerPredicate); + } + // added for use in LoggerAdapter implementations mainly @PerformanceSensitive public static Class<?> getCallerClass(final Class<?> anchor) { diff --git a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java index e5b0e58..d884e6f 100644 --- a/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java +++ b/log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java @@ -20,18 +20,23 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.LoggingException; import org.apache.logging.log4j.spi.AbstractLoggerAdapter; import org.apache.logging.log4j.spi.LoggerContext; +import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.StackLocatorUtil; import org.slf4j.ILoggerFactory; import org.slf4j.Logger; +import java.util.function.Predicate; + /** * Log4j implementation of SLF4J ILoggerFactory interface. */ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements ILoggerFactory { - private static final String FQCN = Log4jLoggerFactory.class.getName(); - private static final String PACKAGE = "org.slf4j"; + private static final StatusLogger LOGGER = StatusLogger.getLogger(); + private static final String SLF4J_PACKAGE = "org.slf4j"; private static final String TO_SLF4J_CONTEXT = "org.apache.logging.slf4j.SLF4JLoggerContext"; + private static final Predicate<Class<?>> CALLER_PREDICATE = clazz -> + !AbstractLoggerAdapter.class.equals(clazz) && !clazz.getName().startsWith(SLF4J_PACKAGE); @Override protected Logger newLogger(final String name, final LoggerContext context) { @@ -42,8 +47,9 @@ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements @Override protected LoggerContext getContext() { final Class<?> anchor = LogManager.getFactory().isClassLoaderDependent() - ? StackLocatorUtil.getCallerClass(FQCN, PACKAGE, 1) + ? StackLocatorUtil.getCallerClass(Log4jLoggerFactory.class, CALLER_PREDICATE) : null; + LOGGER.trace("Log4jLoggerFactory.getContext() found anchor {}", anchor); return anchor == null ? LogManager.getContext(false) : getContext(anchor); diff --git a/log4j-slf4j-impl/src/test/java/org/apache/logging/other/pkg/LoggerContextAnchorTest.java b/log4j-slf4j-impl/src/test/java/org/apache/logging/other/pkg/LoggerContextAnchorTest.java new file mode 100644 index 0000000..2b0fe91 --- /dev/null +++ b/log4j-slf4j-impl/src/test/java/org/apache/logging/other/pkg/LoggerContextAnchorTest.java @@ -0,0 +1,91 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache license, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the license for the specific language governing permissions and + * limitations under the license. + */ +package org.apache.logging.other.pkg; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.status.StatusData; +import org.apache.logging.log4j.status.StatusListener; +import org.apache.logging.log4j.status.StatusLogger; +import org.junit.Test; +import org.slf4j.LoggerFactory; + +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; + +import static org.junit.Assert.assertEquals; + +/** + * Test LoggerContext lookups by verifying the anchor class representing calling code. + */ +public class LoggerContextAnchorTest { + private static final String PREFIX = "Log4jLoggerFactory.getContext() found anchor class "; + + @Test + public void testLoggerFactoryLookupClass() { + String fqcn = getAnchorFqcn(() -> LoggerFactory.getLogger(LoggerContextAnchorTest.class)); + assertEquals(getClass().getName(), fqcn); + } + + @Test + public void testLoggerFactoryLookupString() { + String fqcn = getAnchorFqcn(() -> LoggerFactory.getLogger("custom.logger")); + assertEquals(getClass().getName(), fqcn); + } + + @Test + public void testLoggerFactoryGetILoggerFactoryLookup() { + String fqcn = getAnchorFqcn(() -> LoggerFactory.getILoggerFactory().getLogger("custom.logger")); + assertEquals(getClass().getName(), fqcn); + } + + private static String getAnchorFqcn(Runnable runnable) { + List<String> results = new CopyOnWriteArrayList<>(); + StatusListener listener = new StatusListener() { + @Override + public void log(StatusData data) { + String formattedMessage = data.getMessage().getFormattedMessage(); + if (formattedMessage.startsWith(PREFIX)) { + results.add(formattedMessage.substring(PREFIX.length())); + } + } + + @Override + public Level getStatusLevel() { + return Level.TRACE; + } + + @Override + public void close() { + // nop + } + }; + StatusLogger statusLogger = StatusLogger.getLogger(); + statusLogger.registerListener(listener); + try { + runnable.run(); + if (results.isEmpty()) { + throw new AssertionError("Failed to locate an anchor lookup status message"); + } + if (results.size() > 1) { + throw new AssertionError("Found multiple anchor lines: " + results); + } + return results.get(0); + } finally { + statusLogger.removeListener(listener); + } + } +} diff --git a/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java b/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java index bb2a2d8..2e3fb2f 100644 --- a/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java +++ b/log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java @@ -20,25 +20,30 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.LoggingException; import org.apache.logging.log4j.spi.AbstractLoggerAdapter; import org.apache.logging.log4j.spi.LoggerContext; +import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.StackLocatorUtil; import org.slf4j.ILoggerFactory; import org.slf4j.Logger; +import java.util.function.Predicate; + /** * Log4j implementation of SLF4J ILoggerFactory interface. */ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements ILoggerFactory { - private static final String FQCN = Log4jLoggerFactory.class.getName(); - private static final String PACKAGE = "org.slf4j"; - private final Log4jMarkerFactory markerFactory; + private static final StatusLogger LOGGER = StatusLogger.getLogger(); + private static final String SLF4J_PACKAGE = "org.slf4j"; + private static final Predicate<Class<?>> CALLER_PREDICATE = clazz -> + !AbstractLoggerAdapter.class.equals(clazz) && !clazz.getName().startsWith(SLF4J_PACKAGE); private static final String TO_SLF4J_CONTEXT = "org.apache.logging.slf4j.SLF4JLoggerContext"; - public Log4jLoggerFactory(Log4jMarkerFactory markerFactory) { + private final Log4jMarkerFactory markerFactory; + + public Log4jLoggerFactory(final Log4jMarkerFactory markerFactory) { this.markerFactory = markerFactory; } - @Override protected Logger newLogger(final String name, final LoggerContext context) { final String key = Logger.ROOT_LOGGER_NAME.equals(name) ? LogManager.ROOT_LOGGER_NAME : name; @@ -48,14 +53,14 @@ public class Log4jLoggerFactory extends AbstractLoggerAdapter<Logger> implements @Override protected LoggerContext getContext() { final Class<?> anchor = LogManager.getFactory().isClassLoaderDependent() - ? StackLocatorUtil.getCallerClass(FQCN, PACKAGE, 1) + ? StackLocatorUtil.getCallerClass(Log4jLoggerFactory.class, CALLER_PREDICATE) : null; + LOGGER.trace("Log4jLoggerFactory.getContext() found anchor {}", anchor); return anchor == null ? LogManager.getContext(false) : getContext(anchor); } - Log4jMarkerFactory getMarkerFactory() { return markerFactory; } diff --git a/log4j-slf4j18-impl/src/test/java/org/apache/logging/other/pkg/LoggerContextAnchorTest.java b/log4j-slf4j18-impl/src/test/java/org/apache/logging/other/pkg/LoggerContextAnchorTest.java new file mode 100644 index 0000000..2b0fe91 --- /dev/null +++ b/log4j-slf4j18-impl/src/test/java/org/apache/logging/other/pkg/LoggerContextAnchorTest.java @@ -0,0 +1,91 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache license, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the license for the specific language governing permissions and + * limitations under the license. + */ +package org.apache.logging.other.pkg; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.status.StatusData; +import org.apache.logging.log4j.status.StatusListener; +import org.apache.logging.log4j.status.StatusLogger; +import org.junit.Test; +import org.slf4j.LoggerFactory; + +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; + +import static org.junit.Assert.assertEquals; + +/** + * Test LoggerContext lookups by verifying the anchor class representing calling code. + */ +public class LoggerContextAnchorTest { + private static final String PREFIX = "Log4jLoggerFactory.getContext() found anchor class "; + + @Test + public void testLoggerFactoryLookupClass() { + String fqcn = getAnchorFqcn(() -> LoggerFactory.getLogger(LoggerContextAnchorTest.class)); + assertEquals(getClass().getName(), fqcn); + } + + @Test + public void testLoggerFactoryLookupString() { + String fqcn = getAnchorFqcn(() -> LoggerFactory.getLogger("custom.logger")); + assertEquals(getClass().getName(), fqcn); + } + + @Test + public void testLoggerFactoryGetILoggerFactoryLookup() { + String fqcn = getAnchorFqcn(() -> LoggerFactory.getILoggerFactory().getLogger("custom.logger")); + assertEquals(getClass().getName(), fqcn); + } + + private static String getAnchorFqcn(Runnable runnable) { + List<String> results = new CopyOnWriteArrayList<>(); + StatusListener listener = new StatusListener() { + @Override + public void log(StatusData data) { + String formattedMessage = data.getMessage().getFormattedMessage(); + if (formattedMessage.startsWith(PREFIX)) { + results.add(formattedMessage.substring(PREFIX.length())); + } + } + + @Override + public Level getStatusLevel() { + return Level.TRACE; + } + + @Override + public void close() { + // nop + } + }; + StatusLogger statusLogger = StatusLogger.getLogger(); + statusLogger.registerListener(listener); + try { + runnable.run(); + if (results.isEmpty()) { + throw new AssertionError("Failed to locate an anchor lookup status message"); + } + if (results.size() > 1) { + throw new AssertionError("Found multiple anchor lines: " + results); + } + return results.get(0); + } finally { + statusLogger.removeListener(listener); + } + } +} diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 9415cab..a848ede 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -220,6 +220,10 @@ Allow a PatternSelector to be specified on GelfLayout. </action> <!-- FIXES --> + <action issue="LOG4J2-3083" dev="ckozak" type="fix"> + log4j-slf4j-impl and log4j-slf4j18-impl correctly detect the calling class using both LoggerFactory.getLogger + methods as well as LoggerFactory.getILoggerFactory().getLogger. + </action> <action issue="LOG4J2-2816" dev="vy" type="fix" due-to="Jacob Shields"> Handle Disruptor event translation exceptions. </action>
