Opened 7 years ago

Closed 7 years ago

#2158 closed enhancement (fixed)

Implement counters into Xfrout (1/3)

Reported by: naokikambe Owned by: naokikambe
Priority: medium Milestone: Sprint-20120918
Component: xfrout Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 6 Add Hours to Ticket: 0
Total Hours: 11.25 Internal?: no

Description (last modified by naokikambe)

Implement the following per-zone counters into Xfrout regarding StatisticsItems:

ModuleItemin BIND 9 Statistics XML (tree representation under isc/bind/statistics)Description from BIND 9 ManualImplemented in BIND 10?
Xfrout
xfrout.[zonename].notifyoutv4server/zsstat/NotifyOutv4Number of IPv4 notifies sent from Xfroutno
xfrout.[zonename].notifyoutv6server/zsstat/NotifyOutv6Number of IPv6 notifies sent from Xfroutno
xfrout.[zonename].xfrrejserver/nsstat/XfrRejNumber of XFR requests rejectedno
xfrout.[zonename].xfrreqdoneserver/nsstat/XfrReqDoneNumber of requested zone transfers completedno

IMO at least updating three classes(XfroutServer, XfroutSession and NotifyOut) is needed for implementing counters. And adding 'getstats' command for returning statistics to the stats module is also needed. A dedicated statistics counter class might need to be added. Some test, e.g. lettuce scenario, might be needed for testing the increasing values of the new Xfrout counters.

Subtickets

Change History (13)

comment:1 Changed 7 years ago by naokikambe

  • Description modified (diff)

comment:2 Changed 7 years ago by jinmei

I'm not sure if we want to do this on top of the current
implementation as we plan to revise the whole xfr* fundamentally.

comment:3 Changed 7 years ago by naokikambe

After that fundamental revision, we should implement Xfrin/Xfrout counters. This ticket doesn't need to be worked on until completion of the revision. Due to that, some of counters might not be implemented by the end of Sep. as we initially planned.

comment:4 Changed 7 years ago by shane

  • Milestone changed from New Tasks to StatsRedesign

comment:5 Changed 7 years ago by naokikambe

  • Description modified (diff)
  • Owner set to naokikambe
  • Status changed from new to accepted

Though that fundamental revision is not done yet, I try to add the counters based on the current Xfrout codes.

comment:6 Changed 7 years ago by naokikambe

  • Component changed from statistics to xfrout

comment:7 Changed 7 years ago by naokikambe

  • Owner changed from naokikambe to UnAssigned
  • Status changed from accepted to reviewing

The branch can be reviewed. Regarding a proposed ChangeLog entry, please refer to the branch. Thanks,

comment:8 Changed 7 years ago by jelte

  • Milestone changed from StatsRedesign to Sprint-20120918

comment:9 Changed 7 years ago by jelte

  • Owner changed from UnAssigned to jelte

comment:10 follow-up: Changed 7 years ago by jelte

  • Owner changed from jelte to naokikambe

Just a few minor comments:

notify_out.py:

when checking the counter_notifyoutvX arguments, I'd probably raise an exception if they are not callable (instead of silently ignoring them).

I'm also not sure whether putting in a noop callable (lambda: x: None) is more efficient than checking for 'is not None' in send_notify_message. (do you happen to know if python optimizes that away?)

xfrout.py[.in]:

And the same goes for xfrout, btw :)

There is some dead code at lines 1026 and 1027

For the rest it looks good

comment:11 in reply to: ↑ 10 Changed 7 years ago by naokikambe

  • Owner changed from naokikambe to jelte

Hello, thank you for the comments.

Replying to jelte:

when checking the counter_notifyoutvX arguments, I'd probably raise an exception if they are not callable (instead of silently ignoring them).

I'm also not sure whether putting in a noop callable (lambda: x: None) is more efficient than checking for 'is not None' in send_notify_message. (do you happen to know if python optimizes that away?)

I've updated notify_out.py and xfrout.py not to put in lambda: x: None. It checks it's not NoneType then invokes them. Unless they are callable, an exception would be raised. Please see:

2c05e5b [2158] updated counters handling in the classes

Regards,

comment:12 follow-up: Changed 7 years ago by jelte

  • Owner changed from jelte to naokikambe

Ok, looks good, I think this branch is good to go, so unless it depends on anything else, I think it can be merged.

comment:13 in reply to: ↑ 12 Changed 7 years ago by naokikambe

  • Resolution set to fixed
  • Status changed from reviewing to closed
  • Total Hours changed from 0 to 11.25

Merging was done. I'm closing it. Thanks,

Note: See TracTickets for help on using tickets.