BIND 10 #736: Implement logging configuration

[BIND 10 Development]

#736: Implement logging configuration
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jelte
  stephen                            |                Status:  reviewing
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20110614
                   Priority:         |            Resolution:
  critical                           |             Sensitive:  0
                  Component:         |           Sub-Project:  DNS
  logging                            |  Estimated Difficulty:  7.0
                   Keywords:         |           Total Hours:  0
            Defect Severity:  N/A    |
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => jelte


Comment:

 Reviewed commit 542a15d604018ea73a5a28f26b30b92fb668e399

 '''src/bin/cfgmgr/plugins/b10logging.py'''
 As each logger should have a name, perhaps it would be useful to include
 its name in any error message?

 The check for "filename" occurs whether or not the destination is a file
 (contrary to what the error message says).

 There could be a check that if "facility" is present, the output is to
 syslog.

 Question: Although the logging code makes the distinction between "file",
 "console" and "syslog", is there a need for the user-visible configuration
 to do so?  Could we not remove the "console" destination and have it that
 file output to "stdout" or "stderr" routes output to the console?  (In
 fact, how about replacing "filename", "stream" and "facility" with
 "output", and interpret the value of "output" in a destination-specific
 way?)



 '''src/lib/config/ccsession.h'''
 ModuleCCSession constructor: the documentation for the arguments
 "command_handler" and "start_immediately" is not in the correct place in
 the header.

 Is the "handle_logging" argument to the ModuleCCSession constructor a
 temporary construct?  (Under what circumstances will this module not take
 care of logging configuration?)


 '''src/lib/config/config_data.cc'''
 findListOrMapSubSpec: Is, strictly speaking, the header correct?  It finds
 the inner-most _spec if the specification is nested.

 findItemInSpecList: (throwing an exception when nothing is found.)
 Presumably not finding the element is an unexpected occurrence?  Does this
 make the handling of optional elements more awkward?  Would returning a
 null pointer be better?

 find_spec_part: not in the changes made here, but would use of tokens()
 (in util/strutil.{h,cc}) simplify the code?

 Comment: again not in the changes here, but some of the functions in the
 file are either in the anonymous namespace or are declared static.  This
 means they do not have unit tests.

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