Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make StatusLogger self-contained and testable #2249

Merged
merged 31 commits into from
Feb 8, 2024
Merged
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
94c813c
Make `StatusLogger` self-contained and testable
vy Jan 25, 2024
fd67e38
Extend `UsingStatusLoggerMock` with `ExtensionContextAnchor`
vy Jan 26, 2024
21a47ac
Speed-up `isEnabled()` by caching the least specific listener level
vy Jan 26, 2024
cd8912d
Merge remote-tracking branch 'origin/2.x' into 2.x-StatusLogger-revamp
vy Jan 26, 2024
cd3fadb
Bring the fallback `StatusListener` concept back
vy Jan 29, 2024
f1e3e2f
Make `StatusLogger()` ctor private
vy Jan 29, 2024
c4c941e
Document unmodifiable collection usage
vy Jan 29, 2024
3684a2e
Initialize the default `StatusLogger` instance lazy
vy Jan 29, 2024
2c01bcd
Fix `@Version`s
vy Jan 29, 2024
fdcb05b
Fix Spotless issues
vy Jan 29, 2024
40cc721
Rework `StatusLogger` effective level
vy Jan 29, 2024
88eb8fc
Improve `StatusLoggerAdmin` conditional registration
vy Jan 29, 2024
f3fa918
Fix NPE in `AsyncLoggerConfigDisruptor`
vy Jan 30, 2024
325c371
Merge remote-tracking branch 'origin/2.x' into 2.x-StatusLogger-revamp
vy Jan 30, 2024
df82860
Fix code typo in `StatusLogger`
vy Jan 30, 2024
35e78bb
Fix Spotless failures
vy Jan 30, 2024
682604b
Revert "Fix NPE in `AsyncLoggerConfigDisruptor`"
ppkarwasz Jan 30, 2024
e0090d4
Merge branch '2.x' into 2.x-StatusLogger-revamp
ppkarwasz Jan 30, 2024
e2e983f
Merge remote-tracking branch 'origin/2.x' into 2.x-StatusLogger-revamp
vy Feb 6, 2024
8632e09
Update auto-generated files
vy Feb 6, 2024
36c4d48
Remove `announce@apache.org` in `generate-email.sh`
vy Feb 6, 2024
73cd685
Remove nested logging tests
vy Feb 6, 2024
bfb5197
Merge remote-tracking branch 'origin/2.x' into 2.x-StatusLogger-revamp
vy Feb 6, 2024
9720939
Remove `QueueFullAsync*` tests relying on nested logging
vy Feb 7, 2024
c0235e3
Trim recently added public methods to `StatusLogger`
vy Feb 7, 2024
b8e490c
Deprecate buffering provided by `StatusLogger`
vy Feb 7, 2024
bfdbde4
Match level comparison in `StatusConsoleListener` and `StatusLogger`
vy Feb 7, 2024
fab3f15
Add getter for the fallback listener in `StatusLogger`
vy Feb 8, 2024
9f7533f
Merge remote-tracking branch 'origin/2.x' into 2.x-StatusLogger-revamp
vy Feb 8, 2024
4d43e82
Improve `StatusLogger` javadoc
vy Feb 8, 2024
c48bb4f
Fix `javadoc:javadoc` failures
vy Feb 8, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion log4j-api-test/pom.xml
Original file line number Diff line number Diff line change
@@ -138,7 +138,6 @@
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
Original file line number Diff line number Diff line change
@@ -16,21 +16,16 @@
*/
package org.apache.logging.log4j.test.junit;

import java.io.IOException;
import java.time.Instant;
import java.time.ZoneId;
import java.time.format.DateTimeFormatter;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory;
import org.apache.logging.log4j.simple.SimpleLogger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.status.StatusConsoleListener;
import org.apache.logging.log4j.status.StatusData;
import org.apache.logging.log4j.status.StatusListener;
import org.apache.logging.log4j.status.StatusLogger;
import org.apache.logging.log4j.test.ListStatusListener;
import org.apache.logging.log4j.util.PropertiesUtil;
import org.junit.jupiter.api.extension.BeforeAllCallback;
import org.junit.jupiter.api.extension.BeforeEachCallback;
import org.junit.jupiter.api.extension.ExtensionContext;
@@ -43,12 +38,12 @@
import org.junit.platform.commons.support.ModifierSupport;
import org.junit.platform.commons.support.ReflectionSupport;

class StatusLoggerExtension extends TypeBasedParameterResolver<ListStatusListener>
class StatusListenerExtension extends TypeBasedParameterResolver<ListStatusListener>
implements BeforeAllCallback, BeforeEachCallback, TestExecutionExceptionHandler {

private static final Object KEY = ListStatusListener.class;

public StatusLoggerExtension() {
public StatusListenerExtension() {
super(ListStatusListener.class);
}

@@ -130,33 +125,19 @@ public ListStatusListener getStatusListener() {
}

@Override
public void close() throws Throwable {
public void close() {
statusLogger.removeListener(statusListener);
}

public void handleException(final ExtensionContext context, final Throwable throwable) {
final Logger logger = new SimpleLogger(
"StatusLoggerExtension",
Level.ALL,
false,
false,
false,
false,
final StatusListener listener = new StatusConsoleListener(Level.ALL, System.err);
listener.log(new StatusData(
null,
ParameterizedNoReferenceMessageFactory.INSTANCE,
PropertiesUtil.getProperties(),
System.err);
logger.error("Test {} failed.\nDumping status data:", context.getDisplayName());
final DateTimeFormatter formatter = DateTimeFormatter.ISO_LOCAL_TIME.withZone(ZoneId.systemDefault());
statusListener.getStatusData().forEach(data -> {
logger.atLevel(data.getLevel())
.withThrowable(data.getThrowable())
.withLocation(data.getStackTraceElement())
.log(
"{} {}",
formatter.format(Instant.ofEpochMilli(data.getTimestamp())),
data.getMessage().getFormattedMessage());
});
Level.ERROR,
new ParameterizedMessage("Test `{}` has failed, dumping status data...", context.getDisplayName()),
throwable,
null));
statusListener.getStatusData().forEach(listener::log);
}
}

@@ -186,14 +167,14 @@ public Level getStatusLevel() {
}

@Override
public void close() throws IOException {
public void close() {
// NOP
}

@Override
public Stream<StatusData> getStatusData() {
synchronized (statusData) {
final List<StatusData> clone = (List<StatusData>) statusData.clone();
final List<StatusData> clone = new ArrayList<>(statusData);
return parent != null ? Stream.concat(parent.getStatusData(), clone.stream()) : clone.stream();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* 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.test.junit;

import static org.apache.logging.log4j.test.junit.ExtensionContextAnchor.getAttribute;
import static org.apache.logging.log4j.test.junit.ExtensionContextAnchor.setAttribute;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.when;

import org.apache.logging.log4j.status.StatusConsoleListener;
import org.apache.logging.log4j.status.StatusLogger;
import org.junit.jupiter.api.extension.AfterAllCallback;
import org.junit.jupiter.api.extension.BeforeAllCallback;
import org.junit.jupiter.api.extension.BeforeEachCallback;
import org.junit.jupiter.api.extension.ExtensionContext;

/**
* Replaces {@link StatusLogger} static instance with a mocked one.
* <p>
* <b>Warning!</b>
* Many classes store the result of {@link StatusLogger#getLogger()} in {@code static} field.
* Hence, the mock replacement must be performed before anybody tries to access it.
* Similarly, we cannot replace the mock in between tests, since it is already stored in {@code static} fields.
* That is why we only reset the mocked instance before each test.
* </p>
*
* @see UsingStatusLoggerMock
*/
class StatusLoggerMockExtension implements BeforeAllCallback, BeforeEachCallback, AfterAllCallback {

private static final String KEY_PREFIX = StatusLoggerMockExtension.class.getSimpleName() + '.';

private static final String INITIAL_STATUS_LOGGER_KEY = KEY_PREFIX + "initialStatusLogger";

@Override
public void beforeAll(final ExtensionContext context) throws Exception {
setAttribute(INITIAL_STATUS_LOGGER_KEY, StatusLogger.getLogger(), context);
final StatusLogger statusLogger = mock(StatusLogger.class);
stubFallbackListener(statusLogger);
StatusLogger.setLogger(statusLogger);
}

@Override
public void beforeEach(final ExtensionContext context) throws Exception {
final StatusLogger statusLogger = StatusLogger.getLogger();
reset(statusLogger); // Stubs get reset too!
stubFallbackListener(statusLogger);
}

private static void stubFallbackListener(final StatusLogger statusLogger) {
final StatusConsoleListener fallbackListener = mock(StatusConsoleListener.class);
when(statusLogger.getFallbackListener()).thenReturn(fallbackListener);
}

@Override
public void afterAll(final ExtensionContext context) {
final StatusLogger statusLogger = getAttribute(INITIAL_STATUS_LOGGER_KEY, StatusLogger.class, context);
StatusLogger.setLogger(statusLogger);
}
}
Original file line number Diff line number Diff line change
@@ -37,5 +37,5 @@
@Documented
@ExtendWith(ExtensionContextAnchor.class)
@ExtendWith(TestPropertyResolver.class)
@ExtendWith(StatusLoggerExtension.class)
@ExtendWith(StatusListenerExtension.class)
public @interface UsingStatusListener {}
Original file line number Diff line number Diff line change
@@ -14,24 +14,22 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.logging.log4j.core.async;
package org.apache.logging.log4j.test.junit;

import org.apache.logging.log4j.core.LoggerContext;
import org.apache.logging.log4j.core.test.junit.Tags;
import org.apache.logging.log4j.test.junit.SetTestProperty;
import org.junit.jupiter.api.Tag;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.TYPE;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import org.junit.jupiter.api.extension.ExtendWith;

/**
* Tests queue full scenarios with AsyncLoggers in configuration.
* Shortcut to {@link StatusLoggerMockExtension}.
*/
@SetTestProperty(key = "log4j2.formatMsgAsync", value = "true")
@Tag(Tags.ASYNC_LOGGERS)
public class QueueFullAsyncLoggerConfigLoggingFromToStringTest2
extends QueueFullAsyncLoggerConfigLoggingFromToStringTest {

@Override
protected void checkConfig(final LoggerContext ctx) throws ReflectiveOperationException {
super.checkConfig(ctx);
assertFormatMessagesInBackground();
}
}
@Retention(RUNTIME)
@Target({TYPE, METHOD})
@Documented
@ExtendWith({ExtensionContextAnchor.class, StatusLoggerMockExtension.class})
public @interface UsingStatusLoggerMock {}
Original file line number Diff line number Diff line change
@@ -42,9 +42,12 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.ResourceAccessMode;
import org.junit.jupiter.api.parallel.ResourceLock;
import org.junitpioneer.jupiter.SetSystemProperty;

@StatusLoggerLevel("WARN")
@ResourceLock(value = Resources.MARKER_MANAGER, mode = ResourceAccessMode.READ)
@SetSystemProperty(key = "log4j2.status.entries", value = "200")
@SetSystemProperty(key = "log4j2.StatusLogger.level", value = "WARN")
public class AbstractLoggerTest {

private static final StringBuilder CHAR_SEQ = new StringBuilder("CharSeq");
Original file line number Diff line number Diff line change
@@ -19,11 +19,9 @@
import java.io.ByteArrayOutputStream;
import java.io.PrintStream;
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;
@@ -33,38 +31,19 @@ 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);
void StatusData_getFormattedStatus_should_be_used() {

// 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);
final StatusConsoleListener listener = new StatusConsoleListener(Level.ALL, stream);

// 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);
final StatusData statusData = Mockito.spy(new StatusData(null, Level.TRACE, message, null, 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));
Mockito.verify(statusData).getFormattedStatus();
}

@Test
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* 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 static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import org.apache.logging.log4j.Level;
import org.junit.jupiter.api.Test;

class StatusLoggerLevelTest {

@Test
void effective_level_should_be_the_least_specific_one() {

// Verify the initial level
final StatusLogger logger = StatusLogger.getLogger();
final Level fallbackListenerLevel = Level.ERROR;
assertThat(logger.getLevel()).isEqualTo(fallbackListenerLevel);

// Register a less specific listener
final StatusListener listener1 = mock(StatusListener.class);
Level listener1Level = Level.WARN;
when(listener1.getStatusLevel()).thenReturn(listener1Level);
logger.registerListener(listener1);
assertThat(listener1Level).isNotEqualTo(fallbackListenerLevel); // Verify that the level is distinct
assertThat(logger.getLevel()).isEqualTo(listener1Level); // Verify that the logger level is changed

// Register a less specific listener
final StatusListener listener2 = mock(StatusListener.class);
final Level listener2Level = Level.INFO;
when(listener2.getStatusLevel()).thenReturn(listener2Level);
logger.registerListener(listener2);
assertThat(listener2Level)
.isNotEqualTo(fallbackListenerLevel)
.isNotEqualTo(listener1Level); // Verify that the level is distinct
assertThat(logger.getLevel()).isEqualTo(listener2Level); // Verify that the logger level is changed

// Register a more specific listener
final StatusListener listener3 = mock(StatusListener.class);
final Level listener3Level = Level.ERROR;
when(listener3.getStatusLevel()).thenReturn(listener3Level);
logger.registerListener(listener3);
assertThat(listener3Level)
.isNotEqualTo(listener1Level)
.isNotEqualTo(listener2Level); // Verify that the level is distinct
assertThat(logger.getLevel()).isEqualTo(listener2Level); // Verify that the logger level is not changed

// Update a registered listener level
listener1Level = Level.DEBUG;
when(listener1.getStatusLevel()).thenReturn(listener1Level);
assertThat(listener1Level) // Verify that the level is distinct
.isNotEqualTo(fallbackListenerLevel)
.isNotEqualTo(listener2Level)
.isNotEqualTo(listener3Level);
assertThat(logger.getLevel()).isEqualTo(listener1Level); // Verify that the logger level is changed

// Remove the least specific listener
logger.removeListener(listener2);
assertThat(logger.getLevel()).isEqualTo(listener1Level); // Verify that the level is changed

// Remove the most specific listener
logger.removeListener(listener3);
assertThat(logger.getLevel()).isEqualTo(listener1Level); // Verify that the level is not changed

// Remove the last listener
logger.removeListener(listener1);
assertThat(logger.getLevel()).isEqualTo(fallbackListenerLevel); // Verify that the level is changed
}
}
Loading