Re: [PR] Make `StatusLogger` self-contained and testable (logging-log4j2)

2024-03-05 Thread via GitHub


vy commented on code in PR #2249:
URL: https://github.com/apache/logging-log4j2/pull/2249#discussion_r1513500994


##
log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java:
##
@@ -16,311 +16,590 @@
  */
 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 
} 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.
+ * Listeners
+ * 
+ * Each recorded event will first get buffered and used to notify the 
registered {@link StatusListener}s.
+ * Listener registry is always initialized with a default listener, 
which is a {@link StatusConsoleListener}.
+ * 
+ * 
+ * You can programmatically register listeners using {@link 
#registerListener(StatusListener)} method.
+ * 
+ * Configuration
+ * 
+ * The {@code StatusLogger} can be configured in following ways:
+ * 
+ * 
+ * Passing system properties to the Java process (e.g., {@code 
-Dlog4j2.StatusLogger.level=INFO})
+ * Providing properties in a {@value StatusLogger#PROPERTIES_FILE_NAME} 
file in the classpath
+ * Using Log4j configuration (i.e., {@code } in a {@code log4j2.xml} in the classpath)
+ * 
+ * 
+ * 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:
+ * 
+ * 
+ * The default level is {@code ERROR}
+ * You have } in your {@code log4j2.xml}
+ * Until your {@code log4j2.xml} configuration is read, the effective 
level will be {@code ERROR}
+ * Once your {@code log4j2.xml} configuration is read, the effective level 
will be {@code WARN} as you configured
+ * 
+ * 
+ * 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.
+ * 
+ * 
+ * {@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 } while the other {@code 
}), the last loaded configuration will be 
effective one.
+ * 
+ * Debug mode
+ * 
+ * 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.
+ * 
  */
-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.
+ * 
+ * This is a local clone of {@link Constants#LOG4J2_DEBUG}.
+ * The cloning is necessary to avoid cyclic initialization.
+ * 
+ */
+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.
+ * 
+ * Once the limit is reached, older entries will 

Re: [PR] Make `StatusLogger` self-contained and testable (logging-log4j2)

2024-03-05 Thread via GitHub


carterkozak commented on code in PR #2249:
URL: https://github.com/apache/logging-log4j2/pull/2249#discussion_r1513116254


##
log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java:
##
@@ -16,311 +16,590 @@
  */
 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 
} 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.
+ * Listeners
+ * 
+ * Each recorded event will first get buffered and used to notify the 
registered {@link StatusListener}s.
+ * Listener registry is always initialized with a default listener, 
which is a {@link StatusConsoleListener}.
+ * 
+ * 
+ * You can programmatically register listeners using {@link 
#registerListener(StatusListener)} method.
+ * 
+ * Configuration
+ * 
+ * The {@code StatusLogger} can be configured in following ways:
+ * 
+ * 
+ * Passing system properties to the Java process (e.g., {@code 
-Dlog4j2.StatusLogger.level=INFO})
+ * Providing properties in a {@value StatusLogger#PROPERTIES_FILE_NAME} 
file in the classpath
+ * Using Log4j configuration (i.e., {@code } in a {@code log4j2.xml} in the classpath)
+ * 
+ * 
+ * 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:
+ * 
+ * 
+ * The default level is {@code ERROR}
+ * You have } in your {@code log4j2.xml}
+ * Until your {@code log4j2.xml} configuration is read, the effective 
level will be {@code ERROR}
+ * Once your {@code log4j2.xml} configuration is read, the effective level 
will be {@code WARN} as you configured
+ * 
+ * 
+ * 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.
+ * 
+ * 
+ * {@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 } while the other {@code 
}), the last loaded configuration will be 
effective one.
+ * 
+ * Debug mode
+ * 
+ * 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.
+ * 
  */
-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.
+ * 
+ * This is a local clone of {@link Constants#LOG4J2_DEBUG}.
+ * The cloning is necessary to avoid cyclic initialization.
+ * 
+ */
+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.
+ * 
+ * Once the limit is reached, older 

Re: [PR] Make `StatusLogger` self-contained and testable (logging-log4j2)

2024-02-23 Thread via GitHub


ppkarwasz commented on PR #2249:
URL: https://github.com/apache/logging-log4j2/pull/2249#issuecomment-1961844850

   Well, _logMessage_**Track**_Recursion_ only tracks recursion, it doesn't do 
anything about it. Other parts of the system like async logger read the 
recursion level (and log synchronously for example).
   
   


-- 
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



Re: [PR] Make `StatusLogger` self-contained and testable (logging-log4j2)

2024-02-23 Thread via GitHub


lauredogit commented on PR #2249:
URL: https://github.com/apache/logging-log4j2/pull/2249#issuecomment-1961744492

   Perfect, thanks!
   
   I think there are 2 distinct issues at play.
   
   The easy one to fix is the actual _UnsupportedTemporalTypeException_ on Java 
8, which will be fixed easily by adding the ".withZone(ZoneId.systemDefault())".
   
   However, the other issue is that having such an exception thrown by the 
StatusLogger was causing a _StackOverflowError_... 
   and this was not mitigated by AbstractLogger._logMessageTrackRecursion()_.
   
   This can be investigated later on, though.
   
   
   


-- 
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



Re: [PR] Make `StatusLogger` self-contained and testable (logging-log4j2)

2024-02-23 Thread via GitHub


vy commented on PR #2249:
URL: https://github.com/apache/logging-log4j2/pull/2249#issuecomment-1961651800

   @lauredogit, thanks so much for the report. I will submit a fix. We are 
considering a `2.23.1` release sometime soon, maybe next week?


-- 
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



Re: [PR] Make `StatusLogger` self-contained and testable (logging-log4j2)

2024-02-23 Thread via GitHub


lauredogit commented on PR #2249:
URL: https://github.com/apache/logging-log4j2/pull/2249#issuecomment-1960973519

   Hi,
   This changes caused an issue when running on Java 8 and having configured a 
DateFormat.
   
   It will cause a StackOverflowError such as:
   
   `Caused by: java.lang.StackOverflowError
at 
org.apache.logging.log4j.message.ParameterFormatter.analyzePattern(ParameterFormatter.java:93)
at 
org.apache.logging.log4j.message.ParameterizedMessage.(ParameterizedMessage.java:142)
at 
org.apache.logging.log4j.message.ParameterizedMessage.(ParameterizedMessage.java:176)
at 
org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory.newMessage(ParameterizedNoReferenceMessageFactory.java:105)
at 
org.apache.logging.log4j.message.AbstractMessageFactory.newMessage(AbstractMessageFactory.java:103)
at 
org.apache.logging.log4j.spi.AbstractLogger.logMessage(AbstractLogger.java:2686)
at 
org.apache.logging.log4j.spi.AbstractLogger.logIfEnabled(AbstractLogger.java:2460)
at 
org.apache.logging.log4j.spi.AbstractLogger.warn(AbstractLogger.java:3674)
at 
org.apache.logging.log4j.spi.AbstractLogger.handleLogMessageException(AbstractLogger.java:2927)
at 
org.apache.logging.log4j.spi.AbstractLogger.tryLogMessage(AbstractLogger.java:2909)
at 
org.apache.logging.log4j.spi.AbstractLogger.logMessageTrackRecursion(AbstractLogger.java:2859)
at 
org.apache.logging.log4j.spi.AbstractLogger.logMessageSafely(AbstractLogger.java:2841)
at 
org.apache.logging.log4j.spi.AbstractLogger.logMessage(AbstractLogger.java:2687)
at 
org.apache.logging.log4j.spi.AbstractLogger.logIfEnabled(AbstractLogger.java:2460)
at 
org.apache.logging.log4j.spi.AbstractLogger.warn(AbstractLogger.java:3674)
at 
org.apache.logging.log4j.spi.AbstractLogger.handleLogMessageException(AbstractLogger.java:2927)`
   
   which is caused by
   
   `java.time.temporal.UnsupportedTemporalTypeException: Unsupported field: 
YearOfEra
   at java.time.Instant.getLong(Instant.java:603)
   at 
java.time.format.DateTimePrintContext.getValue(DateTimePrintContext.java:298)
   at 
java.time.format.DateTimeFormatterBuilder$NumberPrinterParser.format(DateTimeFormatterBuilder.java:2551)
   at 
java.time.format.DateTimeFormatterBuilder$CompositePrinterParser.format(DateTimeFormatterBuilder.java:2190)
   at 
java.time.format.DateTimeFormatter.formatTo(DateTimeFormatter.java:1746)
   at 
java.time.format.DateTimeFormatter.format(DateTimeFormatter.java:1720)
   at 
org.apache.logging.log4j.status.StatusData.getFormattedStatus(StatusData.java:170)
   at 
org.apache.logging.log4j.status.StatusConsoleListener.log(StatusConsoleListener.java:138)
   at 
org.apache.logging.log4j.status.StatusLogger.notifyListener(StatusLogger.java:555)
   at 
org.apache.logging.log4j.status.StatusLogger.notifyListeners(StatusLogger.java:549)
   at 
org.apache.logging.log4j.status.StatusLogger.logMessage(StatusLogger.java:526)
   at 
org.apache.logging.log4j.spi.AbstractLogger.log(AbstractLogger.java:2812)
   at 
org.apache.logging.log4j.spi.AbstractLogger.tryLogMessage(AbstractLogger.java:2906)
   at 
org.apache.logging.log4j.spi.AbstractLogger.logMessageTrackRecursion(AbstractLogger.java:2859)
   at 
org.apache.logging.log4j.spi.AbstractLogger.logMessageSafely(AbstractLogger.java:2841)
   at 
org.apache.logging.log4j.spi.AbstractLogger.logMessage(AbstractLogger.java:2620)
   at 
org.apache.logging.log4j.spi.AbstractLogger.logIfEnabled(AbstractLogger.java:2567)
   at 
org.apache.logging.log4j.spi.AbstractLogger.warn(AbstractLogger.java:3469)`
   
   because in 
org.apache.logging.log4j.status.StatusLogger.Config#readInstantFormatter, the 
code is merely
   
   `private static DateTimeFormatter readInstantFormatter(final 
Properties fileProvidedProperties) {
   final String format = readProperty(fileProvidedProperties, 
STATUS_DATE_FORMAT);
   return format != null ? DateTimeFormatter.ofPattern(format) : 
null;
   }
   `
   
   with only **DateTimeFormatter.ofPattern(format)**
   
   This works on Java 11, but on Java 8, it breaks as in 
https://stackoverflow.com/questions/25229124/unsupportedtemporaltypeexception-when-formatting-instant-to-string
   
   A very simple fix is to use instead
   
   `private static DateTimeFormatter readInstantFormatter(final 
Properties fileProvidedProperties) {
   final String format = readProperty(fileProvidedProperties, 
STATUS_DATE_FORMAT);
   return format != null ? 
DateTimeFormatter.ofPattern(format).withZone(ZoneId.systemDefault()) : null;
   }`
   
   
   
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the

Re: [PR] Make `StatusLogger` self-contained and testable (logging-log4j2)

2024-02-08 Thread via GitHub


vy merged PR #2249:
URL: https://github.com/apache/logging-log4j2/pull/2249


-- 
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