diff options
7 files changed, 243 insertions, 44 deletions
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 7a86fe5dd4a..6e513626d8f 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 @@ -37,13 +37,13 @@ import ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy; import ch.qos.logback.core.rolling.TimeBasedRollingPolicy; import java.io.File; import java.util.Arrays; +import java.util.Collection; import java.util.HashSet; import java.util.Set; import javax.annotation.CheckForNull; import org.apache.commons.lang.StringUtils; import org.slf4j.LoggerFactory; -import static java.util.Collections.unmodifiableSet; import static java.util.Objects.requireNonNull; import static org.slf4j.Logger.ROOT_LOGGER_NAME; @@ -60,7 +60,11 @@ public class LogbackHelper { private static final String ROLLING_POLICY_PROPERTY = "sonar.log.rollingPolicy"; private static final String MAX_FILES_PROPERTY = "sonar.log.maxFiles"; private static final String LOG_FORMAT = "%d{yyyy.MM.dd HH:mm:ss} %-5level " + PROCESS_NAME_PLACEHOLDER + "[" + THREAD_ID_PLACEHOLDER + "][%logger{20}] %msg%n"; - public static final Set<Level> ALLOWED_ROOT_LOG_LEVELS = unmodifiableSet(setOf(Level.TRACE, Level.DEBUG, Level.INFO)); + private static final Level[] ALLOWED_ROOT_LOG_LEVELS = new Level[] {Level.TRACE, Level.DEBUG, Level.INFO}; + + public static Collection<Level> allowedLogLevels() { + return Arrays.asList(ALLOWED_ROOT_LOG_LEVELS); + } public LoggerContext getRootContext() { org.slf4j.Logger logger; @@ -165,8 +169,8 @@ public class LogbackHelper { public String buildLogPattern(LogbackHelper.RootLoggerConfig config) { return LOG_FORMAT - .replace(PROCESS_NAME_PLACEHOLDER, config.getProcessName()) - .replace(THREAD_ID_PLACEHOLDER, config.getThreadIdFieldPattern()); + .replace(PROCESS_NAME_PLACEHOLDER, config.getProcessName()) + .replace(THREAD_ID_PLACEHOLDER, config.getThreadIdFieldPattern()); } /** @@ -257,8 +261,8 @@ public class LogbackHelper { */ public Level configureRootLogLevel(Props props, ProcessId processId) { return configureRootLogLevel(props, - SONAR_LOG_LEVEL_PROPERTY, - SONAR_PROCESS_LOG_LEVEL_PROPERTY.replace(PROCESS_NAME_PLACEHOLDER, processId.getKey())); + SONAR_LOG_LEVEL_PROPERTY, + SONAR_PROCESS_LOG_LEVEL_PROPERTY.replace(PROCESS_NAME_PLACEHOLDER, processId.getKey())); } /** @@ -268,14 +272,37 @@ public class LogbackHelper { * @throws IllegalArgumentException if the value of the specified property is not one of {@link #ALLOWED_ROOT_LOG_LEVELS} */ public Level configureRootLogLevel(Props props, String... propertyKeys) { + return configureLoggerLogLevel(getRootContext().getLogger(ROOT_LOGGER_NAME), props, propertyKeys); + } + + private Level configureLoggerLogLevel(Logger logger, Props props, String... propertyKeys) { Level newLevel = Level.INFO; for (String propertyKey : propertyKeys) { - Level level = Level.toLevel(props.value(propertyKey, Level.INFO.toString()), Level.INFO); + Level level = getPropertyValueAsLevel(props, propertyKey); if (!level.isGreaterOrEqual(newLevel)) { newLevel = level; } } - return configureRootLogLevel(newLevel); + logger.setLevel(newLevel); + return newLevel; + } + + private static Level getPropertyValueAsLevel(Props props, String propertyKey) { + Level level = Level.toLevel(props.value(propertyKey, Level.INFO.toString()), Level.INFO); + if (!isAllowed(level)) { + throw new IllegalArgumentException(String.format("log level %s in property %s is not a supported value (allowed levels are %s)", + level, propertyKey, Arrays.toString(ALLOWED_ROOT_LOG_LEVELS))); + } + return level; + } + + private static boolean isAllowed(Level level) { + for (Level allowedRootLogLevel : ALLOWED_ROOT_LOG_LEVELS) { + if (level == allowedRootLogLevel) { + return true; + } + } + return false; } /** @@ -285,13 +312,17 @@ public class LogbackHelper { */ public Level configureRootLogLevel(Level newLevel) { Logger rootLogger = getRootContext().getLogger(ROOT_LOGGER_NAME); - if (!ALLOWED_ROOT_LOG_LEVELS.contains(newLevel)) { - throw new IllegalArgumentException(String.format("%s log level is not supported (allowed levels are %s)", newLevel, ALLOWED_ROOT_LOG_LEVELS)); - } + ensureSupportedLevel(newLevel); rootLogger.setLevel(newLevel); return newLevel; } + private static void ensureSupportedLevel(Level newLevel) { + if (!isAllowed(newLevel)) { + throw new IllegalArgumentException(String.format("%s log level is not supported (allowed levels are %s)", newLevel, Arrays.toString(ALLOWED_ROOT_LOG_LEVELS))); + } + } + /** * Configure the log level of the specified logger to specified level. * <p> 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 5a35bd69c65..a7167c9b33e 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 @@ -39,7 +39,7 @@ public class SearchLogging { helper.configureGlobalFileLog(props, config, logPattern); helper.configureForSubprocessGobbler(props, logPattern); // SQ's global log level must not change ES's log level - helper.configureRootLogLevel(props, "sonar.log.es.level"); + helper.configureRootLogLevel(props, "sonar.log.level.es"); return ctx; } diff --git a/server/sonar-search/src/test/java/org/sonar/search/SearchLoggingTest.java b/server/sonar-search/src/test/java/org/sonar/search/SearchLoggingTest.java index cd4533691b9..b7f2fa60afd 100644 --- a/server/sonar-search/src/test/java/org/sonar/search/SearchLoggingTest.java +++ b/server/sonar-search/src/test/java/org/sonar/search/SearchLoggingTest.java @@ -19,6 +19,7 @@ */ package org.sonar.search; +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; @@ -38,6 +39,7 @@ import org.sonar.process.ProcessProperties; import org.sonar.process.Props; import static org.assertj.core.api.Assertions.assertThat; +import static org.slf4j.Logger.ROOT_LOGGER_NAME; public class SearchLoggingTest { @Rule @@ -63,7 +65,7 @@ public class SearchLoggingTest { public void do_not_log_to_console() { LoggerContext ctx = underTest.configure(props); - Logger root = ctx.getLogger(Logger.ROOT_LOGGER_NAME); + Logger root = ctx.getLogger(ROOT_LOGGER_NAME); Appender appender = root.getAppender("CONSOLE"); assertThat(appender).isNull(); } @@ -72,7 +74,7 @@ public class SearchLoggingTest { public void log_to_es_file() { LoggerContext ctx = underTest.configure(props); - Logger root = ctx.getLogger(Logger.ROOT_LOGGER_NAME); + Logger root = ctx.getLogger(ROOT_LOGGER_NAME); Appender<ILoggingEvent> appender = root.getAppender("file_es"); assertThat(appender).isInstanceOf(FileAppender.class); FileAppender fileAppender = (FileAppender) appender; @@ -81,4 +83,24 @@ public class SearchLoggingTest { PatternLayoutEncoder encoder = (PatternLayoutEncoder) fileAppender.getEncoder(); assertThat(encoder.getPattern()).isEqualTo("%d{yyyy.MM.dd HH:mm:ss} %-5level es[][%logger{20}] %msg%n"); } + + @Test + public void root_log_level_does_not_change_with_global_property_value() { + props.set("sonar.log.level", "TRACE"); + + LoggerContext ctx = underTest.configure(props); + + Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME); + assertThat(rootLogger.getLevel()).isEqualTo(Level.INFO); + } + + @Test + public void root_log_level_change_with_es_property_value() { + props.set("sonar.log.level.es", "TRACE"); + + LoggerContext ctx = underTest.configure(props); + + Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME); + assertThat(rootLogger.getLevel()).isEqualTo(Level.TRACE); + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/ws/ChangeLogLevelAction.java b/server/sonar-server/src/main/java/org/sonar/server/platform/ws/ChangeLogLevelAction.java index e64eac0db8f..74917769dab 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/ws/ChangeLogLevelAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/ws/ChangeLogLevelAction.java @@ -28,7 +28,7 @@ import org.sonar.db.Database; import org.sonar.server.platform.ServerLogging; import org.sonar.server.user.UserSession; -import static org.sonar.process.LogbackHelper.ALLOWED_ROOT_LOG_LEVELS; +import static org.sonar.process.LogbackHelper.allowedLogLevels; public class ChangeLogLevelAction implements SystemWsAction { @@ -57,7 +57,7 @@ public class ChangeLogLevelAction implements SystemWsAction { newAction.createParam(PARAM_LEVEL) .setDescription("The new level. Be cautious: DEBUG, and even more TRACE, may have performance impacts.") - .setPossibleValues(ALLOWED_ROOT_LOG_LEVELS) + .setPossibleValues(allowedLogLevels()) .setRequired(true); } diff --git a/server/sonar-server/src/test/java/org/sonar/ce/log/CeProcessLoggingTest.java b/server/sonar-server/src/test/java/org/sonar/ce/log/CeProcessLoggingTest.java index 6b2dbfd704d..8d2999e23e6 100644 --- a/server/sonar-server/src/test/java/org/sonar/ce/log/CeProcessLoggingTest.java +++ b/server/sonar-server/src/test/java/org/sonar/ce/log/CeProcessLoggingTest.java @@ -34,19 +34,21 @@ import org.junit.AfterClass; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.rules.TemporaryFolder; import org.sonar.process.LogbackHelper; import org.sonar.process.ProcessProperties; import org.sonar.process.Props; import static org.assertj.core.api.Assertions.assertThat; +import static org.slf4j.Logger.ROOT_LOGGER_NAME; public class CeProcessLoggingTest { - private static final String LOG_LEVEL_PROPERTY = "sonar.log.level"; - @Rule public TemporaryFolder temp = new TemporaryFolder(); + @Rule + public ExpectedException expectedException = ExpectedException.none(); private File logDir; private Props props = new Props(new Properties()); @@ -87,16 +89,69 @@ public class CeProcessLoggingTest { } @Test - public void enable_debug_logs() { - props.set(LOG_LEVEL_PROPERTY, "DEBUG"); + public void default_level_for_root_logger_is_INFO() { + LoggerContext ctx = underTest.configure(props); + + verifyRootLogLevel(ctx, Level.INFO); + } + + @Test + public void root_logger_level_changes_with_global_property() { + props.set("sonar.log.level", "TRACE"); + + LoggerContext ctx = underTest.configure(props); + + verifyRootLogLevel(ctx, Level.TRACE); + } + + @Test + public void root_logger_level_changes_with_app_property() { + props.set("sonar.log.level.ce", "TRACE"); + + LoggerContext ctx = underTest.configure(props); + + verifyRootLogLevel(ctx, Level.TRACE); + } + + @Test + public void root_logger_level_changes_with_app_property_and_is_case_insensitive() { + props.set("sonar.log.level.ce", "debug"); + LoggerContext ctx = underTest.configure(props); - assertThat(ctx.getLogger(Logger.ROOT_LOGGER_NAME).getLevel()).isEqualTo(Level.DEBUG); + + verifyRootLogLevel(ctx, Level.DEBUG); } @Test - public void enable_trace_logs() { - props.set(LOG_LEVEL_PROPERTY, "TRACE"); + public void default_to_INFO_if_app_property_has_invalid_value() { + props.set("sonar.log.level.ce", "DodoDouh!"); + LoggerContext ctx = underTest.configure(props); - assertThat(ctx.getLogger(Logger.ROOT_LOGGER_NAME).getLevel()).isEqualTo(Level.TRACE); + verifyRootLogLevel(ctx, Level.INFO); + } + + @Test + public void fail_with_IAE_if_global_property_unsupported_level() { + props.set("sonar.log.level", "ERROR"); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("log level ERROR in property sonar.log.level is not a supported value (allowed levels are [TRACE, DEBUG, INFO])"); + + underTest.configure(props); + } + + @Test + public void fail_with_IAE_if_app_property_unsupported_level() { + props.set("sonar.log.level.ce", "ERROR"); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("log level ERROR in property sonar.log.level.ce is not a supported value (allowed levels are [TRACE, DEBUG, INFO])"); + + underTest.configure(props); + } + + private void verifyRootLogLevel(LoggerContext ctx, Level expected) { + Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME); + assertThat(rootLogger.getLevel()).isEqualTo(expected); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/app/WebServerProcessLoggingTest.java b/server/sonar-server/src/test/java/org/sonar/server/app/WebServerProcessLoggingTest.java index 0ab5bc96715..02e6127f882 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/app/WebServerProcessLoggingTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/app/WebServerProcessLoggingTest.java @@ -34,19 +34,21 @@ import org.junit.AfterClass; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.rules.TemporaryFolder; import org.sonar.process.LogbackHelper; import org.sonar.process.ProcessProperties; import org.sonar.process.Props; import static org.assertj.core.api.Assertions.assertThat; +import static org.slf4j.Logger.ROOT_LOGGER_NAME; public class WebServerProcessLoggingTest { - private static final String LOG_LEVEL_PROPERTY = "sonar.log.level"; - @Rule public TemporaryFolder temp = new TemporaryFolder(); + @Rule + public ExpectedException expectedException = ExpectedException.none(); private File logDir; private Props props = new Props(new Properties()); @@ -87,17 +89,70 @@ public class WebServerProcessLoggingTest { } @Test - public void enable_debug_logs() { - props.set(LOG_LEVEL_PROPERTY, "DEBUG"); + public void default_level_for_root_logger_is_INFO() { + LoggerContext ctx = underTest.configure(props); + + verifyRootLogLevel(ctx, Level.INFO); + } + + @Test + public void root_logger_level_changes_with_global_property() { + props.set("sonar.log.level", "TRACE"); + + LoggerContext ctx = underTest.configure(props); + + verifyRootLogLevel(ctx, Level.TRACE); + } + + @Test + public void root_logger_level_changes_with_app_property() { + props.set("sonar.log.level.web", "TRACE"); + + LoggerContext ctx = underTest.configure(props); + + verifyRootLogLevel(ctx, Level.TRACE); + } + + @Test + public void root_logger_level_changes_with_app_property_and_is_case_insensitive() { + props.set("sonar.log.level.web", "debug"); + LoggerContext ctx = underTest.configure(props); - assertThat(ctx.getLogger(Logger.ROOT_LOGGER_NAME).getLevel()).isEqualTo(Level.DEBUG); + + verifyRootLogLevel(ctx, Level.DEBUG); } @Test - public void enable_trace_logs() { - props.set(LOG_LEVEL_PROPERTY, "TRACE"); + public void default_to_INFO_if_app_property_has_invalid_value() { + props.set("sonar.log.level.web", "DodoDouh!"); + LoggerContext ctx = underTest.configure(props); - assertThat(ctx.getLogger(Logger.ROOT_LOGGER_NAME).getLevel()).isEqualTo(Level.TRACE); + verifyRootLogLevel(ctx, Level.INFO); + } + + @Test + public void fail_with_IAE_if_global_property_unsupported_level() { + props.set("sonar.log.level", "ERROR"); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("log level ERROR in property sonar.log.level is not a supported value (allowed levels are [TRACE, DEBUG, INFO])"); + + underTest.configure(props); + } + + @Test + public void fail_with_IAE_if_app_property_unsupported_level() { + props.set("sonar.log.level.web", "ERROR"); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("log level ERROR in property sonar.log.level.web is not a supported value (allowed levels are [TRACE, DEBUG, INFO])"); + + underTest.configure(props); + } + + private void verifyRootLogLevel(LoggerContext ctx, Level expected) { + Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME); + assertThat(rootLogger.getLevel()).isEqualTo(expected); } } diff --git a/sonar-application/src/test/java/org/sonar/application/AppLoggingTest.java b/sonar-application/src/test/java/org/sonar/application/AppLoggingTest.java index d53a868482b..d9505785f70 100644 --- a/sonar-application/src/test/java/org/sonar/application/AppLoggingTest.java +++ b/sonar-application/src/test/java/org/sonar/application/AppLoggingTest.java @@ -37,6 +37,7 @@ import org.junit.AfterClass; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.rules.TemporaryFolder; import org.sonar.process.LogbackHelper; import org.sonar.process.ProcessProperties; @@ -50,6 +51,8 @@ public class AppLoggingTest { @Rule public TemporaryFolder temp = new TemporaryFolder(); + @Rule + public ExpectedException expectedException = ExpectedException.none(); private File logDir; @@ -172,35 +175,63 @@ public class AppLoggingTest { @Test public void default_level_for_root_logger_is_INFO() { LoggerContext ctx = underTest.configure(props); - Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME); - assertThat(rootLogger.getLevel()).isEqualTo(Level.INFO); + + verifyRootLogLevel(ctx, Level.INFO); } @Test - public void root_logger_level_can_be_changed_with_a_property() { + public void root_logger_level_changes_with_global_property() { + props.set("sonar.log.level", "TRACE"); + + LoggerContext ctx = underTest.configure(props); + + verifyRootLogLevel(ctx, Level.TRACE); + } + + @Test + public void root_logger_level_changes_with_app_property() { props.set("sonar.log.level.app", "TRACE"); LoggerContext ctx = underTest.configure(props); - Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME); - assertThat(rootLogger.getLevel()).isEqualTo(Level.TRACE); + + verifyRootLogLevel(ctx, Level.TRACE); } @Test - public void property_changing_root_logger_level_is_case_insensitive() { - props.set("sonar.log.level.app", "trace"); + public void root_logger_level_changes_with_app_property_and_is_case_insensitive() { + props.set("sonar.log.level.app", "debug"); LoggerContext ctx = underTest.configure(props); - Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME); - assertThat(rootLogger.getLevel()).isEqualTo(Level.TRACE); + + verifyRootLogLevel(ctx, Level.DEBUG); } @Test - public void default_to_INFO_if_property_changing_root_logger_level_has_invalid_value() { + public void default_to_INFO_if_app_property_has_invalid_value() { props.set("sonar.log.level.app", "DodoDouh!"); LoggerContext ctx = underTest.configure(props); - Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME); - assertThat(rootLogger.getLevel()).isEqualTo(Level.INFO); + verifyRootLogLevel(ctx, Level.INFO); + } + + @Test + public void fail_with_IAE_if_global_property_unsupported_level() { + props.set("sonar.log.level", "ERROR"); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("log level ERROR in property sonar.log.level is not a supported value (allowed levels are [TRACE, DEBUG, INFO])"); + + underTest.configure(props); + } + + @Test + public void fail_with_IAE_if_app_property_unsupported_level() { + props.set("sonar.log.level.app", "ERROR"); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("log level ERROR in property sonar.log.level.app is not a supported value (allowed levels are [TRACE, DEBUG, INFO])"); + + underTest.configure(props); } private void emulateRunFromSonarScript() { @@ -251,4 +282,9 @@ public class AppLoggingTest { PatternLayoutEncoder patternEncoder = (PatternLayoutEncoder) encoder; assertThat(patternEncoder.getPattern()).isEqualTo(logPattern); } + + private void verifyRootLogLevel(LoggerContext ctx, Level expected) { + Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME); + assertThat(rootLogger.getLevel()).isEqualTo(expected); + } } |