feat(perf): Extract span.status_code tag for HTTP spans
#3245
Conversation
| @@ -260,6 +264,10 @@ fn span_metrics() -> impl IntoIterator<Item = MetricSpec> { | |||
| Tag::with_key("resource.render_blocking_status") | |||
| .from_field("span.sentry_tags.resource.render_blocking_status") | |||
| .when(is_resource.clone()), | |||
| // HTTP module: | |||
| Tag::with_key("span.status_code") | |||
| .from_field("span.sentry_tags.status_code") | |||
There was a problem hiding this comment.
Do we need the tag in d:spans/exclusive_time_light@millisecond too?
There was a problem hiding this comment.
Unsure what you mean—from what I can see I added the tag to that metric, did I do something wrong?
There was a problem hiding this comment.
The tag is added to two metrics: d:spans/exclusive_time@millisecond and d:spans/exclusive_time_light@millisecond. Do we need the tag in both metrics, or just one of the two metrics is good enough?
There was a problem hiding this comment.
Oh I see! I need it for both, since the light metric is only used on the landing page, and the regular one is used on the domain landing pages, where we have to break down by transaction
relay-dynamic-config/src/defaults.rs
Outdated
| // HTTP module: | ||
| Tag::with_key("span.status_code") | ||
| .from_field("span.sentry_tags.status_code") | ||
| .always(), |
There was a problem hiding this comment.
| .always(), | |
| .when(is_http.clone()), |
| @@ -260,6 +264,10 @@ fn span_metrics() -> impl IntoIterator<Item = MetricSpec> { | |||
| Tag::with_key("resource.render_blocking_status") | |||
| .from_field("span.sentry_tags.resource.render_blocking_status") | |||
| .when(is_resource.clone()), | |||
| // HTTP module: | |||
| Tag::with_key("span.status_code") | |||
| .from_field("span.sentry_tags.status_code") | |||
relay-dynamic-config/src/defaults.rs
Outdated
| // HTTP module: | ||
| Tag::with_key("span.status_code") | ||
| .from_field("span.sentry_tags.status_code") | ||
| .always(), |
There was a problem hiding this comment.
| .always(), | |
| .when(is_http.clone()), |
Note this is worst case a multiplicator of 12x on the entire cardinality, it might seem low, but every additional tag is a multiplicator of the existing cardinality, so if there is no use-case for it, I would consider just going with the broader category of |
gggritso
changed the title
span.status_code tag for HTTP spans in exclusive time metricspan.status_code tag for HTTP spans
|
@iker-barriocanal updated changelog 👍🏻 |
|
Fair enough! |
We need this to do status code breakdowns for the HTTP module. Note: The HTTP module, as designed, doesn't need to store the full code, but rather just the first digit. We break down by 2xx, 4xx, and 5xx codes. That said, it seems simpler and more robust to just extract all the response codes, since in practice we don't expect more than half a dozen codes to actually come in per span.
I added a banal test, but this is my first PR in Relay so please save me from myself 🙏🏻 let me know what you need in terms of code changes and/or test coverage.