Skip to content
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][Alerts] Format alerts for per-alert action context variables #155829

Merged

Conversation

e40pud
Copy link
Contributor

@e40pud e40pud commented Apr 26, 2023

Summary

Closes #155812

In #155384, detection rules were switched to support per-alert actions. When passing the context variable, it was suggested that we should be calling formatAlert to format the alert for notifications, however doing that causes some test failures because formatAlert is fairly heavyweight and bunch of tests were timing out.

Thanks to @marshallmain we have this much faster expandDottedObject that solves the issue with the very slow formatAlert.

@e40pud e40pud requested review from ymao1 and marshallmain April 26, 2023 07:35
@e40pud e40pud requested review from a team as code owners April 26, 2023 07:35
@e40pud e40pud self-assigned this Apr 26, 2023
@e40pud e40pud added the release_note:skip Skip the PR/issue when compiling release notes label Apr 26, 2023
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 9.1MB 9.1MB -315.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 17 19 +2
securitySolution 399 402 +3
total +5

Total ESLint disabled count

id before after diff
enterpriseSearch 18 20 +2
securitySolution 479 482 +3
total +5

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @e40pud

Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@marshallmain marshallmain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@e40pud e40pud merged commit 8f59720 into elastic:main Apr 26, 2023
@kibanamachine kibanamachine added v8.8.0 backport:skip This commit does not require backporting labels Apr 26, 2023
e40pud added a commit to e40pud/kibana that referenced this pull request Apr 27, 2023
…t variables (elastic#155829)

## Summary

Closes [elastic#155812](elastic#155812)

In elastic#155384, detection rules were
switched to support per-alert actions. When passing the context
variable, it was suggested that we should be calling formatAlert to
format the alert for notifications, however doing that causes some test
failures because formatAlert is fairly heavyweight and bunch of tests
were timing out.

Thanks to @marshallmain we have this much faster `expandDottedObject`
that solves the issue with the very slow `formatAlert`.

(cherry picked from commit 8f59720)
@e40pud
Copy link
Contributor Author

e40pud commented Apr 27, 2023

💚 All backports created successfully

Status Branch Result
8.8

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

e40pud added a commit that referenced this pull request Apr 27, 2023
…context variables (#155829) (#156009)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Security Solution][Alerts] Format alerts for per-alert action
context variables
(#155829)](#155829)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Ievgen
Sorokopud","email":"ievgen.sorokopud@elastic.co"},"sourceCommit":{"committedDate":"2023-04-26T16:16:41Z","message":"[Security
Solution][Alerts] Format alerts for per-alert action context variables
(#155829)\n\n## Summary\r\n\r\nCloses
[#155812](https://github.com/elastic/kibana/issues/155812)\r\n\r\nIn
#155384, detection rules
were\r\nswitched to support per-alert actions. When passing the
context\r\nvariable, it was suggested that we should be calling
formatAlert to\r\nformat the alert for notifications, however doing that
causes some test\r\nfailures because formatAlert is fairly heavyweight
and bunch of tests\r\nwere timing out.\r\n\r\nThanks to @marshallmain we
have this much faster `expandDottedObject`\r\nthat solves the issue with
the very slow
`formatAlert`.","sha":"8f597207a222f02b1c7664bc555a9f6e744bc4aa","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:skip","v8.8.0"],"number":155829,"url":"https://github.com/elastic/kibana/pull/155829","mergeCommit":{"message":"[Security
Solution][Alerts] Format alerts for per-alert action context variables
(#155829)\n\n## Summary\r\n\r\nCloses
[#155812](https://github.com/elastic/kibana/issues/155812)\r\n\r\nIn
#155384, detection rules
were\r\nswitched to support per-alert actions. When passing the
context\r\nvariable, it was suggested that we should be calling
formatAlert to\r\nformat the alert for notifications, however doing that
causes some test\r\nfailures because formatAlert is fairly heavyweight
and bunch of tests\r\nwere timing out.\r\n\r\nThanks to @marshallmain we
have this much faster `expandDottedObject`\r\nthat solves the issue with
the very slow
`formatAlert`.","sha":"8f597207a222f02b1c7664bc555a9f6e744bc4aa"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/155829","number":155829,"mergeCommit":{"message":"[Security
Solution][Alerts] Format alerts for per-alert action context variables
(#155829)\n\n## Summary\r\n\r\nCloses
[#155812](https://github.com/elastic/kibana/issues/155812)\r\n\r\nIn
#155384, detection rules
were\r\nswitched to support per-alert actions. When passing the
context\r\nvariable, it was suggested that we should be calling
formatAlert to\r\nformat the alert for notifications, however doing that
causes some test\r\nfailures because formatAlert is fairly heavyweight
and bunch of tests\r\nwere timing out.\r\n\r\nThanks to @marshallmain we
have this much faster `expandDottedObject`\r\nthat solves the issue with
the very slow
`formatAlert`.","sha":"8f597207a222f02b1c7664bc555a9f6e744bc4aa"}}]}]
BACKPORT-->

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes v8.8.0
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

[Security Solution][Alerts] Format alerts for per-alert action context variables
5 participants