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

From: Christian Amsüss via Datatracker <[email-masked]>
Date: Mon, 18 Oct 2021 07:02:08 -0700

Reviewer: Christian Amsüss
Review result: Ready with Issues

## Summary for the IoT Directorate

This document extends the Application-Layer Traffic Optimization (ALTO)
protocol, by which applications that need to a peer from a selection can ask
their uplink for preferences. It adds more fine-grained vocabulary to pick the
peer with best expected bandwidth, or best latency.

As for conventions around the Internet of Things, this makes no choices -- if
follows the path set out by ALTO, and adds terms and considerations for metrics
established outside of ALTO.

The mechanisms (more of ALTO than this particular document) can be applicable
to IoT applications when unconstrained devices relay data into cloud systems --
then they might make better choices in presence of decentralized backends. If
this document gains traction in deployments, these systems will need deeper
application knowledge pertaining to the application's requirements (selecting
the low-latency vs. the high-bandwidth option).

## High-level

The document can be followed well after brief familiarization with ALTO; it is
in a good over-all shape.

2.2 Performance Metric Statistics:

* The metric-identifier definition seems disconnected with the rest of the
  terminology. I assume from context that this is a way for building a
  CostMetric value. If this assumption is wrong, some of the later points are
  moot.

  (By the way, what language is the metric-identifier definition?  It's not
  ABNF, and I don't find any other formal language in thenormative references.)

* p0 and p100 are aliases to min and max IIUC. Is there a preferred form, and
  why is the other form allowed? (Same for p50 and median.)

* Allowing decimals into the cost metric identifier introduces a dot which is
  reserved as per RFC7285 Section 10.6.

* comparing to 7 IANA Considerations: The registered identifeirs are only the
  metric-base-identifiers.

  Registering all combined values from metric-base-identifier x stat is not
  practical (especially with the numeric percentiles); however, the 'priv:'
  prefix indicates that some structure was intended in RFC7285.

  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).

* Section 2.2 (Performance Metric Statistics) allows adding -stdvar to
  metric-base-identifiers; delay-variation-mean is probably similar to
  delay-ow-stdvar. It's only similar (and not identical) due to the offset from
  minimum detailed in 3.3.3; anyhow, pointing out that out in the "Note that in
  statistics" paragraph, e.g. as in "... networking convention. Due to this, it
  is expressed as a dedicated metric and not just as delay-ow-stddev".

  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?)

3 Packet Performance Metrics:

* Cost-Context Specification Considerations: There is a lot of duplication
  here, especially around SLA and the topic of "link" descriptions. (For the
  technically interesting parts, the later sections already refer back to
  3.1.4).

6 Security Considerations:

* On the topic of confidentiality, can these metrics be used with the "ordinal"
  cost mode? A client might just want to pick the peer that has the "best"
  round-trip time, and could thus avoid the need to obtain the confidential
  metrics.

## Nits

* Table 1: The current XML RFC format can do tables that render to HTML, this
  would also make it easier to follow the links.

  For tables and figures, also more accessible labeling is available there.

* Section 3 Packet Performance Metrics: The pkt.* notation appears very ad-hoc,
  and is not used anywhere outside the paragraph. If these are referring to
  external definitions, please consider referencing that definition; otherwise:
  do these labels add value?

* "smaller than milliseconds": smaller than one millisecond?

* "Content-Length: TBA": Does this add value to the examples?

* Example in 3.1.3: Does the example make sense in terms of addresses? Querying
  metrics between an IPv4 and an IPv6 address seems strange. (And is this
  primarily used in a field where IPv4 is still dominant?)

  In particular, the hop count example has an unreasonably high number of hops
  between 192.0.2.2 and 192.168.2.89...

* "the -<percentile> component": "any 'stat' component"?

## Slightly off topic

(ie. this is out my expertise and would be checked later by someone else, but
it may be helpful to revisit it right away)

* IANA considerations: Creating a new regisry usually comes with ore than
  "reqeusts the creation of" and some values;
  https://datatracker.ietf.org/doc/html/rfc8126#section-2.2 in particular asks
  for a registration policy.

  (Given the document says "MUST be one of three", there may not be a need to
  set up a registry in the first place).

---

From: Christian Amsüss <[email-masked]>
Date: Mon, 18 Oct 2021 16:14:26 +0200

Hello,

> ## Summary for the IoT Directorate

I performed the review with the wrong hat on, sorry for the mixup.

Refocusing on the ART review criteria brings up nothing new -- but the
point about the formal language used with metric-identifier deserves
more emphasis.

Likewise, the point about registration of the stat-typed metric
identifiers (and the possible structuring of the registry into
registered prefixes and per-prefix semantics for what is behind a colon)
now falls more directly into the scope of the review.

Otherwise, what was said with focus on the IoT applies likewise to the

> established outside of ALTO.

Best regards
Christian

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

---

From: "Y. Richard Yang" <[email-masked]>
Date: Mon, 18 Oct 2021 15:43:50 -0400

Hi Christian,

Thank you so much for the review. Please see below.

On Mon, Oct 18, 2021 at 10:02 AM Christian Amsüss via Datatracker <
[email-masked]> 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:

<metric-identifier> ::= <metric-base-identifier> [ '-' <stat> ]

=>

<metric-identifier> ::= <metric-base-identifier> [ '-' <stat> ]

<metric-base-identifier> ::= `delay-ow` | `delay-rt` | `delay-variation`
                               | `hop-count` | `lossrate` | `tput`

                               | `bw-residual` | `bw-maxres`

<stat> ::= `min` | `max` | `median` | `mean` | `stddev` | `stdvar`
             | `p<percent>`

<percent> := ASICC number between 0 and 100, inclusive.

Is the above what you have in mind?

> p<percent> is a uniform notation. Singling out 0, 50, 100 creates
complexity, although there is the more common
min/max/median. How about we add a sentence at the end of the paragraph:

"...  Note that some systems use quantile, which is in the range [0, 1].
This document uses percentile to make the identifier easier to read."
=>
"...  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".

>   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?

>   the colon (instead of the dash).

So the suggestion is that ":" as the consistent internal structure
separator. This is quite reasonable.

>   is expressed as a dedicated metric and not just as delay-ow-stddev".
OK. Sounds good. We will add the note.

>   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.

>   technically interesting parts, the later sections already refer back to
Agree. We will take a look and try to compress a bit, say by referring to
previous sections.

>   round-trip time, and could thus avoid the need to obtain the confidential
Yes. They can. Then the mapping (to the ordinal value space) should use
these metrics. It is a good idea to point this out. We plan to add this
sentence in the introduction and security:
Intro (right above Table 1) => Note that these metrics can be used to
support both numerical and ordinal use cases. When it is the ordinal mode,
the mapping to the ordinal value space should use these metrics.

We will add a sentence in Security, on the benefit as you mentioned.

>   For tables and figures, also more accessible labeling is available there.
Good idea. We will use the format.

>   do these labels add value?

See your point. We will revise the pkt. notation to refer to metrics.

> OK. Will fix.

> * "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.

> 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.

>   between 192.0.2.2 and 192.168.2.89...
Good catch. We will make it smaller :-)

> * "the -<percentile> component": "any 'stat' component"?

Not sure what this is? Any more specific locator?

> * IANA considerations: Creating a new regisry usually comes with more than
> Very good comment! We have made the changes.  The newer version fixed this
issue.
Please see -18 which is just loaded today.

Thank you so much!!

Richard

---

From: "Y. Richard Yang" <[email-masked]>
Date: Mon, 18 Oct 2021 15:45:42 -0400

Hi Christian,

Thanks for the update. Can you please take a look at the responses to your
review in the previous email to see if they are addressing the two issues
below?

Richard

On Mon, Oct 18, 2021 at 10:14 AM Christian Amsüss <[email-masked]>
wrote:

> _______________________________________________
> alto mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/alto
>

-- 
-- 
 =====================================
| Y. Richard Yang <[email-masked]>   |
| Professor of Computer Science       |
| http://www.cs.yale.edu/~yry/        |
 =====================================

---

From: Christian Amsüss <[email-masked]>
Date: Tue, 19 Oct 2021 08:53:49 +0200

Hello Richard,

> 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.

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?

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

  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.

> p50, use median; instead of p100, use max".

Fine with me.

> > * Allowing decimals into the cost metric identifier introduces a dot which
> >   is reserved as per RFC7285 Section 10.6.
>

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).

> >   metric-base-identifiers for use with and without a stat parameter
> >   following the colon (instead of the dash).
> 

Just beware that this is formally an update to the registry; not sure
whether that incurs an "updates" to 7285.

> >   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?)
>

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.

> We will add the final exact value when publishing, as it is part of HTTP

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.

> 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.

> Not sure what this is? Any more specific locator?

3.1.3

> 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?

BR
c

---

From: "Y. Richard Yang" <[email-masked]>
Date: Thu, 21 Oct 2021 15:47:06 -0400

Hi Christian,

>   be ABNF.
I see. See below, as we can address them together.

> The object notation is higher granularity and hence cannot handle strings
structure.

> Correct.

>   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".

> > p50, use median; instead of p100, use max".
Great.

> Agree.

> 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.

> Sounds good to me. We will use your wording.

> ignored by all readers anyway.
OK. We will add the final value now.

> Indeed. They are illustrative and we can drop the mix use to avoid
confusion.

> 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

---

From: "Y. Richard Yang" <[email-masked]>
Date: Sat, 23 Oct 2021 23:08:21 -0400

Hi Christian,

We made two changes to address your comments. Please see below.

> the minus and exp components).

The idea of making the number following p as a non-negative integer 0..100
is quite interesting.
If we limit the precision to only 2 digits, we are all set. One issue is
that we may need higher
precision such as 99.9%. One possibility then is to consider delay-rt:p<y>
as the 0.<y>
quantile. For example, delay-rt:p999 is the 99.9%, delay-rt:p01 is the
0.1%. One issue of
this design, however, is that we cannot represent 100% precisely.

As a result, this is the current version to address your comments:

NEW:

percentile, with letter 'p' followed by a number:

      gives the percentile specified by the number following the letter
      'p'.  The number MUST be a non-negative JSON integer in the range
      [0, 100] (i.e., greater than or equal to 0 and less than or equal
      to 100), followed by an optional decimal part, if a higher precision
      is needed.  The decimal part should start with the '.' separator
      (U+002E), and followed by a sequence of one or more ASCII numbers
      between '0' and '9'.  The total length of the cost metric string
      MUST not exceed 32, as required by [RFC7285].  Assume this number
      is y and consider the samples coming from a random variable X.
      Then the metric returns x, such that the probability of X is less
      than or equal to x, i.e., Prob(X <= x), = y/100.  For example,
      delay-ow:p99 gives the 99% percentile of observed one-way delay;
      delay-ow:p99.9 gives the 99.9% percentile.  Note that some systems
      use quantile, which is in the range [0, 1].  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.

> | indicator, but is to be used in the base form.
After some thinking, we think the best way to address your comments is to
introduce
a new statistical operator, and this is a nice fix. Specifically, here is
the proposal:

NEW:

2.2.  Performance Metric Statistics

   The measurement of a performance metric often yields a set of samples
   from an observation distribution ([Prometheus]), instead of a single
   value.  A statistical operator is applied to the samples to obtain a
   value to be reported to the client.  Multiple statistical operators
   (e.g., min, median, max) are being used.

   Hence, this document extends the general US-ASCII alphanumeric cost
   metric strings, formally specified as the CostMetric type (defined in
   Section 10.6 of [RFC7285]; see above in the CostType definition), as
   follows:

      A cost metric string consists of a base metric identifier (or base
      identifier for short) string, followed by an optional statistical
      operator string, connected by the ASCII character colon (':',
      U+003A), if the statistical operator string exists.

   Examples of cost metric strings then include "delay-ow", "delay-
   ow:min", "delay-ow:p99", where "delay-ow" is the base metric
   identifier string; "min" and "p99" are example statistical operator
   strings.

   The statistical operator string MUST be one of the following:

   cur:

      the instantaneous observation value of the metric from the most
      recent sample (i.e., the current value).

   percentile, with letter 'p' followed by a number:

   ...

  If a cost metric string does not have the optional statical operator
   string, the statistical operator SHOULD be interpreted as the default
   statical operator in the definition of the base metric.  If the
   definition of the base metric does not provide a definition for the
   default statistical operator, the metric MUST be considered as the
   median value.
"

Then, for the bw-maxres, we have the following:
"4.3.3.  Intended Semantics and Use

   Intended Semantics: To specify spatial and temporal maximum
   reservable bandwidth from the specified source to the specified
   destination.  The value corresponds to the maximum bandwidth that can
   be reserved (motivated from [RFC3630] Section 2.5.7).  The spatial
   aggregation unit is specified in the query context (e.g., PID to PID,
   or endpoint to endpoint).

   The default statical operator for maximum reservable bandwidth is the
   current instantaneous sample; that is, the default is assumed to be
   "cur".

Is the above better?

Thank you so much!
Richard