Re: [art] Artart last call review of draft-ietf-alto-performance-metrics-17

["Y. Richard Yang" <[e-mail]>]

Hi Christian,

On Tue, Oct 19, 2021 at 2:54 AM Christian Amsüss <[e-mail]>
wrote:

> Hello Richard,
>
> On Mon, Oct 18, 2021 at 03:43:50PM -0400, Y. Richard Yang wrote:
> > Good comment. The document gives the high-level grammar (1 line) at the
> > beginning of Sec. 2.2.
> > It looks that your suggestion is the write out the complete grammar
> upfront:
>
> I was not so much looking for the details of the terms "where <stat>
> MUST be one of the following" was fine) but for
>
> a) naming the formal language this is in. Reading, I've guessed that the
>   square brackets mean "optional" because I've seen that in DOS-time
>   documentation, and it looks formal, but does not declare a language.


>
  A formal language that could be used here and is common in RFCs would
>   be ABNF.
>
>
I see. See below, as we can address them together.


> b) the tying in of the output -- the the term "metric-identifier" is not
>   used anywhere else, and RFC7285 uses a CostMetric type in the
>   cost-metric field; I'd appreciate if these identifiers were somehow
>   linked.
>
  This would be trivial had ALTO used CDDL to describe its JSON, as
>   then there could be a line like `cost-metric //= metric-identifier`;
>   maybe the `object { ... }` notation has something similar?
>
> The object notation is higher granularity and hence cannot handle strings
structure.


>   RFC7285 generally does without constructing string identifier, so
>   there is no ABNF in there.
>
> Correct.


>   If you go with the suggested colon structuring and have text for that,
>   possibly there the optionality and string concatenation be described
>   in ABNF (or just in words with an example), and the need for a formal
>   language would go away here.
>
>
Good comment. How about the following:

Old Sec. 2.2
"Hence, each performance metric's identifier
   should indicate the statistic (i.e., an aggregation operation), to
   become
       <metric-identifier> ::= <metric-base-identifier> [ '-' <stat> ]
   where <stat> MUST be one of the following:
 "

=>

"Hence, this document extends the general US-ASCII alphanumeric cost metric
strings (formally specified as the CostMetric type in Section 10.6 of
[RFC7285]) as follows: A cost metric string (denoted <cost-metric>)
consists of a base metric identifier string (denoted
<metric-base-identifier>) followed by an optional statistics string
(denoted <stat>), connected by the ASCII character colon (':', U+003A), if
the statistics field exists. Examples of cost metric strings include
"delay-ow", "delay-ow:min", "delay-ow:p99".

> "...  Note that some systems use quantile, which is in the range [0, 1].
> > This document uses percentile to make the identifier easier to read. When
> > there is a more common form for a given percentile, it is RECOMMENDED
> that
> > the common form being used; that is, instead of p0, use min; instead of
> > p50, use median; instead of p100, use max".
>
> Fine with me.
>
>
Great.


> > > * Allowing decimals into the cost metric identifier introduces a dot
> which
> > >   is reserved as per RFC7285 Section 10.6.
> >
> > Yes. Correct. From the grammar above, we made sure that we would not
> > introduce a dot. We can add a sentence to point out that when choosing
> the
> > base, we should follow this. Should we do this?
>
> I think it'd be easiest (with the grammar or without) to to introduce
> the number as a nonnegative integer. (That'd allow dropping mentions of
> the minus and exp components).
>
> Agree.


> > >   A way out could be to formalize this structure and register the
> > >   metric-base-identifiers for use with and without a stat parameter
> > >   following the colon (instead of the dash).
> >
> > So the suggestion is that ":" as the consistent internal structure
> > separator. This is quite reasonable.
>
> Just beware that this is formally an update to the registry; not sure
> whether that incurs an "updates" to 7285.
>
>
No. This will not update 7285. Sec. 10.6 of RFC7285 requires registration
and we will do so.



> > >   A few words on which statistic can be used with which metric could
> > >   also help with bw-maxres. (What does bw-maxres-p50 mean, is it
> > >   meaningful at all?)
> >
> > Mathematically, any percentile of a set of a single value is the value
> > itself. But it is indeed a good idea to clarify it. We will add a
> sentence
> > at the end of 2.2
> >
> > => Note that although one can use generic statistics (i.e., any
> percentile
> > in [0, 100]) and multiple specifications may give the same value, it
> helps
> > to choose the more intuitive and robust definition. For example, when the
> > set is expected to be a single value. The max operator is more robust and
> > hence recommended.
>
> That sounds more confusing to me (the max operator ... so I should use
> bw-maxres-max?); maybe (if correct) something like
>
> | Note that unlike the other metrics, maxres is a single value and not
> | sampled over time. Thus, it MUST NOT be specialized with a stat
> | indicator, but is to be used in the base form.
>
> Sounds good to me. We will use your wording.


> > > * "Content-Length: TBA": Does this add value to the examples?
> >
> > We will add the final exact value when publishing, as it is part of HTTP
> > header.
>
> Sure it is part of the HTTP header, but so are many other headers
> (Accept-Content-Encoding, User-Agent, what so not). Accept and
> Content-Type add to the understanding, Content-Length will IMO be
> ignored by all readers anyway.
>
>
OK. We will add the final value now.


> > It is an example to illustrate both ipv4 and ipv6. If we do ipv4->ip4 and
> > ipv6-ipv6, we will need two examples, and it takes more space.
>
> Does it need examples of all of them? These are not unit tests, they are
> illustrative.
>
> Indeed. They are illustrative and we can drop the mix use to avoid
confusion.


> > > * "the -<percentile> component": "any 'stat' component"?
> >
> > Not sure what this is? Any more specific locator?
>
> 3.1.3
>
> > Very good comment! We have made the changes.  The newer version fixed
> this
> > issue.
> > Please see -18 which is just loaded today.
>
> Didn't see that at first ... something was changed in the uplaod process
> and now there's no HTML version any more?
>
>
We will upload a newer version to address all issues soon and then I will
debug to make sure that is an HTML version.

Thank you so much!
Richard



> BR
> c
>
> --
> To use raw power is to make yourself infinitely vulnerable to greater
> powers.
>   -- Bene Gesserit axiom
>