From 12ad1060151fb075c685c104bb9bd60999bb0ef3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Thu, 11 Aug 2016 14:40:35 +0200 Subject: [PATCH] SONAR-7833 task start/stop logs in sonar.log only if DEBUG is enabled start log is always at DEBUG level stop log is at DEBUG level in case of success, at ERROR level in case of error --- .../main/java/org/sonar/ce/app/CeServer.java | 2 +- .../java/org/sonar/process/LogbackHelper.java | 5 +- .../org/sonar/process/LogbackHelperTest.java | 2 +- .../java/org/sonar/search/SearchLogging.java | 2 +- .../ce/log/CeActivityLogAcceptFilter.java | 2 +- .../sonar/ce/log/CeActivityLogDenyFilter.java | 37 ++++++++ .../app => ce/log}/CeProcessLogging.java | 20 +++-- .../server/app/ServerProcessLogging.java | 14 +-- .../server/app/WebServerProcessLogging.java | 8 +- .../taskprocessor/CeWorkerCallableImpl.java | 8 +- .../ce/log/CeActivityLogAcceptFilterTest.java | 63 +++++++++++++ .../ce/log/CeActivityLogDenyFilterTest.java | 62 +++++++++++++ .../app => ce/log}/CeProcessLoggingTest.java | 7 +- .../app/ProgrammaticLogbackValveTest.java | 2 +- ....java => WebServerProcessLoggingTest.java} | 11 +-- .../CeWorkerCallableImplTest.java | 88 +++++++++++++------ .../org/sonar/application/AppLogging.java | 4 +- 17 files changed, 268 insertions(+), 69 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/ce/log/CeActivityLogDenyFilter.java rename server/sonar-server/src/main/java/org/sonar/{server/app => ce/log}/CeProcessLogging.java (59%) create mode 100644 server/sonar-server/src/test/java/org/sonar/ce/log/CeActivityLogAcceptFilterTest.java create mode 100644 server/sonar-server/src/test/java/org/sonar/ce/log/CeActivityLogDenyFilterTest.java rename server/sonar-server/src/test/java/org/sonar/{server/app => ce/log}/CeProcessLoggingTest.java (94%) rename server/sonar-server/src/test/java/org/sonar/server/app/{ServerProcessLoggingTest.java => WebServerProcessLoggingTest.java} (89%) diff --git a/server/sonar-ce/src/main/java/org/sonar/ce/app/CeServer.java b/server/sonar-ce/src/main/java/org/sonar/ce/app/CeServer.java index 2760bc9365b..678494f1f45 100644 --- a/server/sonar-ce/src/main/java/org/sonar/ce/app/CeServer.java +++ b/server/sonar-ce/src/main/java/org/sonar/ce/app/CeServer.java @@ -32,7 +32,7 @@ import org.sonar.process.MinimumViableSystem; import org.sonar.process.Monitored; import org.sonar.process.ProcessEntryPoint; import org.sonar.process.Props; -import org.sonar.server.app.CeProcessLogging; +import org.sonar.ce.log.CeProcessLogging; import static com.google.common.base.Preconditions.checkState; import static org.sonar.process.ProcessUtils.awaitTermination; diff --git a/server/sonar-process/src/main/java/org/sonar/process/LogbackHelper.java b/server/sonar-process/src/main/java/org/sonar/process/LogbackHelper.java index 7da1a0ebf01..819c754597f 100644 --- a/server/sonar-process/src/main/java/org/sonar/process/LogbackHelper.java +++ b/server/sonar-process/src/main/java/org/sonar/process/LogbackHelper.java @@ -36,7 +36,6 @@ import ch.qos.logback.core.rolling.RollingFileAppender; import ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy; import ch.qos.logback.core.rolling.TimeBasedRollingPolicy; import java.io.File; -import javax.annotation.Nullable; import org.apache.commons.lang.StringUtils; import org.slf4j.LoggerFactory; @@ -73,7 +72,7 @@ public class LogbackHelper { return propagator; } - public ConsoleAppender newConsoleAppender(Context loggerContext, String name, String pattern, @Nullable Filter filter) { + public ConsoleAppender newConsoleAppender(Context loggerContext, String name, String pattern, Filter... filters) { PatternLayoutEncoder consoleEncoder = new PatternLayoutEncoder(); consoleEncoder.setContext(loggerContext); consoleEncoder.setPattern(pattern); @@ -83,7 +82,7 @@ public class LogbackHelper { consoleAppender.setEncoder(consoleEncoder); consoleAppender.setName(name); consoleAppender.setTarget("System.out"); - if (filter != null) { + for (Filter filter : filters) { consoleAppender.addFilter(filter); } consoleAppender.start(); diff --git a/server/sonar-process/src/test/java/org/sonar/process/LogbackHelperTest.java b/server/sonar-process/src/test/java/org/sonar/process/LogbackHelperTest.java index 3eff0626275..6eeacdebea9 100644 --- a/server/sonar-process/src/test/java/org/sonar/process/LogbackHelperTest.java +++ b/server/sonar-process/src/test/java/org/sonar/process/LogbackHelperTest.java @@ -82,7 +82,7 @@ public class LogbackHelperTest { @Test public void newConsoleAppender() { LoggerContext ctx = underTest.getRootContext(); - ConsoleAppender appender = underTest.newConsoleAppender(ctx, "MY_APPENDER", "%msg%n", null); + ConsoleAppender appender = underTest.newConsoleAppender(ctx, "MY_APPENDER", "%msg%n"); assertThat(appender.getName()).isEqualTo("MY_APPENDER"); assertThat(appender.getContext()).isSameAs(ctx); diff --git a/server/sonar-search/src/main/java/org/sonar/search/SearchLogging.java b/server/sonar-search/src/main/java/org/sonar/search/SearchLogging.java index 8944bb6a537..6bbf88ab2c8 100644 --- a/server/sonar-search/src/main/java/org/sonar/search/SearchLogging.java +++ b/server/sonar-search/src/main/java/org/sonar/search/SearchLogging.java @@ -36,7 +36,7 @@ public class SearchLogging { LoggerContext ctx = helper.getRootContext(); ctx.reset(); - ConsoleAppender consoleAppender = helper.newConsoleAppender(ctx, "CONSOLE", LOG_FORMAT, null); + ConsoleAppender consoleAppender = helper.newConsoleAppender(ctx, "CONSOLE", LOG_FORMAT); Logger rootLogger = helper.configureLogger(Logger.ROOT_LOGGER_NAME, Level.INFO); rootLogger.addAppender(consoleAppender); return ctx; diff --git a/server/sonar-server/src/main/java/org/sonar/ce/log/CeActivityLogAcceptFilter.java b/server/sonar-server/src/main/java/org/sonar/ce/log/CeActivityLogAcceptFilter.java index 0be5deb67c2..efd9091e231 100644 --- a/server/sonar-server/src/main/java/org/sonar/ce/log/CeActivityLogAcceptFilter.java +++ b/server/sonar-server/src/main/java/org/sonar/ce/log/CeActivityLogAcceptFilter.java @@ -25,7 +25,7 @@ import java.util.Objects; import org.slf4j.MDC; /** - * Keeps only the Compute Engine logs. + * Keeps only Compute Engine activity logs. */ public class CeActivityLogAcceptFilter extends Filter { diff --git a/server/sonar-server/src/main/java/org/sonar/ce/log/CeActivityLogDenyFilter.java b/server/sonar-server/src/main/java/org/sonar/ce/log/CeActivityLogDenyFilter.java new file mode 100644 index 00000000000..b1e361ab71d --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/ce/log/CeActivityLogDenyFilter.java @@ -0,0 +1,37 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.ce.log; + +import ch.qos.logback.core.filter.Filter; +import ch.qos.logback.core.spi.FilterReply; +import java.util.Objects; +import org.slf4j.MDC; + +/** + * Ignores Compute Engine activity logs. + */ +public class CeActivityLogDenyFilter extends Filter { + + @Override + public FilterReply decide(E o) { + return Objects.equals("true", MDC.get(CeLogging.MDC_CE_ACTIVITY_FLAG)) ? FilterReply.DENY : FilterReply.ACCEPT; + } + +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/app/CeProcessLogging.java b/server/sonar-server/src/main/java/org/sonar/ce/log/CeProcessLogging.java similarity index 59% rename from server/sonar-server/src/main/java/org/sonar/server/app/CeProcessLogging.java rename to server/sonar-server/src/main/java/org/sonar/ce/log/CeProcessLogging.java index 0a72d1a371e..6fdfd886812 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/app/CeProcessLogging.java +++ b/server/sonar-server/src/main/java/org/sonar/ce/log/CeProcessLogging.java @@ -17,18 +17,21 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -package org.sonar.server.app; +package org.sonar.ce.log; -import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; -import org.sonar.ce.log.CeLogging; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.ConsoleAppender; import org.sonar.process.LogbackHelper; import org.sonar.process.Props; +import org.sonar.server.app.ServerProcessLogging; + +import static org.slf4j.Logger.ROOT_LOGGER_NAME; /** * Configure logback for the Compute Engine process. * Logs are written to console, which is forwarded to file logs/sonar.log by the app master process. - * In addition, CE writes activity to "logs/ce_activity.log". + * In addition, CE writes activity logs to "logs/ce_activity.log". */ public class CeProcessLogging extends ServerProcessLogging { @@ -37,9 +40,12 @@ public class CeProcessLogging extends ServerProcessLogging { } @Override - protected void configureExtraAppenders(LoggerContext ctx, LogbackHelper helper, Props props) { - ctx.getLogger(Logger.ROOT_LOGGER_NAME).addAppender(CeLogging.createPerTaskAppenderConfiguration(ctx, props)); - ctx.getLogger(Logger.ROOT_LOGGER_NAME).addAppender(CeLogging.createCeActivityAppenderConfiguration(helper, ctx, props)); + protected void configureAppenders(String logFormat, LoggerContext ctx, LogbackHelper helper, Props props) { + ConsoleAppender consoleAppender = helper.newConsoleAppender(ctx, "CONSOLE", logFormat, + new CeTaskLogDenyFilter(), new CeActivityLogDenyFilter()); + ctx.getLogger(ROOT_LOGGER_NAME).addAppender(consoleAppender); + ctx.getLogger(ROOT_LOGGER_NAME).addAppender(CeLogging.createPerTaskAppenderConfiguration(ctx, props)); + ctx.getLogger(ROOT_LOGGER_NAME).addAppender(CeLogging.createCeActivityAppenderConfiguration(helper, ctx, props)); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/app/ServerProcessLogging.java b/server/sonar-server/src/main/java/org/sonar/server/app/ServerProcessLogging.java index ddd0fd142dd..783d46bac4a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/app/ServerProcessLogging.java +++ b/server/sonar-server/src/main/java/org/sonar/server/app/ServerProcessLogging.java @@ -19,26 +19,22 @@ */ package org.sonar.server.app; -import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; -import ch.qos.logback.classic.spi.ILoggingEvent; -import ch.qos.logback.core.ConsoleAppender; import java.util.logging.LogManager; import org.slf4j.bridge.SLF4JBridgeHandler; import org.sonar.api.utils.MessageException; import org.sonar.api.utils.log.LoggerLevel; -import org.sonar.ce.log.CeTaskLogDenyFilter; import org.sonar.process.LogbackHelper; import org.sonar.process.Props; import org.sonar.server.platform.ServerLogging; -abstract class ServerProcessLogging { +public abstract class ServerProcessLogging { private static final String LOG_LEVEL_PROPERTY = "sonar.log.level"; private static final String LOG_FORMAT = "%d{yyyy.MM.dd HH:mm:ss} %-5level XXXX[%logger{20}] %msg%n"; private final String processName; private final LogbackHelper helper = new LogbackHelper(); - ServerProcessLogging(String processName) { + protected ServerProcessLogging(String processName) { this.processName = processName; } @@ -58,12 +54,10 @@ abstract class ServerProcessLogging { private void configureAppenders(LoggerContext ctx, Props props) { String logFormat = LOG_FORMAT.replace("XXXX", processName); - ConsoleAppender consoleAppender = helper.newConsoleAppender(ctx, "CONSOLE", logFormat, new CeTaskLogDenyFilter()); - ctx.getLogger(Logger.ROOT_LOGGER_NAME).addAppender(consoleAppender); - configureExtraAppenders(ctx, helper, props); + configureAppenders(logFormat, ctx, helper, props); } - protected abstract void configureExtraAppenders(LoggerContext ctx, LogbackHelper helper, Props props); + protected abstract void configureAppenders(String logFormat, LoggerContext ctx, LogbackHelper helper, Props props); private void configureLevels(Props props) { String levelCode = props.value(LOG_LEVEL_PROPERTY, "INFO"); diff --git a/server/sonar-server/src/main/java/org/sonar/server/app/WebServerProcessLogging.java b/server/sonar-server/src/main/java/org/sonar/server/app/WebServerProcessLogging.java index 63b80e2cfe7..50709c12565 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/app/WebServerProcessLogging.java +++ b/server/sonar-server/src/main/java/org/sonar/server/app/WebServerProcessLogging.java @@ -19,7 +19,10 @@ */ package org.sonar.server.app; +import ch.qos.logback.classic.Logger; import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.ConsoleAppender; import org.sonar.process.LogbackHelper; import org.sonar.process.Props; @@ -34,8 +37,9 @@ public class WebServerProcessLogging extends ServerProcessLogging { } @Override - protected void configureExtraAppenders(LoggerContext ctx, LogbackHelper helper, Props props) { - // nothing to do + protected void configureAppenders(String logFormat, LoggerContext ctx, LogbackHelper helper, Props props) { + ConsoleAppender consoleAppender = helper.newConsoleAppender(ctx, "CONSOLE", logFormat); + ctx.getLogger(Logger.ROOT_LOGGER_NAME).addAppender(consoleAppender); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/taskprocessor/CeWorkerCallableImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/taskprocessor/CeWorkerCallableImpl.java index b4fd1c0d182..3f890a1d893 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/taskprocessor/CeWorkerCallableImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/taskprocessor/CeWorkerCallableImpl.java @@ -99,7 +99,7 @@ public class CeWorkerCallableImpl implements CeWorkerCallable { private static Profiler startProfiler(CeTask task) { Profiler profiler = Profiler.create(LOG); addContext(profiler, task); - return profiler.startInfo("Execute task"); + return profiler.startDebug("Execute task"); } private static Profiler startActivityProfiler(CeTask task) { @@ -109,11 +109,15 @@ public class CeWorkerCallableImpl implements CeWorkerCallable { } private static void stopProfiler(Profiler profiler, CeTask task, CeActivityDto.Status status) { + if (!profiler.isDebugEnabled()) { + return; + } + addContext(profiler, task); if (status == CeActivityDto.Status.FAILED) { profiler.stopError("Executed task"); } else { - profiler.stopInfo("Executed task"); + profiler.stopDebug("Executed task"); } } diff --git a/server/sonar-server/src/test/java/org/sonar/ce/log/CeActivityLogAcceptFilterTest.java b/server/sonar-server/src/test/java/org/sonar/ce/log/CeActivityLogAcceptFilterTest.java new file mode 100644 index 00000000000..3dcb4b4abb9 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/ce/log/CeActivityLogAcceptFilterTest.java @@ -0,0 +1,63 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.ce.log; + +import ch.qos.logback.core.spi.FilterReply; +import org.apache.log4j.MDC; +import org.junit.After; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.ce.log.CeLogging.MDC_CE_ACTIVITY_FLAG; + +public class CeActivityLogAcceptFilterTest { + private static final Object UNUSED = ""; + + private CeActivityLogAcceptFilter underTest = new CeActivityLogAcceptFilter(); + + @After + public void tearDown() { + MDC.clear(); + } + + @Test + public void rejects_logs_when_property_is_not_set_in_MDC() { + assertThat(underTest.decide(UNUSED)).isEqualTo(FilterReply.DENY); + } + + @Test + public void rejects_logs_when_property_is_not_true_in_MDC() { + MDC.put(MDC_CE_ACTIVITY_FLAG, "bla"); + assertThat(underTest.decide(UNUSED)).isEqualTo(FilterReply.DENY); + } + + @Test + public void rejects_logs_when_property_is_not_true_in_lowercase_in_MDC() { + MDC.put(MDC_CE_ACTIVITY_FLAG, "TrUE"); + assertThat(underTest.decide(UNUSED)).isEqualTo(FilterReply.DENY); + } + + @Test + public void accepts_logs_when_property_is_true_in_lowercase_in_MDC() { + MDC.put(MDC_CE_ACTIVITY_FLAG, "true"); + assertThat(underTest.decide(UNUSED)).isEqualTo(FilterReply.ACCEPT); + } + +} diff --git a/server/sonar-server/src/test/java/org/sonar/ce/log/CeActivityLogDenyFilterTest.java b/server/sonar-server/src/test/java/org/sonar/ce/log/CeActivityLogDenyFilterTest.java new file mode 100644 index 00000000000..f6ad1ad5e8c --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/ce/log/CeActivityLogDenyFilterTest.java @@ -0,0 +1,62 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.ce.log; + +import ch.qos.logback.core.spi.FilterReply; +import org.apache.log4j.MDC; +import org.junit.After; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.ce.log.CeLogging.MDC_CE_ACTIVITY_FLAG; + +public class CeActivityLogDenyFilterTest { + private static final Object UNUSED = ""; + + private CeActivityLogDenyFilter underTest = new CeActivityLogDenyFilter(); + + @After + public void tearDown() { + MDC.clear(); + } + + @Test + public void accepts_logs_when_property_is_not_set_in_MDC() { + assertThat(underTest.decide(UNUSED)).isEqualTo(FilterReply.ACCEPT); + } + + @Test + public void acceots_logs_when_property_is_not_true_in_MDC() { + MDC.put(MDC_CE_ACTIVITY_FLAG, "bla"); + assertThat(underTest.decide(UNUSED)).isEqualTo(FilterReply.ACCEPT); + } + + @Test + public void accepts_logs_when_property_is_not_true_in_lowercase_in_MDC() { + MDC.put(MDC_CE_ACTIVITY_FLAG, "TrUE"); + assertThat(underTest.decide(UNUSED)).isEqualTo(FilterReply.ACCEPT); + } + + @Test + public void rejects_logs_when_property_is_true_in_lowercase_in_MDC() { + MDC.put(MDC_CE_ACTIVITY_FLAG, "true"); + assertThat(underTest.decide(UNUSED)).isEqualTo(FilterReply.DENY); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/app/CeProcessLoggingTest.java b/server/sonar-server/src/test/java/org/sonar/ce/log/CeProcessLoggingTest.java similarity index 94% rename from server/sonar-server/src/test/java/org/sonar/server/app/CeProcessLoggingTest.java rename to server/sonar-server/src/test/java/org/sonar/ce/log/CeProcessLoggingTest.java index 31cc050f6cf..50bc9f95413 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/app/CeProcessLoggingTest.java +++ b/server/sonar-server/src/test/java/org/sonar/ce/log/CeProcessLoggingTest.java @@ -17,7 +17,7 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -package org.sonar.server.app; +package org.sonar.ce.log; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.Logger; @@ -32,7 +32,6 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; -import org.sonar.ce.log.CeFileAppenderFactory; import org.sonar.process.LogbackHelper; import org.sonar.process.ProcessProperties; import org.sonar.process.Props; @@ -46,8 +45,8 @@ public class CeProcessLoggingTest { @Rule public TemporaryFolder temp = new TemporaryFolder(); - Props props = new Props(new Properties()); - CeProcessLogging underTest = new CeProcessLogging(); + private Props props = new Props(new Properties()); + private CeProcessLogging underTest = new CeProcessLogging(); /** * Path to data dir must be set for Compute Engine logging. diff --git a/server/sonar-server/src/test/java/org/sonar/server/app/ProgrammaticLogbackValveTest.java b/server/sonar-server/src/test/java/org/sonar/server/app/ProgrammaticLogbackValveTest.java index a84a7368a04..d8a4ae95ffe 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/app/ProgrammaticLogbackValveTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/app/ProgrammaticLogbackValveTest.java @@ -41,7 +41,7 @@ public class ProgrammaticLogbackValveTest { ProgrammaticLogbackValve valve = new ProgrammaticLogbackValve(); valve.setContainer(mock(Container.class)); LogbackHelper helper = new LogbackHelper(); - ConsoleAppender appender = helper.newConsoleAppender(valve, "CONSOLE", "combined", null); + ConsoleAppender appender = helper.newConsoleAppender(valve, "CONSOLE", "combined"); valve.addAppender(appender); valve.start(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/app/ServerProcessLoggingTest.java b/server/sonar-server/src/test/java/org/sonar/server/app/WebServerProcessLoggingTest.java similarity index 89% rename from server/sonar-server/src/test/java/org/sonar/server/app/ServerProcessLoggingTest.java rename to server/sonar-server/src/test/java/org/sonar/server/app/WebServerProcessLoggingTest.java index 8a180c5c3a4..68d4ef48292 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/app/ServerProcessLoggingTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/app/WebServerProcessLoggingTest.java @@ -33,18 +33,12 @@ import org.sonar.process.Props; import static org.assertj.core.api.Assertions.assertThat; -public class ServerProcessLoggingTest { +public class WebServerProcessLoggingTest { + private WebServerProcessLogging underTest = new WebServerProcessLogging(); private static final String LOG_LEVEL_PROPERTY = "sonar.log.level"; - private static final String PROCESS_NAME = "pr1"; Props props = new Props(new Properties()); - ServerProcessLogging underTest = new ServerProcessLogging(PROCESS_NAME) { - @Override - protected void configureExtraAppenders(LoggerContext ctx, LogbackHelper helper, Props props) { - // nothing to do - } - }; @AfterClass public static void resetLogback() throws JoranException { @@ -78,4 +72,5 @@ public class ServerProcessLoggingTest { LoggerContext ctx = underTest.configure(props); assertThat(ctx.getLogger(Logger.ROOT_LOGGER_NAME).getLevel()).isEqualTo(Level.TRACE); } + } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/taskprocessor/CeWorkerCallableImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/taskprocessor/CeWorkerCallableImplTest.java index e9b41a03b07..27c7c388794 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/taskprocessor/CeWorkerCallableImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/taskprocessor/CeWorkerCallableImplTest.java @@ -105,10 +105,6 @@ public class CeWorkerCallableImplTest { inOrder.verify(ceLogging).clearForTask(); } - private void makeTaskProcessorFail(CeTask task) { - doThrow(new IllegalStateException("simulate exception thrown by TaskProcessor#process")).when(taskProcessor).process(task); - } - @Test public void do_not_display_submitter_param_in_log_when_submitterLogin_is_not_set_in_case_of_success() throws Exception { when(queue.peek()).thenReturn(Optional.of(createCeTask(null))); @@ -117,8 +113,8 @@ public class CeWorkerCallableImplTest { underTest.call(); List logs = logTester.logs(LoggerLevel.INFO); - assertThat(logs).hasSize(4); - for (int i = 0; i < 4; i++) { + assertThat(logs).hasSize(2); + for (int i = 0; i < 2; i++) { assertThat(logs.get(i)).doesNotContain(" | submitter="); } } @@ -133,15 +129,14 @@ public class CeWorkerCallableImplTest { underTest.call(); List logs = logTester.logs(LoggerLevel.INFO); + assertThat(logs).hasSize(1); + assertThat(logs.get(0)).doesNotContain(" | submitter="); + logs = logTester.logs(LoggerLevel.ERROR); assertThat(logs).hasSize(2); for (int i = 0; i < 2; i++) { assertThat(logs.get(i)).doesNotContain(" | submitter="); } - logs = logTester.logs(LoggerLevel.ERROR); - assertThat(logs).hasSize(3); - for (int i = 0; i < 3; i++) { - assertThat(logs.get(i)).doesNotContain(" | submitter="); - } + assertThat(logTester.logs(LoggerLevel.DEBUG)).isEmpty(); } @Test @@ -152,13 +147,11 @@ public class CeWorkerCallableImplTest { underTest.call(); List logs = logTester.logs(LoggerLevel.INFO); - assertThat(logs).hasSize(4); - for (int i = 0; i < 2; i++) { - assertThat(logs.get(i)).contains(" | submitter=FooBar"); - } - for (int i = 2; i < 4; i++) { - assertThat(logs.get(i)).contains(" | submitter=FooBar | time="); - } + assertThat(logs).hasSize(2); + assertThat(logs.get(0)).contains(" | submitter=FooBar"); + assertThat(logs.get(1)).contains(" | submitter=FooBar | time="); + assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); + assertThat(logTester.logs(LoggerLevel.DEBUG)).isEmpty(); } @Test @@ -171,20 +164,63 @@ public class CeWorkerCallableImplTest { underTest.call(); List logs = logTester.logs(LoggerLevel.INFO); + assertThat(logs).hasSize(1); + assertThat(logs.iterator().next()).contains(" | submitter=FooBar"); + logs = logTester.logs(LoggerLevel.ERROR); assertThat(logs).hasSize(2); - for (int i = 0; i < 2; i++) { - assertThat(logs.get(i)).contains(" | submitter=FooBar"); - } + assertThat(logs.get(0)).isEqualTo("Failed to execute task " + ceTask.getUuid()); + assertThat(logs.get(1)).contains(" | submitter=FooBar | time="); + } + + @Test + public void display_start_stop_at_debug_level_for_console_if_DEBUG_is_enabled_and_task_successful() throws Exception { + logTester.setLevel(LoggerLevel.DEBUG); + + when(queue.peek()).thenReturn(Optional.of(createCeTask("FooBar"))); + taskProcessorRepository.setProcessorForTask(CeTaskTypes.REPORT, taskProcessor); + + underTest.call(); + + List logs = logTester.logs(LoggerLevel.INFO); + assertThat(logs).hasSize(2); + assertThat(logs.get(0)).contains(" | submitter=FooBar"); + assertThat(logs.get(1)).contains(" | submitter=FooBar | time="); + assertThat(logTester.logs(LoggerLevel.ERROR)).isEmpty(); + logs = logTester.logs(LoggerLevel.DEBUG); + assertThat(logs).hasSize(2); + assertThat(logs.get(0)).contains(" | submitter=FooBar"); + assertThat(logs.get(1)).contains(" | submitter=FooBar | time="); + } + + @Test + public void display_start_at_debug_level_stop_at_error_level_for_console_if_DEBUG_is_enabled_and_task_failed() throws Exception { + logTester.setLevel(LoggerLevel.DEBUG); + + CeTask ceTask = createCeTask("FooBar"); + when(queue.peek()).thenReturn(Optional.of(ceTask)); + taskProcessorRepository.setProcessorForTask(CeTaskTypes.REPORT, taskProcessor); + makeTaskProcessorFail(ceTask); + + underTest.call(); + + List logs = logTester.logs(LoggerLevel.INFO); + assertThat(logs).hasSize(1); + assertThat(logs.iterator().next()).contains(" | submitter=FooBar"); logs = logTester.logs(LoggerLevel.ERROR); assertThat(logs).hasSize(3); - for (int i = 0; i < 3; i++) { - String log = logs.get(i); - assertThat(log.contains(" | submitter=FooBar | time=") || log.equals("Failed to execute task " + ceTask.getUuid())) - .isTrue(); - } + assertThat(logs.get(0)).isEqualTo("Failed to execute task " + ceTask.getUuid()); + assertThat(logs.get(1)).contains(" | submitter=FooBar | time="); + assertThat(logs.get(2)).contains(" | submitter=FooBar | time="); + logs = logTester.logs(LoggerLevel.DEBUG); + assertThat(logs).hasSize(1); + assertThat(logs.get(0)).contains(" | submitter=FooBar"); } private static CeTask createCeTask(@Nullable String submitterLogin) { return new CeTask.Builder().setUuid("TASK_1").setType(CeTaskTypes.REPORT).setComponentUuid("PROJECT_1").setSubmitterLogin(submitterLogin).build(); } + + private void makeTaskProcessorFail(CeTask task) { + doThrow(new IllegalStateException("simulate exception thrown by TaskProcessor#process")).when(taskProcessor).process(task); + } } diff --git a/sonar-application/src/main/java/org/sonar/application/AppLogging.java b/sonar-application/src/main/java/org/sonar/application/AppLogging.java index 190a5fecd43..5679f75d56f 100644 --- a/sonar-application/src/main/java/org/sonar/application/AppLogging.java +++ b/sonar-application/src/main/java/org/sonar/application/AppLogging.java @@ -70,7 +70,7 @@ class AppLogging { } private void configureConsole(LoggerContext loggerContext) { - ConsoleAppender consoleAppender = helper.newConsoleAppender(loggerContext, CONSOLE_APPENDER, "%msg%n", null); + ConsoleAppender consoleAppender = helper.newConsoleAppender(loggerContext, CONSOLE_APPENDER, "%msg%n"); Logger consoleLogger = loggerContext.getLogger(CONSOLE_LOGGER); consoleLogger.setAdditive(false); consoleLogger.addAppender(consoleAppender); @@ -95,7 +95,7 @@ class AppLogging { } private void configureRoot(Props props, LoggerContext loggerContext) { - ConsoleAppender consoleAppender = helper.newConsoleAppender(loggerContext, "ROOT_CONSOLE", APP_PATTERN, null); + ConsoleAppender consoleAppender = helper.newConsoleAppender(loggerContext, "ROOT_CONSOLE", APP_PATTERN); Logger rootLogger = loggerContext.getLogger(Logger.ROOT_LOGGER_NAME); rootLogger.setLevel(Level.toLevel(props.value("sonar.app.log.level", Level.INFO.toString()), Level.INFO)); rootLogger.addAppender(consoleAppender); -- 2.39.5