From 7e270ada5f52d2181577c6afa9e923d4f5a7cf7b Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Mon, 10 Jun 2019 12:23:28 +0200 Subject: [PATCH] SC-702 decouple Logback appenders from encoders --- .../org/sonar/application/AppLogging.java | 28 ++++++++++---- .../sonar/process/logging/LogbackHelper.java | 37 ++++++------------- .../process/logging/LogbackHelperTest.java | 6 ++- .../server/log/ServerProcessLogging.java | 17 ++++++--- 4 files changed, 48 insertions(+), 40 deletions(-) diff --git a/server/sonar-main/src/main/java/org/sonar/application/AppLogging.java b/server/sonar-main/src/main/java/org/sonar/application/AppLogging.java index 767453b9e32..24d1d4fea23 100644 --- a/server/sonar-main/src/main/java/org/sonar/application/AppLogging.java +++ b/server/sonar-main/src/main/java/org/sonar/application/AppLogging.java @@ -22,9 +22,11 @@ package org.sonar.application; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.encoder.PatternLayoutEncoder; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.ConsoleAppender; import ch.qos.logback.core.FileAppender; +import ch.qos.logback.core.encoder.Encoder; import org.sonar.application.config.AppSettings; import org.sonar.application.process.StreamGobbler; import org.sonar.process.ProcessId; @@ -152,7 +154,8 @@ public class AppLogging { * It creates a dedicated appender to the System.out which applies no formatting the logs it receives. */ private void configureConsole(LoggerContext loggerContext) { - ConsoleAppender consoleAppender = helper.newConsoleAppender(loggerContext, CONSOLE_PLAIN_APPENDER, "%msg%n"); + Encoder encoder = newPatternLayoutEncoder(loggerContext, "%msg%n"); + ConsoleAppender consoleAppender = helper.newConsoleAppender(loggerContext, CONSOLE_PLAIN_APPENDER, encoder); Logger consoleLogger = loggerContext.getLogger(CONSOLE_LOGGER); consoleLogger.setAdditive(false); @@ -192,7 +195,8 @@ public class AppLogging { // logs are written to the console because we want them to be in sonar.log and the wrapper will write any log // from APP's System.out and System.err to sonar.log Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME); - rootLogger.addAppender(createAppConsoleAppender(ctx, helper.buildLogPattern(APP_ROOT_LOGGER_CONFIG))); + Encoder encoder = newPatternLayoutEncoder(ctx, helper.buildLogPattern(APP_ROOT_LOGGER_CONFIG)); + rootLogger.addAppender(createAppConsoleAppender(ctx, encoder)); // in regular configuration, sub processes are not copying their logs to their System.out, so, the only logs to be // expected in LOGGER_GOBBLER are those before logback is setup in subprocesses or when JVM crashes @@ -205,10 +209,10 @@ public class AppLogging { private void configureRootWithLogbackWritingToFile(LoggerContext ctx) { Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME); - String appLogPattern = helper.buildLogPattern(APP_ROOT_LOGGER_CONFIG); - FileAppender fileAppender = helper.newFileAppender(ctx, appSettings.getProps(), APP_ROOT_LOGGER_CONFIG, appLogPattern); + Encoder encoder = newPatternLayoutEncoder(ctx, helper.buildLogPattern(APP_ROOT_LOGGER_CONFIG)); + FileAppender fileAppender = helper.newFileAppender(ctx, appSettings.getProps(), APP_ROOT_LOGGER_CONFIG, encoder); rootLogger.addAppender(fileAppender); - rootLogger.addAppender(createAppConsoleAppender(ctx, appLogPattern)); + rootLogger.addAppender(createAppConsoleAppender(ctx, encoder)); } /** @@ -224,11 +228,19 @@ public class AppLogging { private void configureGobbler(LoggerContext ctx) { Logger gobblerLogger = ctx.getLogger(LOGGER_GOBBLER); gobblerLogger.setAdditive(false); - gobblerLogger.addAppender(helper.newConsoleAppender(ctx, GOBBLER_PLAIN_CONSOLE, "%msg%n")); + Encoder encoder = newPatternLayoutEncoder(ctx, "%msg%n"); + gobblerLogger.addAppender(helper.newConsoleAppender(ctx, GOBBLER_PLAIN_CONSOLE, encoder)); } - private ConsoleAppender createAppConsoleAppender(LoggerContext ctx, String appLogPattern) { - return helper.newConsoleAppender(ctx, APP_CONSOLE_APPENDER, appLogPattern); + private ConsoleAppender createAppConsoleAppender(LoggerContext ctx, Encoder encoder) { + return helper.newConsoleAppender(ctx, APP_CONSOLE_APPENDER, encoder); } + private static Encoder newPatternLayoutEncoder(LoggerContext context, String pattern) { + PatternLayoutEncoder encoder = new PatternLayoutEncoder(); + encoder.setContext(context); + encoder.setPattern(pattern); + encoder.start(); + return encoder; + } } diff --git a/server/sonar-process/src/main/java/org/sonar/process/logging/LogbackHelper.java b/server/sonar-process/src/main/java/org/sonar/process/logging/LogbackHelper.java index b396133e5f9..3af237048df 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/logging/LogbackHelper.java +++ b/server/sonar-process/src/main/java/org/sonar/process/logging/LogbackHelper.java @@ -22,7 +22,6 @@ package org.sonar.process.logging; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; -import ch.qos.logback.classic.encoder.PatternLayoutEncoder; import ch.qos.logback.classic.joran.JoranConfigurator; import ch.qos.logback.classic.jul.LevelChangePropagator; import ch.qos.logback.classic.spi.ILoggingEvent; @@ -30,6 +29,7 @@ import ch.qos.logback.classic.spi.LoggerContextListener; import ch.qos.logback.core.ConsoleAppender; import ch.qos.logback.core.Context; import ch.qos.logback.core.FileAppender; +import ch.qos.logback.core.encoder.Encoder; import ch.qos.logback.core.joran.spi.JoranException; import ch.qos.logback.core.rolling.FixedWindowRollingPolicy; import ch.qos.logback.core.rolling.RollingFileAppender; @@ -150,18 +150,12 @@ public class LogbackHelper extends AbstractLogHelper { } /** - * Creates a new {@link ConsoleAppender} to {@code System.out} with the specified name and log pattern. - * - * @see #buildLogPattern(RootLoggerConfig) + * Creates a new {@link ConsoleAppender} to {@code System.out} with the specified name and log encoder. */ - public ConsoleAppender newConsoleAppender(Context loggerContext, String name, String logPattern) { - PatternLayoutEncoder consoleEncoder = new PatternLayoutEncoder(); - consoleEncoder.setContext(loggerContext); - consoleEncoder.setPattern(logPattern); - consoleEncoder.start(); + public ConsoleAppender newConsoleAppender(Context loggerContext, String name, Encoder encoder) { ConsoleAppender consoleAppender = new ConsoleAppender<>(); consoleAppender.setContext(loggerContext); - consoleAppender.setEncoder(consoleEncoder); + consoleAppender.setEncoder(encoder); consoleAppender.setName(name); consoleAppender.setTarget("System.out"); consoleAppender.start(); @@ -175,29 +169,22 @@ public class LogbackHelper extends AbstractLogHelper { *
  • the file's name will use the prefix defined in {@link RootLoggerConfig#getProcessId()#getLogFilenamePrefix()}.
    • *
  • the file will follow the rotation policy defined in property {@link #ROLLING_POLICY_PROPERTY} and * the max number of files defined in property {@link #MAX_FILES_PROPERTY}
    • - *
  • the logs will follow the specified log pattern
    • + *
  • the logs will follow the specified log encoder
    • * *

    - * - * @see #buildLogPattern(RootLoggerConfig) */ - public FileAppender configureGlobalFileLog(Props props, RootLoggerConfig config, String logPattern) { + public void configureGlobalFileLog(Props props, RootLoggerConfig config, Encoder encoder) { LoggerContext ctx = getRootContext(); Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME); - FileAppender fileAppender = newFileAppender(ctx, props, config, logPattern); + FileAppender fileAppender = newFileAppender(ctx, props, config, encoder); rootLogger.addAppender(fileAppender); - return fileAppender; } - public FileAppender newFileAppender(LoggerContext ctx, Props props, RootLoggerConfig config, String logPattern) { + public FileAppender newFileAppender(LoggerContext ctx, Props props, RootLoggerConfig config, Encoder encoder) { RollingPolicy rollingPolicy = createRollingPolicy(ctx, props, config.getProcessId().getLogFilenamePrefix()); FileAppender fileAppender = rollingPolicy.createAppender("file_" + config.getProcessId().getLogFilenamePrefix()); fileAppender.setContext(ctx); - PatternLayoutEncoder fileEncoder = new PatternLayoutEncoder(); - fileEncoder.setContext(ctx); - fileEncoder.setPattern(logPattern); - fileEncoder.start(); - fileAppender.setEncoder(fileEncoder); + fileAppender.setEncoder(encoder); fileAppender.start(); return fileAppender; } @@ -205,13 +192,11 @@ public class LogbackHelper extends AbstractLogHelper { /** * Make the logback configuration for a sub process to correctly push all its logs to be read by a stream gobbler * on the sub process's System.out. - * - * @see #buildLogPattern(RootLoggerConfig) */ - public void configureForSubprocessGobbler(Props props, String logPattern) { + public void configureForSubprocessGobbler(Props props, Encoder encoder) { if (isAllLogsToConsoleEnabled(props)) { LoggerContext ctx = getRootContext(); - ctx.getLogger(ROOT_LOGGER_NAME).addAppender(newConsoleAppender(ctx, "root_console", logPattern)); + ctx.getLogger(ROOT_LOGGER_NAME).addAppender(newConsoleAppender(ctx, "root_console", encoder)); } } diff --git a/server/sonar-process/src/test/java/org/sonar/process/logging/LogbackHelperTest.java b/server/sonar-process/src/test/java/org/sonar/process/logging/LogbackHelperTest.java index e865ef0c1bb..4d07ae721ea 100644 --- a/server/sonar-process/src/test/java/org/sonar/process/logging/LogbackHelperTest.java +++ b/server/sonar-process/src/test/java/org/sonar/process/logging/LogbackHelperTest.java @@ -206,7 +206,11 @@ public class LogbackHelperTest { @Test public void newConsoleAppender() { LoggerContext ctx = underTest.getRootContext(); - ConsoleAppender appender = underTest.newConsoleAppender(ctx, "MY_APPENDER", "%msg%n"); + PatternLayoutEncoder encoder = new PatternLayoutEncoder(); + encoder.setContext(ctx); + encoder.setPattern("%msg%n"); + encoder.start(); + ConsoleAppender appender = underTest.newConsoleAppender(ctx, "MY_APPENDER", encoder); assertThat(appender.getName()).isEqualTo("MY_APPENDER"); assertThat(appender.getContext()).isSameAs(ctx); diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/log/ServerProcessLogging.java b/server/sonar-server-common/src/main/java/org/sonar/server/log/ServerProcessLogging.java index 33f3a585331..5e453dfbbe3 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/log/ServerProcessLogging.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/log/ServerProcessLogging.java @@ -22,6 +22,7 @@ package org.sonar.server.log; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.encoder.PatternLayoutEncoder; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.ConsoleAppender; import com.google.common.collect.ImmutableSet; @@ -127,10 +128,13 @@ public abstract class ServerProcessLogging { .setProcessId(processId) .setThreadIdFieldPattern(threadIdFieldPattern) .build(); - String logPattern = helper.buildLogPattern(config); + PatternLayoutEncoder encoder = new PatternLayoutEncoder(); + encoder.setContext(helper.getRootContext()); + encoder.setPattern(helper.buildLogPattern(config)); + encoder.start(); - helper.configureGlobalFileLog(props, config, logPattern); - helper.configureForSubprocessGobbler(props, logPattern); + helper.configureGlobalFileLog(props, config, encoder); + helper.configureForSubprocessGobbler(props, encoder); } /** @@ -142,8 +146,11 @@ public abstract class ServerProcessLogging { .setProcessId(ProcessId.APP) .setThreadIdFieldPattern("") .build(); - String logPattern = helper.buildLogPattern(config); - ConsoleAppender consoleAppender = helper.newConsoleAppender(context, "CONSOLE", logPattern); + PatternLayoutEncoder encoder = new PatternLayoutEncoder(); + encoder.setContext(context); + encoder.setPattern(helper.buildLogPattern(config)); + encoder.start(); + ConsoleAppender consoleAppender = helper.newConsoleAppender(context, "CONSOLE", encoder); for (String loggerName : loggerNames) { Logger consoleLogger = context.getLogger(loggerName); -- 2.39.5