BIND 10 #762: Conversion of xfrout to use the new logging interface

[BIND 10 Development]

#762: Conversion of xfrout to use the new logging interface
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  stephen                            |                Status:  reviewing
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20110628
                   Priority:  major  |            Resolution:
                  Component:         |             Sensitive:  0
  logging                            |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  0.0
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 '''General'''

 I've noticed some IDs in .mes are not used in .py and some other IDs
 in .py are not defined in .mes.  Some specific cases are noted below,
 but that's not a comprehensive check.  I guess we need a generic
 checker script to identify these types of errors.  Also, ideally (as
 also commented below), we should have test cases for log messages to
 check whether intended log messages are really produced.

 '''Specifics'''
 - XFROUT_IMPORT_DNS: this can also fail due to failure of loading
   libutil..?  Also, it would be nicer if the detailed explanation
   could provide some more details of "a PYTHONPATH problem" and of how
   it can be fixed.

 - XFROUT_HANDLE_QUERY: should %e be %1?  If so, it also seems to
   suggest that ideally we should have tests to check if expected log
   messages are produced (although it may not be easy, and would
   probably be beyond the scope of this ticket).

 - XFROUT_PARSE_QUERY
   - "to xfrout" may be redundant?
   - "in principle" this should not happen except for really unexpected
     events like memory allocation failure because b10-auth should have
     handled bogus packets.

 - XFROUT_AXFR_TRANSFER_FAILED
  - maybe we should print RR class, too, as the commented-out original
    log tried to do (but if we do, don't hardcode it please:-)?
  - according to the code, "REFUSED" happens the xfrout server reaches
    its max-transfer-out quota.  "not allowed" (as written in the
    detailed description) is not necessarily wrong, but to me this term
    sounds more like refusal due to ACL

 - XFROUT_AXFR_TRANSFER_STARTED
  - print RR class?
  - should we want to log the "END" event as the original
    (commented-out) log tried?

 - XFROUT_AXFR_TRANSFER_ERROR
  - print RR class?

 - XFROUT_STOPPING: this is a specific event happening in the middle of
   transfer.  I'd clarify that fact.

 - XFROUT_REQUEST_FETCH
   - This doesn't seem to be defined. (another case that would suggest
     the need for tests...)
   - since this is an error event, I'd name it accordingly (FETCH seems
     like a normal event).

 - XFROUT_SOCKET_SELECT
   - need %1?
   - better name to indicate it's an error?
   - does this really indicate "an error in the internal communication with
 b10-auth"?

 - XFROUT_PROCESS_REQUEST
   - I suspect "client_address" here means b10-auth ('s unix domain
     socket?) and is mostly meaningless.
   - better name to indicate it's an error?..hmm...this point seems to
     be common for many others, so I won't repeat.  it at least looks
     awkward that some error logs are named xxx_ERROR and others
     aren't.

 - XFROUT_RECEIVE_FILE_DESCRIPTOR: I'm not sure if the description is
   accurate, but for now I'm okay with it.

 - XFROUT_UNIX_SOCKET_FILE_IN_USE
  - need %1?
  - I don't understand this: "This xfrout daemon will now shut down."
    Is "this xfrout daemon" the other process?  The daemon that emits
    this message actual exits (if that means 'shut down').

 - Is the log ID correct here?
 {{{
         try:
             os.unlink(self._sock_file)
         except Exception as e:
             logger.error(XFROUT_REMOVE_UNIX_SOCKET_FILE, str(e))
 }}}
   Note that XFROUT_REMOVE_UNIX_SOCKET_FILE should take two args.

 - XFROUT_REMOVE_OLD_UNIX_SOCKET_FILE: doesn't seem to be used in .py.

 - XFROUT_CC_SESSION_TIMEOUT_ERROR: for the detailed description, "The
   most likely cause it that..." should be "The most likely cause of it
   that..."?

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