Adds total_bytes_received metric emitted by all blackholes#1702
Adds total_bytes_received metric emitted by all blackholes#1702
total_bytes_received metric emitted by all blackholes#1702Conversation
479bb5d to
1eec465
Compare
Signed-off-by: Caleb Metz <caleb.metz@datadoghq.com> changelog Signed-off-by: Claude <noreply@anthropic.com>
1eec465 to
948b64e
Compare
| /// | ||
| /// Note: the meaning of "bytes" varies by transport — HTTP-based blackholes report wire bytes, | ||
| /// while gRPC-based blackholes (OTLP gRPC, Datadog Stateful Logs) report protobuf `encoded_len()`. | ||
| pub(super) static COMMON_BLACKHOLE_LABELS: &[(&str, &str)] = &[("component", "blackhole")]; |
There was a problem hiding this comment.
are there other instances of us emitting the total_bytes_received metric with different components?
If a tag can only have a single possible value, it's not too too useful on it's own.
Could you elaborate on why we need this tag/facet?
There was a problem hiding this comment.
There are not any other instances of total_bytes_received with different components, intentionally.
My thought process here was to have this label be applied to all the different blackholes so that in the future if we do define some new component that will emit total_bytes_received we have the separation.
We could choose to remove this for now, im not opposed, or rework the labels applied to the pre existing bytes_received to extend from common labels
There was a problem hiding this comment.
My thought process here was to have this label be applied to all the different blackholes so that in the future if we do define some new component that will emit total_bytes_received we have the separation.
I'm always hesitant to merge changes "in case of something". I'd prefer to tackle that need then when the use case comes up.
imo we should drop it
There was a problem hiding this comment.
Im convinced. Will simplify things as well.
What does this PR do?
Adds another
bytes_receivedmetric calledtotal_bytes_receivedemitted by every blackhole that is guaranteed to be a single series.Motivation
In current state if we were to configure lading to have multiple different types of blackholes we would emit multiple
bytes_receivedseries since the labels passed through to blackhole metrics havecomponent:blackholeandcomponent_name:{type of blackhole}.In addition, if you supply an ID when defining a blackhole, if there are multiple (even if they are the same type) we will similarly have multiple
bytes_receivedseriesFor clarity I find it appealing to have one
total_bytes_receivedmetric for all blackholes while maintaining the current behavior forbytes_receivedAdditional Notes
See this notebook for more context