-
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
[RAM] Mark disabled alerts as Untracked in both Stack and o11y #164788
[RAM] Mark disabled alerts as Untracked in both Stack and o11y #164788
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
x-pack/plugins/rule_registry/server/utils/create_lifecycle_executor.ts
Outdated
Show resolved
Hide resolved
Pinging @elastic/response-ops (Team:ResponseOps) |
Pinging @elastic/actionable-observability (Team: Actionable Observability) |
…sabled-alerts # Conflicts: # x-pack/plugins/alerting/server/rule_type_registry.test.ts # x-pack/plugins/alerting/server/rule_type_registry.ts
x-pack/test/detection_engine_api_integration/security_and_spaces/rule_execution_logic/query.ts
Outdated
Show resolved
Hide resolved
@@ -151,7 +151,7 @@ export default function createDisableRuleTests({ getService }: FtrProviderContex | |||
savedObjects: [ | |||
{ type: 'alert', id: ruleId, rel: 'primary', type_id: 'test.cumulative-firing' }, | |||
], | |||
message: "instance 'instance-0' has recovered due to the rule was disabled", | |||
message: "instance 'instance-0' has been untracked because the rule was disabled", |
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.
Would be nice to update this functional test (or add a new FT) that creates a rule with alerts, disables it and checks that the alert is marked as untracked
and then re-enables it and sees a new alert doc created.
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! Verified alerts are marked as untracked when rule is disabled and new alerts are created when alert is re-enabled. Left one question about a change to a detection rule test. Also left a comment about a FT but that can be added as a followup.
This reverts commit 29d3192.
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.
Pulled down and just checked all good in security alerts page - LGTM.
💔 Build FailedFailed CI StepsTest Failures
Metrics [docs]Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
History
To update your PR or re-run it, just comment with: |
## Summary Part of #164059 <img width="301" alt="Screenshot 2023-09-28 at 5 38 45 PM" src="https://github.com/elastic/kibana/assets/1445834/1b9ae224-7dad-43d7-a930-adf9458e1613"> <img width="486" alt="Screenshot 2023-09-28 at 5 38 11 PM" src="https://github.com/elastic/kibana/assets/1445834/82eeec3d-af2c-4257-b78e-99aea5a6b66f"> This PR: - Moves the `setAlertStatusToUntracked` function from the `AlertsClient` into the `AlertsService`. This function doesn't actually need any Rule IDs to do what it's supposed to do, only indices and Alert UUIDs. Therefore, we want to make it possible to use outside of a created `AlertsClient`, which requires a Rule to initialize. - Creates a versioned internal API to bulk untrack a given set of `alertUuids` present on `indices`. Both of these pieces of information are readily available from the ECS fields sent to the alert table component, from where this bulk action will be called. - Switches the `setAlertStatusToUntracked` query to look for alert UUIDs instead of alert instance IDs. #164788 dealt with untracking alerts that were bound to a single rule at a time, but this PR could be untracking alerts generated by many different rules at once. Multiple rules may generate the same alert instance ID names with different UUIDs, so using UUID increases the specificity and prevents us from untracking alert instances that the user didn't intend. - Adds a `bulkUpdateState` method to the task scheduler. #164788 modified the `bulkDisable` method to clear untracked alerts from task states, but this new method allows us to untrack a given set of alert instances without disabling the task that generated them. #### Why omit rule ID from this API? The rule ID is technically readily available from the alert table, but it becomes redundant when we already have immediate access to the alert document's index. #164788 used the rule ID to get the `ruleTypeId` and turn this into a corresponding index, which we don't have to do anymore. Furthermore, it helps to omit the rule ID from the `updateByQuery` request, because the user can easily select alerts that were generated by a wide variety of different rules, and untrack them all at once. We could include the rule ID in a separate `should` query, but this adds needless complexity to the query. We do need to know the rule ID after performing `updateByQuery`, because it corresponds to the task state we want to modify, but it's easier to retrieve this using the same query params provided. ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Jiawei Wu <jiawei.wu@cmd.com> Co-authored-by: Xavier Mouligneau <xavier.mouligneau@elastic.co>
Summary
Part of #164059
Implements the
Untracked
lifecycle status, and applies it to alerts when their corresponding rule is disabled.Checklist