SC-702 decouple Logback appenders from encoders

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<ILoggingEvent> consoleAppender = helper.newConsoleAppender(loggerContext, CONSOLE_PLAIN_APPENDER, "%msg%n");
+    Encoder<ILoggingEvent> encoder = newPatternLayoutEncoder(loggerContext, "%msg%n");
+    ConsoleAppender<ILoggingEvent> 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<ILoggingEvent> 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<ILoggingEvent> fileAppender = helper.newFileAppender(ctx, appSettings.getProps(), APP_ROOT_LOGGER_CONFIG, appLogPattern);
+    Encoder<ILoggingEvent> encoder = newPatternLayoutEncoder(ctx, helper.buildLogPattern(APP_ROOT_LOGGER_CONFIG));
+    FileAppender<ILoggingEvent> 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<ILoggingEvent> encoder = newPatternLayoutEncoder(ctx, "%msg%n");
+    gobblerLogger.addAppender(helper.newConsoleAppender(ctx, GOBBLER_PLAIN_CONSOLE, encoder));
   }
 
-  private ConsoleAppender<ILoggingEvent> createAppConsoleAppender(LoggerContext ctx, String appLogPattern) {
-    return helper.newConsoleAppender(ctx, APP_CONSOLE_APPENDER, appLogPattern);
+  private ConsoleAppender<ILoggingEvent> createAppConsoleAppender(LoggerContext ctx, Encoder<ILoggingEvent> encoder) {
+    return helper.newConsoleAppender(ctx, APP_CONSOLE_APPENDER, encoder);
   }
 
+  private static Encoder<ILoggingEvent> newPatternLayoutEncoder(LoggerContext context, String pattern) {
+    PatternLayoutEncoder encoder = new PatternLayoutEncoder();
+    encoder.setContext(context);
+    encoder.setPattern(pattern);
+    encoder.start();
+    return encoder;
+  }
 }

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<ILoggingEvent> newConsoleAppender(Context loggerContext, String name, String logPattern) {
-    PatternLayoutEncoder consoleEncoder = new PatternLayoutEncoder();
-    consoleEncoder.setContext(loggerContext);
-    consoleEncoder.setPattern(logPattern);
-    consoleEncoder.start();
+  public ConsoleAppender<ILoggingEvent> newConsoleAppender(Context loggerContext, String name, Encoder<ILoggingEvent> encoder) {
     ConsoleAppender<ILoggingEvent> 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 {
    * <li>the file's name will use the prefix defined in {@link RootLoggerConfig#getProcessId()#getLogFilenamePrefix()}.</li>
    * <li>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}</li>
-   * <li>the logs will follow the specified log pattern</li>
+   * <li>the logs will follow the specified log encoder</li>
    * </ul>
    * </p>
-   *
-   * @see #buildLogPattern(RootLoggerConfig)
    */
-  public FileAppender<ILoggingEvent> configureGlobalFileLog(Props props, RootLoggerConfig config, String logPattern) {
+  public void configureGlobalFileLog(Props props, RootLoggerConfig config, Encoder<ILoggingEvent> encoder) {
     LoggerContext ctx = getRootContext();
     Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME);
-    FileAppender<ILoggingEvent> fileAppender = newFileAppender(ctx, props, config, logPattern);
+    FileAppender<ILoggingEvent> fileAppender = newFileAppender(ctx, props, config, encoder);
     rootLogger.addAppender(fileAppender);
-    return fileAppender;
   }
 
-  public FileAppender<ILoggingEvent> newFileAppender(LoggerContext ctx, Props props, RootLoggerConfig config, String logPattern) {
+  public FileAppender<ILoggingEvent> newFileAppender(LoggerContext ctx, Props props, RootLoggerConfig config, Encoder<ILoggingEvent> encoder) {
     RollingPolicy rollingPolicy = createRollingPolicy(ctx, props, config.getProcessId().getLogFilenamePrefix());
     FileAppender<ILoggingEvent> 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<ILoggingEvent> 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));
     }
   }
 

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);

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<ILoggingEvent> consoleAppender = helper.newConsoleAppender(context, "CONSOLE", logPattern);
+    PatternLayoutEncoder encoder = new PatternLayoutEncoder();
+    encoder.setContext(context);
+    encoder.setPattern(helper.buildLogPattern(config));
+    encoder.start();
+    ConsoleAppender<ILoggingEvent> consoleAppender = helper.newConsoleAppender(context, "CONSOLE", encoder);
 
     for (String loggerName : loggerNames) {
       Logger consoleLogger = context.getLogger(loggerName);