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
[SLO] Add support for group by to SLO burn rate rule #163434
[SLO] Add support for group by to SLO burn rate rule #163434
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
…d-support-for-group-by-to-burn-rate-rule
…he SavedObjects client; added callout for group by rules
…d-support-for-group-by-to-burn-rate-rule
…d-support-for-group-by-to-burn-rate-rule
x-pack/plugins/observability/public/components/burn_rate_rule_editor/burn_rate_rule_editor.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/observability/public/hooks/slo/use_fetch_slo_definitions.ts
Show resolved
Hide resolved
Pinging @elastic/actionable-observability (Team: Actionable Observability) |
x-pack/plugins/observability/public/hooks/slo/use_fetch_active_alerts.ts
Show resolved
Hide resolved
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.
x-pack/plugins/observability/public/hooks/slo/use_fetch_active_alerts.ts
Outdated
Show resolved
Hide resolved
…d-support-for-group-by-to-burn-rate-rule
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.
Thanks for the refactoring, LGTM!
import { useKibana } from '../../utils/kibana_react'; | ||
import { sloKeys } from './query_key_factory'; | ||
|
||
type SLO = Pick<SLOResponse, 'id' | 'instanceId'>; | ||
|
||
export class ActiveAlerts { |
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.
👍🏻 Nice abstraction
…d-support-for-group-by-to-burn-rate-rule
…d-support-for-group-by-to-burn-rate-rule
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.
response ops changes lgtm
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
Summary
This PR fixes #163120 by adding support for group by's to the SLO Burn Rate rule. I refactored the rule to push all the executions down to Elasticsearch, it uses the same technique as the Metric Threshold rule. I also added a callout to the rule form to let the user know when they've selected an SLO with a group by so it's clear what the behavior will be.