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

[Task Manager] Support excluding certain task types from executing #111036

Merged

Conversation

chrisronline
Copy link
Contributor

@chrisronline chrisronline commented Sep 2, 2021

Resolves #108757

We want to prevent users from disabling the alerting teams plugins (which we did here) but still want to provide an option to disable rules/actions from executing if necessary (usually to isolate some performance issue).

This PR exposes a new, undocumented config to task manager, called xpack.task_manager.internal.exclude_task_types, that allows users to specify an array of task types that will not execute. This config supports using a wildcard * to do pattern matching on the task type. All task types that match any of the configured patterns will not execute but remain in the queue (they won't ever be claimed by Kibanas configured to exclude them)

@chrisronline chrisronline changed the title Support excluding certain action types [Task Manager] Support excluding certain action types Sep 3, 2021
@chrisronline chrisronline changed the title [Task Manager] Support excluding certain action types [Task Manager] Support excluding certain task types Sep 3, 2021
@chrisronline chrisronline changed the title [Task Manager] Support excluding certain task types [Task Manager] Support excluding certain task types from executing Sep 3, 2021
@chrisronline chrisronline marked this pull request as ready for review September 3, 2021 17:05
@chrisronline chrisronline requested a review from a team as a code owner September 3, 2021 17:05
@chrisronline chrisronline added Feature:Task Manager review Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.16.0 v8.0.0 labels Sep 3, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@chrisronline chrisronline added the release_note:skip Skip the PR/issue when compiling release notes label Sep 3, 2021
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 that adding different values to the excluded array works as expected. It would be nice to add a functional test for this...not sure how hard that would be?

x-pack/plugins/task_manager/server/config.ts Outdated Show resolved Hide resolved
x-pack/plugins/task_manager/server/config.ts Show resolved Hide resolved
@chrisronline chrisronline requested a review from ymao1 September 7, 2021 17:48
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. Thanks for adding the functional test! Left a suggestion for the README since I don't think this is meant just for debugging purposes?

x-pack/plugins/task_manager/README.md Outdated Show resolved Hide resolved
@chrisronline chrisronline requested review from a team as code owners September 8, 2021 21:00
@chrisronline
Copy link
Contributor Author

@elasticmachine merge upstream

@chrisronline chrisronline force-pushed the task_manager/exclude_task_types branch from 8da2603 to 587f6a2 Compare September 9, 2021 19:09
Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM - left a note about the logging when the new config is set. Seems like it could be useful to see this at an info or warning level, as opposed to debug level.

}

if (this.config.unsafe.exclude_task_types.length) {
this.logger.debug(
Copy link
Member

Choose a reason for hiding this comment

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

Since we've labelled this as unsafe/experimental, wondering if we should elevate this message to an info, or perhaps even warning.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to a warning. It would make it clear what's happening without a way to ignore it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@chrisronline chrisronline merged commit 17150b2 into elastic:master Sep 13, 2021
chrisronline added a commit to chrisronline/kibana that referenced this pull request Sep 13, 2021
…lastic#111036)

* Support excluding certain action types

* Fix types

* Fix jest tests

* Flip this

* Add functional test

* Add to README

* Updated README

* Add startup log

* Update x-pack/plugins/task_manager/README.md

Co-authored-by: Mike Côté <[email protected]>

* Add telemetry

* Add test

* Rename internal to unsafe

* Update test config

* Fix tests

* Update x-pack/plugins/task_manager/README.md

Co-authored-by: gchaps <[email protected]>

* PR feedback

Co-authored-by: Mike Côté <[email protected]>
Co-authored-by: gchaps <[email protected]>
chrisronline added a commit that referenced this pull request Sep 13, 2021
…111036) (#112022)

* Support excluding certain action types

* Fix types

* Fix jest tests

* Flip this

* Add functional test

* Add to README

* Updated README

* Add startup log

* Update x-pack/plugins/task_manager/README.md

Co-authored-by: Mike Côté <[email protected]>

* Add telemetry

* Add test

* Rename internal to unsafe

* Update test config

* Fix tests

* Update x-pack/plugins/task_manager/README.md

Co-authored-by: gchaps <[email protected]>

* PR feedback

Co-authored-by: Mike Côté <[email protected]>
Co-authored-by: gchaps <[email protected]>

Co-authored-by: Mike Côté <[email protected]>
Co-authored-by: gchaps <[email protected]>
@chrisronline chrisronline deleted the task_manager/exclude_task_types branch September 14, 2021 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Task Manager release_note:skip Skip the PR/issue when compiling release notes review Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide way to easily "switch off alerts" in a running Kibana instance
8 participants