BIND 10 #393: Marking places where logging would happen

[BIND 10 Development]

#393: Marking places where logging would happen
-------------------------------+--------------------------------------------
      Reporter:  vorner        |        Owner:  vorner   
          Type:  enhancement   |       Status:  reviewing
      Priority:  major         |    Milestone:           
     Component:  Unclassified  |   Resolution:           
      Keywords:                |    Sensitive:  0        
Estimatedhours:  0.0           |        Hours:  0        
      Billable:  1             |   Totalhours:  0        
      Internal:  0             |  
-------------------------------+--------------------------------------------
Changes (by stephen):

  * owner:  UnAssigned => vorner


Comment:

 Only when reviewing I discovered how difficult it is to decide where
 logging should be without knowing the details of the framework and in
 particular, what granularity you will be logging.

 I have assumed in this review that ultimately there will be both high-
 level event logging ("server started" etc) and some debug logging (tracing
 queries), and that we need to mark both types of log entries.

 Using dlog to mark the log messages is a good idea - it does provide an
 easy way of turning basic logging on which will be useful in development.


 '''asiolink.cc'''[[BR]]
 RecursiveQuery::sendQuery[[BR]]
 I think that what is being queried for (qname/qtype/qclass) would also be
 relevant here in addition to the nameserver to which the query is being
 sent.  It will help to trace the upstream query chain for a particular
 inbound request.

 However, given that what happens is that a UDPquery object is instantiated
 and posted, it might be better to put the log message into UDPquery
 itself; in particular, add a log message before the asynch_send_to call
 and one after the async_receive_from call.  That way we can trace both the
 query sent and the fact that a response was received.  (The same goes for
 TCPQuery.)

 '''main.cc'''[[BR]]
 Logging the command line the server was started with might be useful.

 '''recursor.cc'''[[BR]]
 In "Recursor::processMessage", there is a block that is commented as
 "Perform further protocol-level validation", only some of the checks
 result in a log message. Either all should or none should.

 Other points that are noted here for reference, but do not need changing
 until the logging framework is complete are:

 '''recursor.cc'''[[BR]]
 The "Adding RRset to message" message would benefit from including the
 section to which the RRset is being added and the type or RRset being
 added.

 "unsupported opcode" - it would be helpful to know what opcode was
 received.

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