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

BIND 10 Development do-not-reply at isc.org
Mon Aug 27 11:33:40 UTC 2012


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

 * owner:  y-aharen => jinmei


Comment:

 Replying to [comment:7 jinmei]:
 > I've read some part of the branch, but I now think it's better to
 > review this stuff after #2154 is completed (or at least after we are
 > sure what to do with it).
 >
 > Here are some intermediate comments, which may or may not matter
 > depending on the #2154 discussion:
 Thank you for your comment. Please make this ticket unassigned if it is
 inappropriate to reassign review to you at this time.

 > '''auth_srv'''
 >
 > - get()/dump() are too generic.  I'd rename them, e.g.,
 >   getStatistics(), dumpStatistics(), etc.
 I renamed them as in stats2012-auth-merge. (git d706823)

 > - Similarly, the description is quite ambiguous:
 > {{{#!cpp
 >     /// \brief Get the values of specified counters.
 >     ///
 >     /// This function returns names and values of counter.
 > }}}
 >   For example, it's not clear what "counters" means.
 > - Description for get/dump doesn't seem to be sufficient in general.
 >   Need to describe parameters; describe in which case an exception is
 >   thrown, describe return value in more detail, and maybe describe the
 >   overall method behavior in more detail.
 I added some doxygen comments. (git 87c49c1)

 > - the const in the return type for get/dump is meaningless:
 > {{{#!cpp
 >     const isc::auth::statistics::Counters::item_tree_type get(
 >         const isc::auth::statistics::Counters::item_node_name_set_type&
 items)
 >         const;
 > }}}
 >   If you intended something like `const TreeType*` by this, what we
 >   should do is to define item_tree_type as `ConstElementPtr`
 >   (and then remove the unnecessary const).  Same comment applies to
 >   the `Counters` class.
 I simply removed consts for in case caller of getStatistics() will add
 some extra statistics data into it. (git 0880746)

 > '''statistics.h'''
 > - item_tree_type does not seem to be aligned with our type naming
 >   convention.  It would normally be named `ItemTreeType`.
 >   same for item_node_name_type and item_node_name_set_type.
 I renamed them in accordance with naming convention. (git ffe7ffd)

 For improving performance and readability, I modified Counters::inc() to
 use lookup table for converting from protocol code to counter item. (git
 6760e04)

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


More information about the bind10-tickets mailing list