feat(suspect-spans): Add exclusive time for root span #1083
Conversation
Following up with #1061, this change adds the exclusive time calculation for the root span of a transaction.
relay-general/src/protocol/event.rs
Outdated
| /// The amount of time in milliseconds spent in this transaction span, excluding its immediate | ||
| /// child spans. (relevant for event type = "transaction") | ||
| #[metastructure(omit_from_schema)] // we only document error events for now | ||
| pub exclusive_time: Annotated<f64>, |
There was a problem hiding this comment.
Please add this in the trace context instead
Ahh my bad, I did not push the changes in the preview commit. Should be updated now. |
CHANGELOG.md
Outdated
| @@ -24,7 +24,7 @@ | |||
| **Internal**: | |||
|
|
|||
| - Add new metrics on Relay's performance in dealing with buckets of metric aggregates, as well as the amount of aggregated buckets. ([#1070](https://github.com/getsentry/relay/pull/1070)) | |||
| - Add the exclusive time of a span. ([#1061](https://github.com/getsentry/relay/pull/1061)) | |||
| - Add the exclusive time of a span. ([#1061](https://github.com/getsentry/relay/pull/1061)), ([#1083](https://github.com/getsentry/relay/pull/1083)) | |||
There was a problem hiding this comment.
@Zylphrex the changelog for 1083 should be moved to the top at the Unreleased section. This line is associated with v21.9.0.
| } | ||
|
|
||
| set_event_exclusive_time(event, &mut span_map); | ||
|
|
||
| let spans = event.spans.value_mut().get_or_insert_with(|| Vec::new()); |
There was a problem hiding this comment.
I notice this inserts a new vector if event.spans doesn't have one set. @untitaker would this be fine?
There was a problem hiding this comment.
apparently we already did that before so I am not too worried about that
you can avoid the double-borrowing here if you change set_event_exclusive_time to take timestamps and a mutable reference to contexts instead of the entire event. then you can still hold a mutable reference to spans throughout this entire function
| } | ||
|
|
||
| set_event_exclusive_time(event, &mut span_map); | ||
|
|
||
| let spans = event.spans.value_mut().get_or_insert_with(|| Vec::new()); |
There was a problem hiding this comment.
apparently we already did that before so I am not too worried about that
you can avoid the double-borrowing here if you change set_event_exclusive_time to take timestamps and a mutable reference to contexts instead of the entire event. then you can still hold a mutable reference to spans throughout this entire function
Following up with #1061, this change adds the exclusive time calculation for the
root span of a transaction.