feat(s2n-quic-dc): implement connection-level counter aggregation #2397
Conversation
| @@ -419,3 +410,13 @@ struct DcStateChanged { | |||
| #[nominal_counter("state")] | |||
| state: DcState, | |||
| } | |||
|
|
|||
| // NOTE - This event MUST come last, since connection-level aggregation depends on it | |||
There was a problem hiding this comment.
You mean last in this file? (Asking since you moved it in the file..)
There was a problem hiding this comment.
Based on s2n-quic-events changes I don't think order matters so approving, but maybe worth fixing up the comment.
There was a problem hiding this comment.
yeah last in this file. basically i didn't want to have scan everything twice - once to look for all the counters, then to process the connection close.
|
|
||
| if units == "Duration" { | ||
| on_connection_closed.extend(quote!( | ||
| self.measure(#info, #id, core::time::Duration::from_micros(context.#ctx_name #counter_load)); |
There was a problem hiding this comment.
Hm, I guess I missed before that we have u64 granularity of microseconds in our metrics. Maybe we should switch to nanoseconds? Anyway, not important for this PR.
There was a problem hiding this comment.
Yeah we do currently:
The tricky bit is our timestamps aren't more granular than that either, so it would be a bit of a change:
Description of changes:
This change implements connection-level counter aggregation. This allows aggregating counters at the connection level and then aggregating them as metrics, as opposed to them being pure counters or measurements, which don't have enough granularity to give insight into distribution of connection behaviors. For example, this change allows getting a distribution of the number of bytes sent and received on streams.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.