-
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
[Security Solution][Detections] Reading last 5 failures from Event Log v1 - raw implementation #115574
[Security Solution][Detections] Reading last 5 failures from Event Log v1 - raw implementation #115574
Conversation
dff0590
to
66be0c7
Compare
💔 Build Failed
Failed CI StepsTest FailuresKibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/ml/jobs/categorization_field_examples·ts.apis Machine Learning jobs Categorization example endpoint - partially valid, more than 75% are nullStandard Out
Stack Trace
Kibana Pipeline / general / Creates and activates a new custom rule with override option.Detection rules, override Creates and activates a new custom rule with override optionStack Trace
Kibana Pipeline / general / displays the data provider action menu when Enter is pressed.timeline data providers displays the data provider action menu when Enter is pressedStack Trace
Metrics [docs]Page load bundle
History
To update your PR or re-run it, just comment with: cc @banderror |
There's a failed Cypress test for Timelines and it's skipped in #115738. I will rebase when the skip is merged. |
66be0c7
to
7aaa316
Compare
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
7aaa316
to
b9a4e4c
Compare
Reviewed stages 2-4 in description and the path forward proposed sounds good to me. 👍 I do wonder with @xcrzx's work this cycle to allow generic fields on solution Rule SO's has any play here, and if we might be able to skip the single status SO approach in favor of just storing any necessary fields on our Security Rule SO directly. I'm probably being too ambitious here (😅), especially with the flexibility a custom SO will give with regards to quickly implementing features in the next few minor releases. That said, it'll still be a join and so we'd still be limited on features like sorting, and of course potential perf considerations (no worse than today at least). I think that was just a long-winded "LGTM 👍" and me convincing myself the near-term flexibility of another Status SO > managing the risk/timing around getting reaching consensus and implementing generic fields on rules... 🙂 |
b9a4e4c
to
a79477f
Compare
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 implementing these changes, Georgii 🚀 I love to see how ExecLog usages become cleaner with the new, improved interface—looking forward to seeing further refinements in that direction!
I checked out the PR locally and tested it. All affected API routes work as expected with both event-log-based implementation, and SO-based 👍 Added some comments to discuss before we can merge this in. Not everything requires immediate action. We could address some of them in follow-up PRs.
...ecurity_solution/server/lib/detection_engine/routes/rules/internal_find_rule_status_route.ts
Outdated
Show resolved
Hide resolved
...erver/lib/detection_engine/rule_execution_log/saved_objects_adapter/saved_objects_adapter.ts
Outdated
Show resolved
Hide resolved
...erver/lib/detection_engine/rule_execution_log/saved_objects_adapter/saved_objects_adapter.ts
Outdated
Show resolved
Hide resolved
...olution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_client.ts
Outdated
Show resolved
Hide resolved
...olution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_client.ts
Outdated
Show resolved
Hide resolved
...olution/server/lib/detection_engine/rule_execution_log/event_log_adapter/event_log_client.ts
Show resolved
Hide resolved
a854593
to
5baac92
Compare
@spong I think eventually we definitely need to store this data in the rule object itself (the simplest way to enable sorting etc), but it's difficult to estimate when this could happen because IMHO this is a much more complicated stuff + cross-team dependencies + an RFC. Meanwhile, we could work on improving the data model and the HTTP API, so that when the mechanism of storing this stuff in the rule is ready, we'll already have a working "blueprint" for this data - the new SO, which we ideally will just transfer 1:1 to the rule. So yeah, I hope that the new custom SO will give us some flexibility + ability to clean up the code for the time being + an opportunity to introduce breaking changes in the API in 8.0.
Unfortunately, yeah, so we need this data in the rule itself. |
c1f182a
to
59084ed
Compare
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👍
67d57e1
to
8f32a94
Compare
8f32a94
to
dc16bb8
Compare
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @banderror |
…g v1 - raw implementation (elastic#115574) **Ticket:** elastic#106469, elastic#101013 ## Summary TL;DR: New internal endpoint for reading data from Event Log (raw version), legacy status SO under the hood. With this PR we now read the Failure History (last 5 failures) on the Rule Details page from Event Log. We continue getting the Current Status from the legacy `siem-detection-engine-rule-status` saved objects. Rule Management page also gets data from the legacy saved objects. - [x] Deprecate existing methods for reading data in `IRuleExecutionLogClient`: `.find()` and `.findBulk()` - [x] Introduce new methods for reading data in IRuleExecutionLogClient: - for reading last N execution events for 1 rule from event log - for reading current status and metrics for 1 rule from legacy status SOs - for reading current statuses and metrics for N rules from legacy status SOs - [x] New methods should return data in the legacy status SO format. - [x] Update all the existing endpoints that depend on `IRuleExecutionLogClient` to use the new methods. - [x] Implement a new internal endpoint for fetching current status of the rule execution and execution events from Event Log for a given rule. - [x] The API of the new endpoint should be the same as `rules/_find_statuses` to minimise changes in the app. - [x] Use the new endpoint on the Rule Details page. ## Near-term plan for technical implementation of the Rule Execution Log (elastic#101013) **Stage 1. Reading last 5 failures from Event Log v1 - raw implementation** - ✔️ done in this PR TL;DR: New internal endpoint for reading data from Event Log (raw version), legacy status SO under the hood. - Deprecate existing methods for reading data in `IRuleExecutionLogClient`: `.find()` and `.findBulk()` - Introduce new methods for reading data in IRuleExecutionLogClient: - for reading last N execution events for 1 rule from event log - for reading current status and metrics for 1 rule from legacy status SOs - for reading current statuses and metrics for N rules from legacy status SOs - New methods should return data in the legacy status SO format. - Update all the existing endpoints that depend on `IRuleExecutionLogClient` to use the new methods. - Implement a new internal endpoint for fetching current status of the rule execution and execution events from Event Log for a given rule. - The API of the new endpoint should be the same as `rules/_find_statuses` to minimise changes in the app. - Use the new endpoint on the Rule Details page. **Stage 2: Reading last 5 failures from Event Log v2 - clean implementation** TL;DR: Clean HTTP API, legacy Rule Status SO under the hood. 🚨🚨🚨 Possible breaking changes in Detections API 🚨🚨🚨 - Design a new data model for the Current Rule Execution Info (the TO-BE new SO type and later the TO-BE data in the rule object itself). - Design a new data model for the Rule Execution Event (read model to be used on the Rule Details page) - Think over changes in `IRuleExecutionLogClient` to support the new data model. - Think over changes in all the endpoints that return any data related to rule monitoring (statuses, metrics, etc). Make sure to check our docs to identify what's documented there regarding rule monitoring. - Update `IRuleExecutionLogClient` to return data in the new format. - Update all the endpoints (including the raw new one) to return data in the new format. - Update Rule Details page to consume data in the new format. - Update Rule Management page to consume data in the new format. **Stage 3: Reading last 5 failures from Event Log v3 - new SO** TL;DR: Clean HTTP API, new Rule Execution Info SO under the hood. - Implement a new SO type for storing the current rule execution info. Relation type: 1 rule - 1 current execution info. - Swap the legacy SO with the new SO in the implementation of `IRuleExecutionLogClient`. **Stage 4: Cleanup and misc** - Revisit the problem of deterministic ordering ([comment](elastic#115574 (comment))) - Remove rule execution log's glue code: adapters, feature switch. - Remove the legacy rule status SO. - Mark the legacy rule status SO as deleted in Kibana Core. - Encapsulate the current space id in the instance of IRuleExecutionLogClient. Remove it from parameters of its methods. - Introduce a Rule Execution Logger scoped to a rule instance. For use in rule executors. - Add test coverage. ### Checklist Delete any items that are not applicable to this PR. - [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
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
…g v1 - raw implementation (#115574) (#116947) **Ticket:** #106469, #101013 ## Summary TL;DR: New internal endpoint for reading data from Event Log (raw version), legacy status SO under the hood. With this PR we now read the Failure History (last 5 failures) on the Rule Details page from Event Log. We continue getting the Current Status from the legacy `siem-detection-engine-rule-status` saved objects. Rule Management page also gets data from the legacy saved objects. - [x] Deprecate existing methods for reading data in `IRuleExecutionLogClient`: `.find()` and `.findBulk()` - [x] Introduce new methods for reading data in IRuleExecutionLogClient: - for reading last N execution events for 1 rule from event log - for reading current status and metrics for 1 rule from legacy status SOs - for reading current statuses and metrics for N rules from legacy status SOs - [x] New methods should return data in the legacy status SO format. - [x] Update all the existing endpoints that depend on `IRuleExecutionLogClient` to use the new methods. - [x] Implement a new internal endpoint for fetching current status of the rule execution and execution events from Event Log for a given rule. - [x] The API of the new endpoint should be the same as `rules/_find_statuses` to minimise changes in the app. - [x] Use the new endpoint on the Rule Details page. ## Near-term plan for technical implementation of the Rule Execution Log (#101013) **Stage 1. Reading last 5 failures from Event Log v1 - raw implementation** - ✔️ done in this PR TL;DR: New internal endpoint for reading data from Event Log (raw version), legacy status SO under the hood. - Deprecate existing methods for reading data in `IRuleExecutionLogClient`: `.find()` and `.findBulk()` - Introduce new methods for reading data in IRuleExecutionLogClient: - for reading last N execution events for 1 rule from event log - for reading current status and metrics for 1 rule from legacy status SOs - for reading current statuses and metrics for N rules from legacy status SOs - New methods should return data in the legacy status SO format. - Update all the existing endpoints that depend on `IRuleExecutionLogClient` to use the new methods. - Implement a new internal endpoint for fetching current status of the rule execution and execution events from Event Log for a given rule. - The API of the new endpoint should be the same as `rules/_find_statuses` to minimise changes in the app. - Use the new endpoint on the Rule Details page. **Stage 2: Reading last 5 failures from Event Log v2 - clean implementation** TL;DR: Clean HTTP API, legacy Rule Status SO under the hood. 🚨🚨🚨 Possible breaking changes in Detections API 🚨🚨🚨 - Design a new data model for the Current Rule Execution Info (the TO-BE new SO type and later the TO-BE data in the rule object itself). - Design a new data model for the Rule Execution Event (read model to be used on the Rule Details page) - Think over changes in `IRuleExecutionLogClient` to support the new data model. - Think over changes in all the endpoints that return any data related to rule monitoring (statuses, metrics, etc). Make sure to check our docs to identify what's documented there regarding rule monitoring. - Update `IRuleExecutionLogClient` to return data in the new format. - Update all the endpoints (including the raw new one) to return data in the new format. - Update Rule Details page to consume data in the new format. - Update Rule Management page to consume data in the new format. **Stage 3: Reading last 5 failures from Event Log v3 - new SO** TL;DR: Clean HTTP API, new Rule Execution Info SO under the hood. - Implement a new SO type for storing the current rule execution info. Relation type: 1 rule - 1 current execution info. - Swap the legacy SO with the new SO in the implementation of `IRuleExecutionLogClient`. **Stage 4: Cleanup and misc** - Revisit the problem of deterministic ordering ([comment](#115574 (comment))) - Remove rule execution log's glue code: adapters, feature switch. - Remove the legacy rule status SO. - Mark the legacy rule status SO as deleted in Kibana Core. - Encapsulate the current space id in the instance of IRuleExecutionLogClient. Remove it from parameters of its methods. - Introduce a Rule Execution Logger scoped to a rule instance. For use in rule executors. - Add test coverage. ### Checklist Delete any items that are not applicable to this PR. - [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: Georgii Gorbachev <[email protected]>
Ticket: #106469, #101013
Summary
TL;DR: New internal endpoint for reading data from Event Log (raw version), legacy status SO under the hood.
With this PR we now read the Failure History (last 5 failures) on the Rule Details page from Event Log. We continue getting the Current Status from the legacy
siem-detection-engine-rule-status
saved objects. Rule Management page also gets data from the legacy saved objects.IRuleExecutionLogClient
:.find()
and.findBulk()
IRuleExecutionLogClient
to use the new methods.rules/_find_statuses
to minimise changes in the app.Near-term plan for technical implementation of the Rule Execution Log (#101013)
Stage 1. Reading last 5 failures from Event Log v1 - raw implementation - ✔️ done in this PR
TL;DR: New internal endpoint for reading data from Event Log (raw version), legacy status SO under the hood.
IRuleExecutionLogClient
:.find()
and.findBulk()
IRuleExecutionLogClient
to use the new methods.rules/_find_statuses
to minimise changes in the app.Stage 2: Reading last 5 failures from Event Log v2 - clean implementation
TL;DR: Clean HTTP API, legacy Rule Status SO under the hood.
🚨🚨🚨 Possible breaking changes in Detections API 🚨🚨🚨
IRuleExecutionLogClient
to support the new data model.IRuleExecutionLogClient
to return data in the new format.Stage 3: Reading last 5 failures from Event Log v3 - new SO
TL;DR: Clean HTTP API, new Rule Execution Info SO under the hood.
IRuleExecutionLogClient
.Stage 4: Cleanup and misc
Checklist
Delete any items that are not applicable to this PR.