BIND 10 #758: Conversion of bind10 program to use new logging interface

[BIND 10 Development]

#758: Conversion of bind10 program to use new logging interface
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jelte
  stephen                            |                Status:  closed
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20110712
                   Priority:  major  |            Resolution:  complete
                  Component:         |             Sensitive:  0
  logging                            |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  2.0
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by jelte):

 * status:  reviewing => closed
 * resolution:   => complete


Comment:

 Replying to [comment:10 jinmei]:
 > Replying to [comment:9 jelte]:
 >
 > I don't have any more blocking issues.  Please feel free to merge.
 >

 Ok, thanks, done. Closing ticket. Some final thoughts below.


 > I wanted to make further comments on some selected points, though,
 > which follow:
 >
 > > > '''specifics'''
 >
 > I don't have an objection to the change, and I tend to agree that
 > bind10[.py] is not a good (if not "wrong") name.  I have one note,
 > though, that (if I remember it correctly) we add the b10- prefix to
 > the child programs so that we can easily find them by, e.g., ps +
 > grep.  It doesn't apply to the "boss" program, and we intentionally
 > didn't add "b10-" to it.  Probably you knew that but chose to make log
 > outputs consistent, in which case I don't oppose to that (actually I
 > don't have a strong opinion on this point).  But we'll probably need
 > comprehensive documentation for logging in general, and we should
 > describe these conventions there.
 >

 Ah. I thought it was mostly to be able to start everything with 'bind10'
 (which could simply be achieved with a script), if that was the original
 decision, I may have changed my mind, and I see the boss as 'just' a
 module (granted, with the responsibility to start and stop others, but
 still)


 > > > - BIND10_START_AS_NON_ROOT: If we have a ticket for the socket
 > > >   creator, I'd add a note to it about this description:
 > >
 > > ok, there's two tickets, neither of which is 'the ticket' for
 inclusion (#800 and #801), but i added them anyway.
 >
 > Actually, what I meant was to add comments *to* the tickets that when
 > they are done the log message for the boss will have to be updated.
 > I'd leave it to you whether to keep the (added) reference to the
 > ticket numbers in the log message.
 >

 ah, sorry, i misunderstood you. Added comment to #800, leaving the comment
 in.

 >
 > See #1086.  The use of # is purely for compatibility with BIND 9.  I
 > agree the format is uncommon per se, and I'm okay with reconsidering
 > it (if other points are neutral, consistency/compatibility with BIND 9
 > would generally be good, but when a particular behavior of it is
 > considered a bad practice we should rather change it).
 >

 ok i've now been using it in another logging ticket as well.

 > > > - in my test run (with -v, from source), I couldn't find logs in
 > > >   log_starting().
 >
 > > no i originally set them to DEBUG, since i didn't like the 3 terminal
 screens of STARTING/STARTED messages. So they are not printed by default
 (since default is INFO). Changed STARTING message to severity INFO, and i
 left STARTED at DEBUG, to make it the output a bit less. But perhaps we
 should remove the STARTED log completely, and only log something if
 starting failed.
 >
 > Ah, okay.  Hmm, I don't have a strong opinion about info vs debug, but
 > if they are not so frequent (I suspect not), I'd slightly prefer info.
 > Also, considering the option of starting a selected subset of
 > processes, I think logging the (successful) event would be still
 > useful.
 >
 > > > - BIND10_SEND_SIGTERM: I'd use "notice" (if we have it) because it's
 > > >   not an unexpected case even if not considered an error.  also, you
 > > >   may want to refer to BIND10_SHUTDOWN to explain when this happens.
 > > >   Same for BIND10_SEND_SIGKILL.
 > >
 > > there's no notice, so info seems closest (which it already is).
 There's only one case where this isn't unexpected, and that is for msgq,
 btw.
 >
 > Okay about the change.  As for the 'btw' part, I actually meant
 > "because it's an unexpected case" (although I'm still not sure if I
 > understand your btw comment).
 >

 my btw comment was that sending sigterm *is* unexpected, or it should be
 (but it looks like bind10 is too impatient). I think that the only case
 where it always sends a SIGTERM is to stop msgq, all other modules should
 get a shutdown command first.


 > > > - BIND10_PROCESS_ENDED_WITH_EXIT_STATUS: as I said on jabber in a
 > > >   different context I personally don't like to repeat the same
 content
 > > >   in multiple places (most of it is a duplicate of NO_EXIT_STATUS),
 > > >   especially for text this long.  But understanding this may be a
 > > >   matter of opinion, I'd leave it to you.
 > >
 > > I actually disagree with the general premise, but in this case a
 reference will do (due to the way these messages are tied together in the
 way that they are conditionally called)
 >
 > Okay.  But I'd like to clarify one thing: my major point in this
 > context is "don't repeat yourself".  Using a reference to another log
 > ID is just one possible solution to the problem.  It was not clear
 > whether you agree with the major point but are just not comfortable
 > with the solution using the reference, or you disagree with the major
 > point in the first place.  If it's the former, we could consider other
 > solutions (e.g., introducing a macro block that can be shared by
 > multiple log messages).  If it's the latter, that's fine, too; I
 > understand that may be a matter of opinion.
 >

 I personally don't have a big problem with repeated descriptions, *if* the
 descriptions are about separate code paths which happen to have similar
 logic/results. This may point to a refactoring possibility in the code,
 but as long as the different code paths are there, they can be changed,
 and their description should be changed as well, which isn't possible if
 there is just one description (since if you change it it may be wrong for
 the other code path). I do agree on the 'repetition is bad' in general, I
 just wanted to point out there are cases where it can be less bad :) (yes
 this is a matter of opinion)

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