qlog
qlog copied to clipboard
packet_sent doesn't allow logging of IP address (for packets sent outside of a connection)
This would be interesting for Version Negotiation, Retry, CONNECTION_CLOSE etc. sent by a server.
Discussed during meeting.
Robin: not a big fan of this; use datagrams_*
instead or group_id
/path_id
or custom field or separate trace
Lucas: not super-strong opinion there
Marten: imo server log sending out retries should log everything in a single event, not spilt out
Robin: will think about it some more; get back to you
see also #211
So, still not a big fan of having this separately from path_id
(see #336) or conceptually group_id
.
I get that logging a separate path_assigned
event is annoying for this type of thing, but there you could also choose to just log the IP addresses as the pathID
as that's a string (see for example slide 5 here: https://datatracker.ietf.org/meeting/118/materials/slides-118-quic-qlog-00).
I'm mostly afraid that if you add ip fields to all these events, people will use them for "normal" logs as well, not just for the use cases you describe, which is something I -really- want to discourage.
@marten-seemann now that https://github.com/quicwg/qlog/pull/336 has been merged, and with my comments above, do you think we can close this issue?
Discussed during editor's meeting.
Marten: really useful to log events outside connections for debugging Lucas: true, but in practice we don't do this at scale because we get so much bad stuff. Have whole DDoS protection layers to protect against this. Also annoying to link outside connection events to actual connections (e.g., VNEG/RETRY).
Marten: proposed adding separate events to explicitly log this stuff
Robin: seems too much work/overhead just to add ip address info. Feel like the pathID "hack" proposed above allows for this use case.
Marten: I can work, but it's dirty. Paths aren't necessarily 4-tuples; abusing fact that pathID is a string
Robin: true... but if you look at how PathEndpointInfo
is defined, that's basically also just a 4-tuple? You would just skip the path_assigned
step by serializing it yourself.
Marten: don't want to make it more complicated than necessary. I don't like the pathID solution, really don't, but creating new events/category seems like a lot of overhead.
Robin: ok. For now then just closing this issue without action; use pathID for this use case. Open to revisiting later if more feedback from the field comes in.