IGNITE-4863: Disallow change RootLogger log-level if it can have negative effect on other loggers. This closes #1687.
Project: http://git-wip-us.apache.org/repos/asf/ignite/repo Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/01ceeb13 Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/01ceeb13 Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/01ceeb13 Branch: refs/heads/ignite-4932 Commit: 01ceeb13420b68edf12b0262fe0991e84c085dd8 Parents: bb3ff12 Author: Andrey V. Mashenkov <[email protected]> Authored: Thu Apr 6 14:43:50 2017 +0300 Committer: Andrey V. Mashenkov <[email protected]> Committed: Mon Apr 10 17:12:12 2017 +0300 ---------------------------------------------------------------------- .../apache/ignite/logger/log4j/Log4JLogger.java | 64 +++++- .../log4j/GridLog4jInitializationTest.java | 212 +++++++++++++++++++ .../logger/log4j/GridLog4jInitializedTest.java | 55 ----- .../log4j/GridLog4jNotInitializedTest.java | 46 ---- .../ignite/testsuites/IgniteLog4jTestSuite.java | 6 +- 5 files changed, 268 insertions(+), 115 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ignite/blob/01ceeb13/modules/log4j/src/main/java/org/apache/ignite/logger/log4j/Log4JLogger.java ---------------------------------------------------------------------- diff --git a/modules/log4j/src/main/java/org/apache/ignite/logger/log4j/Log4JLogger.java b/modules/log4j/src/main/java/org/apache/ignite/logger/log4j/Log4JLogger.java index d5b0f02..f6ed830 100644 --- a/modules/log4j/src/main/java/org/apache/ignite/logger/log4j/Log4JLogger.java +++ b/modules/log4j/src/main/java/org/apache/ignite/logger/log4j/Log4JLogger.java @@ -35,10 +35,12 @@ import org.apache.ignite.internal.util.typedef.internal.U; import org.apache.ignite.lang.IgniteClosure; import org.apache.ignite.logger.LoggerNodeIdAware; import org.apache.log4j.Appender; +import org.apache.log4j.AppenderSkeleton; import org.apache.log4j.Category; import org.apache.log4j.ConsoleAppender; import org.apache.log4j.FileAppender; import org.apache.log4j.Level; +import org.apache.log4j.LogManager; import org.apache.log4j.Logger; import org.apache.log4j.PatternLayout; import org.apache.log4j.varia.LevelRangeFilter; @@ -78,6 +80,9 @@ import static org.apache.ignite.IgniteSystemProperties.IGNITE_QUIET; * injection. */ public class Log4JLogger implements IgniteLogger, LoggerNodeIdAware, Log4jFileAware { + /** */ + public static final String CONSOLE_ERR_APPENDER_NAME = "CONSOLE_ERR"; + /** Appenders. */ private static Collection<FileAppender> fileAppenders = new GridConcurrentHashSet<>(); @@ -308,7 +313,7 @@ public class Log4JLogger implements IgniteLogger, LoggerNodeIdAware, Log4jFileAw Appender appender = (Appender)appenders.nextElement(); if (appender instanceof ConsoleAppender) { - if ("CONSOLE_ERR".equals(appender.getName())) { + if (CONSOLE_ERR_APPENDER_NAME.equals(appender.getName())) { // Treat CONSOLE_ERR appender as a system one and don't count it. errAppender = (ConsoleAppender)appender; @@ -347,17 +352,27 @@ public class Log4JLogger implements IgniteLogger, LoggerNodeIdAware, Log4jFileAw if (errAppender.getThreshold() == Level.ERROR) errAppender.setThreshold(Level.WARN); } - else - // No error console appender => create console appender with no level limit. - rootCategory.addAppender(createConsoleAppender(Level.OFF)); + else { + // No error console appender => create console appender with. + final AppenderSkeleton consoleAppender = createConsoleAppender(Level.OFF); + + consoleAppender.setThreshold(Level.INFO); - if (logLevel != null) + rootCategory.addAppender(consoleAppender); + } + + // Won't raise LogLevel if there is other loggers configured. As LogLevel can be inherited. + if (logLevel != null && !logLevel.isGreaterOrEqual(impl.getEffectiveLevel())) { impl.setLevel(logLevel); - } - // If still don't have appenders, disable logging. - if (!isConfigured()) + impl.warn("RootLogger logging level has been dropped by Apache Ignite.\n"+ + "Set lower log level or configure ConsoleAppender manually or disable ConsoleAppender automatic creation."); + } + } + else if (!isConfigured() && !hasOtherLoggers()) { + // If still don't have appenders and other loggers configured, disable logging. impl.setLevel(Level.OFF); + } quiet0 = quiet; inited = true; @@ -365,16 +380,34 @@ public class Log4JLogger implements IgniteLogger, LoggerNodeIdAware, Log4jFileAw } /** + * Checks if there is other loggers configured. + * + * @return {@code True} if other logger found. + */ + private boolean hasOtherLoggers() { + final Enumeration loggers = LogManager.getCurrentLoggers(); + + while (loggers.hasMoreElements()) { + Logger c = (Logger)loggers.nextElement(); + + if (c != impl && c.getAllAppenders().hasMoreElements()) + return true; + } + + return false; + } + + /** * Creates console appender with some reasonable default logging settings. * * @param maxLevel Max logging level. * @return New console appender. */ - private Appender createConsoleAppender(Level maxLevel) { + private AppenderSkeleton createConsoleAppender(Level maxLevel) { String fmt = "[%d{ABSOLUTE}][%-5p][%t][%c{1}] %m%n"; // Configure output that should go to System.out - Appender app = new ConsoleAppender(new PatternLayout(fmt), ConsoleAppender.SYSTEM_OUT); + AppenderSkeleton app = new ConsoleAppender(new PatternLayout(fmt), ConsoleAppender.SYSTEM_OUT); LevelRangeFilter lvlFilter = new LevelRangeFilter(); @@ -532,4 +565,15 @@ public class Log4JLogger implements IgniteLogger, LoggerNodeIdAware, Log4jFileAw } } } + + /** + * For test purposes only. + */ + static void reset(){ + inited = false; + + quiet0 = false; + + fileAppenders.clear(); + } } http://git-wip-us.apache.org/repos/asf/ignite/blob/01ceeb13/modules/log4j/src/test/java/org/apache/ignite/logger/log4j/GridLog4jInitializationTest.java ---------------------------------------------------------------------- diff --git a/modules/log4j/src/test/java/org/apache/ignite/logger/log4j/GridLog4jInitializationTest.java b/modules/log4j/src/test/java/org/apache/ignite/logger/log4j/GridLog4jInitializationTest.java new file mode 100644 index 0000000..2a98490 --- /dev/null +++ b/modules/log4j/src/test/java/org/apache/ignite/logger/log4j/GridLog4jInitializationTest.java @@ -0,0 +1,212 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.logger.log4j; + +import junit.framework.TestCase; +import org.apache.ignite.IgniteLogger; +import org.apache.ignite.IgniteSystemProperties; +import org.apache.ignite.testframework.junits.common.GridCommonTest; +import org.apache.log4j.BasicConfigurator; +import org.apache.log4j.Level; +import org.apache.log4j.LogManager; +import org.apache.log4j.Logger; +import org.apache.log4j.varia.NullAppender; + +/** + * Log4j not initialized test. + */ +@GridCommonTest(group = "Logger") +public class GridLog4jInitializationTest extends TestCase { + /** */ + private static final boolean VERBOSE = true; + + /** {@inheritDoc} */ + @Override public void setUp() throws Exception { + super.setUp(); + + resetLogger(); + } + + /** {@inheritDoc} */ + @Override public void tearDown() throws Exception { + super.tearDown(); + + resetLogger(); + } + + /** */ + private void resetLogger() { + Log4JLogger.reset(); + + LogManager.resetConfiguration(); + + System.clearProperty(IgniteSystemProperties.IGNITE_QUIET); + } + + /** */ + public void testLogNotInitialized() { + IgniteLogger log = new Log4JLogger().getLogger(GridLog4jInitializationTest.class); + + if (VERBOSE) + printLoggerResults(log); + + assertTrue(log instanceof Log4JLogger); + + assertEquals(Level.OFF, LogManager.getRootLogger().getEffectiveLevel()); + } + + /** */ + public void testLogInitialized() { + BasicConfigurator.configure(); + + IgniteLogger log = new Log4JLogger().getLogger(GridLog4jInitializationTest.class); + + if (VERBOSE) + printLoggerResults(log); + + assertTrue(log instanceof Log4JLogger); + + assertEquals(Level.DEBUG, LogManager.getRootLogger().getEffectiveLevel()); + } + + /** */ + public void testNoAppendersConfigured() { + LogManager.getRootLogger().setLevel(Level.WARN); + + final Logger logger = LogManager.getLogger(GridLog4jInitializationTest.class); + + assertEquals(Level.WARN, logger.getEffectiveLevel()); + + IgniteLogger log = new Log4JLogger().getLogger(GridLog4jInitializationTest.class); + + if (VERBOSE) + printLoggerResults(log); + + assertEquals(Level.OFF, logger.getEffectiveLevel()); + } + + /** */ + public void testAutoAddConsoleAppender() { + System.setProperty(IgniteSystemProperties.IGNITE_QUIET, String.valueOf(false)); + + LogManager.getRootLogger().setLevel(Level.WARN); + + final Logger logger = LogManager.getLogger(GridLog4jInitializationTest.class); + + assertEquals(Level.WARN, logger.getEffectiveLevel()); + + IgniteLogger log = new Log4JLogger().getLogger(GridLog4jInitializationTest.class); + + if (VERBOSE) + printLoggerResults(log); + + assertEquals(Level.INFO, logger.getEffectiveLevel()); // LogLevel is allowed to be dropped. + } + + /** */ + public void testAutoAddConsoleAppender2() { + System.setProperty(IgniteSystemProperties.IGNITE_QUIET, String.valueOf(false)); + + LogManager.getRootLogger().setLevel(Level.DEBUG); + + final Logger logger = LogManager.getLogger(GridLog4jInitializationTest.class); + + assertEquals(Level.DEBUG, logger.getEffectiveLevel()); + + IgniteLogger log = new Log4JLogger().getLogger(GridLog4jInitializationTest.class); + + if (VERBOSE) + printLoggerResults(log); + + assertEquals(Level.DEBUG, logger.getEffectiveLevel()); // LogLevel should not change. + } + + /** */ + public void testOtherLoggerConfigured() { + LogManager.getRootLogger().setLevel(Level.DEBUG); + + final Logger logger = LogManager.getLogger(GridLog4jInitializationTest.class); + + logger.addAppender(new NullAppender()); + + assertEquals(Level.DEBUG, logger.getEffectiveLevel()); + + IgniteLogger log = new Log4JLogger().getLogger(GridLog4jInitializationTest.class); + + if (VERBOSE) + printLoggerResults(log); + + assertEquals(Level.DEBUG, logger.getEffectiveLevel()); // LogLevel should not be OFF. + } + + /** */ + public void testAutoAddConsoleAppenderWithOtherLoggerConfigured() { + System.setProperty(IgniteSystemProperties.IGNITE_QUIET, String.valueOf(false)); + + LogManager.getRootLogger().setLevel(Level.DEBUG); + + final Logger logger = LogManager.getLogger(GridLog4jInitializationTest.class); + + logger.addAppender(new NullAppender()); + + assertEquals(Level.DEBUG, logger.getEffectiveLevel()); + + IgniteLogger log = new Log4JLogger().getLogger(GridLog4jInitializationTest.class); + + if (VERBOSE) + printLoggerResults(log); + + assertEquals(Level.DEBUG, logger.getEffectiveLevel()); // LogLevel should not be raised. + } + + /** */ + public void testAutoAddConsoleAppenderWithOtherLoggerConfigured2() { + System.setProperty(IgniteSystemProperties.IGNITE_QUIET, String.valueOf(false)); + + LogManager.getRootLogger().setLevel(Level.WARN); + + final Logger logger = LogManager.getLogger(GridLog4jInitializationTest.class); + + logger.addAppender(new NullAppender()); + + assertEquals(Level.WARN, logger.getEffectiveLevel()); + + IgniteLogger log = new Log4JLogger().getLogger(GridLog4jInitializationTest.class); + + if (VERBOSE) + printLoggerResults(log); + + assertEquals(Level.INFO, logger.getEffectiveLevel()); // LogLevel is allowed to be dropped. + } + + /** */ + private void printLoggerResults(IgniteLogger log) { + if (log.isDebugEnabled()) + log.debug("This is 'debug' message."); + else + System.out.println("DEBUG level is not enabled."); + + if (log.isInfoEnabled()) + log.info("This is 'info' message."); + else + System.out.println("INFO level is not enabled."); + + log.warning("This is 'warning' message."); + log.error("This is 'error' message."); + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/ignite/blob/01ceeb13/modules/log4j/src/test/java/org/apache/ignite/logger/log4j/GridLog4jInitializedTest.java ---------------------------------------------------------------------- diff --git a/modules/log4j/src/test/java/org/apache/ignite/logger/log4j/GridLog4jInitializedTest.java b/modules/log4j/src/test/java/org/apache/ignite/logger/log4j/GridLog4jInitializedTest.java deleted file mode 100644 index 94907f0..0000000 --- a/modules/log4j/src/test/java/org/apache/ignite/logger/log4j/GridLog4jInitializedTest.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.ignite.logger.log4j; - -import junit.framework.TestCase; -import org.apache.ignite.IgniteLogger; -import org.apache.ignite.testframework.junits.common.GridCommonTest; -import org.apache.log4j.BasicConfigurator; - -/** - * Log4j initialized test. - */ -@GridCommonTest(group = "Logger") -public class GridLog4jInitializedTest extends TestCase { - - /** - * @throws Exception If failed. - */ - @Override protected void setUp() throws Exception { - BasicConfigurator.configure(); - } - - /** */ - public void testLogInitialize() { - IgniteLogger log = new Log4JLogger(); - - assert log.isInfoEnabled() == true; - - if (log.isDebugEnabled()) - log.debug("This is 'debug' message."); - - log.info("This is 'info' message."); - log.warning("This is 'warning' message."); - log.warning("This is 'warning' message.", new Exception("It's a test warning exception")); - log.error("This is 'error' message."); - log.error("This is 'error' message.", new Exception("It's a test error exception")); - - assert log.getLogger(GridLog4jInitializedTest.class.getName()) instanceof Log4JLogger; - } -} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/ignite/blob/01ceeb13/modules/log4j/src/test/java/org/apache/ignite/logger/log4j/GridLog4jNotInitializedTest.java ---------------------------------------------------------------------- diff --git a/modules/log4j/src/test/java/org/apache/ignite/logger/log4j/GridLog4jNotInitializedTest.java b/modules/log4j/src/test/java/org/apache/ignite/logger/log4j/GridLog4jNotInitializedTest.java deleted file mode 100644 index 390fdcb..0000000 --- a/modules/log4j/src/test/java/org/apache/ignite/logger/log4j/GridLog4jNotInitializedTest.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.ignite.logger.log4j; - -import junit.framework.TestCase; -import org.apache.ignite.IgniteLogger; -import org.apache.ignite.testframework.junits.common.GridCommonTest; - -/** - * Log4j not initialized test. - */ -@GridCommonTest(group = "Logger") -public class GridLog4jNotInitializedTest extends TestCase { - /** */ - public void testLogInitialize() { - IgniteLogger log = new Log4JLogger().getLogger(GridLog4jNotInitializedTest.class); - - if (log.isDebugEnabled()) - log.debug("This is 'debug' message."); - else - System.out.println("DEBUG level is not enabled."); - - if (log.isInfoEnabled()) - log.info("This is 'info' message."); - else - System.out.println("INFO level is not enabled."); - - log.warning("This is 'warning' message."); - log.error("This is 'error' message."); - } -} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/ignite/blob/01ceeb13/modules/log4j/src/test/java/org/apache/ignite/testsuites/IgniteLog4jTestSuite.java ---------------------------------------------------------------------- diff --git a/modules/log4j/src/test/java/org/apache/ignite/testsuites/IgniteLog4jTestSuite.java b/modules/log4j/src/test/java/org/apache/ignite/testsuites/IgniteLog4jTestSuite.java index f5f13d9..e20d32c 100644 --- a/modules/log4j/src/test/java/org/apache/ignite/testsuites/IgniteLog4jTestSuite.java +++ b/modules/log4j/src/test/java/org/apache/ignite/testsuites/IgniteLog4jTestSuite.java @@ -19,8 +19,7 @@ package org.apache.ignite.testsuites; import junit.framework.TestSuite; import org.apache.ignite.logger.log4j.GridLog4jCorrectFileNameTest; -import org.apache.ignite.logger.log4j.GridLog4jInitializedTest; -import org.apache.ignite.logger.log4j.GridLog4jNotInitializedTest; +import org.apache.ignite.logger.log4j.GridLog4jInitializationTest; /** * Log4j logging tests. @@ -33,8 +32,7 @@ public class IgniteLog4jTestSuite extends TestSuite { public static TestSuite suite() throws Exception { TestSuite suite = new TestSuite("Log4j Logging Test Suite"); - suite.addTest(new TestSuite(GridLog4jInitializedTest.class)); - suite.addTest(new TestSuite(GridLog4jNotInitializedTest.class)); + suite.addTest(new TestSuite(GridLog4jInitializationTest.class)); suite.addTest(new TestSuite(GridLog4jCorrectFileNameTest.class)); return suite;
