From 8e8c8f852e1532a346e7c43b45c6bc65d455db8b Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Tue, 5 Oct 2021 11:24:07 -0700 Subject: [PATCH] [Alerting] Allow rule types to specify custom timeout values (#113487) (#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 * fixed tests and docs * Update plugin.test.ts Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: ymao1 Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: ymao1 --- docs/settings/alert-action-settings.asciidoc | 7 +++ .../resources/base/bin/kibana-docker | 1 + x-pack/plugins/alerting/README.md | 2 + x-pack/plugins/alerting/server/config.test.ts | 1 + x-pack/plugins/alerting/server/config.ts | 1 + .../alerting/server/health/get_state.test.ts | 8 +++ x-pack/plugins/alerting/server/plugin.test.ts | 14 ++++++ x-pack/plugins/alerting/server/plugin.ts | 10 +++- .../server/rule_type_registry.test.ts | 50 +++++++++++++++++++ .../alerting/server/rule_type_registry.ts | 17 +++++++ x-pack/plugins/alerting/server/types.ts | 2 +- 11 files changed, 111 insertions(+), 2 deletions(-) diff --git a/docs/settings/alert-action-settings.asciidoc b/docs/settings/alert-action-settings.asciidoc index 51c1d286eb71..d790bf2f8af1 100644 --- a/docs/settings/alert-action-settings.asciidoc +++ b/docs/settings/alert-action-settings.asciidoc @@ -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 <> + +`xpack.alerting.defaultRuleTaskTimeout`:: +Specifies the default timeout for the all rule types tasks. The time is formatted as: ++ +`[ms,s,m,h,d,w,M,Y]` ++ +For example, `20m`, `24h`, `7d`, `1w`. Default: `60s`. diff --git a/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker b/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker index 3e2fc1dc8f8f..2de8dbf8929b 100755 --- a/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker +++ b/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker @@ -221,6 +221,7 @@ kibana_vars=( xpack.alerting.healthCheck.interval xpack.alerting.invalidateApiKeysTask.interval xpack.alerting.invalidateApiKeysTask.removalDelay + xpack.alerting.defaultRuleTaskTimeout xpack.alerts.healthCheck.interval xpack.alerts.invalidateApiKeysTask.interval xpack.alerts.invalidateApiKeysTask.removalDelay diff --git a/x-pack/plugins/alerting/README.md b/x-pack/plugins/alerting/README.md index 94cdeadee97e..58d2ca35dea7 100644 --- a/x-pack/plugins/alerting/README.md +++ b/x-pack/plugins/alerting/README.md @@ -118,6 +118,7 @@ The following table describes the properties of the `options` object. |executor|This is where the code for the rule type lives. This is a function to be called when executing a rule on an interval basis. For full details, see the executor section below.|Function| |producer|The id of the application producing this rule type.|string| |minimumLicenseRequired|The value of a minimum license. Most of the rules are licensed as "basic".|string| +|ruleTaskTimeout|The length of time a rule can run before being cancelled due to timeout. By default, this value is "5m".|string| |useSavedObjectReferences.extractReferences|(Optional) When developing a rule type, you can choose to implement hooks for extracting saved object references from rule parameters. This hook will be invoked when a rule is created or updated. Implementing this hook is optional, but if an extract hook is implemented, an inject hook must also be implemented.|Function |useSavedObjectReferences.injectReferences|(Optional) When developing a rule type, you can choose to implement hooks for injecting saved object references into rule parameters. This hook will be invoked when a rule is retrieved (get or find). Implementing this hook is optional, but if an inject hook is implemented, an extract hook must also be implemented.|Function |isExportable|Whether the rule type is exportable from the Saved Objects Management UI.|boolean| @@ -344,6 +345,7 @@ const myRuleType: AlertType< }; }, producer: 'alerting', + ruleTaskTimeout: '10m', useSavedObjectReferences: { extractReferences: (params: Params): RuleParamsAndRefs => { const { testSavedObjectId, ...otherParams } = params; diff --git a/x-pack/plugins/alerting/server/config.test.ts b/x-pack/plugins/alerting/server/config.test.ts index a1ae77596ccb..63d93b9d6776 100644 --- a/x-pack/plugins/alerting/server/config.test.ts +++ b/x-pack/plugins/alerting/server/config.test.ts @@ -12,6 +12,7 @@ describe('config validation', () => { const config: Record = {}; expect(configSchema.validate(config)).toMatchInlineSnapshot(` Object { + "defaultRuleTaskTimeout": "5m", "healthCheck": Object { "interval": "60m", }, diff --git a/x-pack/plugins/alerting/server/config.ts b/x-pack/plugins/alerting/server/config.ts index 47ef451ceab9..277f0c7297df 100644 --- a/x-pack/plugins/alerting/server/config.ts +++ b/x-pack/plugins/alerting/server/config.ts @@ -20,6 +20,7 @@ export const configSchema = schema.object({ maxEphemeralActionsPerAlert: schema.number({ defaultValue: DEFAULT_MAX_EPHEMERAL_ACTIONS_PER_ALERT, }), + defaultRuleTaskTimeout: schema.string({ validate: validateDurationSchema, defaultValue: '5m' }), }); export type AlertsConfig = TypeOf; diff --git a/x-pack/plugins/alerting/server/health/get_state.test.ts b/x-pack/plugins/alerting/server/health/get_state.test.ts index 9429dcc07d92..f4306b8250b8 100644 --- a/x-pack/plugins/alerting/server/health/get_state.test.ts +++ b/x-pack/plugins/alerting/server/health/get_state.test.ts @@ -72,6 +72,7 @@ describe('getHealthServiceStatusWithRetryAndErrorHandling', () => { removalDelay: '1h', }, maxEphemeralActionsPerAlert: 100, + defaultRuleTaskTimeout: '20m', }), pollInterval ).subscribe(); @@ -106,6 +107,7 @@ describe('getHealthServiceStatusWithRetryAndErrorHandling', () => { removalDelay: '1h', }, maxEphemeralActionsPerAlert: 100, + defaultRuleTaskTimeout: '20m', }), pollInterval, retryDelay @@ -151,6 +153,7 @@ describe('getHealthServiceStatusWithRetryAndErrorHandling', () => { removalDelay: '1h', }, maxEphemeralActionsPerAlert: 100, + defaultRuleTaskTimeout: '20m', }) ).toPromise(); @@ -182,6 +185,7 @@ describe('getHealthServiceStatusWithRetryAndErrorHandling', () => { removalDelay: '1h', }, maxEphemeralActionsPerAlert: 100, + defaultRuleTaskTimeout: '20m', }) ).toPromise(); @@ -213,6 +217,7 @@ describe('getHealthServiceStatusWithRetryAndErrorHandling', () => { removalDelay: '1h', }, maxEphemeralActionsPerAlert: 100, + defaultRuleTaskTimeout: '20m', }) ).toPromise(); @@ -241,6 +246,7 @@ describe('getHealthServiceStatusWithRetryAndErrorHandling', () => { removalDelay: '1h', }, maxEphemeralActionsPerAlert: 100, + defaultRuleTaskTimeout: '20m', }), retryDelay ).subscribe((status) => { @@ -272,6 +278,7 @@ describe('getHealthServiceStatusWithRetryAndErrorHandling', () => { removalDelay: '1h', }, maxEphemeralActionsPerAlert: 100, + defaultRuleTaskTimeout: '20m', }), retryDelay ).subscribe((status) => { @@ -309,6 +316,7 @@ describe('getHealthServiceStatusWithRetryAndErrorHandling', () => { removalDelay: '1h', }, maxEphemeralActionsPerAlert: 100, + defaultRuleTaskTimeout: '20m', }) ).toPromise(); diff --git a/x-pack/plugins/alerting/server/plugin.test.ts b/x-pack/plugins/alerting/server/plugin.test.ts index 4cfa1d91758e..6419a3ccc5c9 100644 --- a/x-pack/plugins/alerting/server/plugin.test.ts +++ b/x-pack/plugins/alerting/server/plugin.test.ts @@ -38,6 +38,7 @@ describe('Alerting Plugin', () => { removalDelay: '1h', }, maxEphemeralActionsPerAlert: 10, + defaultRuleTaskTimeout: '5m', }); plugin = new AlertingPlugin(context); @@ -71,6 +72,7 @@ describe('Alerting Plugin', () => { removalDelay: '1h', }, maxEphemeralActionsPerAlert: 10, + defaultRuleTaskTimeout: '5m', }); plugin = new AlertingPlugin(context); @@ -142,6 +144,15 @@ describe('Alerting Plugin', () => { minimumLicenseRequired: 'basic', }); }); + + it('should apply default config value for ruleTaskTimeout', async () => { + const ruleType = { + ...sampleAlertType, + minimumLicenseRequired: 'basic', + } as AlertType; + await setup.registerType(ruleType); + expect(ruleType.ruleTaskTimeout).toBe('5m'); + }); }); }); @@ -157,6 +168,7 @@ describe('Alerting Plugin', () => { removalDelay: '1h', }, maxEphemeralActionsPerAlert: 10, + defaultRuleTaskTimeout: '5m', }); const plugin = new AlertingPlugin(context); @@ -197,6 +209,7 @@ describe('Alerting Plugin', () => { removalDelay: '1h', }, maxEphemeralActionsPerAlert: 10, + defaultRuleTaskTimeout: '5m', }); const plugin = new AlertingPlugin(context); @@ -251,6 +264,7 @@ describe('Alerting Plugin', () => { removalDelay: '1h', }, maxEphemeralActionsPerAlert: 100, + defaultRuleTaskTimeout: '5m', }); const plugin = new AlertingPlugin(context); diff --git a/x-pack/plugins/alerting/server/plugin.ts b/x-pack/plugins/alerting/server/plugin.ts index 41de44c0b0e8..231787a88969 100644 --- a/x-pack/plugins/alerting/server/plugin.ts +++ b/x-pack/plugins/alerting/server/plugin.ts @@ -286,6 +286,7 @@ export class AlertingPlugin { encryptedSavedObjects: plugins.encryptedSavedObjects, }); + const alertingConfig = this.config; return { registerType< Params extends AlertTypeParams = AlertTypeParams, @@ -309,7 +310,14 @@ export class AlertingPlugin { if (!(alertType.minimumLicenseRequired in LICENSE_TYPE)) { throw new Error(`"${alertType.minimumLicenseRequired}" is not a valid license type`); } - ruleTypeRegistry.register(alertType); + if (!alertType.ruleTaskTimeout) { + alertingConfig.then((config) => { + alertType.ruleTaskTimeout = config.defaultRuleTaskTimeout; + ruleTypeRegistry.register(alertType); + }); + } else { + ruleTypeRegistry.register(alertType); + } }, }; } diff --git a/x-pack/plugins/alerting/server/rule_type_registry.test.ts b/x-pack/plugins/alerting/server/rule_type_registry.test.ts index f8067a2281f6..1c44e862c261 100644 --- a/x-pack/plugins/alerting/server/rule_type_registry.test.ts +++ b/x-pack/plugins/alerting/server/rule_type_registry.test.ts @@ -112,6 +112,32 @@ describe('register()', () => { ); }); + test('throws if AlertType ruleTaskTimeout is not a valid duration', () => { + const alertType: AlertType = { + id: 123 as unknown as string, + name: 'Test', + actionGroups: [ + { + id: 'default', + name: 'Default', + }, + ], + ruleTaskTimeout: '23 milisec', + defaultActionGroupId: 'default', + minimumLicenseRequired: 'basic', + isExportable: true, + executor: jest.fn(), + producer: 'alerts', + }; + const registry = new RuleTypeRegistry(ruleTypeRegistryParams); + + expect(() => registry.register(alertType)).toThrowError( + new Error( + `Rule type \"123\" has invalid timeout: string is not a valid duration: 23 milisec.` + ) + ); + }); + test('throws if RuleType action groups contains reserved group id', () => { const alertType: AlertType = { id: 'test', @@ -181,6 +207,28 @@ describe('register()', () => { `); }); + test('allows an AlertType to specify a custom rule task timeout', () => { + const alertType: AlertType = { + id: 'test', + name: 'Test', + actionGroups: [ + { + id: 'default', + name: 'Default', + }, + ], + defaultActionGroupId: 'default', + ruleTaskTimeout: '13m', + executor: jest.fn(), + producer: 'alerts', + minimumLicenseRequired: 'basic', + isExportable: true, + }; + const registry = new RuleTypeRegistry(ruleTypeRegistryParams); + registry.register(alertType); + expect(registry.get('test').ruleTaskTimeout).toBe('13m'); + }); + test('throws if the custom recovery group is contained in the AlertType action groups', () => { const alertType: AlertType< never, @@ -237,6 +285,7 @@ describe('register()', () => { isExportable: true, executor: jest.fn(), producer: 'alerts', + ruleTaskTimeout: '20m', }; const registry = new RuleTypeRegistry(ruleTypeRegistryParams); registry.register(alertType); @@ -246,6 +295,7 @@ describe('register()', () => { Object { "alerting:test": Object { "createTaskRunner": [Function], + "timeout": "20m", "title": "Test", }, }, diff --git a/x-pack/plugins/alerting/server/rule_type_registry.ts b/x-pack/plugins/alerting/server/rule_type_registry.ts index 3cd21d0c64dd..dc72b644b2c7 100644 --- a/x-pack/plugins/alerting/server/rule_type_registry.ts +++ b/x-pack/plugins/alerting/server/rule_type_registry.ts @@ -25,6 +25,7 @@ import { getBuiltinActionGroups, RecoveredActionGroupId, ActionGroup, + validateDurationSchema, } from '../common'; import { ILicenseState } from './lib/license_state'; import { getAlertTypeFeatureUsageName } from './lib/get_alert_type_feature_usage_name'; @@ -170,6 +171,21 @@ export class RuleTypeRegistry { }) ); } + // validate ruleTypeTimeout here + if (alertType.ruleTaskTimeout) { + const invalidTimeout = validateDurationSchema(alertType.ruleTaskTimeout); + if (invalidTimeout) { + throw new Error( + i18n.translate('xpack.alerting.ruleTypeRegistry.register.invalidTimeoutAlertTypeError', { + defaultMessage: 'Rule type "{id}" has invalid timeout: {errorMessage}.', + values: { + id: alertType.id, + errorMessage: invalidTimeout, + }, + }) + ); + } + } alertType.actionVariables = normalizedActionVariables(alertType.actionVariables); const normalizedAlertType = augmentActionGroupsWithReserved< @@ -190,6 +206,7 @@ export class RuleTypeRegistry { this.taskManager.registerTaskDefinitions({ [`alerting:${alertType.id}`]: { title: alertType.name, + timeout: alertType.ruleTaskTimeout, createTaskRunner: (context: RunContext) => this.taskRunnerFactory.create< Params, diff --git a/x-pack/plugins/alerting/server/types.ts b/x-pack/plugins/alerting/server/types.ts index 67565271fedc..87648e3f633b 100644 --- a/x-pack/plugins/alerting/server/types.ts +++ b/x-pack/plugins/alerting/server/types.ts @@ -157,8 +157,8 @@ export interface AlertType< injectReferences: (params: ExtractedParams, references: SavedObjectReference[]) => Params; }; isExportable: boolean; + ruleTaskTimeout?: string; } - export type UntypedAlertType = AlertType< AlertTypeParams, AlertTypeState,