[RAM] Add query conditional action filter to Security Solution UI #154680
Conversation
Zacqary
added
Team:ResponseOps
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
|
Pinging @elastic/response-ops (Team:ResponseOps) |
There was a problem hiding this comment.
The code looks good, I saw a few inconsistencies between this query filter and the 1 in discovery:
When trying to use DSLs:
Discovery:
New alert filter query:
The new alert filter query also seems to lack spacing, and the left button seems to be smaller, just small UI things.
I will test the filter with Xavier after he shows me how to.
|
The Discover plugin seems to be processing the query DSL and adding it to the filter's It's also unclear where the Discover plugin is actually implementing the @elastic/kibana-data-discovery please advise? What do we need to be passing into the |
|
Hey! This is a really cool feature! Tested it out and am seeing some strange behaviors. Let me know if you need more details from me but I noticed the following:
154680.movI did see some React errors like |
|
@yctercero, I think everything will start to work when @e40pud merged his PR. |
@yctercero |
Hi @spong |
Thanks @ersin-erdal! I'll test again later today. That said, since the rule query was the same as the alert query ( If this is still an issue when I test later I'll provide detailed reproduction steps 👍 |
It looks like EuiDatePicker doesn't allow us to use 24:00, replaces it with 00:00. I changed default end time to 23:59 to avoid confusion. |
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @ersin-erdal |
There was a problem hiding this comment.
Checked out, tested locally, and code reviewed relevant detection-rules area files -- LGTM! 👍 🎉
All previous issues I saw appear to be resolved @ersin-erdal -- I was able to receive notifications with each new setting, both configured together and independently. 🚀
Only remaining nits I see are some styling issues with the KQL bar -- filter options button seems squished compared to the add filter button (see over diff in image), and as I think someone else mentioned the filter pills need some margin and are being stacked vertically instead of horizontally added:
Main KQL bar from Definition step:
Lastly, I have one question about the 23:59 fix w/ the ending time window:
Does this leave a 1min gap (23:59:00 -> 00:00:00) that an action wouldn't fire, or do you round up somewhere downstream to cover the full 24hrs?
spong
added
needs_docs
Feature:Alerting/RuleActions
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
There will be another PR for Conditonal Actions for For |
Sounds good 👍
Thank you for clarifying the behavior here. 🙂 From a UX perspective, I think the UI should default the time-range to the full day, and users can modify from there. So if it's |
…astic#154680) ## Summary Closes elastic#152611 <img width="788" alt="Screenshot 2023-04-10 at 2 09 29 PM" src="https://user-images.githubusercontent.com/1445834/230977273-9d00aa3d-af2c-4dff-80ef-3034213df90d.png"> ### 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] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
Fixes: #156878 As discussed in the issue just turning `if alert is generated within timeframe` on should not change the filter scope. And the UI should not confuse the users by setting some days as default. And as discussed in #154680, default hours filter should cover the whole day. Therefore, this PR sets default alerts filter options as: ``` { days: [], hours: { start: '00:00', end: '00:00', }, }; ``` empty days array maps to all weekdays `[1,2,3,4,5,6,7]` and hours `00:00 -> 00:00` maps to `00:00 -> 24:00`
…#156913) Fixes: elastic#156878 As discussed in the issue just turning `if alert is generated within timeframe` on should not change the filter scope. And the UI should not confuse the users by setting some days as default. And as discussed in elastic#154680, default hours filter should cover the whole day. Therefore, this PR sets default alerts filter options as: ``` { days: [], hours: { start: '00:00', end: '00:00', }, }; ``` empty days array maps to all weekdays `[1,2,3,4,5,6,7]` and hours `00:00 -> 00:00` maps to `00:00 -> 24:00` (cherry picked from commit d157389) # Conflicts: # x-pack/plugins/rule_registry/server/utils/create_get_summarized_alerts_fn.ts
…156913) (#157211) # Backport This will backport the following commits from `main` to `8.8`: - [Not selecting any days in alerts filter maps to all weekdays (#156913)](#156913) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Ersin Erdal","email":"92688503+ersin-erdal@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-05-09T15:29:14Z","message":"Not selecting any days in alerts filter maps to all weekdays (#156913)\n\nFixes: #156878\r\n\r\nAs discussed in the issue just turning `if alert is generated within\r\ntimeframe` on should not change the filter scope.\r\nAnd the UI should not confuse the users by setting some days as default.\r\n\r\nAnd as discussed in #154680, default hours filter should cover the whole\r\nday.\r\n\r\nTherefore, this PR sets default alerts filter options as:\r\n```\r\n {\r\n days: [],\r\n hours: {\r\n start: '00:00',\r\n end: '00:00',\r\n },\r\n };\r\n```\r\n\r\nempty days array maps to all weekdays `[1,2,3,4,5,6,7]`\r\nand hours `00:00 -> 00:00` maps to `00:00 -> 24:00`","sha":"d15738989a6544c9b939cde5afc3533c9850d7d4","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:ResponseOps","v8.8.0","v8.9.0"],"number":156913,"url":"#156913 selecting any days in alerts filter maps to all weekdays (#156913)\n\nFixes: #156878\r\n\r\nAs discussed in the issue just turning `if alert is generated within\r\ntimeframe` on should not change the filter scope.\r\nAnd the UI should not confuse the users by setting some days as default.\r\n\r\nAnd as discussed in #154680, default hours filter should cover the whole\r\nday.\r\n\r\nTherefore, this PR sets default alerts filter options as:\r\n```\r\n {\r\n days: [],\r\n hours: {\r\n start: '00:00',\r\n end: '00:00',\r\n },\r\n };\r\n```\r\n\r\nempty days array maps to all weekdays `[1,2,3,4,5,6,7]`\r\nand hours `00:00 -> 00:00` maps to `00:00 -> 24:00`","sha":"d15738989a6544c9b939cde5afc3533c9850d7d4"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"#156913 selecting any days in alerts filter maps to all weekdays (#156913)\n\nFixes: #156878\r\n\r\nAs discussed in the issue just turning `if alert is generated within\r\ntimeframe` on should not change the filter scope.\r\nAnd the UI should not confuse the users by setting some days as default.\r\n\r\nAnd as discussed in #154680, default hours filter should cover the whole\r\nday.\r\n\r\nTherefore, this PR sets default alerts filter options as:\r\n```\r\n {\r\n days: [],\r\n hours: {\r\n start: '00:00',\r\n end: '00:00',\r\n },\r\n };\r\n```\r\n\r\nempty days array maps to all weekdays `[1,2,3,4,5,6,7]`\r\nand hours `00:00 -> 00:00` maps to `00:00 -> 24:00`","sha":"d15738989a6544c9b939cde5afc3533c9850d7d4"}}]}] BACKPORT--> --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Summary
Closes #152611
Fixes #155146 too
Checklist
Delete any items that are not applicable to this PR.