-
Notifications
You must be signed in to change notification settings - Fork 93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ref(spans): Reduced set of metrics/tags for all orgs #2924
Conversation
let mut span = Span::default(); | ||
span.sentry_tags | ||
.get_or_insert_with(Default::default) | ||
.insert("mobile".to_owned(), "true".to_owned().into()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is necessary to trigger the is_mobile
condition in defaults.rs.
tags: { | ||
"environment": "fake_environment", | ||
"span.action": "GET", | ||
"span.category": "http", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HTTP spans are gone for now, except for those inui.load
transactions.
"span.status": "ok", | ||
"span.system": "postgresql", | ||
"transaction": "gEt /api/:version/users/", | ||
"transaction.method": "POST", | ||
"transaction.op": "myop", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tags are not required for the db
category.
@@ -626,10 +173,6 @@ expression: metrics | |||
"span.domain": "table", | |||
"span.group": "f4a7fef06db3d88e", | |||
"span.op": "db.sql.query", | |||
"span.status": "ok", | |||
"span.system": "postgresql", | |||
"transaction.method": "POST", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transaction.method
is not required on the exclusive_time_light
metric.
"transaction.method": "POST", | ||
"transaction.op": "myop", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tags are not needed on resource spans.
Bucket { | ||
timestamp: UnixTimestamp(1695255136), | ||
width: 0, | ||
name: "d:spans/exclusive_time@millisecond", | ||
value: Distribution( | ||
[ | ||
1668.516159, | ||
], | ||
), | ||
tags: { | ||
"environment": "fake_environment", | ||
"span.category": "app", | ||
"span.description": "Cold Start", | ||
"span.group": "2d675185edfeb30c", | ||
"span.op": "app.start.cold", | ||
"span.status_code": "200", | ||
"transaction": "gEt /api/:version/users/", | ||
"transaction.method": "POST", | ||
"transaction.op": "myop", | ||
}, | ||
}, | ||
Bucket { | ||
timestamp: UnixTimestamp(1695255136), | ||
width: 0, | ||
name: "d:spans/exclusive_time_light@millisecond", | ||
value: Distribution( | ||
[ | ||
1668.516159, | ||
], | ||
), | ||
tags: { | ||
"environment": "fake_environment", | ||
"span.category": "app", | ||
"span.description": "Cold Start", | ||
"span.group": "2d675185edfeb30c", | ||
"span.op": "app.start.cold", | ||
"span.status_code": "200", | ||
"transaction.method": "POST", | ||
"transaction.op": "myop", | ||
}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
app.start.*
metrics are only extracted for mobile SDKs, but there's no SDK info on the event in this test case. I figured we can omit these test inputs, because there's already a separate mobile test case.
extract_span_metrics_op_duration("app.start.cold", 180000.0).len() | ||
); | ||
let metrics = extract_span_metrics_mobile("app.start.cold", 180000.0); | ||
assert!(!metrics.is_empty()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we validate the names of the extracted metrics?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Follow-up to #2907: Apply reduced set of span metrics and tags regardless of feature flag.
ref: https://github.com/getsentry/team-ingest/issues/252