search

quicwg / qlog

The IETF I-D documents for the qlog format
Other
86 stars 13 forks source link

How is "trigger" a common subfield of "data"? #429

Open LPardue opened 4 months ago

LPardue commented 4 months ago

In https://quicwg.org/qlog/draft-ietf-quic-qlog-main-schema.html#section-8.2-5 we say:

Any generic key-value map type can be assigned to $ProtocolEventData (the only common "data" subfield defined in this document is the optional trigger field, see Section 7.4).

I don't quite understand how this is a common field, since there is no CDDL for it. Should we be defining $ProtocolEventData with the actual optional ?trigger field? Or is is more like "common by convention" or some other magic.

Or is it actually the case that "trigger" is a design pattern that concrete event definitions can adopt but that they must explicitly declare the trigger field in their event definition?

LPardue commented 4 months ago

FWIW: Of this 40 events we define for QUIC and H3, only 10 of them currently define a trigger field.

rmarx commented 1 month ago

It's impossible to add the trigger field to $ProtocolEventData since new events aren't really inheriting from $ProtocolEventData but rather just adding themselves to it as a list of all possible events that can be used (put differently: $ProtocolEventData is an extensible ENUM, not a base class).

We don't really have a base class for events at this point, and so trigger is left as a bit of a loosely defined thing (more like as you say "a design pattern that concrete event definitions can adopt but that they must explicitly declare the trigger field in their event definition").

Given that we only have the one trigger field that would be common and that it's optional and not super-widespread, I personally think this is fine?

LPardue commented 1 month ago

Thanks for the explanation, that makes more sense. I think we can probably wordsmith the text on triggers a bit. I just reread section 8.2 and 8.2.1 and its seems a bit verbose. I think we can probably simplify this to something along the lines of "if its useful for an event to include a trigger, then it is RECOMMENDED that the event data contains an optional field named trigger that has a string value."