[netconf] Re: WGLC for trace-ctx-extension-03

[Per Andersson <[e-mail]>]

Hi!

I have reviewed draft-ietf-netconf-trace-ctx-extension-01.

The document is well written and needed, thanks!
We have already implemented it.

My review notes below.


The document could benefit from a terminology section explaining the
following terms:

* OTLP

* OTLP Protocol

* M.E.L.T

* gNMI


Figure 3 probably means to have only bidirectional arrows since the
referenced retrieval mechanisms are either "pull or push". Currently
only the top arrow is bidirectional while the two are unidirectional.


As can be seen below, the YANG module validation the recommended
namespace value is

    urn:ietf:params:xml:ns:yang:ietf-netconf-otlp-context

This ties better to the YANG module name and the presented xmlns prefix

    xmlns:ietf-netconf-otlp-context

It is noted that the namespaces for

     urn:ietf:params:xml:ns:yang:traceparent:1.0

and

     urn:ietf:params:xml:ns:yang:tracestate:1.0

correspondingly should have longer names containing the YANG module
name. However, this would be very long. Perhaps a shorter, but
corresponding namespace such as

    urn:ietf:params:xml:ns:yang:otlp-context-traceparent

could be used? If so maybe the shorter "otlp-context" namespace can be
kept instead of the recommended "ietf-netconf-otlp-context", which would
signal relation?


YANG module

* Suggest explaining what "otlp" is in identity meta-error. Or maybe it
  is enough to use upper case if it is explained in the suggested
  terminology section?

Running pyang --ietf ietf-netconf-otlp-context.yang reports:

ietf-netconf-otlp-context.yang:3: warning: RFC 8407: 4.9: namespace
value should be
"urn:ietf:params:xml:ns:yang:ietf-netconf-otlp-context"
ietf-netconf-otlp-context.yang:45: warning: RFC 8407: Appendix B: The
text about which RFC this module is part of seems to be missing or is
not correct (see pyang --ietf-help for details).

Suggest updating the YANG module to accomodate for these warnings. See
above for discussion on namespace.

yanglint is happy!


The following commands

pyang -f yang --keep-comments --yang-line-length 69 \
      ietf-netconf-otlp-context.yang > new.yang \
&& diff ietf-netconf-otlp-context.yang new.yang

yields a diff (formatting) which should update the YANG module:

8c8,9
<     reference "RFC8791: YANG Data Structure Extensions";
---
>     reference
>       "RFC8791: YANG Data Structure Extensions";
12,13c13
<      "IETF NETCONF (Network Configuration) Working Group";
<
---
>     "IETF NETCONF (Network Configuration) Working Group";
16,17c16
<     WG List:  <mailto:[e-mail]>";
<
---
>      WG List:  <mailto:[e-mail]>";
20,44c19,43
<     client and servers needs to share some specific contextual
<     information. This NETCONF extensions aligns the NETCONF
<     protocol to the W3C trace-context document:
<     https://www.w3.org/TR/2021/REC-trace-context-1-20211123
<
<     Copyright (c) 2024 IETF Trust and the persons identified as
<     authors of the code.  All rights reserved.
<
<     Redistribution and use in source and binary forms, with or
<     without modification, is permitted pursuant to, and subject to
<     the license terms contained in, the Revised BSD License set
<     forth in Section 4.c of the IETF Trust's Legal Provisions
<     Relating to IETF Documents
<     (https://trustee.ietf.org/license-info)
<
<     This version of this YANG module is part of RFC XXXX
<
<     (https://www.rfc-editor.org/info/rfcXXXX) see the RFC itself
<     for full legal notices
<
<     The key words 'MUST', 'MUST NOT', 'REQUIRED', 'SHALL', 'SHALL
<     NOT', 'SHOULD', 'SHOULD NOT', 'RECOMMENDED', 'NOT RECOMMENDED',
<     'MAY', and 'OPTIONAL' in this document are to be interpreted as
<     described in BCP 14 (RFC 2119) (RFC 8174) when, and only when,
<     they appear in all capitals, as shown here.
---
>      client and servers needs to share some specific contextual
>      information. This NETCONF extensions aligns the NETCONF
>      protocol to the W3C trace-context document:
>      https://www.w3.org/TR/2021/REC-trace-context-1-20211123
>
>      Copyright (c) 2024 IETF Trust and the persons identified as
>      authors of the code.  All rights reserved.
>
>      Redistribution and use in source and binary forms, with or
>      without modification, is permitted pursuant to, and subject to
>      the license terms contained in, the Revised BSD License set
>      forth in Section 4.c of the IETF Trust's Legal Provisions
>      Relating to IETF Documents
>      (https://trustee.ietf.org/license-info)
>
>      This version of this YANG module is part of RFC XXXX
>
>      (https://www.rfc-editor.org/info/rfcXXXX) see the RFC itself
>      for full legal notices
>
>      The key words 'MUST', 'MUST NOT', 'REQUIRED', 'SHALL', 'SHALL
>      NOT', 'SHOULD', 'SHOULD NOT', 'RECOMMENDED', 'NOT RECOMMENDED',
>      'MAY', and 'OPTIONAL' in this document are to be interpreted as
>      described in BCP 14 (RFC 2119) (RFC 8174) when, and only when,
>      they appear in all capitals, as shown here.
55c54,55
<     description "Base identity for otlp attribute errors.";
---
>     description
>       "Base identity for otlp attribute errors.";
60,61c60,62
<     description "Indicates an attribute or header that is required
<       (in the current situation) is missing.";
---
>     description
>       "Indicates an attribute or header that is required
>        (in the current situation) is missing.";
62a64
>
65,66c67,69
<     description "Indicates an attribute or header value where the
<       value is incorrectly formatted.";
---
>     description
>       "Indicates an attribute or header value where the
>        value is incorrectly formatted.";
67a71
>
70,71c74,76
<     description "Indicates that the server encountered a processing
<       error while processing the attribute or header value.";
---
>     description
>       "Indicates that the server encountered a processing
>        error while processing the attribute or header value.";
78c83,84
<     description "Error type";
---
>     description
>       "Error type";
84c90
<          "This error is returned by a NETCONF or RESTCONF server
---
>         "This error is returned by a NETCONF or RESTCONF server
94,98c100,104
<           In NETCONF, the qualified XML attribute name.
<           In RESTCONF, the HTTP header name.
<           If a client sent a NETCONF RPC with the attribute
<           w3ctc:traceparent='incorrect-format'
<           this leaf would have the value 'w3ctc:traceparent'";
---
>            In NETCONF, the qualified XML attribute name.
>            In RESTCONF, the HTTP header name.
>            If a client sent a NETCONF RPC with the attribute
>            w3ctc:traceparent='incorrect-format'
>            this leaf would have the value 'w3ctc:traceparent'";
104,107c110,113
<           by the server.
<           If a client sent a NETCONF RPC with the attribute
<           w3ctc:traceparent='incorrect-format'
<           this leaf would have the value 'incorrect-format'.";
---
>            by the server.
>            If a client sent a NETCONF RPC with the attribute
>            w3ctc:traceparent='incorrect-format'
>            this leaf would have the value 'incorrect-format'.";
113,117c119,123
<           detected by the server.
<           If a client sent an RPC annotated with the attribute
<           w3ctc:traceparent='incorrect-format'
<           this leaf might have the value
<           'ietf-netconf-otlp-context:bad-format'.";
---
>            detected by the server.
>            If a client sent an RPC annotated with the attribute
>            w3ctc:traceparent='incorrect-format'
>            this leaf might have the value
>            'ietf-netconf-otlp-context:bad-format'.";


Nits:

* nc_trace as short title. Suggest using something more descriptive.
  (Also noticed short illegible titles in in
  draft-ietf-netconf-restconf-trace-ctx-headers and
  draft-ietf-netconf-transaction-id.)

* Section 1.2 YANG DataStore.; several more instances of "DataStore"
  -> Suggest using "YANG Datastore", this is what can be found in other
     documents.

* Inconsistency using DataStore, Datastore, and datastore throughout the
  document.
  -> Suggest using Datastore and datastore, since this is usually found
     in other WG documents.

* Section 2.2 "versionning" typo -> versioning

* "complemetary" typo -> complementary

* "faciliate" typo -> facilitate

* "infomration" typo -> information

* Markdown formatting leaking into the text rendering of the document:
  _traceparent_, _tracestate_, _w3ctc:traceparent_, _w3ctc:tracestate_,
  _nc:rpc_, _nc:rpc-reply_, _notif:notification_, _yp:push-update_, and
  _yp:push-change-update_.

* "additonal" typo -> additional

Additionally idnits reports a few warnings and comments. However,
some of them seem to be false positives.

  Checking nits according to https://www.ietf.org/id-info/checklist :
  ----------------------------------------------------------------------------

  == There are 1 instance of lines with non-RFC6890-compliant IPv4 addresses
     in the document.  If these are example addresses, they should be changed.

  -- The draft header indicates that this document obsoletes
     draft-netconf-trace-ctx-extension-00, but the abstract doesn't seem to
     mention this, which it should.


  Miscellaneous warnings:
  ----------------------------------------------------------------------------

  == The document seems to lack the recommended RFC 2119 boilerplate, even if
     it appears to use RFC 2119 keywords -- however, there's a paragraph with
     a matching beginning. Boilerplate error?

     (The document does seem to have the reference to RFC 2119 which the
     ID-Checklist requires).
  -- No information found for rfcdraft-netconf-trace-ctx-extension-00 - is
     the name correct?

  -- The document date (8 July 2024) is 87 days in the past.  Is this
     intentional?


  Checking references for intended status: Proposed Standard
  ----------------------------------------------------------------------------

     (See RFCs 3967 and 4897 for information about using normative references
     to lower-maturity documents in RFCs)

  -- Possible downref: Non-RFC (?) normative reference: ref.
     'W3C-Trace-Context'

  == Outdated reference: A later version (-06) exists of
     draft-ietf-netconf-transaction-id-05


     Summary: 0 errors (**), 0 flaws (~~), 4 warnings (==), 4 comments (--).


Thanks for your contribution!


--
Per, as a contributor