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

Replace specific events with a single encompassing event #24

Closed
rmarx opened this issue Oct 7, 2019 · 0 comments
Closed

Replace specific events with a single encompassing event #24

rmarx opened this issue Oct 7, 2019 · 0 comments

Comments

@rmarx
Copy link
Contributor

rmarx commented Oct 7, 2019

We want to reduce the total amount of events as much as possible.

Especially specific events for things happening in reaction to the receipt of a specific frame in a packet (e.g., ACK, MAX_DATA, etc.) can be removed, since they can usually be inferred from that frame. Initially we had separate events for these (e.g., "packet_acknowledged" or "flow_control_updated" but those were rarely used in addition to packet_received events + many implementations do not defer frame handling from reception.

One notable exception is @nibank's msquic, which does not log all frames in a received packet, but rather does log only specific events (e.g., packet_acknowledged). One of the reasons is because he feels logging each packet in full does not scale. Another reason for this pattern might be that an implementation does not wish to log all types of frames OR conversely, does not want to log packet-level information at all but only very select frames.

To support this use case and still keep a low amount of event types, I will add a "frame_parsed" encompassing event. This will log the frame with its associated data, but without the encompassing packet-level data. This prevents re-defining semantics for many events. The downside is that you sometimes might want to log e.g., "packet_acknowledged" a long time after frame receipt. In that case, you would pretend you're parsing the frame only then. I feel this is a good trade-off to make here though.

@rmarx rmarx closed this as completed in 4f30c9c Oct 7, 2019
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

1 participant