BIND 10 #2225: Implement counters into Xfrout (3/3)

[BIND 10 Development]

#2225: Implement counters into Xfrout (3/3)
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  naokikambe                         |                Status:  reviewing
                       Type:         |             Milestone:
  enhancement                        |  Sprint-20121120
                   Priority:         |            Resolution:
  medium                             |             Sensitive:  0
                  Component:         |           Sub-Project:  DNS
  xfrout                             |  Estimated Difficulty:  7
                   Keywords:         |           Total Hours:  0
            Defect Severity:  N/A    |
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by naokikambe):

 * owner:  naokikambe => jinmei


Comment:

 Thank you for the comments.

 The purposes of my introducing such a statistics library are:

  - to put the counter code in a place, e.g. counter.py
  - to make less dependency between the counter code and the target code,
 e.g. xfrout.py
  - to simplify implementation of counters by inheriting the parent class
  - to simplify creation of the counters for the target code, e.g. by
 invoking ``init()``
  - to make coding test codes easier, e.g. by using inc_xxx() and get_xxx()

 So I'll try to answer your questions along with these purposes.

 Replying to [comment:9 jinmei]:
 > I can't understand the point of the things defined in
 > statistics/counter.py.  Defining a common base class for counters
 > is probably a good idea, but I don't understand what it buys
 > with the convoluted trick of `_COUNTER` and `init()`.  After all,
 > most of xfrout-specific work is implemented in `XfroutCounter`,
 > so it seems to be sufficient if we just use a straightforward mapping
 > (dict) from a counter name to value.

 ``_COUNTER`` and ``init()`` are used for initialized counters of
 b10-xfrout.

 In the future, I planned to put common codes into the parent class.  In
 #2300, which is unreviewed ticket, I tried to put most of common codes
 into the parent class as much as possible.

 > I'm also concerned about the singleton nature of the counter object.
 > IMO, a singleton is generally a source of all problems (very
 > unfriendly with tests to name one) and should be avoided unless
 > absolutely necessary.  Since I don't see the need for the trick, I
 > don't understand why it's absolutely necessary either.

 For example, b10-xfrout should walk with having the counters in each
 library, e.g. notify_out.py. If the target module creates the counters
 once, then it can use them in such a library without any further creation.
 So the counters have to be statically implemented and created. If not,
 multiple instances of the same counter may be created and it may become
 difficult to keep consistency between the values of them.

 > In any case, it's very awkward to see things like inc_notifyoutv4()
 > and `XfroutCounter` are defined in the generic counter module.  It
 > should be defined somewhere else, somewhere more specific to the
 > relevant module(s).  Same sense of comment applies to the module
 > description.  Using some specific module as an example is probably
 > good to help understand it, but I think the general description should
 > be independent from a specific module.

 noftify_out.py can be imported in modules other than xfrout.py like
 zonemgr.py. But in such a module, such counters doesn't need to be used.
 Because it doesn't either import or initialize them.

 > One more specific comment:
 >
 > - I don't understand what this means:
 > {{{#!python
 > These accessors are effective in other module. For example, in case
 > that this module `counter.py` is once imported in such a main module
 > as b10-xfrout, Regarding the item `notifyoutv4`, the incrementer
 > inc_notifyoutv4() can be invoked via other module like notify_out.py,
 > which is firstly imported in the main module.
 > }}}


 Sorry for my poor English ability. I will explain again here by using the
 actual codes.

 The counters such as  inc_notifyoutv4() are imported and initialized at
 b10-xfrout:
 {{{#!python
 30  from isc.statistics import counter
 ..
 112 # setup statistics counter
 113 counter.init(SPECFILE_LOCATION)
 }}}

 Then b10-xfrout's counters, e.g. ``inc_notifyoutv4()``, can be called at
 notify_out.py:
 {{{#!python
  28 from isc.statistics import counter
 ..
 482             # count notifying by IPv4 or IPv6 for statistics
 483             if zone_notify_info.get_socket().family == socket.AF_INET:
 484                 counter.inc_notifyoutv4(zone_notify_info.zone_name)
 }}}

 Unless counters are imported and initialized in such a way, counters like
 ``inc_notifyoutv4()`` cannot be workable in notify_out.py. Because just an
 empty counter is called in notify_out.py, which is directly defined at
 counter.py:
 {{{#!python
  94 # These method are dummies for notify_out in case XfroutCounter is not
  95 # loaded.
  96 def inc_notifyoutv4(self, arg):
  97     """An empty method to be disclosed"""
  98     pass
 }}}

 Regards,

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