[Security Solution][Detections] Rule Execution Log Feedback and Fixes #129003
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spong
added
bug
… cardinality overflow count for status filters
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @spong |
xcrzx
approved these changes
Apr 5, 2022
There was a problem hiding this comment.
Thanks for these fixes and improvements, @spong! The io-ts-based schema validation with default values looks cool 👍
I verified the changes locally; everything works as expected. Code changes LGTM. Added a nit on types and a question to discuss the execution events API params.
x-pack/plugins/security_solution/common/detection_engine/schemas/common/rule_monitoring.ts
Show resolved
Hide resolved
...ecurity_solution/common/detection_engine/schemas/request/get_rule_execution_events_schema.ts
Show resolved
Hide resolved
kibanamachine
pushed a commit
that referenced
this pull request
Apr 5, 2022
…#129003) ## Summary Addresses feedback and fixes identified in #126215 Feedback addressed includes: * Adds route validation via io-ts decode and schema tests * Fixed styling of max execution events error by wrapping text (#129321) * Fixed types within `view alerts for execution` action onClick * Caps auto-refresh minimum to `1min` (#129332) * Adds cardinality aggs to initial `execution_uuid` query to properly report total counts when filtering by status * Disabled `View Alerts from Execution` action as current UX was too cumbersome with erasing users existing filters --- Additional follow-ups for another PR: - [ ] UI Unit tests - [ ] Finalize API Integration tests for gap remediation events - [ ] Persist table state (DatePicker/StatusFilter/SortField/SortOrder/Pagination) when navigating to other tabs on the same page - [ ] Update global DatePicker to daterange of execution for `view alerts for execution` action (and clear all other filters) - [ ] Support `disabled rule` platform error #126215 (comment) - [ ] Verify StatusFilter issue #126215 (comment) --- ### 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 - [X] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [X] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) (cherry picked from commit bc413c6)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Apr 5, 2022
…#129003) (#129518) ## Summary Addresses feedback and fixes identified in #126215 Feedback addressed includes: * Adds route validation via io-ts decode and schema tests * Fixed styling of max execution events error by wrapping text (#129321) * Fixed types within `view alerts for execution` action onClick * Caps auto-refresh minimum to `1min` (#129332) * Adds cardinality aggs to initial `execution_uuid` query to properly report total counts when filtering by status * Disabled `View Alerts from Execution` action as current UX was too cumbersome with erasing users existing filters --- Additional follow-ups for another PR: - [ ] UI Unit tests - [ ] Finalize API Integration tests for gap remediation events - [ ] Persist table state (DatePicker/StatusFilter/SortField/SortOrder/Pagination) when navigating to other tabs on the same page - [ ] Update global DatePicker to daterange of execution for `view alerts for execution` action (and clear all other filters) - [ ] Support `disabled rule` platform error #126215 (comment) - [ ] Verify StatusFilter issue #126215 (comment) --- ### 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 - [X] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [X] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) (cherry picked from commit bc413c6) Co-authored-by: Garrett Spong <spong@users.noreply.github.com>
4 tasks
spong
added a commit
that referenced
this pull request
May 4, 2022
… Part Deux (#130072) ## Summary Addresses feedback and fixes identified in #126215 & #129003 ##### Feedback addressed includes: * Adds toast for restoring global query state after performing `view alerts for execution` action <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164511565-b77d3dc8-a8b5-4927-a947-54966a58c74f.gif" /> </p> * Updates global SuperDatePicker to daterange of execution (+/- day) for `view alerts for execution` action (and clear all other filters) * See above gif * Remove redundant `RuleExecutionStatusType` (#129003 (comment)) * Persist table state (DatePicker/StatusFilter/SortField/SortOrder/Pagination) when navigating to other tabs on the same page <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164512498-59416601-d967-4a27-b0cc-0715cc0662c0.gif" /> </p> * Fix duration hours bug (`7 hours (25033167ms)` as `06:417:13:000`) <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164511478-bf0bb6d8-d8b7-4c86-8fbd-b60090f00555.png" /> </p> * Support `disabled rule` platform error (#126215 (comment)) * Updated `getAggregateExecutionEvents` to fallback to platform status from `event.outcome` if `security_status` is empty, and also falls back to `error.message` is `security_message` is empty. This also now queries for corresponding `event.outcome` if filter is provided so that platform-only events can still be displayed when filtering. <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164510056-1e0bce86-8360-4d46-b591-2041457e3244.png" /> </p> * Verify StatusFilter issue #126215 (comment) * Unable to reproduce, I believe the query updates around first querying for status may've fixed this? * Provide helpful defaults for `to`/`from` and support datemath strings again (#129003 (comment)) * Created enhancement for this here: #131095 * Adds UI Unit tests for RuleExecutionLog Table * Finalize API Integration tests for gap remediation events * Test methods developed for injecting arbitrary execution events while still working with event-log RBAC. See last [API integration test](https://github.com/elastic/kibana/blob/22cc0c8dbd2a1300675caf4c6d471d211ed44858/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/get_rule_execution_events.ts#L121-L166) for technique. This can further be used to inject many execution events and expand tests around pagination, sorting, filters, etc. * Fixes `gap_duration`'s of `1-499`ms showing up as `-` instead of `0` * Fixes restore filters action to restore either absolute or relative datepicker as it originally was * Resolves #130946 * Adds `min-height` to tab container * Removes scroll-pane from ExceptionsViewer to match Alerts/Execution Log --- ##### Remaining follow-ups: None! 🎉 ### 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 - [X] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [X] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
spong
added a commit
to spong/kibana
that referenced
this pull request
May 4, 2022
… Part Deux (elastic#130072) ## Summary Addresses feedback and fixes identified in elastic#126215 & elastic#129003 ##### Feedback addressed includes: * Adds toast for restoring global query state after performing `view alerts for execution` action <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164511565-b77d3dc8-a8b5-4927-a947-54966a58c74f.gif" /> </p> * Updates global SuperDatePicker to daterange of execution (+/- day) for `view alerts for execution` action (and clear all other filters) * See above gif * Remove redundant `RuleExecutionStatusType` (elastic#129003 (comment)) * Persist table state (DatePicker/StatusFilter/SortField/SortOrder/Pagination) when navigating to other tabs on the same page <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164512498-59416601-d967-4a27-b0cc-0715cc0662c0.gif" /> </p> * Fix duration hours bug (`7 hours (25033167ms)` as `06:417:13:000`) <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164511478-bf0bb6d8-d8b7-4c86-8fbd-b60090f00555.png" /> </p> * Support `disabled rule` platform error (elastic#126215 (comment)) * Updated `getAggregateExecutionEvents` to fallback to platform status from `event.outcome` if `security_status` is empty, and also falls back to `error.message` is `security_message` is empty. This also now queries for corresponding `event.outcome` if filter is provided so that platform-only events can still be displayed when filtering. <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164510056-1e0bce86-8360-4d46-b591-2041457e3244.png" /> </p> * Verify StatusFilter issue elastic#126215 (comment) * Unable to reproduce, I believe the query updates around first querying for status may've fixed this? * Provide helpful defaults for `to`/`from` and support datemath strings again (elastic#129003 (comment)) * Created enhancement for this here: elastic#131095 * Adds UI Unit tests for RuleExecutionLog Table * Finalize API Integration tests for gap remediation events * Test methods developed for injecting arbitrary execution events while still working with event-log RBAC. See last [API integration test](https://github.com/elastic/kibana/blob/22cc0c8dbd2a1300675caf4c6d471d211ed44858/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/get_rule_execution_events.ts#L121-L166) for technique. This can further be used to inject many execution events and expand tests around pagination, sorting, filters, etc. * Fixes `gap_duration`'s of `1-499`ms showing up as `-` instead of `0` * Fixes restore filters action to restore either absolute or relative datepicker as it originally was * Resolves elastic#130946 * Adds `min-height` to tab container * Removes scroll-pane from ExceptionsViewer to match Alerts/Execution Log --- ##### Remaining follow-ups: None! 🎉 ### 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 - [X] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [X] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) (cherry picked from commit 683463e) # Conflicts: # x-pack/plugins/security_solution/cypress/tasks/alerts.ts # x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/details/execution_log_table/execution_log_columns.tsx # x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/event_log/event_log_reader.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/event_log/get_execution_event_aggregation/index.test.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/event_log/get_execution_event_aggregation/index.ts # x-pack/test/detection_engine_api_integration/utils/index_event_log_execution_events.ts
spong
added a commit
that referenced
this pull request
May 5, 2022
… Fixes Part Deux (#130072) (#131574) * [Security Solution][Detections] Rule Execution Log Feedback and Fixes Part Deux (#130072) ## Summary Addresses feedback and fixes identified in #126215 & #129003 ##### Feedback addressed includes: * Adds toast for restoring global query state after performing `view alerts for execution` action <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164511565-b77d3dc8-a8b5-4927-a947-54966a58c74f.gif" /> </p> * Updates global SuperDatePicker to daterange of execution (+/- day) for `view alerts for execution` action (and clear all other filters) * See above gif * Remove redundant `RuleExecutionStatusType` (#129003 (comment)) * Persist table state (DatePicker/StatusFilter/SortField/SortOrder/Pagination) when navigating to other tabs on the same page <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164512498-59416601-d967-4a27-b0cc-0715cc0662c0.gif" /> </p> * Fix duration hours bug (`7 hours (25033167ms)` as `06:417:13:000`) <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164511478-bf0bb6d8-d8b7-4c86-8fbd-b60090f00555.png" /> </p> * Support `disabled rule` platform error (#126215 (comment)) * Updated `getAggregateExecutionEvents` to fallback to platform status from `event.outcome` if `security_status` is empty, and also falls back to `error.message` is `security_message` is empty. This also now queries for corresponding `event.outcome` if filter is provided so that platform-only events can still be displayed when filtering. <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164510056-1e0bce86-8360-4d46-b591-2041457e3244.png" /> </p> * Verify StatusFilter issue #126215 (comment) * Unable to reproduce, I believe the query updates around first querying for status may've fixed this? * Provide helpful defaults for `to`/`from` and support datemath strings again (#129003 (comment)) * Created enhancement for this here: #131095 * Adds UI Unit tests for RuleExecutionLog Table * Finalize API Integration tests for gap remediation events * Test methods developed for injecting arbitrary execution events while still working with event-log RBAC. See last [API integration test](https://github.com/elastic/kibana/blob/22cc0c8dbd2a1300675caf4c6d471d211ed44858/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/get_rule_execution_events.ts#L121-L166) for technique. This can further be used to inject many execution events and expand tests around pagination, sorting, filters, etc. * Fixes `gap_duration`'s of `1-499`ms showing up as `-` instead of `0` * Fixes restore filters action to restore either absolute or relative datepicker as it originally was * Resolves #130946 * Adds `min-height` to tab container * Removes scroll-pane from ExceptionsViewer to match Alerts/Execution Log --- ##### Remaining follow-ups: None! 🎉 ### 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 - [X] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [X] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) (cherry picked from commit 683463e) # Conflicts: # x-pack/plugins/security_solution/cypress/tasks/alerts.ts # x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/details/execution_log_table/execution_log_columns.tsx # x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/event_log/event_log_reader.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/event_log/get_execution_event_aggregation/index.test.ts # x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/event_log/get_execution_event_aggregation/index.ts # x-pack/test/detection_engine_api_integration/utils/index_event_log_execution_events.ts * Fixing import
kertal
pushed a commit
to kertal/kibana
that referenced
this pull request
May 24, 2022
… Part Deux (elastic#130072) ## Summary Addresses feedback and fixes identified in elastic#126215 & elastic#129003 ##### Feedback addressed includes: * Adds toast for restoring global query state after performing `view alerts for execution` action <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164511565-b77d3dc8-a8b5-4927-a947-54966a58c74f.gif" /> </p> * Updates global SuperDatePicker to daterange of execution (+/- day) for `view alerts for execution` action (and clear all other filters) * See above gif * Remove redundant `RuleExecutionStatusType` (elastic#129003 (comment)) * Persist table state (DatePicker/StatusFilter/SortField/SortOrder/Pagination) when navigating to other tabs on the same page <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164512498-59416601-d967-4a27-b0cc-0715cc0662c0.gif" /> </p> * Fix duration hours bug (`7 hours (25033167ms)` as `06:417:13:000`) <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164511478-bf0bb6d8-d8b7-4c86-8fbd-b60090f00555.png" /> </p> * Support `disabled rule` platform error (elastic#126215 (comment)) * Updated `getAggregateExecutionEvents` to fallback to platform status from `event.outcome` if `security_status` is empty, and also falls back to `error.message` is `security_message` is empty. This also now queries for corresponding `event.outcome` if filter is provided so that platform-only events can still be displayed when filtering. <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/164510056-1e0bce86-8360-4d46-b591-2041457e3244.png" /> </p> * Verify StatusFilter issue elastic#126215 (comment) * Unable to reproduce, I believe the query updates around first querying for status may've fixed this? * Provide helpful defaults for `to`/`from` and support datemath strings again (elastic#129003 (comment)) * Created enhancement for this here: elastic#131095 * Adds UI Unit tests for RuleExecutionLog Table * Finalize API Integration tests for gap remediation events * Test methods developed for injecting arbitrary execution events while still working with event-log RBAC. See last [API integration test](https://github.com/elastic/kibana/blob/22cc0c8dbd2a1300675caf4c6d471d211ed44858/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/get_rule_execution_events.ts#L121-L166) for technique. This can further be used to inject many execution events and expand tests around pagination, sorting, filters, etc. * Fixes `gap_duration`'s of `1-499`ms showing up as `-` instead of `0` * Fixes restore filters action to restore either absolute or relative datepicker as it originally was * Resolves elastic#130946 * Adds `min-height` to tab container * Removes scroll-pane from ExceptionsViewer to match Alerts/Execution Log --- ##### Remaining follow-ups: None! 🎉 ### 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 - [X] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [X] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Addresses feedback and fixes identified in #126215
Feedback addressed includes:
view alerts for executionaction onClick1min([Security Solution]Refresh every parameter can be set in negative under Rule Execution Log #129332)execution_uuidquery to properly report total counts when filtering by statusView Alerts from Executionaction as current UX was too cumbersome with erasing users existing filtersAdditional follow-ups for another PR (#130072):
view alerts for executionaction (and clear all other filters)disabled ruleplatform error [Security Solution][Detections] Adds rule execution log table #126215 (comment)to/fromand support datemath strings again ([Security Solution][Detections] Rule Execution Log Feedback and Fixes #129003 (comment))RuleExecutionStatusType([Security Solution][Detections] Rule Execution Log Feedback and Fixes #129003 (comment))Checklist
Delete any items that are not applicable to this PR.