You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by vy...@apache.org on 2022/10/17 18:44:59 UTC
[logging-log4j2] branch release-2.x updated: LOG4J2-3584 Make StatusConsoleListener use SimpleLogger internally. (#1107)
This is an automated email from the ASF dual-hosted git repository.
vy pushed a commit to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/release-2.x by this push:
new 258fd0c6cf LOG4J2-3584 Make StatusConsoleListener use SimpleLogger internally. (#1107)
258fd0c6cf is described below
commit 258fd0c6cfaa27f7d64b6626c6a866b52eda3dc5
Author: Volkan Yazıcı <vo...@yazi.ci>
AuthorDate: Mon Oct 17 20:44:54 2022 +0200
LOG4J2-3584 Make StatusConsoleListener use SimpleLogger internally. (#1107)
---
.../log4j/status/StatusConsoleListenerTest.java | 172 +++++++++++++++++++++
.../logging/log4j/status/SimpleLoggerFactory.java | 59 +++++++
.../log4j/status/StatusConsoleListener.java | 66 +++++---
.../apache/logging/log4j/status/StatusLogger.java | 56 +++----
src/changes/changes.xml | 3 +
5 files changed, 310 insertions(+), 46 deletions(-)
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusConsoleListenerTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusConsoleListenerTest.java
new file mode 100644
index 0000000000..6b66543387
--- /dev/null
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusConsoleListenerTest.java
@@ -0,0 +1,172 @@
+/*
+ * 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.log4j.status;
+
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.LogBuilder;
+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.assertj.core.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+
+public class StatusConsoleListenerTest {
+
+ public static final MessageFactory MESSAGE_FACTORY = ParameterizedNoReferenceMessageFactory.INSTANCE;
+
+ @Test
+ void SimpleLogger_should_be_used() {
+
+ // Create a mock `SimpleLoggerFactory`.
+ final SimpleLogger logger = Mockito.mock(SimpleLogger.class);
+ final LogBuilder logBuilder = Mockito.mock(LogBuilder.class);
+ Mockito.when(logger.atLevel(Mockito.any())).thenReturn(logBuilder);
+ Mockito.when(logBuilder.withThrowable(Mockito.any())).thenReturn(logBuilder);
+ Mockito.when(logBuilder.withLocation(Mockito.any())).thenReturn(logBuilder);
+ final SimpleLoggerFactory loggerFactory = Mockito.mock(SimpleLoggerFactory.class);
+ Mockito
+ .when(loggerFactory.createSimpleLogger(
+ Mockito.any(),
+ Mockito.any(),
+ Mockito.any(),
+ Mockito.any()))
+ .thenReturn(logger);
+
+ // Create the listener.
+ final PrintStream stream = Mockito.mock(PrintStream.class);
+ final Level level = Mockito.mock(Level.class);
+ final StatusConsoleListener listener = new StatusConsoleListener(level, stream, loggerFactory);
+
+ // Log a message.
+ final StackTraceElement caller = Mockito.mock(StackTraceElement.class);
+ final Message message = Mockito.mock(Message.class);
+ final Throwable throwable = Mockito.mock(Throwable.class);
+ final StatusData statusData = new StatusData(
+ caller,
+ level,
+ message,
+ throwable,
+ null);
+ listener.log(statusData);
+
+ // Verify the call.
+ Mockito
+ .verify(loggerFactory)
+ .createSimpleLogger(
+ Mockito.eq("StatusConsoleListener"),
+ Mockito.same(level),
+ Mockito.any(),
+ Mockito.same(stream));
+ Mockito.verify(logger).atLevel(Mockito.same(level));
+ Mockito.verify(logBuilder).withThrowable(Mockito.same(throwable));
+ Mockito.verify(logBuilder).withLocation(Mockito.same(caller));
+ Mockito.verify(logBuilder).log(Mockito.same(message));
+
+ }
+
+ @Test
+ void level_and_stream_should_be_honored() throws Exception {
+
+ // Create the listener.
+ final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+ final String encoding = "UTF-8";
+ final PrintStream printStream = new PrintStream(outputStream, false, encoding);
+ final StatusConsoleListener listener = new StatusConsoleListener(Level.WARN, printStream);
+
+ // First, log a message that is expected to be logged.
+ final RuntimeException expectedThrowable = new RuntimeException("expectedThrowable");
+ expectedThrowable.setStackTrace(new StackTraceElement[]{
+ new StackTraceElement("expectedThrowableClass", "expectedThrowableMethod", "expectedThrowableFile", 1)
+ });
+ final Message expectedMessage = MESSAGE_FACTORY.newMessage("expectedMessage");
+ listener.log(new StatusData(
+ null, // since ignored by `SimpleLogger`
+ Level.WARN,
+ expectedMessage,
+ expectedThrowable,
+ null)); // as set by `StatusLogger` itself
+
+ // Second, log a message that is expected to be discarded due to its insufficient level.
+ final RuntimeException discardedThrowable = new RuntimeException("discardedThrowable");
+ discardedThrowable.setStackTrace(new StackTraceElement[]{
+ new StackTraceElement("discardedThrowableClass", "discardedThrowableMethod", "discardedThrowableFile", 2)
+ });
+ final Message discardedMessage = MESSAGE_FACTORY.newMessage("discardedMessage");
+ listener.log(new StatusData(
+ null, // since ignored by `SimpleLogger`
+ Level.INFO,
+ discardedMessage,
+ discardedThrowable,
+ null)); // as set by `StatusLogger` itself
+
+ // Collect the output.
+ printStream.flush();
+ final String output = outputStream.toString(encoding);
+
+ // Verify the output.
+ Assertions
+ .assertThat(output)
+ .contains(expectedThrowable.getMessage())
+ .contains(expectedMessage.getFormattedMessage())
+ .doesNotContain(discardedThrowable.getMessage())
+ .doesNotContain(discardedMessage.getFormattedMessage());
+
+ }
+
+ @Test
+ void filters_should_be_honored() throws Exception {
+
+ // Create the listener.
+ final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+ final String encoding = "UTF-8";
+ final PrintStream printStream = new PrintStream(outputStream, false, encoding);
+ final StatusConsoleListener listener = new StatusConsoleListener(Level.TRACE, printStream);
+
+ // Set the filter.
+ final StackTraceElement caller = new StackTraceElement("callerClass", "callerMethod", "callerFile", 1);
+ listener.setFilters(caller.getClassName());
+
+ // Log the message to be filtered.
+ final Message message = MESSAGE_FACTORY.newMessage("foo");
+ listener.log(new StatusData(
+ caller,
+ Level.TRACE,
+ message,
+ null,
+ null)); // as set by `StatusLogger` itself
+
+ // Verify the filtering.
+ printStream.flush();
+ final String output = outputStream.toString(encoding);
+ Assertions.assertThat(output).isEmpty();
+
+ }
+
+ @Test
+ void non_system_streams_should_be_closed() throws Exception {
+ final PrintStream stream = Mockito.mock(PrintStream.class);
+ final StatusConsoleListener listener = new StatusConsoleListener(Level.WARN, stream);
+ listener.close();
+ Mockito.verify(stream).close();
+ }
+
+}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/status/SimpleLoggerFactory.java b/log4j-api/src/main/java/org/apache/logging/log4j/status/SimpleLoggerFactory.java
new file mode 100644
index 0000000000..406f891f65
--- /dev/null
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/status/SimpleLoggerFactory.java
@@ -0,0 +1,59 @@
+/*
+ * 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.log4j.status;
+
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.message.MessageFactory;
+import org.apache.logging.log4j.simple.SimpleLogger;
+import org.apache.logging.log4j.util.Strings;
+
+import java.io.PrintStream;
+
+/**
+ * {@link org.apache.logging.log4j.simple.SimpleLogger} factory to be used by {@link StatusLogger} and {@link StatusConsoleListener}.
+ */
+class SimpleLoggerFactory {
+
+ private static final SimpleLoggerFactory INSTANCE = new SimpleLoggerFactory();
+
+ private SimpleLoggerFactory() {}
+
+ static SimpleLoggerFactory getInstance() {
+ return INSTANCE;
+ }
+
+ SimpleLogger createSimpleLogger(
+ final String name,
+ final Level level,
+ final MessageFactory messageFactory,
+ final PrintStream stream) {
+ final String dateFormat = StatusLogger.PROPS.getStringProperty(StatusLogger.STATUS_DATE_FORMAT);
+ final boolean dateFormatProvided = Strings.isNotBlank(dateFormat);
+ return new SimpleLogger(
+ name,
+ level,
+ false,
+ true,
+ dateFormatProvided,
+ false,
+ dateFormat,
+ messageFactory,
+ StatusLogger.PROPS,
+ stream);
+ }
+
+}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java b/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java
index 1097483e92..bb04cc420d 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java
@@ -16,42 +16,65 @@
*/
package org.apache.logging.log4j.status;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory;
+
import java.io.IOException;
import java.io.PrintStream;
-
-import org.apache.logging.log4j.Level;
+import java.util.Objects;
/**
- * StatusListener that writes to the Console.
+ * {@link StatusListener} that writes to the console.
*/
@SuppressWarnings("UseOfSystemOutOrSystemErr")
public class StatusConsoleListener implements StatusListener {
- private Level level = Level.FATAL;
+ private Level level;
+
private String[] filters;
+
private final PrintStream stream;
+ private final Logger logger;
+
/**
- * Creates the StatusConsoleListener using the supplied Level.
- * @param level The Level of status messages that should appear on the console.
+ * Constructs a {@link StatusConsoleListener} instance writing to {@link System#out} using the supplied level.
+ *
+ * @param level the level of status messages that should appear on the console
+ * @throws NullPointerException on null {@code level}
*/
public StatusConsoleListener(final Level level) {
this(level, System.out);
}
/**
- * Creates the StatusConsoleListener using the supplied Level. Make sure not to use a logger stream of some sort
- * to avoid creating an infinite loop of indirection!
- * @param level The Level of status messages that should appear on the console.
- * @param stream The PrintStream to write to.
- * @throws IllegalArgumentException if the PrintStream argument is {@code null}.
+ * Constructs a {@link StatusConsoleListener} instance using the supplied level and stream.
+ * <p>
+ * Make sure not to use a logger stream of some sort to avoid creating an infinite loop of indirection!
+ * </p>
+ *
+ * @param level the level of status messages that should appear on the console
+ * @param stream the stream to write to
+ * @throws NullPointerException on null {@code level} or {@code stream}
*/
public StatusConsoleListener(final Level level, final PrintStream stream) {
- if (stream == null) {
- throw new IllegalArgumentException("You must provide a stream to use for this listener.");
- }
- this.level = level;
- this.stream = stream;
+ this(level, stream, SimpleLoggerFactory.getInstance());
+ }
+
+ StatusConsoleListener(
+ final Level level,
+ final PrintStream stream,
+ final SimpleLoggerFactory loggerFactory) {
+ this.level = Objects.requireNonNull(level, "level");
+ this.stream = Objects.requireNonNull(stream, "stream");
+ this.logger = Objects
+ .requireNonNull(loggerFactory, "loggerFactory")
+ .createSimpleLogger(
+ "StatusConsoleListener",
+ level,
+ ParameterizedNoReferenceMessageFactory.INSTANCE,
+ stream);
}
/**
@@ -77,8 +100,14 @@ public class StatusConsoleListener implements StatusListener {
*/
@Override
public void log(final StatusData data) {
- if (!filtered(data)) {
- stream.println(data.getFormattedStatus());
+ final boolean filtered = filtered(data);
+ if (!filtered) {
+ logger
+ // Logging using _only_ the following 4 fields set by `StatusLogger#logMessage()`:
+ .atLevel(data.getLevel())
+ .withThrowable(data.getThrowable())
+ .withLocation(data.getStackTraceElement())
+ .log(data.getMessage());
}
}
@@ -110,4 +139,5 @@ public class StatusConsoleListener implements StatusListener {
this.stream.close();
}
}
+
}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java b/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java
index 32accf9b21..1f3a93737f 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java
@@ -16,6 +16,17 @@
*/
package org.apache.logging.log4j.status;
+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;
+
import java.io.Closeable;
import java.io.IOException;
import java.util.ArrayList;
@@ -29,18 +40,6 @@ 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;
-import org.apache.logging.log4j.util.Strings;
-
/**
* 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
@@ -75,15 +74,21 @@ public final class StatusLogger extends AbstractLogger {
private static final String NOT_AVAIL = "?";
- private static final PropertiesUtil PROPS = new PropertiesUtil("log4j2.StatusLogger.properties");
+ static final PropertiesUtil PROPS = new PropertiesUtil("log4j2.StatusLogger.properties");
private static final int MAX_ENTRIES = PROPS.getIntegerProperty(MAX_STATUS_ENTRIES, 200);
private static final String DEFAULT_STATUS_LEVEL = PROPS.getStringProperty(DEFAULT_STATUS_LISTENER_LEVEL);
+ static final boolean DEBUG_ENABLED = PropertiesUtil
+ .getProperties()
+ .getBooleanProperty(Constants.LOG4J2_DEBUG, false, true);
+
// LOG4J2-1176: normal parameterized message remembers param object, causing memory leaks.
- private static final StatusLogger STATUS_LOGGER = new StatusLogger(StatusLogger.class.getName(),
- ParameterizedNoReferenceMessageFactory.INSTANCE);
+ private static final StatusLogger STATUS_LOGGER = new StatusLogger(
+ StatusLogger.class.getName(),
+ ParameterizedNoReferenceMessageFactory.INSTANCE,
+ SimpleLoggerFactory.getInstance());
private final SimpleLogger logger;
@@ -129,20 +134,16 @@ public final class StatusLogger extends AbstractLogger {
* @param name The logger name.
* @param messageFactory The message factory.
*/
- private StatusLogger(final String name, final MessageFactory messageFactory) {
+ private StatusLogger(
+ final String name,
+ final MessageFactory messageFactory,
+ final SimpleLoggerFactory loggerFactory) {
super(name, messageFactory);
- final String dateFormat = PROPS.getStringProperty(STATUS_DATE_FORMAT, Strings.EMPTY);
- final boolean showDateTime = !Strings.isEmpty(dateFormat);
- final Level loggerLevel = isDebugPropertyEnabled() ? Level.TRACE : Level.ERROR;
- this.logger = new SimpleLogger("StatusLogger", loggerLevel, false, true, showDateTime, false, dateFormat, messageFactory, PROPS, System.err);
+ 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();
}
- // LOG4J2-1813 if system property "log4j2.debug" is defined, print all status logging
- private boolean isDebugPropertyEnabled() {
- return PropertiesUtil.getProperties().getBooleanProperty(Constants.LOG4J2_DEBUG, false, true);
- }
-
/**
* Retrieve the StatusLogger.
*
@@ -292,7 +293,7 @@ public final class StatusLogger extends AbstractLogger {
msgLock.unlock();
}
// LOG4J2-1813 if system property "log4j2.debug" is defined, all status logging is enabled
- if (isDebugPropertyEnabled() || (listeners.size() <= 0)) {
+ if (DEBUG_ENABLED || (listeners.size() <= 0)) {
logger.logMessage(fqcn, level, marker, msg, t);
} else {
for (final StatusListener listener : listeners) {
@@ -422,8 +423,7 @@ public final class StatusLogger extends AbstractLogger {
@Override
public boolean isEnabled(final Level level, final Marker marker) {
- // LOG4J2-1813 if system property "log4j2.debug" is defined, all status logging is enabled
- if (isDebugPropertyEnabled()) {
+ if (DEBUG_ENABLED) {
return true;
}
if (listeners.size() > 0) {
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index cd68354d80..0f63d2c71a 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -35,6 +35,9 @@
</action>
</release>
<release version="2.19.0" date="2022-09-09" description="GA Release 2.19.0">
+ <action issue="LOG4J2-3584" dev="vy" type="fix">
+ Make StatusConsoleListener use SimpleLogger internally.
+ </action>
<action issue="LOG4J2-3614" dev="vy" type="fix" due-to="strainu">
Harden InstantFormatter against delegate failures.
</action>