qlog
qlog copied to clipboard
Convert H3 setting param types to a real type
Closes #382
Instead of a separate raw_type field, we'll just allow the nae to be a union of enum and uint64.
Heads up that I changed some of this in #417, by adding the name_bytes
field (to be consistent with other places where we allow logging raw values), so that would replace the raw_type
field proposed here.
Heads up that I changed some of this in #417, by adding the
name_bytes
field (to be consistent with other places where we allow logging raw values), so that would replace theraw_type
field proposed here.
So I've rebased and resolved the conflict. However, ISTM on a previous editor call we wanted to make this a union instead, so something like
H3Setting = {
name: H3SettingsName / uint64
value: uint64
}
and obviously a text update to describe how to pick which one.
Do we still want to do that?
So, we've gradually moved away from that pattern IIRC (e.g., packet_number is now always a uint64 instead of uint64 / string to deal with IJSON).
Looking through the other events, there are a few events that use this pattern (ResetStreamFrame, StopSendingFrame, ConnectionClosed, IPAddress), e.g., connectionclosed:
? connection_code: $TransportError /
CryptoError /
uint32
? application_code: $ApplicationError /
uint32
But most events that need something like this, use the
field: string
field_bytes: hexstring
OR
field: string
field_bytes: uint64
It seems like ideally we'd just have a single way of dealing with these. I'm leaning more towards the explicit extra field_bytes
pattern, to make things explicit that this is (likely) an "unexpected" value that's likely to cause problems + to prevent tools from having to special case types for fields (at least most of the time).
If you agree, then we should merge this PR as-is (or with my proposed changes in the upcoming review ;)) and then I can make a new PR adding new XYZ_bytes
fields to the non-conformant events.
I'm more than happy to go with this direction right now. Suggestions merged so I think GTG