Document: draft-ietf-netconf-adaptive-subscription
Title: Adaptive Subscription to YANG Notification
Reviewer: Dhruv Dhody
Review result: Has Issues

# OPSDIR Early Review of draft-ietf-netconf-adaptive-subscription

Reviewer: Dhruv Dhody
Review Result: Has Issues (But on the right track)

I have reviewed this document as part of the Operational directorate's ongoing
effort to review all IETF documents being processed by the IESG.  These
comments were written with the intent of improving the operational aspects of
the IETF drafts.

The document is well-written. The motivation is clear. Thank you for including
the appendix use case with clear examples.

The document does not include a dedicated Operational Considerations section.
It might be useful to think about a dedicated section with operational guidance
that could be provided around setting multiple adaptive periods, granularity of
periods, Xpath guidance, scalability etc.

## Major

- The intended status is experimental, but the scope of the experiment is
unclear. See the draft from Ron/Adrian on some useful tips to include in your
I-D: https://datatracker.ietf.org/doc/draft-bonica-gendispatch-exp/

## Minor

- Suggested rephrasing of the abstract -
````
This document defines a YANG data model and associated
mechanism to enable adaptive subscriptions to YANG
notifications. The publisher can dynamically adjust
the periodic update interval based on the evaluation
of pre-configured conditions (e.g., thresholds or
expressions). This allows for finer-grained telemetry by
increasing update frequency when certain criteria are met,
and reducing it otherwise.
````

- The default value of eval-interval: "defines how often the XPath condition
expression as defined in "eval-expression" is evaluated to decide whether to
switch to another period interval. If an "eval-interval" is not provided, then
the "eval-interval" MUST be set with the minimum time interval that the server
is able to detect wherever changes to the targeted data node occur."; is this
default value of minimum time interval easy to determine by the server? Would
an implementation have difficulty satisfying this MUST condition?

- Section 3, I can't parse "The following complex implementation and use
choices need to be cautious..." and how to connect with the list that follows
(which mixes requirements 'support...' and conditions)? Consider changing
'recommended' to 'RECOMMENDED'?

- The description of the notification is incorrect, seems like you copied it
from "notification push-update" and forgot to update it! ````
  notification adaptive-period-update {
    if-feature "adaptive-subscription";
    sn:subscription-state-notification;
    description
      "This notification contains a push update that in turn
       contains data subscribed to via a subscription.  In the
       case of a periodic subscription, this notification is sent
       for periodic updates.  It can also be used for
       synchronization updates of an on-change subscription.
       This notification shall only be sent to receivers of a
       subscription.  It does not constitute a general-purpose
       notification that would be subscribable as part of the
       NETCONF event stream by any receiver.";
````

- Consider using the RFC 7942 template to provide the information on the gRPC
implementation.

- Section 7 needs more work, it does not provide all the information as per the
template.

- In YANG in B.1, please don't use SHOULD/SHALL in an example YANG module.

## Nits

- The reference to [CHIP] is missing a URL, should it be this -
https://csa-iot.org/all-solutions/matter/

- s/A value of 0indicates/A value of 0 indicates/

- s/WIFI/Wi-Fi/

- In section C.3, replace Figure 2 with Figure 3.

- In figure 4, <period>20000</period> -> should this be 2000 instead (20 sec)

- what is even in "does not affect the even/update record"? Did you mean event?

Thanks!
Dhruv