Skip to content

Commit

Permalink
Allow multiple consoleAppender to be used in peon logging
Browse files Browse the repository at this point in the history
  • Loading branch information
@maytasm
maytasm committed Jul 17, 2023
1 parent 0e3ce0a commit b180154
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 20 deletions.
40 changes: 22 additions & 18 deletions .../apache/druid/indexing/common/tasklogs/ConsoleLoggingEnforcementConfigurationFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import org.apache.logging.log4j.core.config.xml.XmlConfiguration;
import org.apache.logging.log4j.core.layout.PatternLayout;

import javax.annotation.Nullable;
import javax.annotation.Nonnull;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -91,15 +91,18 @@ protected void doConfigure()
{
super.doConfigure();

Appender consoleAppender = findConsoleAppender();
if (consoleAppender == null) {
List<Appender> consoleAppenders = findConsoleAppenders();
if (consoleAppenders.isEmpty()) {
// create a ConsoleAppender with default pattern if no console appender is configured in the configuration file
consoleAppender = ConsoleAppender.newBuilder()
.setName("_Injected_Console_Appender_")
.setLayout(PatternLayout.newBuilder()
.withPattern("%d{ISO8601} %p [%t] %c - %m%n")
.build())
.build();
consoleAppenders.add(
ConsoleAppender.newBuilder()
.setName("_Injected_Console_Appender_")
.setLayout(PatternLayout.newBuilder()
.withPattern("%d{ISO8601} %p [%t] %c - %m%n")
.build()
)
.build()
);
}

List<LoggerConfig> loggerConfigList = new ArrayList<>();
Expand All @@ -112,36 +115,38 @@ protected void doConfigure()
// If not, replace it's appender to ConsoleAppender.
//
for (LoggerConfig logger : loggerConfigList) {
applyConsoleAppender(logger, consoleAppender);
applyConsoleAppender(logger, consoleAppenders);
}
}

@Nullable
private Appender findConsoleAppender()
@Nonnull
private List<Appender> findConsoleAppenders()
{
List<Appender> consoleAppenders = new ArrayList<>();
for (Map.Entry<String, Appender> entry : this.getAppenders().entrySet()) {
Appender appender = entry.getValue();
if (appender instanceof ConsoleAppender) {
return appender;
consoleAppenders.add(appender);
}
}
return null;
return consoleAppenders;
}

/**
* Ensure there is a console logger defined. Without a console logger peon logs wont be able to be stored in deep storage
*/
private void applyConsoleAppender(LoggerConfig logger, Appender consoleAppender)
private void applyConsoleAppender(LoggerConfig logger, List<Appender> consoleAppenders)
{
List<String> consoleAppenderNames = consoleAppenders.stream().map(Appender::getName).collect(Collectors.toList());
for (AppenderRef appenderRef : logger.getAppenderRefs()) {
if (consoleAppender.getName().equals(appenderRef.getRef())) {
if (consoleAppenderNames.contains(appenderRef.getRef())) {
// we need a console logger no matter what, but we want to be able to define a different appender if necessary
return;
}
}
Level level = Level.INFO;
Filter filter = null;

Appender consoleAppender = consoleAppenders.get(0);
if (!logger.getAppenderRefs().isEmpty()) {
AppenderRef appenderRef = logger.getAppenderRefs().get(0);

Expand All @@ -160,7 +165,6 @@ private void applyConsoleAppender(LoggerConfig logger, Appender consoleAppender)
consoleAppender.getName());
}


// add ConsoleAppender to this logger
logger.addAppender(consoleAppender, level, filter);
}
Expand Down
82 changes: 80 additions & 2 deletions ...rc/test/java/org/apache/druid/indexing/common/tasklogs/ConsoleLoggingEnforcementTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,73 @@ public void testEmptyAppender() throws IOException
Assert.assertEquals("%m", layout.getConversionPattern());
}

@Test
public void testMultipleConsoleAppender() throws IOException
{
// this logger configuration contains multiple appenders and appender refers
String log4jConfiguration = "<Configuration status=\"WARN\">\n"
+ " <Appenders>\n"
+ " <Console name=\"Console\" target=\"SYSTEM_OUT\">\n"
+ " <PatternLayout pattern=\"%m\"/>\n"
+ " </Console>\n"
+ " <Console name=\"Console1\" target=\"SYSTEM_OUT\">\n"
+ " <PatternLayout pattern=\"%m\"/>\n"
+ " </Console>\n"
+ " <Console name=\"Console2\" target=\"SYSTEM_OUT\">\n"
+ " <PatternLayout pattern=\"%m\"/>\n"
+ " </Console>\n"
+ " <Http name=\"Http\" url=\"http://localhost:9200/\">\n"
+ " <JsonLayout properties=\"true\"/>\n"
+ " </Http>\n"
+ " </Appenders>\n"
+ " <Loggers>\n"
+ " <Root level=\"info\">\n"
+ " <AppenderRef ref=\"Http\"/>\n"
+ " <AppenderRef ref=\"Console\"/>\n"
+ " </Root>\n"
+ " <Logger level=\"debug\" name=\"org.apache.druid\" additivity=\"false\">\n"
+ " <AppenderRef ref=\"Http\"/>\n"
+ " <AppenderRef ref=\"Console\"/>\n"
+ " </Logger>\n"
+ " <Logger level=\"debug\" name=\"org.apache.mmon1\" additivity=\"false\">\n"
+ " <AppenderRef ref=\"Http\"/>\n"
+ " <AppenderRef ref=\"Console1\"/>\n"
+ " </Logger>\n"
+ " <Logger level=\"debug\" name=\"org.apache.mmon2\" additivity=\"false\">\n"
+ " <AppenderRef ref=\"Http\"/>\n"
+ " <AppenderRef ref=\"Console2\"/>\n"
+ " </Logger>\n"
+ " <Logger level=\"debug\" name=\"org.apache.mmon3\" additivity=\"false\">\n"
+ " <AppenderRef ref=\"Http\"/>\n"
+ " </Logger>\n"
+ " <Logger level=\"debug\" name=\"org.apache.mmon4\" additivity=\"false\">\n"
+ " </Logger>\n"
+ " </Loggers>\n"
+ "</Configuration>";

LoggerContext context = enforceConsoleLogger(log4jConfiguration);

// this logger is not defined in configuration, it derivates ROOT logger configuration
assertHasConsoleAppenderAndHttpAppender(getLogger(context, "name_not_in_config"), Level.INFO);

assertHasConsoleAppenderAndHttpAppender(getLogger(context, "org.apache.druid"), Level.DEBUG);
assertHasConsoleAppenderAndHttpAppender(getLogger(context, ROOT), Level.INFO);

assertHasConsoleAppenderAndHttpAppenderWithNameValidation(getLogger(context, "org.apache.mmon1"), Level.DEBUG, "Console1", "Http");
assertHasConsoleAppenderAndHttpAppenderWithNameValidation(getLogger(context, "org.apache.mmon2"), Level.DEBUG, "Console2", "Http");
assertHasOnlyOneConsoleAppender(getLogger(context, "org.apache.mmon3"), Level.DEBUG);
assertHasOnlyOneConsoleAppender(getLogger(context, "org.apache.mmon4"), Level.DEBUG);

// the ConsoleAppender should be exactly the same as it's in the configuration
PatternLayout layout = (PatternLayout) getLogger(context, "anything").getAppenders()
.values()
.stream()
.findFirst()
.get()
.getLayout();
Assert.assertEquals("%m", layout.getConversionPattern());
}

private LoggerContext enforceConsoleLogger(String configuration) throws IOException
{
LoggerContext context = new LoggerContext("test");
Expand All @@ -239,19 +306,30 @@ private LoggerContext enforceConsoleLogger(String configuration) throws IOExcept
}

private void assertHasConsoleAppenderAndHttpAppender(Logger logger, Level level)
{
assertHasConsoleAppenderAndHttpAppenderWithNameValidation(logger, level, "Console", "Http");
}

private void assertHasConsoleAppenderAndHttpAppenderWithNameValidation(
Logger logger,
Level level,
String consoleName,
String httpName
)
{
// there's two appenders
Assert.assertEquals(2, logger.getAppenders().size());

// and the appenders must be ConsoleAppender and HttpAppender
Assert.assertEquals(ConsoleAppender.class, logger.getAppenders().get("Console").getClass());
Assert.assertEquals(HttpAppender.class, logger.getAppenders().get("Http").getClass());
Assert.assertEquals(ConsoleAppender.class, logger.getAppenders().get(consoleName).getClass());
Assert.assertEquals(HttpAppender.class, logger.getAppenders().get(httpName).getClass());

if (level != null) {
Assert.assertEquals(level, logger.getLevel());
}
}


private void assertHasOnlyOneConsoleAppender(Logger logger, Level level)
{
// there's only one appender
Expand Down

0 comments on commit b180154

Please sign in to comment.