From d2a98d4dfc63fd606418f5bd433be514bbe20d01 Mon Sep 17 00:00:00 2001 From: Robert Gemmell Date: Mon, 17 Aug 2009 22:28:01 +0000 Subject: QPID-2051: relax the parser validation to only halt startup on fatal-errors in the xml file, and relax the level-check to allow undefined system properties. Move the Log4J initialisation override inside Main instead of the startup scripts, and have it check for -Dlog4j.configuration first before engaging. git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@805188 13f79535-47bb-0310-9956-ffa450edef68 --- qpid/java/broker/bin/qpid-server | 8 +- qpid/java/broker/bin/qpid-server.bat | 5 +- .../apache/log4j/xml/QpidLog4JConfigurator.java | 88 +++++++++++++++++++--- .../src/main/java/org/apache/qpid/server/Main.java | 17 ++++- .../logging/management/LoggingManagementMBean.java | 25 +----- .../log4j/xml/QpidLog4JConfiguratorTest.java | 4 +- 6 files changed, 97 insertions(+), 50 deletions(-) diff --git a/qpid/java/broker/bin/qpid-server b/qpid/java/broker/bin/qpid-server index 7482ad63ef..e5a9e998e2 100755 --- a/qpid/java/broker/bin/qpid-server +++ b/qpid/java/broker/bin/qpid-server @@ -23,12 +23,6 @@ if [ -z "$QPID_HOME" ]; then export PATH=${PATH}:${QPID_HOME}/bin fi -if [ -z "$QPID_LOG4J_SETTINGS" ]; then - # Disable the Log4J default initialization process, allowing the broker to - # perform the configuration using etc/log4j.xml - QPID_LOG4J_SETTINGS="-Dlog4j.defaultInitOverride=true" -fi - # Set classpath to include Qpid jar with all required jars in manifest QPID_LIBS=$QPID_HOME/lib/qpid-all.jar:$QPID_HOME/lib/bdbstore-launch.jar @@ -40,6 +34,6 @@ export JAVA=java \ QPID_CLASSPATH=$QPID_LIBS \ QPID_RUN_LOG=2 -QPID_OPTS="$QPID_OPTS $QPID_LOG4J_SETTINGS -Damqj.read_write_pool_size=32" +QPID_OPTS="$QPID_OPTS -Damqj.read_write_pool_size=32" . qpid-run org.apache.qpid.server.Main "$@" diff --git a/qpid/java/broker/bin/qpid-server.bat b/qpid/java/broker/bin/qpid-server.bat index 7fff183192..2687baa111 100644 --- a/qpid/java/broker/bin/qpid-server.bat +++ b/qpid/java/broker/bin/qpid-server.bat @@ -20,9 +20,6 @@ @echo off REM Script to run the Qpid Java Broker -rem stop the Log4J default initialisation, let the broker do it using etc/log4j.xml -if "%QPID_LOG4J_SETTINGS%" == "" set QPID_LOG4J_SETTINGS=-Dlog4j.defaultInitOverride=true - rem Guess QPID_HOME if not defined set CURRENT_DIR=%cd% if not "%QPID_HOME%" == "" goto gotHome @@ -197,7 +194,7 @@ rem QPID_OPTS intended to hold any -D props for use rem user must enclose any value for QPID_OPTS in double quotes :runCommand set MODULE_JARS=%QPID_MODULE_JARS% -set COMMAND="%JAVA_HOME%\bin\java" %JAVA_VM% %JAVA_MEM% %JAVA_GC% %QPID_OPTS% %QPID_LOG4J_SETTINGS% %SYSTEM_PROPS% -cp "%CLASSPATH%;%MODULE_JARS%" org.apache.qpid.server.Main %QPID_ARGS% +set COMMAND="%JAVA_HOME%\bin\java" %JAVA_VM% %JAVA_MEM% %JAVA_GC% %QPID_OPTS% %SYSTEM_PROPS% -cp "%CLASSPATH%;%MODULE_JARS%" org.apache.qpid.server.Main %QPID_ARGS% if "%debug%" == "true" echo %CLASSPATH%;%LAUNCH_JAR%;%MODULE_JARS% if "%debug%" == "true" echo %COMMAND% diff --git a/qpid/java/broker/src/main/java/org/apache/log4j/xml/QpidLog4JConfigurator.java b/qpid/java/broker/src/main/java/org/apache/log4j/xml/QpidLog4JConfigurator.java index aa6694b3b1..4afc209ba7 100644 --- a/qpid/java/broker/src/main/java/org/apache/log4j/xml/QpidLog4JConfigurator.java +++ b/qpid/java/broker/src/main/java/org/apache/log4j/xml/QpidLog4JConfigurator.java @@ -32,9 +32,9 @@ import javax.xml.parsers.ParserConfigurationException; import org.apache.log4j.Level; import org.apache.log4j.Logger; import org.apache.qpid.server.logging.management.LoggingManagementMBean; -import org.apache.qpid.server.logging.management.LoggingManagementMBean.QpidLog4JSaxErrorHandler; import org.xml.sax.ErrorHandler; import org.xml.sax.SAXException; +import org.xml.sax.SAXParseException; /** * Substitute for the Log4J XMLWatchdog (as used by DOMConfigurator.configureAndWatch) @@ -50,6 +50,7 @@ public class QpidLog4JConfigurator //shared with LoggingManagementMBean public static final ReentrantLock LOCK = new ReentrantLock(); private static Logger _logger; + private static DOMConfigurator domConfig = new DOMConfigurator(); private QpidLog4JConfigurator() { @@ -63,10 +64,15 @@ public class QpidLog4JConfigurator { LOCK.lock(); - strictlyParseXMLConfigFile(filename); + parseXMLConfigFile(filename); checkLoggerLevels(filename); DOMConfigurator.configure(filename); + + if(_logger == null) + { + _logger = Logger.getLogger(QpidLog4JConfigurator.class); + } } finally { @@ -77,7 +83,7 @@ public class QpidLog4JConfigurator public static void configureAndWatch(String filename, long delay) throws IOException, ParserConfigurationException, SAXException, IllegalLoggerLevelException { - strictlyParseXMLConfigFile(filename); + parseXMLConfigFile(filename); checkLoggerLevels(filename); QpidLog4JXMLWatchdog watchdog = new QpidLog4JXMLWatchdog(filename); @@ -85,7 +91,7 @@ public class QpidLog4JConfigurator watchdog.start(); } - private static void strictlyParseXMLConfigFile(String fileName) throws IOException, SAXException, + private static void parseXMLConfigFile(String fileName) throws IOException, SAXException, ParserConfigurationException { try @@ -127,6 +133,43 @@ public class QpidLog4JConfigurator } } + public static class QpidLog4JSaxErrorHandler implements ErrorHandler + { + public void error(SAXParseException e) throws SAXException + { + if(_logger != null) + { + _logger.warn(constructMessage("Error parsing XML file", e)); + } + else + { + System.err.println(constructMessage("Error parsing XML file", e)); + } + } + + public void fatalError(SAXParseException e) throws SAXException + { + throw new SAXException(constructMessage("Fatal error parsing XML file", e)); + } + + public void warning(SAXParseException e) throws SAXException + { + if(_logger != null) + { + _logger.warn(constructMessage("Warning parsing XML file", e)); + } + else + { + System.err.println(constructMessage("Warning parsing XML file", e)); + } + } + + private static String constructMessage(final String msg, final SAXParseException ex) + { + return new String(msg + ": Line " + ex.getLineNumber()+" column " +ex.getColumnNumber() + ": " + ex.getMessage()); + } + } + private static class QpidLog4JXMLWatchdog extends XMLWatchdog { public QpidLog4JXMLWatchdog(String filename) @@ -142,7 +185,7 @@ public class QpidLog4JConfigurator try { - strictlyParseXMLConfigFile(filename); + parseXMLConfigFile(filename); } catch (Exception e) { @@ -201,18 +244,43 @@ public class QpidLog4JConfigurator { LOCK.lock(); + //get the Logger levels to check Map loggersLevels; loggersLevels = LoggingManagementMBean.retrieveConfigFileLoggersLevels(filename); + //add the RootLogger to the list too + String rootLoggerlevelString = LoggingManagementMBean.retrieveConfigFileRootLoggerLevel(filename); + loggersLevels.put("Root", rootLoggerlevelString); + for (String loggerName : loggersLevels.keySet()) { String levelString = loggersLevels.get(loggerName); - checkLevel(loggerName,levelString); + + //let log4j replace any properties in the string + String log4jConfiguredString = domConfig.subst(levelString); + + if(log4jConfiguredString.equals("") & ! log4jConfiguredString.equals(levelString)) + { + //log4j has returned an empty string but this isnt what we gave it. + //There may have been an undefined property. Unlike an incorrect + //literal value, we will allow this case to proceed, but warn users. + + if(_logger != null) + { + _logger.warn("Unable to detect Level value from '" + levelString + +"' for logger '" + loggerName + "', Log4J will default this to DEBUG"); + } + else + { + System.err.println("Unable to detect Level value from '" + levelString + +"' for logger " + loggerName + ", Log4J will default this to DEBUG"); + } + + continue; + } + + checkLevel(loggerName,log4jConfiguredString); } - - //check the root logger level - String rootLoggerlevelString = LoggingManagementMBean.retrieveConfigFileRootLoggerLevel(filename); - checkLevel("Root", rootLoggerlevelString); } finally { diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/Main.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/Main.java index 8566ba6270..f8deb95628 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/Main.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/Main.java @@ -69,8 +69,8 @@ import java.util.Properties; @SuppressWarnings({"AccessStaticViaInstance"}) public class Main { - private static final Logger _logger = Logger.getLogger(Main.class); - public static final Logger _brokerLogger = Logger.getLogger("Qpid.Broker"); + private static Logger _logger; + private static Logger _brokerLogger; private static final String DEFAULT_CONFIG_FILE = "etc/config.xml"; @@ -468,7 +468,18 @@ public class Main public static void main(String[] args) { - + //if the -Dlog4j.configuration property has not been set, enable the init override + //to stop Log4J wondering off and picking up the first log4j.xml/properties file it + //finds from the classpath when we get the first Loggers + if(System.getProperty("log4j.configuration") == null) + { + System.setProperty("log4j.defaultInitOverride", "true"); + } + + //now that the override status is know, we can instantiate the Loggers + _logger = Logger.getLogger(Main.class); + _brokerLogger = Logger.getLogger("Qpid.Broker"); + new Main(args); } diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/management/LoggingManagementMBean.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/management/LoggingManagementMBean.java index d98b14e4d5..3c47cdd094 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/management/LoggingManagementMBean.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/logging/management/LoggingManagementMBean.java @@ -40,6 +40,7 @@ import org.apache.log4j.LogManager; import org.apache.log4j.Logger; import org.apache.log4j.xml.Log4jEntityResolver; import org.apache.log4j.xml.QpidLog4JConfigurator; +import org.apache.log4j.xml.QpidLog4JConfigurator.QpidLog4JSaxErrorHandler; import org.apache.log4j.xml.QpidLog4JConfigurator.IllegalLoggerLevelException; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -266,30 +267,6 @@ public class LoggingManagementMBean extends AMQManagedObject implements LoggingM return newLevel; } - //handler to catch errors signalled by the JAXP parser and throw an appropriate exception - public static class QpidLog4JSaxErrorHandler implements ErrorHandler - { - public void error(SAXParseException e) throws SAXException - { - throw new SAXException(constructMessage("Error parsing XML file", e)); - } - - public void fatalError(SAXParseException e) throws SAXException - { - throw new SAXException(constructMessage("Fatal error parsing XML file", e)); - } - - public void warning(SAXParseException e) throws SAXException - { - throw new SAXException(constructMessage("Warning parsing XML file", e)); - } - - private static String constructMessage(final String msg, final SAXParseException ex) - { - return new String(msg + ": Line " + ex.getLineNumber()+" column " +ex.getColumnNumber() + ": " + ex.getMessage()); - } - } - //method to parse the XML configuration file, validating it in the process, and returning a DOM Document of the content. private static synchronized Document parseConfigFile(String fileName) throws IOException { diff --git a/qpid/java/broker/src/test/java/org/apache/log4j/xml/QpidLog4JConfiguratorTest.java b/qpid/java/broker/src/test/java/org/apache/log4j/xml/QpidLog4JConfiguratorTest.java index 643f1fa48e..9f350033d6 100644 --- a/qpid/java/broker/src/test/java/org/apache/log4j/xml/QpidLog4JConfiguratorTest.java +++ b/qpid/java/broker/src/test/java/org/apache/log4j/xml/QpidLog4JConfiguratorTest.java @@ -386,11 +386,11 @@ public class QpidLog4JConfiguratorTest extends TestCase } catch (IllegalLoggerLevelException e) { - fail("Incorrect Exception, expected an IOException"); + //expected, ignore } catch (IOException e) { - //expected, ignore + fail("Incorrect Exception, expected an IllegalLoggerLevelException"); } } } -- cgit v1.2.1