LOG4J2-3083 Fix slf4j calling class lookup using both accessors

log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocator.java

@@ -19,6 +19,7 @@
 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,22 +40,33 @@ private StackLocator() {
     }
 
     @PerformanceSensitive
-    public Class<?> getCallerClass(final String fqcn) {
-        return getCallerClass(fqcn, "");
+    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, final String pkg) {
-        return getCallerClass(fqcn, pkg, 0);
+    public Class<?> getCallerClass(final String fqcn) {
+        return getCallerClass(fqcn, "");
     }
 
     @PerformanceSensitive
-    public Class<?> getCallerClass(final String fqcn, final String pkg, final int skipDepth) {
+    public Class<?> getCallerClass(final String fqcn, final String pkg) {
         return walker.walk(s -> s
                 .dropWhile(f -> !f.getClassName().equals(fqcn))
                 .dropWhile(f -> f.getClassName().equals(fqcn))
                 .dropWhile(f -> !f.getClassName().startsWith(pkg))
-                .skip(skipDepth)
                 .findFirst())
                 .map(StackWalker.StackFrame::getDeclaringClass)
                 .orElse(null);

log4j-api/src/main/java/org/apache/logging/log4j/util/StackLocatorUtil.java

@@ -20,6 +20,7 @@
 
 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>
@@ -58,7 +59,11 @@ public static Class<?> getCallerClass(final String fqcn) {
     }
 
     /**
-     * Equivalent to {@link #getCallerClass(String, String, int)} with {@code skipDepth = 0}.
+     * Search for a calling class.
+     *
+     * @param fqcn Root class name whose caller to search for.
+     * @param pkg Package name prefix that must be matched after the {@code fqcn} has been found.
+     * @return The caller class that was matched, or null if one could not be located.
      */
     @PerformanceSensitive
     public static Class<?> getCallerClass(final String fqcn, final String pkg) {
@@ -68,14 +73,13 @@ public static Class<?> getCallerClass(final String fqcn, final String pkg) {
     /**
      * Search for a calling class.
      *
-     * @param fqcn Root class name whose caller to search for.
-     * @param pkg Package name prefix that must be matched after the {@code fqcn} has been found.
-     * @param skipDepth Number of stack frames to skip after the {@code fqcn} and {@code pkg} have been matched.
-     * @return The caller class that was matched, or null if one could not be located.
+     * @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 String fqcn, final String pkg, final int skipDepth) {
-        return stackLocator.getCallerClass(fqcn, pkg, skipDepth);
+    public static Class<?> getCallerClass(final Class<?> sentinelClass, final Predicate<Class<?>> callerPredicate) {
+        return stackLocator.getCallerClass(sentinelClass, callerPredicate);
     }
 
     // added for use in LoggerAdapter implementations mainly

log4j-slf4j-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java

@@ -20,18 +20,23 @@
 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 @@ protected Logger newLogger(final String name, final LoggerContext context) {
     @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);

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);
+        }
+    }
+}

log4j-slf4j18-impl/src/main/java/org/apache/logging/slf4j/Log4jLoggerFactory.java

@@ -20,25 +20,30 @@
 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 @@ protected Logger newLogger(final String name, final LoggerContext context) {
     @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;
     }

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);
+        }
+    }
+}

src/changes/changes.xml

@@ -217,6 +217,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>