fix(spans): Ingest span metrics for http spans #3111
Conversation
There was a problem hiding this comment.
For an unknown reason
@phacops the decision to stop ingesting them was deliberate & coordinated: #2907 (comment)
Pressing "Request changes" because of open questions.
| @@ -73,6 +73,8 @@ fn span_metrics() -> impl IntoIterator<Item = MetricSpec> { | |||
|
|
|||
| let is_mobile_sdk = RuleCondition::eq("span.sentry_tags.mobile", "true"); | |||
|
|
|||
| let is_http = RuleCondition::eq("span.op", "http.client"); | |||
There was a problem hiding this comment.
Do we need exclusive time metrics for all http.client spans? Or is there a way to narrow it down further?
There was a problem hiding this comment.
🤔 I think we need it for everything, yeah. Since the HTTP module needs to show the total time spent on http spans, I don't think we can omit metrics for any spans
| @@ -216,7 +225,7 @@ fn span_metrics() -> impl IntoIterator<Item = MetricSpec> { | |||
| .always(), | |||
| Tag::with_key("span.domain") | |||
| .from_field("span.sentry_tags.domain") | |||
| .when(is_db.clone() | is_resource.clone()), | |||
| .when(is_db.clone() | is_resource.clone() | is_http.clone()), | |||
There was a problem hiding this comment.
@gggritso Do we already know what tag combinations / filters we want to query exclusive_time by? Does this config accurately reflect those access patterns?
There was a problem hiding this comment.
We have initial product requirements, so we do know, even though it might change:
environmentfor environment filtersspan.domain, to break down and filter by domain in the table, and possibly render top 5 domains chartsspan.status_code, to show 2xx, 4xx, and 5xx counts by span domain and/or by span transactiontransaction, to show a table of transactions for a given span domain, filter by transactiontransaction.methodso we can filter by correct endpoint method in the sample panel
The config looks good to me 👍🏻
For an unknown reason, we stopped ingesting them on January 7th when we shouldn't have.
I also removed the test for
all_modulesas this feature doesn't exist anymore.