ppkarwasz commented on code in PR #2249: URL: https://github.com/apache/logging-log4j2/pull/2249#discussion_r1467374859
########## log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java: ########## @@ -16,311 +16,567 @@ */ package org.apache.logging.log4j.status; -import java.io.Closeable; +import static java.util.Objects.requireNonNull; + +import edu.umd.cs.findbugs.annotations.Nullable; import java.io.IOException; +import java.io.InputStream; +import java.io.PrintStream; +import java.net.URL; +import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; import java.util.List; +import java.util.Properties; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReadWriteLock; -import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Marker; import org.apache.logging.log4j.message.Message; import org.apache.logging.log4j.message.MessageFactory; import org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory; -import org.apache.logging.log4j.simple.SimpleLogger; -import org.apache.logging.log4j.simple.SimpleLoggerContext; import org.apache.logging.log4j.spi.AbstractLogger; import org.apache.logging.log4j.util.Constants; -import org.apache.logging.log4j.util.PropertiesUtil; /** - * Records events that occur in the logging system. By default, only error messages are logged to {@link System#err}. - * Normally, the Log4j StatusLogger is configured via the root {@code <Configuration status="LEVEL"/>} node in a Log4j - * configuration file. However, this can be overridden via a system property named - * {@value #DEFAULT_STATUS_LISTENER_LEVEL} and will work with any Log4j provider. - * - * @see SimpleLogger - * @see SimpleLoggerContext + * Records events that occur in the logging system. + * {@link StatusLogger} is expected to be a standalone, self-sufficient component that the logging system can rely on for low-level logging purposes. + * <h3>Listeners</h3> + * <p> + * Each recorded event will first get buffered and used to notify the registered {@link StatusListener}s. + * Listener registry is always initialized with a <em>default listener</em>, which is a {@link StatusConsoleListener}. + * </p> + * <p> + * You can programmatically register listeners using {@link #registerListener(StatusListener)} method. + * </p> + * <h3>Configuration</h3> + * <p> + * The {@code StatusLogger} can be configured in following ways: + * </p> + * <ol> + * <li>Passing system properties to the Java process (e.g., {@code -Dlog4j2.StatusLogger.level=INFO})</li> + * <li>Providing properties in a {@value StatusLogger#PROPERTIES_FILE_NAME} file in the classpath</li> + * <li>Using Log4j configuration (i.e., {@code <Configuration status="WARN" dest="out">} in a {@code log4j2.xml} in the classpath)</li> + * </ol> + * <p> + * It is crucial to understand that there is a time between the first {@code StatusLogger} access and a configuration file (e.g., {@code log4j2.xml}) read. + * Consider the following example: + * </p> + * <ol> + * <li>The default level is {@code ERROR}</li> + * <li>You have <Configuration status="WARN">} in your {@code log4j2.xml}</li> + * <li>Until your {@code log4j2.xml} configuration is read, the effective level will be {@code ERROR}</li> + * <li>Once your {@code log4j2.xml} configuration is read, the effective level will be {@code WARN} as you configured</li> + * </ol> + * <p> + * Hence, unless you use either system properties or {@value StatusLogger#PROPERTIES_FILE_NAME} file in the classpath, there is a time window that only the defaults will be effective. + * </p> + * <p> + * {@code StatusLogger} is designed as a singleton class accessed statically. + * If you are running an application containing multiple Log4j configurations (e.g., in a servlet environment with multiple containers) and you happen to have differing {@code StatusLogger} configurations (e.g, one {@code log4j2.xml} containing {@code <Configuration status="ERROR">} while the other {@code <Configuration status="INFO">}), the last loaded configuration will be effective one. + * </p> + * <h3>Debug mode</h3> + * <p> + * When the {@value Constants#LOG4J2_DEBUG} system property is present, any level-related filtering will be skipped and all events will be notified to listeners. + * </p> */ -public final class StatusLogger extends AbstractLogger { +public class StatusLogger extends AbstractLogger { + + private static final long serialVersionUID = 2L; + + /** + * The name of the system property that enables debug mode in its presence. + * <p> + * This is a local clone of {@link Constants#LOG4J2_DEBUG}. + * The cloning is necessary to avoid cyclic initialization. + * </p> + */ + public static final String DEBUG_PROPERTY_NAME = "log4j2.debug"; + + /** + * The name of the system property that can be configured with the maximum number of events buffered. + * <p> + * Once the limit is reached, older entries will be removed as new entries are added. + * </p> + * + * @since 2.23.0 + */ + public static final String BUFFER_CAPACITY_PROPERTY_NAME = "log4j2.status.entries"; + + /** + * The default value of the {@link #BUFFER_CAPACITY_PROPERTY_NAME} system property: {@code 0}. + * + * @since 2.23.0 + */ + public static final int BUFFER_CAPACITY_DEFAULT_VALUE = 0; /** - * System property that can be configured with the number of entries in the queue. Once the limit is reached older - * entries will be removed as new entries are added. + * The name of the system property that can be configured with the maximum number of events buffered. + * <p> + * Once the limit is reached, older entries will be removed as new entries are added. + * </p> + * + * @deprecated Use {@link #BUFFER_CAPACITY_PROPERTY_NAME} instead. + */ + @Deprecated + public static final String MAX_STATUS_ENTRIES = BUFFER_CAPACITY_PROPERTY_NAME; + + /** + * The name of the system property that can be configured with the {@link Level} name to use as the default listener level. + * <p> + * The listener registry is initialized with a default listener. + * This default listener will accept entries filtered by the level provided in this configuration. + * </p> + * + * @since 2.23.0 + */ + public static final String DEFAULT_LISTENER_LEVEL_PROPERTY_NAME = "log4j2.StatusLogger.level"; + + /** + * The default value of the {@link #DEFAULT_LISTENER_LEVEL_PROPERTY_NAME} system property: {@code ERROR}. + * + * @since 2.23.0 + */ + public static final Level DEFAULT_LISTENER_LEVEL_DEFAULT_VALUE = Level.ERROR; + + /** + * The name of the system property that can be configured with the {@link Level} name to use as the default listener level. + * <p> + * The listener registry is initialized with a default listener. + * This default listener will accept entries filtered by the level provided in this configuration. + * </p> + * + * @since 2.8 + * @deprecated Use {@link #DEFAULT_LISTENER_LEVEL_PROPERTY_NAME} instead. */ - public static final String MAX_STATUS_ENTRIES = "log4j2.status.entries"; + @Deprecated + public static final String DEFAULT_STATUS_LISTENER_LEVEL = DEFAULT_LISTENER_LEVEL_PROPERTY_NAME; /** - * System property that can be configured with the {@link Level} name to use as the default level for - * {@link StatusListener}s. + * The name of the system property that can be configured with a {@link java.time.format.DateTimeFormatter} pattern that will be passed to the default listener. + * + * @since 2.23.0 */ - public static final String DEFAULT_STATUS_LISTENER_LEVEL = "log4j2.StatusLogger.level"; + public static final String INSTANT_FORMAT_PROPERTY_NAME = "log4j2.StatusLogger.DateFormat"; /** - * System property that can be configured with a date-time format string to use as the format for timestamps - * in the status logger output. See {@link java.text.SimpleDateFormat} for supported formats. + * The name of the system property that can be configured with a {@link java.time.format.DateTimeFormatter} pattern that will be passed to the default listener. + * * @since 2.11.0 + * @deprecated Use {@link #INSTANT_FORMAT_PROPERTY_NAME} instead. */ - public static final String STATUS_DATE_FORMAT = "log4j2.StatusLogger.DateFormat"; + @Deprecated + public static final String STATUS_DATE_FORMAT = INSTANT_FORMAT_PROPERTY_NAME; - private static final long serialVersionUID = 2L; + /** + * The name of the file to be searched in the classpath to read properties from. + * + * @since 2.23.0 + */ + public static final String PROPERTIES_FILE_NAME = "log4j2.StatusLogger.properties"; - private static final String NOT_AVAIL = "?"; + /** + * Holder for user-provided {@link StatusLogger} configurations. + * + * @since 2.23.0 + */ + public static final class Config { + + private static final Config INSTANCE = new Config(); + + private final boolean debugEnabled; + + private final int bufferCapacity; + + private final Level defaultListenerLevel; + + @Nullable + private final DateTimeFormatter instantFormatter; + + /** + * Constructs an instance using the given properties. + * <b>Users should not create new instances, but use {@link #getInstance()} instead</b>! + * + * @param debugEnabled the value of the {@value DEBUG_PROPERTY_NAME} property + * @param bufferCapacity the value of the {@value BUFFER_CAPACITY_PROPERTY_NAME} property + * @param defaultListenerLevel the value of the {@value DEFAULT_LISTENER_LEVEL_PROPERTY_NAME} property + * @param instantFormatter the value of the {@value INSTANT_FORMAT_PROPERTY_NAME} property + */ + public Config( + boolean debugEnabled, + int bufferCapacity, + Level defaultListenerLevel, + @Nullable DateTimeFormatter instantFormatter) { + this.debugEnabled = debugEnabled; + if (bufferCapacity < 0) { + throw new IllegalArgumentException( + "was expecting a positive `bufferCapacity`, found: " + bufferCapacity); + } + this.bufferCapacity = bufferCapacity; + this.defaultListenerLevel = requireNonNull(defaultListenerLevel, "defaultListenerLevel"); + this.instantFormatter = requireNonNull(instantFormatter, "instantFormatter"); + } - static final PropertiesUtil PROPS = new PropertiesUtil("log4j2.StatusLogger.properties"); + /** + * Constructs an instance using either system properties or a property file (i.e., {@value Config#PROPERTIES_FILE_NAME}) in the classpath, if available. + * <b>Users should not create new instances, but use {@link #getInstance()} instead</b>! + */ + public Config() { + final Properties fileProvidedProperties = readPropertiesFile(); + this.debugEnabled = readDebugEnabled(fileProvidedProperties); + this.bufferCapacity = readBufferCapacity(fileProvidedProperties); + this.defaultListenerLevel = readDefaultListenerLevel(fileProvidedProperties); + this.instantFormatter = readInstantFormatter(fileProvidedProperties); + } - private static final int MAX_ENTRIES = PROPS.getIntegerProperty(MAX_STATUS_ENTRIES, 200); + /** + * Gets the static instance. + * + * @return a singleton instance + */ + public static Config getInstance() { + return INSTANCE; + } - private static final String DEFAULT_STATUS_LEVEL = PROPS.getStringProperty(DEFAULT_STATUS_LISTENER_LEVEL); + private static boolean readDebugEnabled(final Properties fileProvidedProperties) { + final String debug = readProperty(fileProvidedProperties, DEBUG_PROPERTY_NAME); + return debug != null; + } - static final boolean DEBUG_ENABLED = - PropertiesUtil.getProperties().getBooleanProperty(Constants.LOG4J2_DEBUG, false, true); + private static int readBufferCapacity(final Properties fileProvidedProperties) { + final String capacityString = readProperty(fileProvidedProperties, BUFFER_CAPACITY_PROPERTY_NAME); + return capacityString != null ? Integer.parseInt(capacityString) : BUFFER_CAPACITY_DEFAULT_VALUE; + } - // LOG4J2-1176: normal parameterized message remembers param object, causing memory leaks. - private static final StatusLogger STATUS_LOGGER = new StatusLogger( - StatusLogger.class.getName(), - ParameterizedNoReferenceMessageFactory.INSTANCE, - SimpleLoggerFactory.getInstance()); + private static Level readDefaultListenerLevel(final Properties fileProvidedProperties) { + final String level = readProperty(fileProvidedProperties, DEFAULT_LISTENER_LEVEL_PROPERTY_NAME); + return level != null ? Level.valueOf(level) : DEFAULT_LISTENER_LEVEL_DEFAULT_VALUE; + } - private final SimpleLogger logger; + private static DateTimeFormatter readInstantFormatter(final Properties fileProvidedProperties) { + final String format = readProperty(fileProvidedProperties, INSTANT_FORMAT_PROPERTY_NAME); + return format != null ? DateTimeFormatter.ofPattern(format) : null; + } - private final Collection<StatusListener> listeners = new CopyOnWriteArrayList<>(); + private static String readProperty(final Properties fileProvidedProperties, final String propertyName) { + final String systemProvidedValue = System.getProperty(propertyName); + return systemProvidedValue != null + ? systemProvidedValue + : (String) fileProvidedProperties.get(propertyName); + } - @SuppressWarnings("NonSerializableFieldInSerializableClass") - // ReentrantReadWriteLock is Serializable - private final ReadWriteLock listenersLock = new ReentrantReadWriteLock(); + // We need to roll out our own `.properties` reader. + // We could have used `PropertiesUtil`, `PropertyFilePropertySource`, etc. + // Consequently, they would delegate to `LoaderUtil`, etc. + // All these mechanisms expect a working `StatusLogger`. + // Hence, in order to be self-sufficient, we cannot rely on them. + private static Properties readPropertiesFile() { + final Properties properties = new Properties(); + final URL url = StatusLogger.class.getResource(PROPERTIES_FILE_NAME); + if (url == null) { + return properties; + } + try (final InputStream stream = url.openStream()) { + properties.load(stream); + } catch (final IOException error) { + // There is no logging system at this stage. + // There is nothing we can do but simply dumping the failure. + error.printStackTrace(System.err); + } + return properties; + } + } + + private static volatile StatusLogger INSTANCE = new StatusLogger(); + + private final Config config; + + private final StatusConsoleListener defaultListener; + + private final Collection<StatusListener> listeners; + + private final transient ReadWriteLock listenerLock = new ReentrantReadWriteLock(); - private final Queue<StatusData> messages = new BoundedQueue<>(MAX_ENTRIES); + private final transient Lock listenerReadLock = listenerLock.readLock(); - @SuppressWarnings("NonSerializableFieldInSerializableClass") - // ReentrantLock is Serializable - private final Lock msgLock = new ReentrantLock(); + private final transient Lock listenerWriteLock = listenerLock.writeLock(); - private int listenersLevel; + private final Queue<StatusData> buffer = new ConcurrentLinkedQueue<>(); /** - * Constructs the singleton instance for the STATUS_LOGGER constant. - * <p> - * This is now the logger level is set: - * </p> - * <ol> - * <li>If the property {@value Constants#LOG4J2_DEBUG} is {@code "true"}, then use {@link Level#TRACE}, otherwise,</li> - * <li>Use {@link Level#ERROR}</li> - * </ol> - * <p> - * This is now the listener level is set: - * </p> - * <ol> - * <li>If the property {@value #DEFAULT_STATUS_LISTENER_LEVEL} is set, then use <em>it</em>, otherwise,</li> - * <li>Use {@link Level#WARN}</li> - * </ol> - * <p> - * See: - * <ol> - * <li>LOG4J2-1813 Provide shorter and more intuitive way to switch on Log4j internal debug logging. If system property - * "log4j2.debug" is defined, print all status logging.</li> - * <li>LOG4J2-3340 StatusLogger's log Level cannot be changed as advertised.</li> - * </ol> - * </p> + * Constructs the default instance. + * <b>Users should not create new instances, but use {@link #getLogger()} instead!</b> * - * @param name The logger name. - * @param messageFactory The message factory. + * @since 2.23.0 */ - private StatusLogger( - final String name, final MessageFactory messageFactory, final SimpleLoggerFactory loggerFactory) { - super(name, messageFactory); - final Level loggerLevel = DEBUG_ENABLED ? Level.TRACE : Level.ERROR; - this.logger = loggerFactory.createSimpleLogger("StatusLogger", loggerLevel, messageFactory, System.err); - this.listenersLevel = Level.toLevel(DEFAULT_STATUS_LEVEL, Level.WARN).intLevel(); + public StatusLogger() { + this( + StatusLogger.class.getSimpleName(), + ParameterizedNoReferenceMessageFactory.INSTANCE, + Config.getInstance(), + new StatusConsoleListener(Config.getInstance().defaultListenerLevel)); } /** - * Retrieve the StatusLogger. + * Constructs an instance using given properties. + * <b>Users should not create new instances, but use {@link #getLogger()} instead!</b> * - * @return The StatusLogger. + * @param name the logger name + * @param messageFactory the message factory + * @param config the configuration + * @param defaultListener the default listener + * @throws NullPointerException on null {@code name}, {@code messageFactory}, {@code config}, or {@code defaultListener} + * @since 2.23.0 + */ + public StatusLogger( + final String name, + final MessageFactory messageFactory, + final Config config, + final StatusConsoleListener defaultListener) { + super(requireNonNull(name, "name"), requireNonNull(messageFactory, "messageFactory")); + this.config = requireNonNull(config, "config"); + this.defaultListener = requireNonNull(defaultListener, "defaultListener"); + this.listeners = new ArrayList<>(Collections.singleton(defaultListener)); + } + + /** + * Gets the static instance. + * + * @return the singleton instance */ public static StatusLogger getLogger() { - return STATUS_LOGGER; + return INSTANCE; + } + + /** + * Sets the static (i.e., singleton) instance returned by {@link #getLogger()}. + * This method is intended for testing purposes and can have unforeseen consequences if used in production code. + * + * @param logger a logger instance + * @throws NullPointerException on null {@code logger} + * @since 2.23.0 + */ + public static void setLogger(final StatusLogger logger) { + INSTANCE = requireNonNull(logger, "logger"); } + /** + * Sets the level of the default listener. + * + * @param level a level + * @since 2.23.0 + */ + public void setDefaultListenerLevel(final Level level) { + requireNonNull(level, "level"); + defaultListener.setLevel(level); + } + + /** + * Sets the output of the default listener. + * + * @param stream a print stream + * @since 2.23.0 + */ + public void setDefaultListenerOutput(final PrintStream stream) { + requireNonNull(stream, "stream"); + defaultListener.setStream(stream); + } + + /** + * Sets the level of the default listener. + * + * @param level a level + * @deprecated Use {@link #setDefaultListenerLevel(Level)} instead. + */ + @Deprecated public void setLevel(final Level level) { - logger.setLevel(level); + requireNonNull(level, "level"); + setDefaultListenerLevel(level); } /** * Registers a new listener. * - * @param listener The StatusListener to register. + * @param listener a listener to register */ public void registerListener(final StatusListener listener) { - listenersLock.writeLock().lock(); + requireNonNull(listener, "listener"); + listenerWriteLock.lock(); try { listeners.add(listener); - final Level lvl = listener.getStatusLevel(); - if (listenersLevel < lvl.intLevel()) { - listenersLevel = lvl.intLevel(); - } } finally { - listenersLock.writeLock().unlock(); + listenerWriteLock.unlock(); } } /** - * Removes a StatusListener. + * Removes the given listener. * - * @param listener The StatusListener to remove. + * @param listener a listener to remove */ public void removeListener(final StatusListener listener) { - closeSilently(listener); - listenersLock.writeLock().lock(); + requireNonNull(listener, "listener"); + listenerWriteLock.lock(); try { + closeListenerSafely(listener); listeners.remove(listener); - int lowest = Level.toLevel(DEFAULT_STATUS_LEVEL, Level.WARN).intLevel(); - for (final StatusListener statusListener : listeners) { - final int level = statusListener.getStatusLevel().intLevel(); - if (lowest < level) { - lowest = level; - } - } - listenersLevel = lowest; } finally { - listenersLock.writeLock().unlock(); + listenerWriteLock.unlock(); } } - public void updateListenerLevel(final Level status) { - if (status.intLevel() > listenersLevel) { - listenersLevel = status.intLevel(); - } + /** + * + * Sets the level of the default listener. + * + * @param level a level + * @deprecated Instead either use {@link #setDefaultListenerLevel(Level)}, or {@link #getListeners() get the specific listener} and change its {@link StatusListener#getStatusLevel() level}. + */ + @Deprecated + public void updateListenerLevel(final Level level) { + requireNonNull(level, "level"); + setDefaultListenerLevel(level); } /** - * Returns a thread safe Iterable for the StatusListener. + * Returns the listener collection. * - * @return An Iterable for the list of StatusListeners. + * @return a thread-safe read-only collection of listeners */ public Iterable<StatusListener> getListeners() { - return listeners; + listenerReadLock.lock(); + try { + return Collections.unmodifiableCollection(listeners); + } finally { + listenerReadLock.unlock(); + } } /** - * Clears the list of status events and listeners. + * Clears the event buffer and listeners. */ public void reset() { - listenersLock.writeLock().lock(); + listenerWriteLock.lock(); try { - for (final StatusListener listener : listeners) { - closeSilently(listener); + Iterator<StatusListener> listenerIterator = listeners.iterator(); + while (listenerIterator.hasNext()) { + StatusListener listener = listenerIterator.next(); + closeListenerSafely(listener); + listenerIterator.remove(); } } finally { - listeners.clear(); - listenersLock.writeLock().unlock(); - // note this should certainly come after the unlock to avoid unnecessary nested locking - clear(); + listenerWriteLock.unlock(); } + buffer.clear(); } - private static void closeSilently(final Closeable resource) { + private static void closeListenerSafely(final StatusListener listener) { try { - resource.close(); - } catch (final IOException ignored) { - // ignored + listener.close(); + } catch (final IOException error) { + final String message = String.format("failed closing listener: %s", listener); + new RuntimeException(message, error).printStackTrace(System.err); } } /** - * Returns a List of all events as StatusData objects. + * Returns buffered events. * - * @return The list of StatusData objects. + * @return a thread-safe read-only collection of buffered events */ public List<StatusData> getStatusData() { - msgLock.lock(); - try { - return new ArrayList<>(messages); - } finally { - msgLock.unlock(); - } + return Collections.unmodifiableList(new ArrayList<>(buffer)); Review Comment: Do we need to prevent modification of the returned data, since it is a copy? A typical application for us would be in tests where we can: - get a snapshot of the data, - assert the presence of some `WARN` messages, - assert that there are no other `WARN` messages. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org