This is an automated email from the ASF dual-hosted git repository. swebb2066 pushed a commit to branch harmonize_internal_logging_messages in repository https://gitbox.apache.org/repos/asf/logging-log4cxx.git
commit 017c2ec5ef3687e2be89398a5b523522b18dd592 Author: Stephen Webb <[email protected]> AuthorDate: Thu Aug 28 10:45:23 2025 +1000 Improve internal debug logging message consistency in configuration file parsing --- src/main/cpp/domconfigurator.cpp | 30 ++++++++------ src/main/cpp/optionconverter.cpp | 28 ++++++------- src/main/cpp/propertyconfigurator.cpp | 75 ++++++++++++++--------------------- 3 files changed, 59 insertions(+), 74 deletions(-) diff --git a/src/main/cpp/domconfigurator.cpp b/src/main/cpp/domconfigurator.cpp index 93102d9c..172eacf9 100644 --- a/src/main/cpp/domconfigurator.cpp +++ b/src/main/cpp/domconfigurator.cpp @@ -229,7 +229,8 @@ AppenderPtr DOMConfigurator::parseAppender(Pool& p, LogString className(subst(getAttribute(utf8Decoder, appenderElement, CLASS_ATTR))); if (LogLog::isDebugEnabled()) { - LogLog::debug(LOG4CXX_STR("Class name: [") + className + LOG4CXX_STR("]")); + LogLog::debug(LOG4CXX_STR("Desired ") + Appender::getStaticClass().getName() + + LOG4CXX_STR(" sub-class: [") + className + LOG4CXX_STR("]")); } try @@ -318,7 +319,7 @@ AppenderPtr DOMConfigurator::parseAppender(Pool& p, { LogLog::debug(LOG4CXX_STR("Attaching appender named [") + refName + LOG4CXX_STR("] to appender named [") + - appender->getName() + LOG4CXX_STR("].")); + appender->getName() + LOG4CXX_STR("]")); } aa->addAppender(findAppenderByReference(p, utf8Decoder, currentElement, doc, appenders)); } @@ -473,7 +474,7 @@ void DOMConfigurator::parseLogger( if (LogLog::isDebugEnabled()) { LogLog::debug(LOG4CXX_STR("Setting [") + logger->getName() + LOG4CXX_STR("] additivity to [") + - (additivity ? LogString(LOG4CXX_STR("true")) : LogString(LOG4CXX_STR("false"))) + LOG4CXX_STR("].")); + (additivity ? LogString(LOG4CXX_STR("true")) : LogString(LOG4CXX_STR("false"))) + LOG4CXX_STR("]")); } logger->setAdditivity(additivity); parseChildrenOfLoggerElement(p, utf8Decoder, loggerElement, logger, false, doc, appenders); @@ -497,7 +498,8 @@ void DOMConfigurator::parseLoggerFactory( { if (LogLog::isDebugEnabled()) { - LogLog::debug(LOG4CXX_STR("Desired logger factory: [") + className + LOG4CXX_STR("]")); + LogLog::debug(LOG4CXX_STR("Desired ") + LoggerFactory::getStaticClass().getName() + + LOG4CXX_STR(" sub-class: [") + className + LOG4CXX_STR("]")); } std::shared_ptr<Object> obj = OptionConverter::instantiateByClassName( className, @@ -562,7 +564,7 @@ void DOMConfigurator::parseChildrenOfLoggerElement( { if (LogLog::isDebugEnabled()) LogLog::debug(LOG4CXX_STR("Adding appender named [") + refName + - LOG4CXX_STR("] to logger [") + logger->getName() + LOG4CXX_STR("].")); + LOG4CXX_STR("] to logger [") + logger->getName() + LOG4CXX_STR("]")); newappenders.push_back(appender); } else @@ -604,7 +606,8 @@ LayoutPtr DOMConfigurator::parseLayout ( LogString className(subst(getAttribute(utf8Decoder, layout_element, CLASS_ATTR))); if (LogLog::isDebugEnabled()) { - LogLog::debug(LOG4CXX_STR("Parsing layout of class: \"") + className + LOG4CXX_STR("\"")); + LogLog::debug(LOG4CXX_STR("Desired ") + Layout::getStaticClass().getName() + + LOG4CXX_STR(" sub-class: [") + className + LOG4CXX_STR("]")); } try @@ -647,7 +650,8 @@ ObjectPtr DOMConfigurator::parseTriggeringPolicy ( LogString className = subst(getAttribute(utf8Decoder, layout_element, CLASS_ATTR)); if (LogLog::isDebugEnabled()) { - LogLog::debug(LOG4CXX_STR("Parsing triggering policy of class: \"") + className + LOG4CXX_STR("\"")); + LogLog::debug(LOG4CXX_STR("Desired ") + TriggeringPolicy::getStaticClass().getName() + + LOG4CXX_STR(" sub-class: [") + className + LOG4CXX_STR("]")); } try @@ -703,7 +707,8 @@ RollingPolicyPtr DOMConfigurator::parseRollingPolicy ( LogString className = subst(getAttribute(utf8Decoder, layout_element, CLASS_ATTR)); if (LogLog::isDebugEnabled()) { - LogLog::debug(LOG4CXX_STR("Parsing rolling policy of class: \"") + className + LOG4CXX_STR("\"")); + LogLog::debug(LOG4CXX_STR("Desired ") + RollingPolicy::getStaticClass().getName() + + LOG4CXX_STR(" sub-class: [") + className + LOG4CXX_STR("]")); } try @@ -755,7 +760,7 @@ void DOMConfigurator::parseLevel( LogString levelStr(subst(getAttribute(utf8Decoder, element, VALUE_ATTR))); if (LogLog::isDebugEnabled()) { - LogLog::debug(LOG4CXX_STR("Level value for ") + loggerName + LOG4CXX_STR(" is [") + levelStr + LOG4CXX_STR("].")); + LogLog::debug(LOG4CXX_STR("Level value for ") + loggerName + LOG4CXX_STR(" is [") + levelStr + LOG4CXX_STR("]")); } if (StringHelper::equalsIgnoreCase(levelStr, LOG4CXX_STR("INHERITED"), LOG4CXX_STR("inherited")) @@ -763,7 +768,7 @@ void DOMConfigurator::parseLevel( { if (isRoot) { - LogLog::error(LOG4CXX_STR("Root level cannot be inherited. Ignoring directive.")); + LogLog::error(LOG4CXX_STR("Root level cannot be ") + levelStr + LOG4CXX_STR(". Ignoring directive.")); } else { @@ -782,7 +787,8 @@ void DOMConfigurator::parseLevel( { if (LogLog::isDebugEnabled()) { - LogLog::debug(LOG4CXX_STR("Desired Level sub-class: [") + className + LOG4CXX_STR("]")); + LogLog::debug(LOG4CXX_STR("Desired ") + Level::getStaticClass().getName() + + LOG4CXX_STR(" sub-class: [") + className + LOG4CXX_STR("]")); } try @@ -880,7 +886,7 @@ spi::ConfigurationStatus DOMConfigurator::doConfigure if (LogLog::isDebugEnabled()) { LogString debugMsg = LOG4CXX_STR("Loading configuration file [") - + filename.getPath() + LOG4CXX_STR("]."); + + filename.getPath() + LOG4CXX_STR("]"); LogLog::debug(debugMsg); } diff --git a/src/main/cpp/optionconverter.cpp b/src/main/cpp/optionconverter.cpp index 178e2437..c225059b 100644 --- a/src/main/cpp/optionconverter.cpp +++ b/src/main/cpp/optionconverter.cpp @@ -322,39 +322,30 @@ LevelPtr OptionConverter::toLevel(const LogString& value, if (hashIndex == LogString::npos) { + // no class name specified : use standard Level class if (value.empty()) { return defaultValue; } else { - if (LogLog::isDebugEnabled()) - { - LogLog::debug( - ((LogString) LOG4CXX_STR("OptionConverter::toLevel: no class name specified, level=[")) - + value - + LOG4CXX_STR("]")); - } - // no class name specified : use standard Level class return Level::toLevelLS(value, defaultValue); } } LogString clazz = value.substr(hashIndex + 1); LogString levelName = value.substr(0, hashIndex); - if (LogLog::isDebugEnabled()) - { - LogLog::debug(LOG4CXX_STR("OptionConverter::toLevel: class=[") - + clazz + LOG4CXX_STR("], level=[") - + levelName + LOG4CXX_STR("]") - ); - } // This is degenerate case but you never know. - if (levelName.empty()) + if (levelName.empty() || clazz.empty()) { return Level::toLevelLS(value, defaultValue); } + if (LogLog::isDebugEnabled()) + { + LogLog::debug(LOG4CXX_STR("Desired ") + Level::getStaticClass().getName() + + LOG4CXX_STR(" sub-class: [") + clazz + LOG4CXX_STR("]")); + } try { @@ -418,6 +409,11 @@ ObjectPtr OptionConverter::instantiateByClassName(const LogString& className, { if (!className.empty()) { + if (LogLog::isDebugEnabled()) + { + LogLog::debug(LOG4CXX_STR("Desired ") + superClass.getName() + + LOG4CXX_STR(" sub-class: [") + className + LOG4CXX_STR("]")); + } try { const Class& classObj = Loader::loadClass(className); diff --git a/src/main/cpp/propertyconfigurator.cpp b/src/main/cpp/propertyconfigurator.cpp index cf3a555b..d2c5b0c5 100644 --- a/src/main/cpp/propertyconfigurator.cpp +++ b/src/main/cpp/propertyconfigurator.cpp @@ -133,7 +133,7 @@ spi::ConfigurationStatus PropertyConfigurator::doConfigure if (LogLog::isDebugEnabled()) { LogString debugMsg = LOG4CXX_STR("Loading configuration file [") - + configFileName.getPath() + LOG4CXX_STR("]."); + + configFileName.getPath() + LOG4CXX_STR("]"); LogLog::debug(debugMsg); } return doConfigure(props, hierarchy); @@ -196,7 +196,7 @@ spi::ConfigurationStatus PropertyConfigurator::doConfigure(helpers::Properties& { LogLog::debug(((LogString) LOG4CXX_STR("Hierarchy threshold set to [")) + hierarchy->getThreshold()->toString() - + LOG4CXX_STR("].")); + + LOG4CXX_STR("]")); } } @@ -245,7 +245,7 @@ void PropertyConfigurator::configureLoggerFactory(helpers::Properties& props) { LogString msg(LOG4CXX_STR("Setting logger factory to [")); msg += factoryClassName; - msg += LOG4CXX_STR("]."); + msg += LOG4CXX_STR("]"); LogLog::debug(msg); } std::shared_ptr<Object> instance = std::shared_ptr<Object>( @@ -339,10 +339,8 @@ bool PropertyConfigurator::parseAdditivityForLogger(helpers::Properties& props, bool additivity = OptionConverter::toBoolean(value, true); if (LogLog::isDebugEnabled()) { - LogLog::debug(((LogString) LOG4CXX_STR("Setting additivity for \"")) - + loggerName - + ((additivity) ? LOG4CXX_STR("\" to true") : - LOG4CXX_STR("\" to false"))); + LogLog::debug(LOG4CXX_STR("Setting [") + loggerName + LOG4CXX_STR("] additivity to [") + + (additivity ? LogString(LOG4CXX_STR("true")) : LogString(LOG4CXX_STR("false")) + LOG4CXX_STR("]"))); } return additivity; @@ -363,7 +361,7 @@ void PropertyConfigurator::parseLogger( LogLog::debug(((LogString) LOG4CXX_STR("Parsing for [")) + loggerName + LOG4CXX_STR("] with value=[") - + value + LOG4CXX_STR("].")); + + value + LOG4CXX_STR("]")); } // We must skip over ',' but not white space @@ -380,12 +378,6 @@ void PropertyConfigurator::parseLogger( } LogString levelStr = st.nextToken(); - if (LogLog::isDebugEnabled()) - { - LogLog::debug((LogString) LOG4CXX_STR("Level token is [") - + levelStr + LOG4CXX_STR("].")); - } - // If the level value is inherited, set logger level value to // null. We also check that the user has not specified inherited for the @@ -393,32 +385,23 @@ void PropertyConfigurator::parseLogger( if (StringHelper::equalsIgnoreCase(levelStr, LOG4CXX_STR("INHERITED"), LOG4CXX_STR("inherited")) || StringHelper::equalsIgnoreCase(levelStr, LOG4CXX_STR("NULL"), LOG4CXX_STR("null"))) { - static const WideLife<LogString> INTERNAL_ROOT_NAME(LOG4CXX_STR("root")); - - if (loggerName == INTERNAL_ROOT_NAME.value()) + if (loggerName == LOG4CXX_STR("root")) { - LogLog::warn(LOG4CXX_STR("The root logger cannot be set to null.")); + LogLog::warn(LOG4CXX_STR("Root level cannot be ") + levelStr + LOG4CXX_STR(". Ignoring directive.")); } else { logger->setLevel(0); - if (LogLog::isDebugEnabled()) - { - LogLog::debug((LogString) LOG4CXX_STR("Logger ") - + loggerName + LOG4CXX_STR(" set to null")); - } } } else { logger->setLevel(OptionConverter::toLevel(levelStr, Level::getDebug())); - - if (LogLog::isDebugEnabled()) - { - LogLog::debug((LogString) LOG4CXX_STR("Logger ") - + loggerName + LOG4CXX_STR(" set to ") - + logger->getLevel()->toString()); - } + } + if (LogLog::isDebugEnabled()) + { + LogLog::debug(loggerName + LOG4CXX_STR(" level set to ") + + logger->getEffectiveLevel()->toString()); } } @@ -438,8 +421,8 @@ void PropertyConfigurator::parseLogger( if (LogLog::isDebugEnabled()) { - LogLog::debug(LOG4CXX_STR("Parsing appender named ") - + appenderName + LOG4CXX_STR("\".")); + LogLog::debug(LOG4CXX_STR("Parsing appender named [") + + appenderName + LOG4CXX_STR("]")); } appender = parseAppender(props, appenderName); @@ -461,8 +444,8 @@ AppenderPtr PropertyConfigurator::parseAppender( { if (LogLog::isDebugEnabled()) { - LogLog::debug((LogString) LOG4CXX_STR("Appender \"") - + appenderName + LOG4CXX_STR("\" was already parsed.")); + LogLog::debug((LogString) LOG4CXX_STR("Appender [") + + appenderName + LOG4CXX_STR("] was already parsed.")); } return appender; @@ -491,8 +474,8 @@ AppenderPtr PropertyConfigurator::parseAppender( if (!appender) { - LogLog::error((LogString) LOG4CXX_STR("Could not instantiate appender named \"") - + appenderName + LOG4CXX_STR("\".")); + LogLog::error((LogString) LOG4CXX_STR("Could not instantiate appender named [") + + appenderName + LOG4CXX_STR("]")); return 0; } @@ -515,15 +498,15 @@ AppenderPtr PropertyConfigurator::parseAppender( appender->setLayout(layout); if (LogLog::isDebugEnabled()) { - LogLog::debug((LogString) LOG4CXX_STR("Parsing layout options for \"") - + appenderName + LOG4CXX_STR("\".")); + LogLog::debug((LogString) LOG4CXX_STR("Parsing layout options for [") + + appenderName + LOG4CXX_STR("]")); } PropertySetter::setProperties(layout, props, layoutPrefix + LOG4CXX_STR("."), p); if (LogLog::isDebugEnabled()) { - LogLog::debug((LogString) LOG4CXX_STR("End of parsing for \"") - + appenderName + LOG4CXX_STR("\".")); + LogLog::debug((LogString) LOG4CXX_STR("End of parsing for [") + + appenderName + LOG4CXX_STR("]")); } } } @@ -545,8 +528,8 @@ AppenderPtr PropertyConfigurator::parseAppender( if (LogLog::isDebugEnabled()) { - LogLog::debug((LogString) LOG4CXX_STR("Parsing rolling policy options for \"") - + appenderName + LOG4CXX_STR("\".")); + LogLog::debug((LogString) LOG4CXX_STR("Parsing rolling policy options for [") + + appenderName + LOG4CXX_STR("]")); } PropertySetter::setProperties(rollingPolicy, props, rollingPolicyKey + LOG4CXX_STR("."), p); } @@ -566,8 +549,8 @@ AppenderPtr PropertyConfigurator::parseAppender( if (LogLog::isDebugEnabled()) { - LogLog::debug((LogString) LOG4CXX_STR("Parsing triggering policy options for \"") - + appenderName + LOG4CXX_STR("\".")); + LogLog::debug((LogString) LOG4CXX_STR("Parsing triggering policy options for [") + + appenderName + LOG4CXX_STR("]")); } PropertySetter::setProperties(triggeringPolicy, props, triggeringPolicyKey + LOG4CXX_STR("."), p); } @@ -577,8 +560,8 @@ AppenderPtr PropertyConfigurator::parseAppender( PropertySetter::setProperties(appender, props, prefix + LOG4CXX_STR("."), p); if (LogLog::isDebugEnabled()) { - LogLog::debug((LogString) LOG4CXX_STR("Parsed \"") - + appenderName + LOG4CXX_STR("\" options.")); + LogLog::debug((LogString) LOG4CXX_STR("Parsed [") + + appenderName + LOG4CXX_STR("] options.")); } }
