feat(s2n-quic-dc): emit top-level stream events #2394
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
camshaft
force-pushed
the
camshaft/dc-stream-events
branch
5 times, most recently
from
e3337a5 to
2384171
Compare
camshaft
changed the title
| /// The amount of time it took to process the write request | ||
| /// | ||
| /// Note that this includes both any syscall and encryption overhead | ||
| #[measure("processing_duration", Duration)] |
There was a problem hiding this comment.
I guess this is OK for now, but I wonder if we should have a duration_per_byte or something like that, to make it easier to actually make sense of what this means.
There was a problem hiding this comment.
yeah that could be an interesting metric to have. i can add that as a follow-up
| #[event("stream:read_socket_flushed")] | ||
| pub struct StreamReadSocketFlushed { | ||
| /// The number of bytes that the stream tried to read from the socket | ||
| #[measure("capacity", Bytes)] |
There was a problem hiding this comment.
This buffer is always an internal buffer IIRC, right? Do we expect meaningful size differences on that?
There was a problem hiding this comment.
Yeah it's always an internal buffer. I was curious to see if we were ever needing to read more for stream sockets to complete a packet, in which case this buffer would be undersized.
Mark-Simulacrum
previously approved these changes
Dec 3, 2024
camshaft
force-pushed
the
camshaft/dc-stream-events
branch
from
2384171 to
ca4038b
Compare
camshaft
force-pushed
the
camshaft/dc-stream-events
branch
from
ca4038b to
fc51fd1
Compare
Mark-Simulacrum
approved these changes
Dec 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes:
This change adds several stream events for s2n-quic-dc streams. This can provide better insight into what is happening to streams both in aggregate and individually.
Testing:
You can see the events showing up in the tracing logs for the tests:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.