BIND 10 #2445: suppress initial log

[BIND 10 Development]

#2445: suppress initial log
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:  jelte
                Type:  defect        |                       Status:
            Priority:  medium        |  reviewing
           Component:  Boss of BIND  |                    Milestone:
            Keywords:                |  Sprint-20121218
           Sensitive:  0             |                   Resolution:
         Sub-Project:  Core          |                 CVSS Scoring:
Estimated Difficulty:  4             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:14 jelte]:

 Okay, I realize I didn't fully understand how the logging
 initialization and reconfiguration work.  Now I've taken a closer look
 at the implementation, and with my revised understanding I'm still not
 convinced a singleton must be inevitably used (meaning other
 alternatives would rather be considered much worse).

 > While I work on the smaller points, let me first comment on two of your
 larger points, since they are somewhat related, and chosen for a reason:
 >
 > 1. Singleton buffer
 > 2. Replacing appenders on specification with empty output options list
 >
 > Currently, behaviour does change on the Impl class level (see below,
 > i'm changing that back), but not on the public API, because (and
 > this is kind of obfuscated by the pimpl implementation; the outer
 > calls contain extra lines of code), in logger_manager.[h|cc]'s
 > existing processSpecfication() calls, the entire logging system is
 > reset; all appenders are destroyed, and new defaults are created
 > (for the root logger). And then it starts working through the
 > specification(s) for the actual logger(s).

 I'd like to clarify a few things here.

 First, by "existing processSpecfication() calls" I guess you actually
 meant `LoggerManager::process()` calls (except for the one you added
 in this branch), which call `processInit()`, which calls
 `LoggerManagerImpl::processInit()`, which calls
 `LoggerManagerImpl::initRootLogger()`, where some default values for
 `b10root` logger object are reset.  But, in my understanding,
 `initRootLogger()` shouldn't destroy existing appenders in `b10root`
 at this point.  Note that `log4cplus::Logger::getInstance()` returns
 an existing logger if one exists.

 Secondly, in `LoggerManagerImpl::processSpecfication()`, existing
 appenders for the specified name of logger can be destroyed, but (in
 the original version before this branch) it seems to happen only if
 output options are given.

 > So the reason I put it in a singleton was to not have to collect all
 possible bufferappenders and keep them around before the call to
 processInit() in loggermanager.h's process() calls; otherwise either
 processInit() would need to change; and in that case behaviour would
 change, since it would no longer automatically support *removing* loggers
 from your specification (they'd remain unchanged), or alternatively, it
 would have to make lists of loggers and bufferappenders before and after
 the change and then go through them, which I suspect would be more brittle
 than this is :)

 Here I'm afraid I don't understand...what do you mean by "no longer
 automatically support removing loggers"?  In my understanding (as I
 said above), there's no current behavior of "automatically removing
 loggers", either in `processInit()` (which actually doesn't touch
 existing ones except for the default log level etc on the root) or in
 `LoggerManagerImpl::processSpecfication()` (unless output options are
 specified).

 With my revised understanding, our goal seems to be reasonably
 achievable by extending processInit():

 {{{#!cpp
 void
 LoggerManagerImpl::processInit() {
     // b10root should exist in the context where this method is called
     log4cplus::Logger b10root = log4cplus::Logger::getInstance(
                                                     getRootLoggerName());
     SharedAppenderPtr buffer_appender = b10root.getAppender("buffer");
     if (buffer_appender) {
         // BufferAppender::flush() is an extension, which would flush the
         // internal buffer
         dynamic_cast<BufferAppender&>(*buffer_appender).flush();
     }

     log4cplus::Logger::getDefaultHierarchy().resetConfiguration();
     initRootLogger();
 }
 }}}

-- 
Ticket URL: <http://bind10.isc.org/ticket/2445#comment:15>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development