Re: [Teas] Yangdoctors early review of draft-ietf-teas-actn-pm-telemetry-autonomics-08

[Dhruv Dhody <[e-mail]>]

Hi Reshad,

Thanks for your early Yangdoctors review. I have posted an update -
https://www.ietf.org/rfcdiff?url2=draft-ietf-teas-actn-pm-telemetry-autonomics-09

Please see inline.

On Sat, Mar 12, 2022 at 11:40 PM Reshad Rahman via Datatracker <
[e-mail]> wrote:

> Reviewer: Reshad Rahman
> Review result: Ready with Issues
>
> This is my YANG Doctor review of
> draft-ietf-teas-actn-pm-telemetry-autonomics-08.
>
> I have reviewed the document and looked at references such as
> draft-ietf-teas-actn-vn-yang, RFC8453, RFC8309 etc. I think I got to grasp
> the
> intent of the document, at least the autonomics part. However I am not
> sure I
> fully understand how everything fits in together. I don't follow the TEAS
> WG
> anymore, I tried to catch up by searching the archive but I didn't see any
> discussions on this document in the archive. Having more examples may help.
>
>
I have added another example and did some rearranging of text.



> Comments on the document:
>
> The document title, abstract and YANG modules all use the term
> "telemetry". I
> do not see anything specific to telemetry in the YANG modules, it is just
> extra
> data being modelled in TE tunnels and VNs. There are a couple of examples
> with
> establish-subscription, but that IMO still doesn't the model telemetry
> specific.
>
>
I added a clarification in the abstract and introduction. I am shying away
from removing the term entirely as it is the key use.



> Introduction, 5th paragraph. I don't get "to provide an ability to program
> their customized performance monitoring subscription". Possible it's
> because
> I'm missing the big picture. My suggestion is to clarify/simplify. But if
> the
> WG is good with this, fine with me.
>
>
I did some re-wording, hope that helps!



> Introduction, 6th paragraph. The following statement is of some concern to
> me
> because of the potential confusion. Again, up to the WG. Nit: missing "way"
> after "different".
>  "The term performance monitoring is used in this document in a
>    different from how the term has been used in TE networks for many
>    years."
>
>
This text is now removed based on the comment from Greg.



> Section 2. Figure 1 is a nice diagram, but it is not clear where/how this
> document fits in.
>
>
It was coming from another document that is no longer progressing. It was
decided to keep the figure bit and remove the reference. I added text to
say it is just an example!



> Section 2, next to last bullet. "other SLA requirements". It would help
> to have
> an example of such an SLA requirement.
>
>
Added



> Section 5.2. There's a mismatch between the text for figure 9 (copy-paste
> from
> text for figure 8?) and figure 9.
>
>
Updated, thanks for spotting it!



> IANA Considerations: For YANG module name registry, the reference should be
> RFC6020, not RFC7950.
>
>
Updated.



> Comments on the YANG models:
>
> Why use identity for scale-op? enum seems fine for up and down.
>
>
Updated.



> For threshold-time and cooldown-time, is it ok to have value 0?
>
>
Yes, added text for it.



> For threshold-value, why use a string. I realize using a union might be
> tricky
> here, but why not uint64? Is it because there might be a threshold which
> cannot
> be expressed as a value?
>
>
Uint64 could be an issue for float bandwidth. I created a union to
accommodate various values.



> Telemetry-id seems unnecessary. First it's not clear whether it's unique
> per TE
> tunnel. And, why invent a new "id", why not refer to the TE tunnel using
> its
> key?
>

Removed.



>
> For scale-in and scale-out, by how much? Is there a missing leaf node or
> did I
> misunderstand how this works?
>
>
leaf scale (that we changed from string to unit64) is that!


> For grouping-op(eration), are "and" and "or" still needed considering
> those are
> in scaling-criteria-operation?
>
>
They are not and thus they are different identities.



> Typos:
>
> - analize (analyze)
>
> - interpritted (interpreted)
>
> - criterias (criteria)
>
>
Thanks for spotting these!

Regards,
Dhruv



> Regards,
> Reshad.
>
>
>