Last Call Review of draft-ietf-ccamp-alarm-module-07
review-ietf-ccamp-alarm-module-07-opsdir-lc-clarke-2019-03-18-00

Request Review of draft-ietf-ccamp-alarm-module
Requested rev. no specific revision (document currently at 09)
Type Last Call Review
Team Ops Directorate (opsdir)
Deadline 2019-03-20
Requested 2019-03-06
Draft last updated 2019-03-18
Completed reviews Yangdoctors Last Call review of -06 by Carl Moberg (diff)
Rtgdir Last Call review of -06 by Joel Halpern (diff)
Secdir Last Call review of -07 by Shawn Emery (diff)
Genart Last Call review of -07 by Dan Romascanu (diff)
Opsdir Last Call review of -07 by Joe Clarke (diff)
Genart Telechat review of -09 by Dan Romascanu
Assignment Reviewer Joe Clarke
State Completed
Review review-ietf-ccamp-alarm-module-07-opsdir-lc-clarke-2019-03-18
Reviewed rev. 07 (document currently at 09)
Review result Has Nits
Review completed: 2019-03-18

Review
review-ietf-ccamp-alarm-module-07-opsdir-lc-clarke-2019-03-18

I have been assigned to review this draft on behalf of the OPS DIR.  Overall, I found the draft ready with some nits.  I found relatively easy to read and well-thought-through.  Broadly speaking, I found the term "Alarm Shelving" to be confusing.  You do define this, but it is not something that many operators are familiar with.  I had originally thought alarm suppression was better, but after I read more, I see what you're trying to do with this.

Additionally, some of the features like alarm-history and alarm-shelving have a potential operational impact with respect to local server resources, and perhaps that's worth calling out.

The leaf "has-clear" seems like it would read better as "has-cleared" from an English standpoint.  Thoughts?

Finally, I can see where it might be desirable to suppress alarm notifications without shelving them per se.  That is, the alarm remains in the list, but notifications are not sent.  That doesn't seem possible, and I'm wondering if it's worth considering.

My per-section nits (typos) are found below.

Section 3.2

s/an hierarchy/a hierarchy/

===

Section 3.5.1

In the example, ((GigabitEthernet0/25, link-
   alarm,""), false, T, major, "Interface GigabitEthernet0/25 down"), maybe replace 'T' with a sample timestamp.  It took me a few seconds to grok that as I was associating it with a boolean.

===

Section 3.5.2

s/criteria/criterion/

===

Section 3.6

s/the the disk full/the disk full/

===

Section 3.7

s/this criteria/these criteria/

===

Section 4.7

s/alarn/alarm/

===

YANG Module section

s/identifiying/identifying/

s/exampla/example/

s/alarm is not longer active using other/alarm is no longer active using other/