From db80217f161ac24023e50bfb8c5628e436df6584 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Fri, 18 Nov 2016 10:39:53 +0100 Subject: [PATCH] SONAR-8335 support global log level in ES JVM as we do for other JVM --- .../java/org/sonar/process/LogbackHelper.java | 28 ++------ .../java/org/sonar/search/SearchLogging.java | 3 +- .../org/sonar/search/SearchLoggingTest.java | 70 +++++++++++++++++-- .../app/WebServerProcessLoggingTest.java | 2 +- .../src/main/assembly/conf/sonar.properties | 6 +- 5 files changed, 77 insertions(+), 32 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 c5ee0e2c66c..ac9351d8ad2 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 @@ -39,7 +39,6 @@ import java.io.File; import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; import java.util.Set; import javax.annotation.CheckForNull; import org.apache.commons.lang.StringUtils; @@ -107,7 +106,7 @@ public class LogbackHelper { return new Builder(); } - public ProcessId getProcessId() { + ProcessId getProcessId() { return processId; } @@ -117,7 +116,7 @@ public class LogbackHelper { public static final class Builder { @CheckForNull - public ProcessId processId; + private ProcessId processId; private String threadIdFieldPattern = ""; private Builder() { @@ -220,40 +219,27 @@ public class LogbackHelper { return props.valueAsBoolean(ALL_LOGS_TO_CONSOLE_PROPERTY, false); } - @SafeVarargs - private static Set setOf(T... args) { - Set res = new HashSet<>(args.length); - res.addAll(Arrays.asList(args)); - return res; - } - /** * Configure the log level of the root logger reading the value of property {@link #SONAR_LOG_LEVEL_PROPERTY}. * * @throws IllegalArgumentException if the value of {@link #SONAR_LOG_LEVEL_PROPERTY} is not one of {@link #ALLOWED_ROOT_LOG_LEVELS} */ 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())); + Level newLevel = resolveLevel(props, SONAR_LOG_LEVEL_PROPERTY, SONAR_PROCESS_LOG_LEVEL_PROPERTY.replace(PROCESS_NAME_PLACEHOLDER, processId.getKey())); + getRootContext().getLogger(ROOT_LOGGER_NAME).setLevel(newLevel); + return newLevel; } /** - * Configure the log level of the root logger reading the value of specified properties. + * Resolve a log level reading the value of specified properties. *

* To compute the applied log level the following rules will be followed: *

    the last property with a defined and valid value in the order of the {@code propertyKeys} argument will be applied
- *
    if there is none, {@link Level#INFO INFO} will apply
+ *
    if there is none, {@link Level#INFO INFO} will be returned
*

* * @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) { - Level newLevel = resolveLevel(props, propertyKeys); - getRootContext().getLogger(ROOT_LOGGER_NAME).setLevel(newLevel); - return newLevel; - } - private static Level resolveLevel(Props props, String... propertyKeys) { Level newLevel = Level.INFO; for (String propertyKey : propertyKeys) { 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 04136ab0698..d0216cc89f6 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,8 +39,7 @@ public class SearchLogging { String logPattern = helper.buildLogPattern(config); 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.level.es"); + helper.configureRootLogLevel(props, ProcessId.ELASTICSEARCH); 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 b7f2fa60afd..c1a18209704 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 @@ -33,6 +33,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; @@ -44,6 +45,8 @@ import static org.slf4j.Logger.ROOT_LOGGER_NAME; public class SearchLoggingTest { @Rule public TemporaryFolder temp = new TemporaryFolder(); + @Rule + public ExpectedException expectedException = ExpectedException.none(); private File logDir; @@ -85,22 +88,79 @@ public class SearchLoggingTest { } @Test - public void root_log_level_does_not_change_with_global_property_value() { + 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); - Logger rootLogger = ctx.getLogger(ROOT_LOGGER_NAME); - assertThat(rootLogger.getLevel()).isEqualTo(Level.INFO); + verifyRootLogLevel(ctx, Level.TRACE); } @Test - public void root_log_level_change_with_es_property_value() { + public void root_logger_level_changes_with_es_property() { props.set("sonar.log.level.es", "TRACE"); LoggerContext ctx = underTest.configure(props); + verifyRootLogLevel(ctx, Level.TRACE); + } + + @Test + public void root_logger_level_is_configured_from_es_property_over_global_property() { + props.set("sonar.log.level", "TRACE"); + props.set("sonar.log.level.es", "DEBUG"); + + LoggerContext ctx = underTest.configure(props); + + verifyRootLogLevel(ctx, Level.DEBUG); + } + + @Test + public void root_logger_level_changes_with_es_property_and_is_case_insensitive() { + props.set("sonar.log.level.es", "deBug"); + + LoggerContext ctx = underTest.configure(props); + + verifyRootLogLevel(ctx, Level.DEBUG); + } + + @Test + public void root_logger_level_defaults_to_INFO_if_es_property_has_invalid_value() { + props.set("sonar.log.level.es", "DodoDouh!"); + + LoggerContext ctx = underTest.configure(props); + 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_es_property_unsupported_level() { + props.set("sonar.log.level.es", "ERROR"); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("log level ERROR in property sonar.log.level.es 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(Level.TRACE); + 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 14bd48fbd21..a81077b6793 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 @@ -124,7 +124,7 @@ public class WebServerProcessLoggingTest { } @Test - public void root_logger_level_changes_with_ce_property_and_is_case_insensitive() { + public void root_logger_level_changes_with_web_property_and_is_case_insensitive() { props.set("sonar.log.level.web", "debug"); LoggerContext ctx = underTest.configure(props); diff --git a/sonar-application/src/main/assembly/conf/sonar.properties b/sonar-application/src/main/assembly/conf/sonar.properties index dd4d6d7623a..9f08e54d864 100644 --- a/sonar-application/src/main/assembly/conf/sonar.properties +++ b/sonar-application/src/main/assembly/conf/sonar.properties @@ -250,7 +250,7 @@ # Some logs, however, will follow the convention to provide data in payload in the format " | key=value" # Especially, log of profiled pieces of code will end with " | time=XXXXms". -# Global level of logs (applies to App, Web and CE processes). +# Global level of logs (applies to all 4 processes). # Supported values are INFO (default), DEBUG and TRACE #sonar.log.level=INFO @@ -279,8 +279,8 @@ #sonar.log.maxFiles=7 # Access log is the list of all the HTTP requests received by server. If enabled, it is stored -# in the file {sonar.path.logs}/access.log. This file follows the same rolling policy as for -# sonar.log (see sonar.log.rollingPolicy and sonar.log.maxFiles). +# in the file {sonar.path.logs}/access.log. This file follows the same rolling policy as other log file +# (see sonar.log.rollingPolicy and sonar.log.maxFiles). #sonar.web.accessLogs.enable=true # Format of access log. It is ignored if sonar.web.accessLogs.enable=false. Possible values are: -- 2.39.5