Skip to content

Commit

Permalink
Add calling class name to mock log appender (opensearch-project#12013)
Browse files Browse the repository at this point in the history
Issue opensearch-project#10799 is caused by a stopped or unstarted appender still being
present in the static logger state. I have not been able to find the
improper usage of the log appender that would cause this error. To help
debug, this commit adds the calling class name to the mock log appender.
Any future occurrence should result in an error message with the class
name that incorrected configured the logger.

I've also added an override to the `stop()` method to force that callers
use the close() method that properly cleans up the logger state as
opposed to stopping the appender directly.

Signed-off-by: Andrew Ross <[email protected]>
Signed-off-by: Shivansh Arora <[email protected]>
  • Loading branch information
andrross authored and shiv0408 committed Apr 25, 2024
1 parent 079b9c9 commit bf3ec17
Showing 1 changed file with 20 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.appender.AbstractAppender;
import org.apache.logging.log4j.core.config.Property;
import org.apache.logging.log4j.core.filter.RegexFilter;
import org.opensearch.common.logging.Loggers;
import org.opensearch.common.regex.Regex;
Expand Down Expand Up @@ -68,11 +69,19 @@ public class MockLogAppender extends AbstractAppender implements AutoCloseable {
* write to a closed MockLogAppender instance.
*/
public static MockLogAppender createForLoggers(Logger... loggers) throws IllegalAccessException {
return createForLoggers(".*(\n.*)*", loggers);
final String callingClass = Thread.currentThread().getStackTrace()[2].getClassName();
return createForLoggersInternal(callingClass, ".*(\n.*)*", loggers);
}

public static MockLogAppender createForLoggers(String filter, Logger... loggers) throws IllegalAccessException {
final String callingClass = Thread.currentThread().getStackTrace()[2].getClassName();
return createForLoggersInternal(callingClass, filter, loggers);
}

private static MockLogAppender createForLoggersInternal(String callingClass, String filter, Logger... loggers)
throws IllegalAccessException {
final MockLogAppender appender = new MockLogAppender(
callingClass + "-mock-log-appender",
RegexFilter.createFilter(filter, new String[0], false, null, null),
Collections.unmodifiableList(Arrays.asList(loggers))
);
Expand All @@ -83,8 +92,8 @@ public static MockLogAppender createForLoggers(String filter, Logger... loggers)
return appender;
}

private MockLogAppender(RegexFilter filter, List<Logger> loggers) {
super("mock", filter, null);
private MockLogAppender(String name, RegexFilter filter, List<Logger> loggers) {
super(name, filter, null, true, Property.EMPTY_ARRAY);
/*
* We use a copy-on-write array list since log messages could be appended while we are setting up expectations. When that occurs,
* we would run into a concurrent modification exception from the iteration over the expectations in #append, concurrent with a
Expand Down Expand Up @@ -116,7 +125,14 @@ public void close() {
for (Logger logger : loggers) {
Loggers.removeAppender(logger, this);
}
this.stop();
super.stop();
}

@Override
public void stop() {
// MockLogAppender should be used with try-with-resources to ensure
// proper clean up ordering and should never be stopped directly.
throw new UnsupportedOperationException("Use close() to ensure proper clean up ordering");
}

public interface LoggingExpectation {
Expand Down

0 comments on commit bf3ec17

Please sign in to comment.