-
Notifications
You must be signed in to change notification settings - Fork 505
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
feat: metric span summaries #2522
Conversation
…y-python into feature/span-aggregation
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.
lgtm
This lands a callback which controls the sample rate of span local metric summaries dynamically. Refs getsentry/sentry-python#2522
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.
LGTM
We could consider renaming LocalAggregator
to SpanAggregator
or SpanMetricAggregator
, but depends on how this is intended to be used in the future and if it's always going to be stored on a span.
This PR adds metric summaries to spans.
Refs this RFC: https://github.com/getsentry/rfcs/blob/main/text/0123-metrics-correlation.md
For now they are guarded behind a sample rate which is 0 by default so we can gradually judge the impact. The sampling is done globally which is not useful from a user experience point of view, but enables us to validate the behavior.