From d28bd659fda693609a26d3598eebeac8c5deef6d Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 16 Nov 2016 12:03:11 +0100 Subject: [PATCH] SONAR-8336 add properties to control SQL logs level in CE and Web --- .../container/ComputeEngineContainerImpl.java | 2 + .../ComputeEngineContainerImplTest.java | 2 +- .../java/org/sonar/process/LogbackHelper.java | 21 ++++- .../org/sonar/ce/log/CeProcessLogging.java | 6 +- .../server/app/ServerProcessLogging.java | 4 +- .../server/app/WebServerProcessLogging.java | 5 +- .../platformlevel/PlatformLevel1.java | 2 + .../sonar/ce/log/CeProcessLoggingTest.java | 84 +++++++++++++++++- .../app/WebServerProcessLoggingTest.java | 85 ++++++++++++++++++- sonar-db/pom.xml | 5 ++ .../java/org/sonar/db/DefaultDatabase.java | 10 ++- .../org/sonar/db/DefaultDatabaseTest.java | 13 +-- .../src/test/java/org/sonar/db/TestDb.java | 3 +- 13 files changed, 220 insertions(+), 22 deletions(-) diff --git a/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java b/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java index 6168cbcc122..fe082dbbc7a 100644 --- a/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java +++ b/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java @@ -63,6 +63,7 @@ import org.sonar.db.DbClient; import org.sonar.db.DefaultDatabase; import org.sonar.db.purge.PurgeProfiler; import org.sonar.db.version.DatabaseVersion; +import org.sonar.process.LogbackHelper; import org.sonar.process.Props; import org.sonar.server.component.ComponentCleanerService; import org.sonar.server.component.ComponentFinder; @@ -213,6 +214,7 @@ public class ComputeEngineContainerImpl implements ComputeEngineContainer { SonarRuntimeImpl.forSonarQube(ApiVersion.load(System2.INSTANCE), SonarQubeSide.COMPUTE_ENGINE), UuidFactoryImpl.INSTANCE, ClusterImpl.class, + LogbackHelper.class, DefaultDatabase.class, DatabaseChecker.class, // must instantiate deprecated class in 5.2 and only this one (and not its replacement) diff --git a/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java b/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java index e1815013c35..a31db40f3ee 100644 --- a/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java +++ b/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java @@ -105,7 +105,7 @@ public class ComputeEngineContainerImplTest { ); assertThat(picoContainer.getParent().getParent().getParent().getComponentAdapters()).hasSize( COMPONENTS_IN_LEVEL_1_AT_CONSTRUCTION - + 25 // level 1 + + 26 // level 1 + 47 // content of DaoModule + 2 // content of EsSearchModule + 63 // content of CorePropertyDefinitions 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 abacb741b7a..875b938f7dd 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 @@ -121,7 +121,7 @@ public class LogbackHelper { public static final class Builder { @CheckForNull - public String processName; + private String processName; private String threadIdFieldPattern = ""; @CheckForNull private String fileNamePrefix; @@ -333,6 +333,11 @@ public class LogbackHelper { } } + public Level configureLoggerLogLevelFromDomain(String loggerName, Props props, ProcessId processId, LogDomain domain) { + String processProperty = SONAR_PROCESS_LOG_LEVEL_PROPERTY.replace(PROCESS_NAME_PLACEHOLDER, processId.getKey()); + return configureLoggerLogLevel(getRootContext().getLogger(loggerName), props, SONAR_LOG_LEVEL_PROPERTY, processProperty, processProperty + "." + domain.key); + } + /** * Configure the log level of the specified logger to specified level. *

@@ -345,6 +350,10 @@ public class LogbackHelper { return logger; } + public Level getLoggerLevel(String loggerName) { + return getRootContext().getLogger(loggerName).getLevel(); + } + /** * Generally used to reset logback in logging tests */ @@ -478,4 +487,14 @@ public class LogbackHelper { return appender; } } + + public enum LogDomain { + SQL("sql"); + + private final String key; + + LogDomain(String key) { + this.key = key; + } + } } diff --git a/server/sonar-server/src/main/java/org/sonar/ce/log/CeProcessLogging.java b/server/sonar-server/src/main/java/org/sonar/ce/log/CeProcessLogging.java index e756cd1c251..2d31981f82a 100644 --- a/server/sonar-server/src/main/java/org/sonar/ce/log/CeProcessLogging.java +++ b/server/sonar-server/src/main/java/org/sonar/ce/log/CeProcessLogging.java @@ -19,11 +19,13 @@ */ package org.sonar.ce.log; +import org.sonar.process.LogbackHelper; import org.sonar.process.ProcessId; import org.sonar.process.Props; import org.sonar.server.app.ServerProcessLogging; import static org.sonar.ce.log.CeLogging.MDC_CE_TASK_UUID; +import static org.sonar.process.LogbackHelper.LogDomain; /** * Configure logback for the Compute Engine process. Logs are written to file "ce.log" in SQ's log directory. @@ -35,7 +37,7 @@ public class CeProcessLogging extends ServerProcessLogging { } @Override - protected void extendConfiguration(Props props) { - // no configuration extension to do + protected void extendConfiguration(LogbackHelper helper, Props props) { + helper.configureLoggerLogLevelFromDomain("sql", props, ProcessId.COMPUTE_ENGINE, LogDomain.SQL); } } 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 933f6800314..842c8f6be52 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 @@ -44,12 +44,12 @@ public abstract class ServerProcessLogging { helper.enableJulChangePropagation(ctx); configureRootLogger(props); - extendConfiguration(props); + extendConfiguration(helper, props); return ctx; } - protected abstract void extendConfiguration(Props props); + protected abstract void extendConfiguration(LogbackHelper helper, Props props); private void configureRootLogger(Props props) { LogbackHelper.RootLoggerConfig config = newRootLoggerConfigBuilder() 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 f4f0753f214..8c3f6e70d8d 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 @@ -21,6 +21,7 @@ package org.sonar.server.app; import java.util.logging.LogManager; import org.slf4j.bridge.SLF4JBridgeHandler; +import org.sonar.process.LogbackHelper; import org.sonar.process.ProcessId; import org.sonar.process.Props; @@ -34,9 +35,11 @@ public class WebServerProcessLogging extends ServerProcessLogging { } @Override - protected void extendConfiguration(Props props) { + protected void extendConfiguration(LogbackHelper helper, Props props) { // Configure java.util.logging, used by Tomcat, in order to forward to slf4j LogManager.getLogManager().reset(); SLF4JBridgeHandler.install(); + + helper.configureLoggerLogLevelFromDomain("sql", props, ProcessId.WEB_SERVER, LogbackHelper.LogDomain.SQL); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel1.java b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel1.java index 2bbe67023bb..d2543fb2b93 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel1.java +++ b/server/sonar-server/src/main/java/org/sonar/server/platform/platformlevel/PlatformLevel1.java @@ -37,6 +37,7 @@ import org.sonar.db.DefaultDatabase; import org.sonar.db.purge.PurgeProfiler; import org.sonar.db.semaphore.SemaphoresImpl; import org.sonar.db.version.DatabaseVersion; +import org.sonar.process.LogbackHelper; import org.sonar.server.app.ProcessCommandWrapperImpl; import org.sonar.server.app.RestartFlagHolderImpl; import org.sonar.server.issue.index.IssueIndex; @@ -85,6 +86,7 @@ public class PlatformLevel1 extends PlatformLevel { UuidFactoryImpl.INSTANCE, UrlSettings.class, EmbeddedDatabaseFactory.class, + LogbackHelper.class, DefaultDatabase.class, DatabaseChecker.class, // must instantiate deprecated class in 5.2 and only this one (and not its replacement) 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 6ca90b862b9..a9e6366a896 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 @@ -133,13 +133,78 @@ public class CeProcessLoggingTest { } @Test - public void default_to_INFO_if_ce_property_has_invalid_value() { + public void sql_logger_level_changes_with_global_property_and_is_case_insensitive() { + props.set("sonar.log.level", "InFO"); + + LoggerContext ctx = underTest.configure(props); + + verifySqlLogLevel(ctx, Level.INFO); + } + + @Test + public void sql_logger_level_changes_with_ce_property_and_is_case_insensitive() { + props.set("sonar.log.level.ce", "TrACe"); + + LoggerContext ctx = underTest.configure(props); + + verifySqlLogLevel(ctx, Level.TRACE); + } + + @Test + public void sql_logger_level_changes_with_ce_sql_property_and_is_case_insensitive() { + props.set("sonar.log.level.ce.sql", "debug"); + + LoggerContext ctx = underTest.configure(props); + + verifySqlLogLevel(ctx, Level.DEBUG); + } + + @Test + public void sql_logger_level_is_configured_from_ce_sql_property_over_ce_property() { + props.set("sonar.log.level.ce.sql", "debug"); + props.set("sonar.log.level.ce", "TRACE"); + + LoggerContext ctx = underTest.configure(props); + + verifySqlLogLevel(ctx, Level.DEBUG); + } + + @Test + public void sql_logger_level_is_configured_from_ce_sql_property_over_global_property() { + props.set("sonar.log.level.ce.sql", "debug"); + props.set("sonar.log.level", "TRACE"); + + LoggerContext ctx = underTest.configure(props); + + verifySqlLogLevel(ctx, Level.DEBUG); + } + + @Test + public void sql_logger_level_is_configured_from_ce_property_over_global_property() { + props.set("sonar.log.level.ce", "debug"); + props.set("sonar.log.level", "TRACE"); + + LoggerContext ctx = underTest.configure(props); + + verifySqlLogLevel(ctx, Level.DEBUG); + } + + @Test + public void root_logger_level_defaults_to_INFO_if_ce_property_has_invalid_value() { props.set("sonar.log.level.ce", "DodoDouh!"); LoggerContext ctx = underTest.configure(props); verifyRootLogLevel(ctx, Level.INFO); } + @Test + public void sql_logger_level_defaults_to_INFO_if_ce_sql_property_has_invalid_value() { + props.set("sonar.log.level.ce.sql", "DodoDouh!"); + + LoggerContext ctx = underTest.configure(props); + verifySqlLogLevel(ctx, Level.INFO); + } + @Test public void fail_with_IAE_if_global_property_unsupported_level() { props.set("sonar.log.level", "ERROR"); @@ -160,8 +225,21 @@ public class CeProcessLoggingTest { underTest.configure(props); } + @Test + public void fail_with_IAE_if_ce_sql_property_unsupported_level() { + props.set("sonar.log.level.ce.sql", "ERROR"); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("log level ERROR in property sonar.log.level.ce.sql 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); + assertThat(ctx.getLogger(ROOT_LOGGER_NAME).getLevel()).isEqualTo(expected); + } + + private void verifySqlLogLevel(LoggerContext ctx, Level expected) { + assertThat(ctx.getLogger("sql").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 2aabd118f6e..9030477c23e 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 @@ -105,7 +105,7 @@ public class WebServerProcessLoggingTest { } @Test - public void root_logger_level_changes_with_ce_property() { + public void root_logger_level_changes_with_web_property() { props.set("sonar.log.level.web", "TRACE"); LoggerContext ctx = underTest.configure(props); @@ -133,13 +133,78 @@ public class WebServerProcessLoggingTest { } @Test - public void default_to_INFO_if_ce_property_has_invalid_value() { + public void sql_logger_level_changes_with_global_property_and_is_case_insensitive() { + props.set("sonar.log.level", "InFO"); + + LoggerContext ctx = underTest.configure(props); + + verifySqlLogLevel(ctx, Level.INFO); + } + + @Test + public void sql_logger_level_changes_with_web_property_and_is_case_insensitive() { + props.set("sonar.log.level.web", "TrACe"); + + LoggerContext ctx = underTest.configure(props); + + verifySqlLogLevel(ctx, Level.TRACE); + } + + @Test + public void sql_logger_level_changes_with_web_sql_property_and_is_case_insensitive() { + props.set("sonar.log.level.web.sql", "debug"); + + LoggerContext ctx = underTest.configure(props); + + verifySqlLogLevel(ctx, Level.DEBUG); + } + + @Test + public void sql_logger_level_is_configured_from_web_sql_property_over_web_property() { + props.set("sonar.log.level.web.sql", "debug"); + props.set("sonar.log.level.web", "TRACE"); + + LoggerContext ctx = underTest.configure(props); + + verifySqlLogLevel(ctx, Level.DEBUG); + } + + @Test + public void sql_logger_level_is_configured_from_web_sql_property_over_global_property() { + props.set("sonar.log.level.web.sql", "debug"); + props.set("sonar.log.level", "TRACE"); + + LoggerContext ctx = underTest.configure(props); + + verifySqlLogLevel(ctx, Level.DEBUG); + } + + @Test + public void sql_logger_level_is_configured_from_web_property_over_global_property() { + props.set("sonar.log.level.web", "debug"); + props.set("sonar.log.level", "TRACE"); + + LoggerContext ctx = underTest.configure(props); + + verifySqlLogLevel(ctx, Level.DEBUG); + } + + @Test + public void root_logger_level_defaults_to_INFO_if_web_property_has_invalid_value() { props.set("sonar.log.level.web", "DodoDouh!"); LoggerContext ctx = underTest.configure(props); verifyRootLogLevel(ctx, Level.INFO); } + @Test + public void sql_logger_level_defaults_to_INFO_if_web_sql_property_has_invalid_value() { + props.set("sonar.log.level.web.sql", "DodoDouh!"); + + LoggerContext ctx = underTest.configure(props); + verifySqlLogLevel(ctx, Level.INFO); + } + @Test public void fail_with_IAE_if_global_property_unsupported_level() { props.set("sonar.log.level", "ERROR"); @@ -151,7 +216,7 @@ public class WebServerProcessLoggingTest { } @Test - public void fail_with_IAE_if_ce_property_unsupported_level() { + public void fail_with_IAE_if_web_property_unsupported_level() { props.set("sonar.log.level.web", "ERROR"); expectedException.expect(IllegalArgumentException.class); @@ -160,9 +225,23 @@ public class WebServerProcessLoggingTest { underTest.configure(props); } + @Test + public void fail_with_IAE_if_web_sql_property_unsupported_level() { + props.set("sonar.log.level.web.sql", "ERROR"); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("log level ERROR in property sonar.log.level.web.sql 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); } + private void verifySqlLogLevel(LoggerContext ctx, Level expected) { + assertThat(ctx.getLogger("sql").getLevel()).isEqualTo(expected); + } + } diff --git a/sonar-db/pom.xml b/sonar-db/pom.xml index 9d95b998e29..ccffa08f014 100644 --- a/sonar-db/pom.xml +++ b/sonar-db/pom.xml @@ -73,6 +73,11 @@ ch.qos.logback logback-core + + ${project.groupId} + sonar-process + ${project.version} + diff --git a/sonar-db/src/main/java/org/sonar/db/DefaultDatabase.java b/sonar-db/src/main/java/org/sonar/db/DefaultDatabase.java index 2b1360c46ab..5a1995161d9 100644 --- a/sonar-db/src/main/java/org/sonar/db/DefaultDatabase.java +++ b/sonar-db/src/main/java/org/sonar/db/DefaultDatabase.java @@ -19,6 +19,7 @@ */ package org.sonar.db; +import ch.qos.logback.classic.Level; import com.google.common.annotations.VisibleForTesting; import java.sql.Connection; import java.sql.SQLException; @@ -39,6 +40,7 @@ import org.sonar.db.dialect.DialectUtils; import org.sonar.db.profiling.NullConnectionInterceptor; import org.sonar.db.profiling.ProfiledConnectionInterceptor; import org.sonar.db.profiling.ProfiledDataSource; +import org.sonar.process.LogbackHelper; import static java.lang.String.format; @@ -54,12 +56,14 @@ public class DefaultDatabase implements Database { private static final String SONAR_JDBC_DIALECT = "sonar.jdbc.dialect"; private static final String SONAR_JDBC_URL = "sonar.jdbc.url"; - private Settings settings; + private final LogbackHelper logbackHelper; + private final Settings settings; private ProfiledDataSource datasource; private Dialect dialect; private Properties properties; - public DefaultDatabase(Settings settings) { + public DefaultDatabase(LogbackHelper logbackHelper, Settings settings) { + this.logbackHelper = logbackHelper; this.settings = settings; } @@ -93,7 +97,7 @@ public class DefaultDatabase implements Database { datasource = new ProfiledDataSource(basicDataSource, NullConnectionInterceptor.INSTANCE); datasource.setConnectionInitSqls(dialect.getConnectionInitStatements()); datasource.setValidationQuery(dialect.getValidationQuery()); - enableSqlLogging(datasource, "TRACE".equals(settings.getString("sonar.log.level"))); + enableSqlLogging(datasource, logbackHelper.getLoggerLevel("sql") == Level.TRACE); } private void checkConnection() { diff --git a/sonar-db/src/test/java/org/sonar/db/DefaultDatabaseTest.java b/sonar-db/src/test/java/org/sonar/db/DefaultDatabaseTest.java index 2ab4bbad182..454dcbac3ac 100644 --- a/sonar-db/src/test/java/org/sonar/db/DefaultDatabaseTest.java +++ b/sonar-db/src/test/java/org/sonar/db/DefaultDatabaseTest.java @@ -25,14 +25,17 @@ import org.junit.Test; import org.sonar.api.config.Settings; import org.sonar.api.config.MapSettings; import org.sonar.db.dialect.PostgreSql; +import org.sonar.process.LogbackHelper; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; public class DefaultDatabaseTest { + private LogbackHelper logbackHelper = mock(LogbackHelper.class); @Test public void shouldLoadDefaultValues() { - DefaultDatabase db = new DefaultDatabase(new MapSettings()); + DefaultDatabase db = new DefaultDatabase(logbackHelper, new MapSettings()); db.initSettings(); Properties props = db.getProperties(); @@ -59,7 +62,7 @@ public class DefaultDatabaseTest { public void shouldCompleteProperties() { Settings settings = new MapSettings(); - DefaultDatabase db = new DefaultDatabase(settings) { + DefaultDatabase db = new DefaultDatabase(logbackHelper, settings) { @Override protected void doCompleteProperties(Properties properties) { properties.setProperty("sonar.jdbc.maxActive", "2"); @@ -81,7 +84,7 @@ public class DefaultDatabaseTest { settings.setProperty("sonar.jdbc.password", "sonar"); settings.setProperty("sonar.jdbc.maxActive", "1"); - DefaultDatabase db = new DefaultDatabase(settings); + DefaultDatabase db = new DefaultDatabase(logbackHelper, settings); db.start(); db.stop(); @@ -94,7 +97,7 @@ public class DefaultDatabaseTest { Settings settings = new MapSettings(); settings.setProperty("sonar.jdbc.url", "jdbc:postgresql://localhost/sonar"); - DefaultDatabase database = new DefaultDatabase(settings); + DefaultDatabase database = new DefaultDatabase(logbackHelper, settings); database.initSettings(); assertThat(database.getDialect().getId()).isEqualTo(PostgreSql.ID); @@ -105,7 +108,7 @@ public class DefaultDatabaseTest { Settings settings = new MapSettings(); settings.setProperty("sonar.jdbc.url", "jdbc:postgresql://localhost/sonar"); - DefaultDatabase database = new DefaultDatabase(settings); + DefaultDatabase database = new DefaultDatabase(logbackHelper, settings); database.initSettings(); assertThat(database.getProperties().getProperty("sonar.jdbc.driverClassName")).isEqualTo("org.postgresql.Driver"); diff --git a/sonar-db/src/test/java/org/sonar/db/TestDb.java b/sonar-db/src/test/java/org/sonar/db/TestDb.java index 4d008a2338c..4f6d0c0c0b0 100644 --- a/sonar-db/src/test/java/org/sonar/db/TestDb.java +++ b/sonar-db/src/test/java/org/sonar/db/TestDb.java @@ -41,6 +41,7 @@ import org.sonar.api.config.MapSettings; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.db.dialect.H2; +import org.sonar.process.LogbackHelper; /** * This class should be call using @ClassRule in order to create the schema once (ft @Rule is used @@ -84,7 +85,7 @@ class TestDb { } String dialect = settings.getString("sonar.jdbc.dialect"); if (dialect != null && !"h2".equals(dialect)) { - db = new DefaultDatabase(settings); + db = new DefaultDatabase(new LogbackHelper(), settings); } else { db = new H2Database("h2Tests" + DigestUtils.md5Hex(StringUtils.defaultString(schemaPath)), schemaPath == null); } -- 2.39.5