-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Improve time window handling and validation #161978
Improve time window handling and validation #161978
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
e05f730
to
b7d0fdd
Compare
b7d0fdd
to
cfb087b
Compare
Pinging @elastic/actionable-observability (Team: Actionable Observability) |
case 'w': | ||
return 'weeks'; | ||
return 'isoWeek'; |
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.
👍
💔 Build FailedFailed CI StepsMetrics [docs]
History
To update your PR or re-run it, just comment with: cc @kdelemme |
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
Summary
This PR enforces the following time window for SLOs:
We enforce these durations so we can use a finite number of summary transforms.
I've also fixed an issue with calendar aligned time window when calculating the date range for a weekly one: relying on
startOf('isoWeek')
which is Monday, as for Elasticsearch.I've cleaned up a bunch of unnecessary tests related to no more valid time window duration as well.