Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
LPardue opened this issue Jul 9, 2024 · 3 comments
Open

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

LPardue opened this issue Jul 9, 2024 · 3 comments

Comments

@LPardue
Copy link
Member

LPardue commented Jul 9, 2024

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
Copy link
Member Author

LPardue commented Jul 9, 2024

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

@rmarx
Copy link
Contributor

rmarx commented Oct 21, 2024

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
Copy link
Member Author

LPardue commented Oct 21, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants