w3ctag / design-reviews

W3C specs and API reviews
Creative Commons Zero v1.0 Universal
333 stars 55 forks source link

ReportingObserver #195

Closed RByers closed 6 years ago

RByers commented 7 years ago

Hello TAG!

This is early, but new blink launch guidelines include asking for TAG feedback early. At this stage I wouldn't recommend reviewing the spec in detail - it's still under review with spec editors. But I'd at least appreciate a look at the explainer / use cases (and spec review is great too - but to avoid wasting your time I can ping again when it has actually landed).

This is part of "Reporting API" being driven by @mikewest which I don't see a TAG review request for yet. I'm personally working only on the JS interface in ReportingObserver and believe the two halves (HTTP vs. JS APIs) are largely separable and independently shippable. But if you prefer to treat this as a TAG review for all of Reporting that's fine with me too.

I'm requesting a TAG review of:

Further details (optional):

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

travisleithead commented 7 years ago

FYI: Re WICG/reporting#51, the current proposal application/reports+json looked good to me--and I commented as such on their issue.

triblondon commented 6 years ago

Picked up at London F2F. Passing on this at this time, there's still work to do for the authors to followup on our earlier feedback

travisleithead commented 6 years ago
torgo commented 6 years ago

Discussed the idea of bringing some people together on a call for this issue or possibly for our f2f.

torgo commented 6 years ago

To agree how we want to proceed by next week. @slightlyoff to reach out to various parties.

slightlyoff commented 6 years ago

Per last week's call, the remaining substantive issue is JSON vs. Structured Headers for this design. There's a question here for @jyasskin about how Web Packaging decided to use Structured Headers. Is SH the new hotness? Or a fad? Is it fit-for-purpose for this API?

slightlyoff commented 6 years ago

Ok, so we spent a bit more time in today's meeting reading the SH spec and note that it doesn't support any sort of nesting in it's data-structures. Nearly every example in the Reporting API Explainer involves nesting, and the JS API specifically returns a list of report.

It seems that it would be possible to flatten some of these report types to confirm to SH restrictions. The Report-To header looks particularly ripe for this. As for report bodies, that's less clear.

@mnot: are your concerns with JSON limited to Report-To? If bodies are sent in JSON, does that still work for you?

jyasskin commented 6 years ago

@mnot and @reschke are better folks to ask than me. https://tools.ietf.org/html/draft-ietf-httpbis-header-structure-01#section-1 (note the non-current draft number) discusses why not to use JSON in HTTP headers, but of course doesn't apply to payloads.

Note that the latest version of draft-reschke-http-jfv is https://tools.ietf.org/html/draft-reschke-http-jfv-08, not the -02 that the Reporting spec refers to.

If you go to Structured Headers, the difficulty is that Report-To is an array containing maps (so far we're good with a Parameterised List), containing an array (uh-oh, but sometimes doable with a space-delimited string), containing maps (nope).

mnot commented 6 years ago

I've added the reasons that I remember coming up in discussion here: https://httpwg.org/http-extensions/draft-ietf-httpbis-header-structure.html#why-not-json

mnot commented 6 years ago

@jyasskin @slightlyoff I see that level of nesting in the payload in that explainer, but not the header field value. What am I missing?

jyasskin commented 6 years ago

@mnot http://wicg.github.io/reporting/#header describes the HTTP header with the amount of nesting I described. However, that specification describes the endpoints array as required, but the explainer doesn't include that field in its example. 🤷🏼‍♂️

Payloads are generated in http://wicg.github.io/reporting/#try-delivery.

jyasskin commented 6 years ago

Folks can follow the resulting HTTPWG discussion at https://lists.w3.org/Archives/Public/ietf-http-wg/2018AprJun/0324.html.

mnot commented 6 years ago

@jyasskin is this a representative example (ignoring the semantic sensibility of the values)?

[
    {
        "group": "foo",
        "include_subdomains": false,
        "max_age": 60,
        "endpoints": [
            {
                "url": "https://example.com/",
                "priority": 15,
                "weight": 10
            },
            {
                "url": "https://other.example.com/",
                "priority": 10,
                "weight": 10
            }
        ]
    },
    {
        "group": "bar",
        "include_subdomains": true,
        "max_age": 30,
        "endpoints": [
            {
                "url": "https://example.org/",
                "priority": 1,
                "weight": 5
            }
        ]        
    }
]
jyasskin commented 6 years ago

I defer to one of the spec's owners. @mikewest?

slightlyoff commented 6 years ago

Hey all,

We discussed again today, and given the new example of the structure that @mnot posted, it seems like JSON is indeed the only available, largely appropriate choice at the moment. Other designs require pre-guessing how to de-structure common, deep data with flat names or a new convention (not part of the SH design) to enable re-construction based on some sort of naming convention. The alternatives (Base64-encoding nested structured) seem just as bad.

Would love to see the issues filed get addressed, but don't see much need to hold this train for the use of JSON modulo previously raised points.

mnot commented 6 years ago

@slightlyoff there's also discussion in https://github.com/WICG/reporting/issues/53 that's currently blocked on https://github.com/WICG/reporting/issues/93.

dcreager commented 6 years ago

w3c/reporting#93 and w3c/reporting#53 have now been resolved. (Note the repo name changes; Reporting and NEL both moved from WICG → WebPerf last week)

travisleithead commented 6 years ago

@dcreager thanks for closing the loop. We appreciate all the consideration and discussions around SH vs. JSON. Given where the design has fallen, we don't have any more meaningful feedback at this time. Thanks for flying TAG!

ylafon commented 6 years ago

Also, it was noted that the fact that JSON contains separators like , (comma) can lead to interoperability issues with some intermediaries, as it is a separator used in common HTTP header syntax (it was an issue in the past with Cookies, for example).

dcreager commented 6 years ago

Thanks Yves! Is there a link to the discussion, or can you provide an example where an intermediary might mangle a JSON-encoded Report-To header? JFV should, for example, do the right thing if Report-To appears twice, and an intermediary decides to concatenate them with a , separator.