This is an automated email from the ASF dual-hosted git repository. vy pushed a commit to branch recycler-api-3.x in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit d96b37020ba6cd52fe1283ae8864a93685e943f9 Author: Piotr P. Karwasz <[email protected]> AuthorDate: Fri Jan 13 06:15:27 2023 +0100 [LOG4J2-3647] Add global filter support to `LogBuilder` `LogBuilder` suppliers in the `Logger` class always return a no-op builder if the log message level is higher than the configured level. This way global filters are always skipped. This patch always returns a functional builder if we detect the presence of a global filter. --- .../logging/log4j/internal/DefaultLogBuilder.java | 21 +- .../apache/logging/log4j/spi/AbstractLogger.java | 43 ++-- .../java/org/apache/logging/log4j/core/Logger.java | 10 + .../org/apache/logging/slf4j/SLF4JLogBuilder.java | 229 +++++---------------- .../java/org/apache/logging/slf4j/SLF4JLogger.java | 32 ++- 5 files changed, 126 insertions(+), 209 deletions(-) diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/DefaultLogBuilder.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/DefaultLogBuilder.java index 184f0fc3c3..2690c6ee72 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/internal/DefaultLogBuilder.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/internal/DefaultLogBuilder.java @@ -43,7 +43,7 @@ public class DefaultLogBuilder implements BridgeAware, LogBuilder { private static final Logger LOGGER = StatusLogger.getLogger(); private static final Message EMPTY_MESSAGE = new SimpleMessage(Strings.EMPTY); - private final ExtendedLogger logger; + private ExtendedLogger logger; private Level level; private Marker marker; private Throwable throwable; @@ -51,19 +51,32 @@ public class DefaultLogBuilder implements BridgeAware, LogBuilder { private final long threadId; private String fqcn = FQCN; - public DefaultLogBuilder(final ExtendedLogger logger) { + public DefaultLogBuilder(final ExtendedLogger logger, final Level level) { this.logger = logger; + this.level = level; this.threadId = Thread.currentThread().getId(); } + public DefaultLogBuilder() { + this(null, null); + } + @Override public void setEntryPoint(String fqcn) { this.fqcn = fqcn; } - @InternalApi - public LogBuilder atLevel(final Level level) { + /** + * This method should be considered internal. It is used to reset the LogBuilder for a new log message. + * @param level The logging level for this event. + * @return This LogBuilder instance. + */ + public LogBuilder reset(ExtendedLogger logger, Level level) { + this.logger = logger; this.level = level; + this.marker = null; + this.throwable = null; + this.location = null; return this; } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java index 6a6d4368a9..c9ed863ae0 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java @@ -16,24 +16,11 @@ */ package org.apache.logging.log4j.spi; -import org.apache.logging.log4j.Level; -import org.apache.logging.log4j.LogBuilder; -import org.apache.logging.log4j.LoggingException; -import org.apache.logging.log4j.Marker; -import org.apache.logging.log4j.MarkerManager; +import org.apache.logging.log4j.*; import org.apache.logging.log4j.internal.DefaultLogBuilder; -import org.apache.logging.log4j.message.EntryMessage; -import org.apache.logging.log4j.message.FlowMessageFactory; -import org.apache.logging.log4j.message.Message; -import org.apache.logging.log4j.message.MessageFactory; -import org.apache.logging.log4j.message.StringFormattedMessage; +import org.apache.logging.log4j.message.*; import org.apache.logging.log4j.status.StatusLogger; -import org.apache.logging.log4j.util.Cast; -import org.apache.logging.log4j.util.LambdaUtil; -import org.apache.logging.log4j.util.MessageSupplier; -import org.apache.logging.log4j.util.PerformanceSensitive; -import org.apache.logging.log4j.util.StackLocatorUtil; -import org.apache.logging.log4j.util.Supplier; +import org.apache.logging.log4j.util.*; /** * Base implementation of a Logger. It is highly recommended that any Logger implementation extend this class. @@ -83,8 +70,7 @@ public abstract class AbstractLogger implements ExtendedLogger { private final FlowMessageFactory flowMessageFactory; private static final ThreadLocal<int[]> recursionDepthHolder = new ThreadLocal<>(); // LOG4J2-1518, LOG4J2-2031 protected final Recycler<LogBuilder> recycler = LoggingSystem.getRecyclerFactory() - .create(() -> new DefaultLogBuilder(this)); - + .create(() -> new DefaultLogBuilder(this, null)); /** * Creates a new logger named after this class (or subclass). @@ -1974,7 +1960,7 @@ public abstract class AbstractLogger implements ExtendedLogger { logMessageTrackRecursion(fqcn, level, marker, msg, throwable); } finally { // LOG4J2-1583 prevent scrambled logs when logging calls are nested (logging in toString()) - messageFactory.recycle(msg); + ReusableMessageFactory.release(msg); } } @@ -2739,8 +2725,7 @@ public abstract class AbstractLogger implements ExtendedLogger { */ @Override public LogBuilder always() { - final DefaultLogBuilder builder = (DefaultLogBuilder) recycler.acquire(); - return builder.atLevel(Level.OFF); + return getLogBuilder(Level.OFF); } /** @@ -2751,11 +2736,19 @@ public abstract class AbstractLogger implements ExtendedLogger { @Override public LogBuilder atLevel(final Level level) { if (isEnabled(level)) { - final DefaultLogBuilder builder = (DefaultLogBuilder) recycler.acquire(); - return builder.atLevel(level); - } else { - return LogBuilder.NOOP; + return getLogBuilder(level); } + return LogBuilder.NOOP; + } + + /** + * Returns a log builder that logs at the specified level. + * + * @since 2.20.0 + */ + protected LogBuilder getLogBuilder(Level level) { + DefaultLogBuilder builder = (DefaultLogBuilder) recycler.acquire(); + return builder.reset(this, level); } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java index 8578bbd92e..9da3d45454 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java @@ -346,6 +346,16 @@ public class Logger extends AbstractLogger implements Supplier<LoggerConfig> { privateConfig.config.setLoggerAdditive(this, additive); } + @Override + public LogBuilder atLevel(Level level) { + // A global filter might accept messages less specific than level. + // Therefore we return always a functional builder. + if (privateConfig.hasFilter()) { + return getLogBuilder(level); + } + return super.atLevel(level); + } + /** * Associates this Logger with a new Configuration. This method is not * exposed through the public API. diff --git a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogBuilder.java b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogBuilder.java index a910694d68..947391739f 100644 --- a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogBuilder.java +++ b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogBuilder.java @@ -17,233 +17,112 @@ package org.apache.logging.slf4j; import org.apache.logging.log4j.Level; -import org.apache.logging.log4j.LogBuilder; -import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.Marker; +import org.apache.logging.log4j.internal.DefaultLogBuilder; import org.apache.logging.log4j.message.Message; -import org.apache.logging.log4j.message.SimpleMessage; import org.apache.logging.log4j.spi.ExtendedLogger; -import org.apache.logging.log4j.status.StatusLogger; -import org.apache.logging.log4j.util.LambdaUtil; -import org.apache.logging.log4j.util.StackLocatorUtil; -import org.apache.logging.log4j.util.Supplier; -public class SLF4JLogBuilder implements LogBuilder { +public class SLF4JLogBuilder extends DefaultLogBuilder { - private static Message EMPTY_MESSAGE = new SimpleMessage(""); - private static final String FQCN = SLF4JLogBuilder.class.getName(); - private static final Logger LOGGER = StatusLogger.getLogger(); - - private ExtendedLogger logger; - private Level level; - private Marker marker; - private Throwable throwable; - private volatile boolean inUse; - private final long threadId; - - public SLF4JLogBuilder(SLF4JLogger logger, Level level) { - this.logger = logger; - this.level = level; - this.threadId = Thread.currentThread().getId(); - this.inUse = level != null; + public SLF4JLogBuilder(ExtendedLogger logger, Level level) { + super(logger, level); } public SLF4JLogBuilder() { - this(null, null); - } - - public LogBuilder reset(SLF4JLogger logger, Level level) { - this.logger = logger; - this.level = level; - this.marker = null; - this.throwable = null; - this.inUse = true; - return this; - } - - public boolean isInUse() { - return this.inUse; - } - - private boolean isValid() { - if (!inUse) { - LOGGER.warn("Attempt to reuse LogBuilder was ignored. {}", StackLocatorUtil.getCallerClass(2)); - return false; - } - if (this.threadId != Thread.currentThread().getId()) { - LOGGER.warn("LogBuilder can only be used on the owning thread. {}", StackLocatorUtil.getCallerClass(2)); - return false; - } - return true; - } - - private void logMessage(Message message) { - try { - logger.logMessage(FQCN, level, marker, message, throwable); - } finally { - inUse = false; - } - } - - @Override - public LogBuilder withMarker(Marker marker) { - this.marker = marker; - return this; - } - - @Override - public LogBuilder withThrowable(Throwable throwable) { - this.throwable = throwable; - return this; - } - - @Override - public LogBuilder withLocation() { - LOGGER.info("Call to withLocation() ignored since SLF4J does not support setting location information."); - return this; - } - - @Override - public LogBuilder withLocation(StackTraceElement location) { - return withLocation(); - } - - @Override - public void log(CharSequence message) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message)); - } - } - - @Override - public void log(String message) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message)); - } - } - - @Override - public void log(String message, Object... params) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message, params)); - } + super(); } @Override - public void log(String message, Supplier<?>... params) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message, LambdaUtil.getAll(params))); - } - } - - @Override - public void log(Message message) { - if (isValid()) { - logMessage(message); - } + protected boolean isEnabled(Message message) { + // SLF4J will check again later + return true; } @Override - public void log(Supplier<Message> messageSupplier) { - if (isValid()) { - logMessage(messageSupplier.get()); - } + protected boolean isEnabled(CharSequence message) { + // SLF4J will check again later + return true; } @Override - public Message logAndGet(Supplier<Message> messageSupplier) { - Message message = null; - if (isValid()) { - logMessage(message = messageSupplier.get()); - } - return message; + protected boolean isEnabled(String message) { + // SLF4J will check again later + return true; } @Override - public void log(Object message) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message)); - } + protected boolean isEnabled(String message, Object... params) { + // SLF4J will check again later + return true; } @Override - public void log(String message, Object p0) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message, p0)); - } + protected boolean isEnabled(Object message) { + // SLF4J will check again later + return true; } @Override - public void log(String message, Object p0, Object p1) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message, p0, p1)); - } + protected boolean isEnabled(String message, Object p0) { + // SLF4J will check again later + return true; } @Override - public void log(String message, Object p0, Object p1, Object p2) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message, p0, p1, p2)); - } + protected boolean isEnabled(String message, Object p0, Object p1) { + // SLF4J will check again later + return true; } @Override - public void log(String message, Object p0, Object p1, Object p2, Object p3) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message, p0, p1, p2, p3)); - } + protected boolean isEnabled(String message, Object p0, Object p1, Object p2) { + // SLF4J will check again later + return true; } @Override - public void log(String message, Object p0, Object p1, Object p2, Object p3, Object p4) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message, p0, p1, p2, p3, p4)); - } + protected boolean isEnabled(String message, Object p0, Object p1, Object p2, Object p3) { + // SLF4J will check again later + return true; } @Override - public void log(String message, Object p0, Object p1, Object p2, Object p3, Object p4, Object p5) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message, p0, p1, p2, p3, p4, p5)); - } + protected boolean isEnabled(String message, Object p0, Object p1, Object p2, Object p3, Object p4) { + // SLF4J will check again later + return true; } @Override - public void log(String message, Object p0, Object p1, Object p2, Object p3, Object p4, Object p5, Object p6) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message, p0, p1, p2, p3, p4, p5, p6)); - } + protected boolean isEnabled(String message, Object p0, Object p1, Object p2, Object p3, Object p4, Object p5) { + // SLF4J will check again later + return true; } @Override - public void log(String message, Object p0, Object p1, Object p2, Object p3, Object p4, Object p5, Object p6, - Object p7) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message, p0, p1, p2, p3, p4, p5, p6, p7)); - } + protected boolean isEnabled(String message, Object p0, Object p1, Object p2, Object p3, Object p4, Object p5, + Object p6) { + // SLF4J will check again later + return true; } @Override - public void log(String message, Object p0, Object p1, Object p2, Object p3, Object p4, Object p5, Object p6, - Object p7, Object p8) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message, p0, p1, p2, p3, p4, p5, p6, p7, p8)); - } + protected boolean isEnabled(String message, Object p0, Object p1, Object p2, Object p3, Object p4, Object p5, + Object p6, Object p7) { + // SLF4J will check again later + return true; } @Override - public void log(String message, Object p0, Object p1, Object p2, Object p3, Object p4, Object p5, Object p6, - Object p7, Object p8, Object p9) { - if (isValid()) { - logMessage(logger.getMessageFactory().newMessage(message, p0, p1, p2, p3, p4, p5, p6, p7, p8, p9)); - } + protected boolean isEnabled(String message, Object p0, Object p1, Object p2, Object p3, Object p4, Object p5, + Object p6, Object p7, Object p8) { + // SLF4J will check again later + return true; } @Override - public void log() { - if (isValid()) { - logMessage(EMPTY_MESSAGE); - } + protected boolean isEnabled(String message, Object p0, Object p1, Object p2, Object p3, Object p4, Object p5, + Object p6, Object p7, Object p8, Object p9) { + // SLF4J will check again later + return true; } } diff --git a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogger.java b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogger.java index c040666555..4034b1ba3f 100644 --- a/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogger.java +++ b/log4j-to-slf4j/src/main/java/org/apache/logging/slf4j/SLF4JLogger.java @@ -23,6 +23,9 @@ import org.apache.logging.log4j.message.LoggerNameAwareMessage; import org.apache.logging.log4j.message.Message; import org.apache.logging.log4j.message.MessageFactory; import org.apache.logging.log4j.spi.AbstractLogger; +import org.apache.logging.log4j.spi.LoggingSystem; +import org.apache.logging.log4j.spi.Recycler; +import org.slf4j.LoggerFactory; import org.slf4j.MarkerFactory; import org.slf4j.spi.LocationAwareLogger; @@ -31,6 +34,15 @@ import org.slf4j.spi.LocationAwareLogger; */ public class SLF4JLogger extends AbstractLogger { + /** + * Logback supports turbo filters, that can override the logger's level. + * Therefore we can never return a no-op builder. + */ + private static final boolean LAZY_LEVEL_CHECK = "ch.qos.logback.classic.LoggerContext" + .equals(LoggerFactory.getILoggerFactory().getClass().getName()); + private static final Recycler<SLF4JLogBuilder> logBuilderRecycler = + LoggingSystem.getRecyclerFactory().create(SLF4JLogBuilder::new); + private final org.slf4j.Logger logger; private final LocationAwareLogger locationAwareLogger; @@ -259,11 +271,6 @@ public class SLF4JLogger extends AbstractLogger { } } - @Override - public LogBuilder always() { - return atLevel(Level.OFF); - } - @Override public LogBuilder atTrace() { return atLevel(Level.TRACE); @@ -294,4 +301,19 @@ public class SLF4JLogger extends AbstractLogger { return atLevel(Level.TRACE); } + @Override + protected LogBuilder getLogBuilder(Level level) { + SLF4JLogBuilder builder = logBuilderRecycler.acquire(); + return builder.reset(this, level); + } + + @Override + public LogBuilder atLevel(Level level) { + // TODO: wrap SLF4J 2.x LoggingEventBuilder + if (LAZY_LEVEL_CHECK) { + return getLogBuilder(level); + } + return super.atLevel(level); + } + }
