#188: Review request for Server-Timing

Visit on Github.

Opened Aug 14, 2017

Hello TAG!

I'm requesting a TAG review of:

Further details (optional):

We'd prefer the TAG provide feedback as (please select one):

  • open issues in our Github repo for each point of feedback
  • open a single issue in our Github repo for the entire review
  • leave review feedback as a comment in this issue and @-notify [github usernames]

Discussions

Comment by @dbaron Aug 15, 2017 (See Github)

It would perhaps be useful to see an explainer document here, to answer questions such as why a new specification is needed here rather than reusing existing mechanisms. (My guess at the answer to that particular question is so that timing information for all the resources that are used by the document, no matter what type of resources they are, can be included easily in the timeline.)

Comment by @slightlyoff Aug 18, 2017 (See Github)

Would also like to see an explainer; we recently put together a dummy outline that could help in drafting yours

Would also be good to see discussion of security and privacy considerations in the draft spec.

In terms of the design, a few questions:

  • The encoding for these values seems very one-off; does it share a common grammar with other systems (e.g., CSP)? Could it possibly use JSON?
  • The exact parsing semantics of the metric-value (and reflection back to JS) seem ambiguous to me. No formalism (or JS conversion) are offered in the text, only the note that they must be representable as a double, but that leaves a lot of room for interpretation. I'd like to see specific numeric conversion steps outlined.
  • The example code in the spec uses a confusing mix of old and new syntax. A fully modern version might read:
// Example 2
// serverTiming entries can live on 'navigation' and 'resource' entries
['navigation', 'resource'].forEach((entryType) => {
  let entries = performance.getEntriesByType(entryType);
  for(let { name: url, serverTiming } of entries) {
      // iterate over the serverTiming array; only the slow ones
      for(let { name, duration, description } of serverTiming.filter(t => { t.duration > 200 }) {
        console.info('Slow server-timing entry =',
            JSON.stringify({url, entryType, name, duration, description}, null, 2))
      }    
  }
Comment by @cvazac Aug 21, 2017 (See Github)

Explainer

Security discussion

The header format that we've chosen for Server Timing is similar to Accept-Language and Content-Type

Regarding "specific numeric conversion steps", is there any prior art you can link me to? cc @slightlyoff

Modernized example code.

Comment by @mnot Aug 31, 2017 (See Github)

A few notes on the header ABNF:

  • Omit the header field-name and colon from the Server-Timing rule.
  • Reference RFC7230, Section 7 for the # extension.
  • Reference RFC5234 for ABNF.
  • metric allows OWS around the "=", unlike RFC7231 parameter. Is this intentional?
  • metric-value allows an empty value (e.g., "foo="); is this intentional? If so, it should be called out in prose.
  • Is using non-ascii characters in metric-name and/or description a desired use? If so, you need to define an encoding, e.g., RFC5987bis.

See also the checklist for new header fields.

Comment by @cvazac Sep 1, 2017 (See Github)

@mnot:

  • Omit the header field-name and colon from the Server-Timing rule.
  • Reference RFC7230, Section 7 for the # extension.
  • Reference RFC5234 for ABNF.

👍 (https://github.com/w3c/server-timing/pull/31)

  • metric allows OWS around the "=", unlike RFC7231 parameter. Is this intentional?

parameter in RFC7231 looks to be specific to Accept and Content-Type. I thought it made sense to allow the optional whitespace for readability. But I'm not finding any prior art for OWS around equal signs... I don't have a problem removing it from spec, but I'd think browsers would still want to allow.

I dug into parameter for Content-Type a little further, and every modern browser supports WS before and after the "=" (excepting Firefox which supports WS after, but not before, the =)

  • metric-value allows an empty value (e.g., "foo="); is this intentional? If so, it should be called out in prose.

I see what you mean, this happened because of a recent change. metric-value is the way it is to allow for foo=.25 (no leading "0"). foo= does not hold more meaning than foo, which we need to be legal. Is there a way you know of in ABNF to tighten that up?

  • Is using non-ascii characters in metric-name and/or description a desired use? If so, you need to define an encoding, e.g., RFC5987bis.

I could have this wrong, but I think both are just ascii, which is what we want. for metric-name: metric-name is a token (link) token resolves to atchar, which resolves to asciis, DIGITs, and ALPHAs (link) ALPHAs are just A-Z / a-z (link)

Comment by @triblondon Sep 5, 2017 (See Github)

A few thoughts from me informed by recent TAG call:

  • I'm slightly concerned that "people" (ie, me) will use Server-Timing as a mechanism to communicate arbitrary page related metadata from server (or middlebox) to client, because it is the only HTTP header, to my knowledge, that is accessible from JavaScript (for a page navigation), without side effects.
  • It is a shame that there is no start time on a timing metric, so it is impossible to describe the concurrency or blocking of tasks in relation to one another, possibly the most valuable use-case for this header. As an alternative we could have some kind of declared relationship between the metrics to indicate that they block.
  • There's a separate discussion of syntax, my preference would be to drop the semicolon delimiter idea. I can't see the need for a millisecond duration AND a description as a common requirement, so you're often going to get key1=;val, key2=;val2 which is weird and looks wrong. My preference would be to interpret the value as a time period if it includes a unit, CSS style.
Comment by @plinss Sep 5, 2017 (See Github)

@cvazac and @yoavweiss are either (or both) of you available to join a TAG call to discuss this further? We meet at 8am PDT (15:00 UTC) on Sept 12 or 19 would be ideal.

Comment by @hadleybeeman Sep 5, 2017 (See Github)

Discussed on TAG call 5 Sept 2017, particularly questions around the choice not to use JSON. We had a quick look at @reschke's internet draft A JSON Encoding for HTTP Header Field Values.

We also had a quick look at its data tracker page... What is the status/momentum of this? Is it implemented anywhere?

@mnot, any additional context/input?

Comment by @yoavweiss Sep 5, 2017 (See Github)

@cvazac and @yoavweiss are either (or both) of you available to join a TAG call to discuss this further? We meet at 8am PDT (15:00 UTC) on Sept 12 or 19 would be ideal.

I can join if needed on September 12th.

We also had a quick look at its data tracker page... What is the status/momentum of this? Is it implemented anywhere?

Implemented with intent to ship in Blink. Intent to implement in WebKit.

Comment by @cvazac Sep 5, 2017 (See Github)
  • AFAIK yes, it's the only header accessible to script, outside of cookies.

  • regarding startTime related discussion here alternates to startTime as a first-class value (that "work" today):

db-start=100, db-duration=50

db=50; 100

  • regarding syntax: the following are all legal: key key=duration key;description (no need for = when no duration specified) key=value;description @triblondon are you suggesting we go from three fields per metric to two - one for the name of the metric, and the other as a string (which should be interpreted as a number if there are known time-based units)?

I will join on 9/12. Can someone please send me the details for the call? cc @hadleybeeman

Comment by @plinss Sep 5, 2017 (See Github)

Great that you both can make it, we'll be using WebRTC via Jitsi: https://meet.jit.si/w3ctag

Comment by @igrigorik Sep 7, 2017 (See Github)

Hey all, lots of great discussions here!

Meta ask and observation: if there is high-level design feedback you think is worth iterating on, please file issues against the spec repo and let's iterate there, where it's visible to other WG members. We shouldn't (re)design features in this forum.

Comment by @plinss Sep 11, 2017 (See Github)

@cvazac and @yoavweiss FYI: agenda for tomorrow's call: https://github.com/w3ctag/meetings/blob/gh-pages/2017/telcons/09-12-agenda.md

Comment by @triblondon Sep 12, 2017 (See Github)

Strawman proposal:

Server-Timing: db-start=100;type=moment, db-duration=50;type=duration, peak-memory=4525345;type=bytes, data-center=LHR;type=enum, cache-status=HIT;type=enum, user-id=3457853;type=str

Obviously I can't predict all the different types, but the point is that tooling is going to want to visualise these in different ways, for instance:

  • Durations as horizontal bars
  • Moments as vertical lines / dots
  • Bytes scaled to an appropriate human-readable unit (tool might also conceivably compare the value with values for the same metric for previous requests)
  • ENUMs as tag-style labels, perhaps with automatic colour coding
  • Freefrom strings as just basic tabulated data

It seems not hard to build support for this kind of extensibility into the syntax, so as to avoid this:

Server-Timing: data=;<URL-ENCODED-JSON>

One might imagine the above is "fine" on the basis that it's not interfering with the intended use of the header, but if the tools for which this header is designed start to look for and depend on this syntax, then it would be very hard to introduce a type attribute later on.

I'm not a big fan of adding an explicit description attribute key, because I do ultimately think that a string value should not have to live in a meta property just because you have reserved the main value property for a millisecond duration. However, a meta property seems like a reasonable solution (and an alternative to a unit) to switch the processing model that should apply to the main value.

Comment by @yoavweiss Sep 13, 2017 (See Github)

Started https://github.com/w3c/server-timing/issues/32 to discuss feedback from this review and from yesterday's meeting.

Comment by @triblondon Sep 27, 2017 (See Github)

Posted https://github.com/w3c/server-timing/issues/32#issuecomment-332450482 to respond to the proposed changes. Everything looks great.