search

quicwg / qlog

The IETF I-D documents for the qlog format
Other
84 stars 12 forks source link

H3Settings is a bit underspecified #382

Closed LPardue closed 3 months ago

LPardue commented 8 months ago

https://quicwg.org/qlog/draft-ietf-quic-qlog-h3-events.html#section-5.3.4

H3Setting = {
    name: text
    value: uint64
}

Looking at IANA https://www.iana.org/assignments/http3-parameters/http3-parameters.xhtml, I think qlog nudges implementations to log the setting name. When an implementation receives an unknown setting (not a greased one) it won't know the name. It could log the setting type value code in the name field, but that is not specified. This makes things annoying for parsers since they need to do some thinking.

One suggestion is to add another field like type that could contain the numerical type value. It would be duplicative to have both, so that might require making type and name optional.

LPardue commented 8 months ago

Also we might want to state that the name should be the name IANA use, possiblyeven define a explicit names in addition to free form string.

rmarx commented 7 months ago

Discussed during meeting. Agree to add this option, similar to how we changed ALPN in #385

rmarx commented 7 months ago

Another thing I'm wondering is if we should clarify what the name field is exactly.

i.e., I've previously logged this myself as QPACK_MAX_TABLE_CAPACITY but the IANA name is prefixed with SETTINGS_, so SETTINGS_QPACK_MAX_TABLE_CAPACITY instead.

You could argue this should be obvious, but e.g., in RFC9204, they use the unprefixed version (see https://www.rfc-editor.org/rfc/rfc9204.html#section-8.1) with a note For formatting reasons, the setting names here are abbreviated by removing the 'SETTINGS_' prefix.... could be quite confusing for novice implementers, so maybe 1 example or clear statement that the name is the IANA registered name would be good.

LPardue commented 7 months ago

Agree 100%, I'll add some text