quicwg / qlog

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

change STREAM and DATAGRAM frame definition to allow logging of frame payload #342

Open marten-seemann opened 10 months ago

marten-seemann commented 10 months ago

Currently the raw field includes the wire representation of the frame. This doesn't seem useful, and would mean that a qlog consumer would need to implement a frame parser. Instead, we should only log the payload.

This could be done by changing the raw field to payload.

rmarx commented 7 months ago

So, I don't agree with this. The reason it includes the frame/packet headers currently is because we (intentionally) lose some information present in the binary form by representing the data in qlog/JSON. Users should have a way to get to the original data for low-level debugging if needed. An argument -might- be made for splitting it into a separate headers and payload (and then also trailers?) field, but not omitting headers.

Your proposal imo also only circumvents 1 layer of parsing at a time (e.g., not logging QUIC STREAM frames still requires implementing an H3/QPACK parser, not logging QUIC Packer Headers still required QUIC frame parser, etc.).

I would suggest closing this without action.

rmarx commented 3 months ago

Are you still willing to fight for this @marten-seemann ?

I just also thought of another counter-argument: since the current RawInfo struct also contains the full length and the payload-only length, an implementation that just wants the payload can calculate the length of the headers and then skip that many bytes ahead in the payload hexstring to get to the payload start (it requires a little bit more logic in the case of trailers, but we really only have those for QUIC packets, and those trailers are 100% predictable in size).

If you do not provide push-back within 1 week, I'll close this without action.

rmarx commented 3 months ago

Discussed during editors' meeting. Indeed makes sense to only log payload. If you need higher up headers, you log previous layer (e.g., if you need QUIC packet headers, you'd get them from the raw UDP packets).