[Alerting] Allow rule types to specify custom timeout values (elastic…

…#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 · Oct 5, 2021 · 7e85f11 · 7e85f11
1 parent d286ba9
commit 7e85f11
Show file tree

Hide file tree

Showing 11 changed files with 111 additions and 2 deletions.
diff --git 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 <<task-manager-settings,`xpack.task_manager.ephemeral_tasks.enabled`>>
+
+`xpack.alerting.defaultRuleTaskTimeout`::
+Specifies the default timeout for the all rule types tasks. The time is formatted as:
++
+`<count>[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
@@ -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
@@ -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<ExtractedParams> => {
 			const { testSavedObjectId, ...otherParams } = params;

diff --git 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<string, unknown> = {};
     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
@@ -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<typeof configSchema>;
diff --git 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
@@ -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<never, never, never, never, never, 'default', never>;
+        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
@@ -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
@@ -112,6 +112,32 @@ describe('register()', () => {
     );
   });
 
+  test('throws if AlertType ruleTaskTimeout is not a valid duration', () => {
+    const alertType: AlertType<never, never, never, never, never, 'default'> = {
+      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<never, never, never, never, never, 'default' | 'NotReserved'> = {
       id: 'test',
@@ -181,6 +207,28 @@ describe('register()', () => {
     `);
   });
 
+  test('allows an AlertType to specify a custom rule task timeout', () => {
+    const alertType: 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,
+    };
+    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
@@ -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
@@ -157,8 +157,8 @@ export interface AlertType<
     injectReferences: (params: ExtractedParams, references: SavedObjectReference[]) => Params;
   };
   isExportable: boolean;
+  ruleTaskTimeout?: string;
 }
-
 export type UntypedAlertType = AlertType<
   AlertTypeParams,
   AlertTypeState,