-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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(alerts): use extrapolated data during creation #53428
feat(alerts): use extrapolated data during creation #53428
Conversation
5d8d338
to
1e6981f
Compare
Co-authored-by: Priscila Oliveira <priscila.oliveira@sentry.io>
Co-authored-by: Priscila Oliveira <priscila.oliveira@sentry.io>
shall we create unit/integration tests for this change? |
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.
left one last comment but it looks good to me! :)
</OnDemandMetricRequest> | ||
); | ||
|
||
expect(doEventsRequest).toHaveBeenCalledWith( |
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 we need to check for the number of calls here too? expect(doEventsRequest).toHaveBeenCalledTimes(2);
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.
Yep, we can, added in d36fc34
Closes: Support chart queries for on-demand metrics#52508
Adds A separate request flow for on-deamnd metric alerts that queries
event-stats
to check if the on-demand metric matching filters is already present. If it is displays that Timeseries data. If not it repeats the request toevent-stats
falling back to indexed data, and extrapolates it using the dynamic sampling rate.