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

[Alerting] Allow rule types to specify custom timeout values #113487

Merged

Conversation

YulNaumenko
Copy link
Contributor

@YulNaumenko YulNaumenko commented Sep 29, 2021

Summary

Resolves #111804

In this PR was extended rules registry ability to specify rule task timeout, which could be useful for some rule types which expecting a log running rules.
Could be set in the rule type properties:

 const ruleType: AlertType<never, never, never, never, never, 'default', 'backToAwesome'> = {
      id: 'test',
      name: 'Test',
      actionGroups: [
        {
          id: 'default',
          name: 'Default',
        },
      ],
      defaultActionGroupId: 'default',
      ruleTaskTimeout: '13m',
      executor: jest.fn(),
      producer: 'alerts',
      minimumLicenseRequired: 'basic',
      isExportable: true,
    };

Added ability to configure default framework rule task timeout using Kibana config setting:
xpack.alerting.defaultRuleTaskTimeout

Checklist

@YulNaumenko YulNaumenko added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.16.0 Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework labels Sep 29, 2021
@YulNaumenko YulNaumenko self-assigned this Sep 29, 2021
@YulNaumenko YulNaumenko marked this pull request as ready for review September 30, 2021 03:49
@YulNaumenko YulNaumenko requested a review from a team as a code owner September 30, 2021 03:49
@elasticmachine
Copy link
Contributor

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

@YulNaumenko
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

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

Looking great! I added a couple of comments and curious to your thoughts!

docs/settings/alert-action-settings.asciidoc Outdated Show resolved Hide resolved
x-pack/plugins/alerting/server/plugin.ts Show resolved Hide resolved
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! Just a comment about allowlists and some wording suggestions for the docs/README

@@ -179,3 +179,10 @@ For example, `20m`, `24h`, `7d`, `1w`. Default: `60s`.

`xpack.alerting.maxEphemeralActionsPerAlert`::
Sets the number of actions that will be executed ephemerally. To use this, enable ephemeral tasks in task manager first with <<task-manager-settings,`xpack.task_manager.ephemeral_tasks.enabled`>>

`xpack.alerting.defaultRuleTaskTimeout`::
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add this to the docker allowlist and also open a cloud PR for this setting?

docs/settings/alert-action-settings.asciidoc Outdated Show resolved Hide resolved
x-pack/plugins/alerting/README.md Outdated Show resolved Hide resolved
@YulNaumenko YulNaumenko requested a review from a team as a code owner October 5, 2021 02:15
@YulNaumenko YulNaumenko merged commit b204b91 into elastic:master Oct 5, 2021
YulNaumenko added a commit to YulNaumenko/kibana that referenced this pull request Oct 5, 2021
…#113487)

* [Alerting] Allow rule types to specify custom timeout values

* fixed tests and docs

* -

* fixed due to comments

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

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

* fixed tests and docs

* Update plugin.test.ts

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: ymao1 <[email protected]>
YulNaumenko added a commit that referenced this pull request Oct 5, 2021
#113957)

* [Alerting] Allow rule types to specify custom timeout values

* fixed tests and docs

* -

* fixed due to comments

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

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

* fixed tests and docs

* Update plugin.test.ts

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: ymao1 <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: ymao1 <[email protected]>
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
alerting 240 241 +1
Unknown metric groups

API count

id before after diff
alerting 248 249 +1

History

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

cc @YulNaumenko

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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) v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Alerting] Allow rule types to specify custom timeout values
6 participants