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.

Sign up for GitHub

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

Jump to bottom

[ResponseOps][Alerting] Optimize the scheduling of rule actions so it can happen in bulk #137781

Merged
merged 30 commits into from
Aug 12, 2022
Merged

[ResponseOps][Alerting] Optimize the scheduling of rule actions so it can happen in bulk #137781

merged 30 commits into from
Aug 12, 2022

Conversation

doakalexi
Copy link
Contributor

@doakalexi doakalexi commented Aug 1, 2022 •
edited
Loading

Resolves #126511

Summary

We wanted to leverage bulk calls to schedule rule actions.

  • Updated the actions client could support enqueueing in bulk
  • Update the alerting task runner so that it could call the new bulk API

Checklist

To verify

  • Create a rule and verify that is runs as expected, you shouldn't notice any changes from the front-end

@doakalexi
Copy link
Contributor Author

@elasticmachine merge upstream

@doakalexi
Copy link
Contributor Author

@elasticmachine merge upstream

@doakalexi doakalexi changed the title Adding bulk scheduler [ResponseOps][Alerting] Optimize the scheduling of rule actions so it can happen in bulk Aug 3, 2022
@doakalexi doakalexi added v8.5.0 Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Aug 3, 2022
@doakalexi doakalexi marked this pull request as ready for review August 3, 2022 13:52
@doakalexi doakalexi requested a review from a team as a code owner August 3, 2022 13:52
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@doakalexi
Copy link
Contributor Author

@elasticmachine merge upstream

ymao1
ymao1 requested changes Aug 8, 2022
View reviewed changes
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.

Some initial comments after a first pass :)

x-pack/plugins/alerting/server/task_runner/create_execution_handler.ts Show resolved Hide resolved
x-pack/plugins/alerting/server/task_runner/task_runner.test.ts Show resolved Hide resolved
x-pack/plugins/actions/server/create_execute_function.ts Show resolved Hide resolved
x-pack/plugins/actions/server/create_execute_function.ts Outdated Show resolved Hide resolved
@doakalexi
Copy link
Contributor Author

@elasticmachine merge upstream

@mikecote mikecote self-requested a review August 10, 2022 13:53
mikecote
mikecote reviewed Aug 10, 2022
View reviewed changes
Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

Left a few questions but overall the code is looking good and looking forward to this optimization!

x-pack/plugins/actions/server/actions_client.ts Outdated Show resolved Hide resolved
x-pack/plugins/actions/server/create_execute_function.ts Outdated Show resolved Hide resolved
x-pack/plugins/actions/server/create_execute_function.ts Outdated Show resolved Hide resolved
x-pack/plugins/actions/server/create_execute_function.ts Show resolved Hide resolved
doakalexi and others added 4 commits August 10, 2022 14:26
@doakalexi @mikecote
Update x-pack/plugins/actions/server/create_execute_function.ts
79702ff 
Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
@doakalexi @mikecote
Update x-pack/plugins/actions/server/create_execute_function.ts
3b0be30 
Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
@doakalexi
Cleaning up auth
d405d58
@doakalexi
Merge branch 'alerting/schedule-rule-actions-in-bulk' of github.com:d…
434482b 
…oakalexi/kibana into alerting/schedule-rule-actions-in-bulk
@doakalexi
Copy link
Contributor Author

@elasticmachine merge upstream

mikecote
mikecote approved these changes Aug 12, 2022
View reviewed changes
Copy link
Contributor

@mikecote mikecote left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Changes LGTM! Can't wait for this optimization!

Feel free to create a follow up issue to remove the code around enqueueExecution (the non-bulk one) if the code won't be used anymore.

@doakalexi
Copy link
Contributor Author

Changes LGTM! Can't wait for this optimization!

Feel free to create a follow up issue to remove the code around enqueueExecution (the non-bulk one) if the code won't be used anymore.

Thanks for reviewing! There is a route that uses enqueueExecution, so I am not sure if we can remove it.

@mikecote
Copy link
Contributor

Thanks for reviewing! There is a route that uses enqueueExecution, so I am not sure if we can remove it.

Ah, good catch. Let's leave it as is then :-)

ymao1
ymao1 approved these changes Aug 12, 2022
View reviewed changes
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! Verified actions are being bulk scheduled 🎉

Left some minor comments but overall looks great!

x-pack/plugins/actions/server/lib/track_legacy_rbac_exemption.ts Show resolved Hide resolved
x-pack/plugins/actions/server/authorization/get_authorization_mode_by_source.ts Outdated Show resolved Hide resolved
x-pack/plugins/actions/server/authorization/get_authorization_mode_by_source.test.ts Show resolved Hide resolved
x-pack/plugins/alerting/server/task_runner/task_runner.test.ts Show resolved Hide resolved
@doakalexi
Copy link
Contributor Author

@elasticmachine merge upstream

@doakalexi
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@doakalexi doakalexi merged commit 4ddc26d into elastic:main Aug 12, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 12, 2022
Mpdreamz pushed a commit to Mpdreamz/kibana that referenced this pull request Sep 6, 2022
@doakalexi @kibanamachine
@mikecote @Mpdreamz
[ResponseOps][Alerting] Optimize the scheduling of rule actions so it…
f662319 
… can happen in bulk (elastic#137781)

* Adding bulk scheduler

* Updating bulk schedule

* Fixing failing tests

* Fixing test

* Adding bulk schedule tests

* Cleaning up enqueue function

* Using bulk getConnectors

* Removing empty line

* Update x-pack/plugins/actions/server/create_execute_function.ts

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

* Update x-pack/plugins/actions/server/create_execute_function.ts

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>

* Cleaning up auth

* Fixing test failure

* Updating bulk auth changes

* Fixed track change

* Fixing test failures

* Addressing pr comments

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
@doakalexi doakalexi deleted the alerting/schedule-rule-actions-in-bulk branch December 6, 2022 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ymao1 ymao1 ymao1 approved these changes

@mikecote mikecote mikecote approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.5.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Optimize the scheduling of rule actions so it can happen in bulk
6 participants
@doakalexi @elasticmachine @mikecote @kibana-ci @ymao1 @kibanamachine