feat(eap): Add payload size as an attribute to logs and spans #5042
Conversation
| log.attributes | ||
| .get_or_insert_with(Default::default) | ||
| .insert_if_missing("sentry.payload_size_bytes", || size_in_bytes); |
There was a problem hiding this comment.
Should do this in processing/logs/store.rs, then it works correctly even with external Relays, where this will not (someone or an external Relay sets the attribute, then it never gets changed).
There was a problem hiding this comment.
I think you can add the size from the header (convert gets WithHeader) to the FieldAttributes struct and add the attribute right next to where we add sentry.severity_text.
FieldAttributes will no longer just be "field attributes" but that's w/e.
relay-server/src/services/store.rs
Outdated
| trace_item.attributes.insert( | ||
| "sentry.payload_size_bytes".into(), | ||
| AnyValue { | ||
| value: Some(Value::IntValue(span.payload_size_bytes as i64)), | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Why do we need it for Spans?
The size of a Span is (currently) not billing relevant. The size used here will completely differ from the one for logs, which makes them not comparable and we will have to change that in the future again (not sure if that is an issue) with more SpanV2 support.
There was a problem hiding this comment.
It is indeed not billing relevant yet but we're looking at an indication of the size of the span when we receive it since we'll start applying limits to them soon.
I thought this was the best approximation right now, and we can replace it for a more accurate one later when we have it.
There was a problem hiding this comment.
Do you need them in Kafka? We can also emit this as a metric in Relay (and I think we already have that because we track the Kafka message sizes).
I thought this was the best approximation right now
It is probably the best we have right now, but I'd expect it to be off by more than 50%, that's what we saw for logs.
| let payload_size_bytes = log | ||
| .header | ||
| .as_ref() | ||
| .unwrap_or(&OurLogHeader { | ||
| ..Default::default() | ||
| }) | ||
| .byte_size | ||
| .unwrap_or_default(); |
There was a problem hiding this comment.
log.header.as_ref().and_then(|h: &OurLogHeader| h.byte_size).unwrap_or_default()
Seems a bit nicer, maybe you can also drop the type annotation in the and_then.
No description provided.