BIND 10 #2157: Create an interface to pass statistics counters in Auth module

[BIND 10 Development]

#2157: Create an interface to pass statistics counters in Auth module
-------------------------------------+-------------------------------------
            Reporter:  y-aharen      |                        Owner:
                Type:  enhancement   |  jinmei
            Priority:  medium        |                       Status:
           Component:  b10-auth      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130205
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  8             |                 CVSS Scoring:
         Total Hours:  36.66         |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:62 y-aharen]:

 > > > I applied your suggestion for b10-auth.8. Thank you.
 > > >
 > > > While testing master with the branch in some environment, I found
 lettuce test
 > > > fails occasionally. I added a workaround for it (git 1c470ce). If
 it's OK,
 > >
 > > Please be more specific: what was wrong and how it solved the issue.
 > > And, in any case, when Jeremy wakes up we should probably create a
 > > merge branch and run it on buildbots.
 > It was insufficient check for log message output.
 > In lettuce test, it checks the counters are expected. Counters are read
 from Stats module.
 > To avoid reading counters before they've got updated, it checks log
 message output; whether
 > Auth module receives 'getstats' command and responded to it. It had
 checked only for
 > Auth module receives 'getstats' command and it failed occasionally in
 some environments.
 > I added a check for response.

 I'd be concerned about the keywords used there.  Both "CC_REPLY" and
 '"v4":' are pretty short and could match other things (especially the
 case for the former); it's also not clear what we are waiting for by
 just seeing them in the test feature.  Further, don't they necessarily
 ensure the statistics are updated at b10-stats, just indicating some
 related messages arrived?

 But, I guess they were chosen because there was no better alternative
 (I've not looked at the code); if so, and if it worked in practice, I
 wouldn't block the merge just for that.  But I'd suggest the
 following:

 - add comment about what these expect at least at one of the places
   where they were used
 {{{
         And wait for new bind10 stderr message CC_REPLY
         And wait for new bind10 stderr message "v4":
 }}}
 - create a ticket to introduce a better keyword (probably adding a log
   message to b10-stats) and update the test.  It should be an easy
   task, so probably do it soonish.

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