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