[Alerting] Showing last execution duration on Rule Management view (#…

…113935) * Adding last duration to execution status and returning in alerting routes * Fixing types * Adding helper function to format duration * Returning rule timeout value in list rules API * Updating rules table to add duration column and tweaks to match mockup * Updating rules table to add duration column and tweaks to match mockup * i18n fix * Only showing duration warning if duration is long * Unit tests * i18n fix * Fixing functional test * Aligning warning icon * Reset last duration when rule is disabled then reenabled * Fixing functional test * Fixing functional test * Restoring muted badge. Fixing scss * Dont show muted badge if rule is disabled * Moving disabled icontip to right of rule name * Updating tooltips * Updating last run Co-authored-by: Kibana Machine <[email protected]>
elastic · Oct 12, 2021 · c926b14 · c926b14
1 parent 44c9611
commit c926b14
Show file tree

Hide file tree

Showing 36 changed files with 512 additions and 219 deletions.
diff --git a/x-pack/plugins/alerting/common/alert.ts b/x-pack/plugins/alerting/common/alert.ts
@@ -35,6 +35,7 @@ export enum AlertExecutionStatusErrorReasons {
 export interface AlertExecutionStatus {
   status: AlertExecutionStatuses;
   lastExecutionDate: Date;
+  lastDuration?: number;
   error?: {
     reason: AlertExecutionStatusErrorReasons;
     message: string;

diff --git a/x-pack/plugins/alerting/common/alert_type.ts b/x-pack/plugins/alerting/common/alert_type.ts
@@ -21,6 +21,7 @@ export interface AlertType<
   producer: string;
   minimumLicenseRequired: LicenseType;
   isExportable: boolean;
+  ruleTaskTimeout?: string;
   defaultScheduleInterval?: string;
   minimumScheduleInterval?: string;
 }

diff --git a/x-pack/plugins/alerting/common/parse_duration.test.ts b/x-pack/plugins/alerting/common/parse_duration.test.ts
@@ -5,7 +5,12 @@
  * 2.0.
  */
 
-import { parseDuration, getDurationNumberInItsUnit, getDurationUnitValue } from './parse_duration';
+import {
+  parseDuration,
+  formatDuration,
+  getDurationNumberInItsUnit,
+  getDurationUnitValue,
+} from './parse_duration';
 
 test('parses seconds', () => {
   const result = parseDuration('10s');
@@ -39,6 +44,38 @@ test('throws error when suffix is missing', () => {
   );
 });
 
+test('formats seconds', () => {
+  const result = formatDuration('10s');
+  expect(result).toEqual('10 sec');
+});
+
+test('formats minutes', () => {
+  const result = formatDuration('10m');
+  expect(result).toEqual('10 min');
+});
+
+test('formats hours', () => {
+  const result = formatDuration('10h');
+  expect(result).toEqual('10 hr');
+});
+
+test('formats days', () => {
+  const result = formatDuration('10d');
+  expect(result).toEqual('10 day');
+});
+
+test('format throws error when the format is invalid', () => {
+  expect(() => formatDuration('10x')).toThrowErrorMatchingInlineSnapshot(
+    `"Invalid duration \\"10x\\". Durations must be of the form {number}x. Example: 5s, 5m, 5h or 5d\\""`
+  );
+});
+
+test('format throws error when suffix is missing', () => {
+  expect(() => formatDuration('1000')).toThrowErrorMatchingInlineSnapshot(
+    `"Invalid duration \\"1000\\". Durations must be of the form {number}x. Example: 5s, 5m, 5h or 5d\\""`
+  );
+});
+
 test('throws error when 0 based', () => {
   expect(() => parseDuration('0s')).toThrowErrorMatchingInlineSnapshot(
     `"Invalid duration \\"0s\\". Durations must be of the form {number}x. Example: 5s, 5m, 5h or 5d\\""`

diff --git a/x-pack/plugins/alerting/common/parse_duration.ts b/x-pack/plugins/alerting/common/parse_duration.ts
@@ -27,6 +27,22 @@ export function parseDuration(duration: string): number {
   );
 }
 
+export function formatDuration(duration: string): string {
+  const parsed = parseInt(duration, 10);
+  if (isSeconds(duration)) {
+    return `${parsed} sec`;
+  } else if (isMinutes(duration)) {
+    return `${parsed} min`;
+  } else if (isHours(duration)) {
+    return `${parsed} hr`;
+  } else if (isDays(duration)) {
+    return `${parsed} day`;
+  }
+  throw new Error(
+    `Invalid duration "${duration}". Durations must be of the form {number}x. Example: 5s, 5m, 5h or 5d"`
+  );
+}
+
 export function getDurationNumberInItsUnit(duration: string): number {
   return parseInt(duration.replace(/[^0-9.]/g, ''), 10);
 }

diff --git a/x-pack/plugins/alerting/server/lib/alert_execution_status.test.ts b/x-pack/plugins/alerting/server/lib/alert_execution_status.test.ts
@@ -81,6 +81,7 @@ describe('AlertExecutionStatus', () => {
       expect(alertExecutionStatusToRaw({ lastExecutionDate: date, status })).toMatchInlineSnapshot(`
         Object {
           "error": null,
+          "lastDuration": 0,
           "lastExecutionDate": "2020-09-03T16:26:58.000Z",
           "status": "ok",
         }
@@ -95,11 +96,24 @@ describe('AlertExecutionStatus', () => {
             "message": "wops",
             "reason": "decrypt",
           },
+          "lastDuration": 0,
           "lastExecutionDate": "2020-09-03T16:26:58.000Z",
           "status": "ok",
         }
       `);
     });
+
+    test('status with a duration', () => {
+      expect(alertExecutionStatusToRaw({ lastExecutionDate: date, status, lastDuration: 1234 }))
+        .toMatchInlineSnapshot(`
+      Object {
+        "error": null,
+        "lastDuration": 1234,
+        "lastExecutionDate": "2020-09-03T16:26:58.000Z",
+        "status": "ok",
+      }
+    `);
+    });
   });
 
   describe('alertExecutionStatusFromRaw()', () => {
@@ -177,6 +191,41 @@ describe('AlertExecutionStatus', () => {
         }
       `);
     });
+
+    test('valid status, date and duration', () => {
+      const result = alertExecutionStatusFromRaw(MockLogger, 'alert-id', {
+        status,
+        lastExecutionDate: date,
+        lastDuration: 1234,
+      });
+      expect(result).toMatchInlineSnapshot(`
+        Object {
+          "lastDuration": 1234,
+          "lastExecutionDate": 2020-09-03T16:26:58.000Z,
+          "status": "active",
+        }
+      `);
+    });
+
+    test('valid status, date, error and duration', () => {
+      const result = alertExecutionStatusFromRaw(MockLogger, 'alert-id', {
+        status,
+        lastExecutionDate: date,
+        error,
+        lastDuration: 1234,
+      });
+      expect(result).toMatchInlineSnapshot(`
+        Object {
+          "error": Object {
+            "message": "wops",
+            "reason": "execute",
+          },
+          "lastDuration": 1234,
+          "lastExecutionDate": 2020-09-03T16:26:58.000Z,
+          "status": "active",
+        }
+      `);
+    });
   });
 });
 

diff --git a/x-pack/plugins/alerting/server/lib/alert_execution_status.ts b/x-pack/plugins/alerting/server/lib/alert_execution_status.ts
@@ -32,11 +32,13 @@ export function executionStatusFromError(error: Error): AlertExecutionStatus {
 
 export function alertExecutionStatusToRaw({
   lastExecutionDate,
+  lastDuration,
   status,
   error,
 }: AlertExecutionStatus): RawAlertExecutionStatus {
   return {
     lastExecutionDate: lastExecutionDate.toISOString(),
+    lastDuration: lastDuration ?? 0,
     status,
     // explicitly setting to null (in case undefined) due to partial update concerns
     error: error ?? null,
@@ -50,7 +52,7 @@ export function alertExecutionStatusFromRaw(
 ): AlertExecutionStatus | undefined {
   if (!rawAlertExecutionStatus) return undefined;
 
-  const { lastExecutionDate, status = 'unknown', error } = rawAlertExecutionStatus;
+  const { lastExecutionDate, lastDuration, status = 'unknown', error } = rawAlertExecutionStatus;
 
   let parsedDateMillis = lastExecutionDate ? Date.parse(lastExecutionDate) : Date.now();
   if (isNaN(parsedDateMillis)) {
@@ -60,12 +62,20 @@ export function alertExecutionStatusFromRaw(
     parsedDateMillis = Date.now();
   }
 
-  const parsedDate = new Date(parsedDateMillis);
+  const executionStatus: AlertExecutionStatus = {
+    status,
+    lastExecutionDate: new Date(parsedDateMillis),
+  };
+
+  if (null != lastDuration) {
+    executionStatus.lastDuration = lastDuration;
+  }
+
   if (error) {
-    return { lastExecutionDate: parsedDate, status, error };
-  } else {
-    return { lastExecutionDate: parsedDate, status };
+    executionStatus.error = error;
   }
+
+  return executionStatus;
 }
 
 export const getAlertExecutionStatusPending = (lastExecutionDate: string) => ({

diff --git a/x-pack/plugins/alerting/server/routes/create_rule.ts b/x-pack/plugins/alerting/server/routes/create_rule.ts
@@ -67,7 +67,7 @@ const rewriteBodyRes: RewriteResponseCase<SanitizedAlert<AlertTypeParams>> = ({
   notifyWhen,
   muteAll,
   mutedInstanceIds,
-  executionStatus: { lastExecutionDate, ...executionStatus },
+  executionStatus: { lastExecutionDate, lastDuration, ...executionStatus },
   ...rest
 }) => ({
   ...rest,
@@ -84,6 +84,7 @@ const rewriteBodyRes: RewriteResponseCase<SanitizedAlert<AlertTypeParams>> = ({
   execution_status: {
     ...executionStatus,
     last_execution_date: lastExecutionDate,
+    last_duration: lastDuration,
   },
   actions: actions.map(({ group, id, actionTypeId, params }) => ({
     group,

diff --git a/x-pack/plugins/alerting/server/routes/find_rules.ts b/x-pack/plugins/alerting/server/routes/find_rules.ts
@@ -93,8 +93,9 @@ const rewriteBodyRes: RewriteResponseCase<FindResult<AlertTypeParams>> = ({
         muted_alert_ids: mutedInstanceIds,
         scheduled_task_id: scheduledTaskId,
         execution_status: executionStatus && {
-          ...omit(executionStatus, 'lastExecutionDate'),
+          ...omit(executionStatus, 'lastExecutionDate', 'lastDuration'),
           last_execution_date: executionStatus.lastExecutionDate,
+          last_duration: executionStatus.lastDuration,
         },
         actions: actions.map(({ group, id, actionTypeId, params }) => ({
           group,

diff --git a/x-pack/plugins/alerting/server/routes/get_rule.ts b/x-pack/plugins/alerting/server/routes/get_rule.ts
@@ -48,8 +48,9 @@ const rewriteBodyRes: RewriteResponseCase<SanitizedAlert<AlertTypeParams>> = ({
   muted_alert_ids: mutedInstanceIds,
   scheduled_task_id: scheduledTaskId,
   execution_status: executionStatus && {
-    ...omit(executionStatus, 'lastExecutionDate'),
+    ...omit(executionStatus, 'lastExecutionDate', 'lastDuration'),
     last_execution_date: executionStatus.lastExecutionDate,
+    last_duration: executionStatus.lastDuration,
   },
   actions: actions.map(({ group, id, actionTypeId, params }) => ({
     group,

diff --git a/x-pack/plugins/alerting/server/routes/resolve_rule.ts b/x-pack/plugins/alerting/server/routes/resolve_rule.ts
@@ -48,8 +48,9 @@ const rewriteBodyRes: RewriteResponseCase<ResolvedSanitizedRule<AlertTypeParams>
   muted_alert_ids: mutedInstanceIds,
   scheduled_task_id: scheduledTaskId,
   execution_status: executionStatus && {
-    ...omit(executionStatus, 'lastExecutionDate'),
+    ...omit(executionStatus, 'lastExecutionDate', 'lastDuration'),
     last_execution_date: executionStatus.lastExecutionDate,
+    last_duration: executionStatus.lastDuration,
   },
   actions: actions.map(({ group, id, actionTypeId, params }) => ({
     group,

diff --git a/x-pack/plugins/alerting/server/routes/rule_types.test.ts b/x-pack/plugins/alerting/server/routes/rule_types.test.ts
@@ -49,6 +49,7 @@ describe('ruleTypesRoute', () => {
         defaultActionGroupId: 'default',
         minimumLicenseRequired: 'basic',
         isExportable: true,
+        ruleTaskTimeout: '10m',
         recoveryActionGroup: RecoveredActionGroup,
         authorizedConsumers: {},
         actionVariables: {
@@ -76,6 +77,7 @@ describe('ruleTypesRoute', () => {
         minimum_license_required: 'basic',
         minimum_schedule_interval: '1m',
         is_exportable: true,
+        rule_task_timeout: '10m',
         recovery_action_group: RecoveredActionGroup,
         authorized_consumers: {},
         action_variables: {
@@ -118,6 +120,7 @@ describe('ruleTypesRoute', () => {
               "id": "recovered",
               "name": "Recovered",
             },
+            "rule_task_timeout": "10m",
           },
         ],
       }

diff --git a/x-pack/plugins/alerting/server/routes/rule_types.ts b/x-pack/plugins/alerting/server/routes/rule_types.ts
@@ -20,6 +20,7 @@ const rewriteBodyRes: RewriteResponseCase<RegistryAlertTypeWithAuth[]> = (result
       defaultActionGroupId,
       minimumLicenseRequired,
       isExportable,
+      ruleTaskTimeout,
       actionVariables,
       authorizedConsumers,
       minimumScheduleInterval,
@@ -33,6 +34,7 @@ const rewriteBodyRes: RewriteResponseCase<RegistryAlertTypeWithAuth[]> = (result
       default_action_group_id: defaultActionGroupId,
       minimum_license_required: minimumLicenseRequired,
       is_exportable: isExportable,
+      rule_task_timeout: ruleTaskTimeout,
       action_variables: actionVariables,
       authorized_consumers: authorizedConsumers,
       minimum_schedule_interval: minimumScheduleInterval,

diff --git a/x-pack/plugins/alerting/server/routes/update_rule.ts b/x-pack/plugins/alerting/server/routes/update_rule.ts
@@ -88,6 +88,7 @@ const rewriteBodyRes: RewriteResponseCase<PartialAlert<AlertTypeParams>> = ({
         execution_status: {
           status: executionStatus.status,
           last_execution_date: executionStatus.lastExecutionDate,
+          last_duration: executionStatus.lastDuration,
         },
       }
     : {}),

diff --git a/x-pack/plugins/alerting/server/rule_type_registry.test.ts b/x-pack/plugins/alerting/server/rule_type_registry.test.ts
@@ -494,6 +494,7 @@ describe('list()', () => {
       ],
       defaultActionGroupId: 'testActionGroup',
       isExportable: true,
+      ruleTaskTimeout: '20m',
       minimumLicenseRequired: 'basic',
       executor: jest.fn(),
       producer: 'alerts',
@@ -530,6 +531,7 @@ describe('list()', () => {
             "id": "recovered",
             "name": "Recovered",
           },
+          "ruleTaskTimeout": "20m",
         },
       }
     `);

diff --git a/x-pack/plugins/alerting/server/rule_type_registry.ts b/x-pack/plugins/alerting/server/rule_type_registry.ts
@@ -48,6 +48,7 @@ export interface RegistryRuleType
     | 'producer'
     | 'minimumLicenseRequired'
     | 'isExportable'
+    | 'ruleTaskTimeout'
     | 'minimumScheduleInterval'
     | 'defaultScheduleInterval'
   > {
@@ -327,6 +328,7 @@ export class RuleTypeRegistry {
             producer,
             minimumLicenseRequired,
             isExportable,
+            ruleTaskTimeout,
             minimumScheduleInterval,
             defaultScheduleInterval,
           },
@@ -340,6 +342,7 @@ export class RuleTypeRegistry {
           producer,
           minimumLicenseRequired,
           isExportable,
+          ruleTaskTimeout,
           minimumScheduleInterval,
           defaultScheduleInterval,
           enabledInLicense: !!this.licenseState.getLicenseCheckForAlertType(

diff --git a/x-pack/plugins/alerting/server/rules_client/rules_client.ts b/x-pack/plugins/alerting/server/rules_client/rules_client.ts
@@ -1133,6 +1133,7 @@ export class RulesClient {
         updatedAt: new Date().toISOString(),
         executionStatus: {
           status: 'pending',
+          lastDuration: 0,
           lastExecutionDate: new Date().toISOString(),
           error: null,
         },

diff --git a/x-pack/plugins/alerting/server/rules_client/tests/enable.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/enable.test.ts
@@ -260,6 +260,7 @@ describe('enable()', () => {
         ],
         executionStatus: {
           status: 'pending',
+          lastDuration: 0,
           lastExecutionDate: '2019-02-12T21:01:22.479Z',
           error: null,
         },
@@ -369,6 +370,7 @@ describe('enable()', () => {
         ],
         executionStatus: {
           status: 'pending',
+          lastDuration: 0,
           lastExecutionDate: '2019-02-12T21:01:22.479Z',
           error: null,
         },

diff --git a/x-pack/plugins/alerting/server/saved_objects/mappings.json b/x-pack/plugins/alerting/server/saved_objects/mappings.json
@@ -101,6 +101,9 @@
           "lastExecutionDate": {
             "type": "date"
           },
+          "lastDuration": {
+            "type": "long"
+          },
           "error": {
             "properties": {
               "reason": {