-
Notifications
You must be signed in to change notification settings - Fork 92
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: Extract normalized dist as metric [INGEST-335] #1158
Conversation
if erase { | ||
*dist = None; | ||
} | ||
} |
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.
Added the erase flag here because I cannot set dist
while checking its contents. I don't like it though - any suggestions welcome.
Co-authored-by: Markus Unterwaditzer <markus-honeypot@unterwaditzer.net>
relay-general/src/store/normalize.rs
Outdated
|
||
#[test] | ||
fn test_normalize_dist_empty() { | ||
let mut dist = Option::Some("".to_owned()); |
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.
my prev code suggestion goes for all testcases here fwiw
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.
Right, I did the lazy thing and pressed "Commit suggestion". Will change other instances as well.
Apply normalization to
event.dist
and add as tag to every transaction metric.There is no further normalization in sentry python code as far as I can see: The dist is accepted in event_manager unaltered:
https://github.com/getsentry/sentry/blob/44d7aa211d67b64884230d833f6e485a96357ad6/src/sentry/event_manager.py#L674