replace actionHash with uuid (#151997)

Resolves: #149831

This PR, replaces the `actionHash` that is used as an identifier of the
throttled actions in the task state, with `uuid`.

## To verify:
Run Es with `path.data`
Create a rule that is creating multiple alerts, in v8.7 (or main branch)
with two actions,
one action `For each alert` and `On custom interval` => `{throttle: 1h,
summary: false, notifyWhen: onThrottleInterval}`
one action `Summary of alerts` and `On custom interval` => `{throttle:
1h, summary: true, notifyWhen: onThrottleInterval}`

Observe the task status like below:
```
{
  "alertTypeState": {...},
  "alertInstances": {
    "host-1": {
      ...
      "meta": {
        ...
        "lastScheduledActions": {
          ...
          "actions": {
            ".server-log:metrics.threshold.fired:1h": {
              "date": "2023-02-23T14:37:48.285Z"
            }
          }
        }
      }
    },
    "host-2": {
     ....
      "meta": {
       ...
        "lastScheduledActions": {
          ...
          "actions": {
            ".server-log:metrics.threshold.fired:1h": {
              "date": "2023-02-23T14:37:48.285Z"
            }
          }
        }
      }
    }
  },
  "summaryActions": {
    ".server-log:metrics.threshold.fired:1h": {
      "date": "2023-02-23T14:36:46.085Z"
    }
  },
}

```

Stop Es and Kibana, and switch to this branch.
Run ES with the same `path.data` again.
The rule still must be running as it was (throttling) and the action
identifier in the task state should be replaced with uuid like shown
below.

```
{
  "alertTypeState": {...},
  "alertInstances": {
    "host-1": {
      ...
      "meta": {
        ...
        "lastScheduledActions": {
          ...
          "actions": {
            "34f542f2-4bc1-4434-94d1-8db5a3a3d282": { <== here comes a uuid
              "date": "2023-02-23T14:37:48.285Z"
            }
          }
        }
      }
    },
    "host-2": {
     ....
      "meta": {
       ...
        "lastScheduledActions": {
          ...
          "actions": {
            "34f542f2-4bc1-4434-94d1-8db5a3a3d282": {
              "date": "2023-02-23T14:37:48.285Z"
            }
          }
        }
      }
    }
  },
  "summaryActions": {
    "34f542f2-4bc1-4434-94d1-8db5a3a3d282": { <=== and here
      "date": "2023-02-23T14:36:46.085Z"
    }
  },
}

```
This commit is contained in:
Ersin Erdal 2023-03-08 00:10:51 +01:00 committed by GitHub
parent 18aa846ef2
commit b78c3a1ef8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 274 additions and 102 deletions

View file

@ -57,7 +57,7 @@ describe('checking migration metadata changes on all registered SO types', () =>
Object {
"action": "6cfc277ed3211639e37546ac625f4a68f2494215",
"action_task_params": "db2afea7d78e00e725486b791554d0d4e81956ef",
"alert": "2568bf6d8ba0876441c61c9e58e08016c1dc1617",
"alert": "785240e3137f5eb1a0f8986e5b8eff99780fc04f",
"api_key_pending_invalidation": "16e7bcf8e78764102d7f525542d5b616809a21ee",
"apm-indices": "d19dd7fb51f2d2cbc1f8769481721e0953f9a6d2",
"apm-server-schema": "1d42f17eff9ec6c16d3a9324d9539e2d123d0a9a",

View file

@ -52,6 +52,25 @@ describe('isThrottled', () => {
expect(alert.isThrottled({ throttle: '1m' })).toEqual(true);
});
test(`should use actionHash if it was used in a legacy action`, () => {
const alert = new Alert<AlertInstanceState, AlertInstanceContext, DefaultActionGroupId>('1', {
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
actions: {
'slack:alert:1h': { date: new Date() },
},
},
},
});
clock.tick(30000);
alert.scheduleActions('default');
expect(
alert.isThrottled({ throttle: '1m', actionHash: 'slack:alert:1h', uuid: '111-222' })
).toEqual(true);
});
test(`shouldn't throttle when group didn't change and throttle period expired`, () => {
const alert = new Alert<AlertInstanceState, AlertInstanceContext, DefaultActionGroupId>('1', {
meta: {
@ -87,14 +106,14 @@ describe('isThrottled', () => {
date: new Date(),
group: 'default',
actions: {
'slack:1h': { date: new Date() },
'111-111': { date: new Date() },
},
},
},
});
clock.tick(5000);
alert.scheduleActions('other-group');
expect(alert.isThrottled({ throttle: '1m', actionHash: 'slack:1h' })).toEqual(false);
expect(alert.isThrottled({ throttle: '1m', uuid: '111-111' })).toEqual(false);
});
test(`shouldn't throttle a specific action when group didn't change and throttle period expired`, () => {
@ -104,14 +123,16 @@ describe('isThrottled', () => {
date: new Date('2020-01-01'),
group: 'default',
actions: {
'slack:1h': { date: new Date() },
'111-111': { date: new Date() },
},
},
},
});
clock.tick(30000);
alert.scheduleActions('default');
expect(alert.isThrottled({ throttle: '15s', actionHash: 'slack:1h' })).toEqual(false);
expect(alert.isThrottled({ throttle: '15s', uuid: '111-111', actionHash: 'slack:1h' })).toEqual(
false
);
});
test(`shouldn't throttle a specific action when group changes`, () => {
@ -121,14 +142,14 @@ describe('isThrottled', () => {
date: new Date(),
group: 'default',
actions: {
'slack:1h': { date: new Date() },
'111-111': { date: new Date() },
},
},
},
});
clock.tick(5000);
alert.scheduleActions('other-group');
expect(alert.isThrottled({ throttle: '1m', actionHash: 'slack:1h' })).toEqual(false);
expect(alert.isThrottled({ throttle: '1m', uuid: '111-111' })).toEqual(false);
});
});
@ -312,7 +333,7 @@ describe('updateLastScheduledActions()', () => {
const alert = new Alert<AlertInstanceState, AlertInstanceContext, DefaultActionGroupId>('1', {
meta: {},
});
alert.updateLastScheduledActions('default', 'actionId1');
alert.updateLastScheduledActions('default', 'actionId1', '111-111');
expect(alert.toJSON()).toEqual({
state: {},
meta: {
@ -321,7 +342,36 @@ describe('updateLastScheduledActions()', () => {
date: new Date().toISOString(),
group: 'default',
actions: {
actionId1: { date: new Date().toISOString() },
'111-111': { date: new Date().toISOString() },
},
},
},
});
});
test('removes the objects with an old actionHash', () => {
const alert = new Alert<AlertInstanceState, AlertInstanceContext, DefaultActionGroupId>('1', {
meta: {
flappingHistory: [],
lastScheduledActions: {
date: new Date(),
group: 'default',
actions: {
'slack:alert:1h': { date: new Date() },
},
},
},
});
alert.updateLastScheduledActions('default', 'slack:alert:1h', '111-111');
expect(alert.toJSON()).toEqual({
state: {},
meta: {
flappingHistory: [],
lastScheduledActions: {
date: new Date().toISOString(),
group: 'default',
actions: {
'111-111': { date: new Date().toISOString() },
},
},
},

View file

@ -67,7 +67,15 @@ export class Alert<
return this.scheduledExecutionOptions !== undefined;
}
isThrottled({ throttle, actionHash }: { throttle: string | null; actionHash?: string }) {
isThrottled({
throttle,
actionHash,
uuid,
}: {
throttle: string | null;
actionHash?: string;
uuid?: string;
}) {
if (this.scheduledExecutionOptions === undefined) {
return false;
}
@ -79,9 +87,12 @@ export class Alert<
this.scheduledExecutionOptions
)
) {
if (actionHash) {
if (uuid && actionHash) {
if (this.meta.lastScheduledActions.actions) {
const lastTriggerDate = this.meta.lastScheduledActions.actions[actionHash]?.date;
const actionInState =
this.meta.lastScheduledActions.actions[uuid] ||
this.meta.lastScheduledActions.actions[actionHash]; // actionHash must be removed once all the hash identifiers removed from the task state
const lastTriggerDate = actionInState?.date;
return !!(lastTriggerDate && lastTriggerDate.getTime() + throttleMills > Date.now());
}
return false;
@ -169,7 +180,7 @@ export class Alert<
return this;
}
updateLastScheduledActions(group: ActionGroupIds, actionHash?: string | null) {
updateLastScheduledActions(group: ActionGroupIds, actionHash?: string | null, uuid?: string) {
if (!this.meta.lastScheduledActions) {
this.meta.lastScheduledActions = {} as LastScheduledActions;
}
@ -179,11 +190,15 @@ export class Alert<
if (this.meta.lastScheduledActions.group !== group) {
this.meta.lastScheduledActions.actions = {};
} else if (actionHash) {
} else if (uuid) {
if (!this.meta.lastScheduledActions.actions) {
this.meta.lastScheduledActions.actions = {};
}
this.meta.lastScheduledActions.actions[actionHash] = { date };
// remove deprecated actionHash
if (!!actionHash && this.meta.lastScheduledActions.actions[actionHash]) {
delete this.meta.lastScheduledActions.actions[actionHash];
}
this.meta.lastScheduledActions.actions[uuid] = { date };
}
}

View file

@ -7,7 +7,6 @@
import { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server';
import { EncryptedSavedObjectsPluginSetup } from '@kbn/encrypted-saved-objects-plugin/server';
import { v4 as uuidv4 } from 'uuid';
import { extractedSavedObjectParamReferenceNamePrefix } from '../../../rules_client/common/constants';
import {
createEsoMigration,
@ -38,27 +37,6 @@ function addGroupByToEsQueryRule(
return doc;
}
function addActionUuid(
doc: SavedObjectUnsanitizedDoc<RawRule>
): SavedObjectUnsanitizedDoc<RawRule> {
const {
attributes: { actions },
} = doc;
return {
...doc,
attributes: {
...doc.attributes,
actions: actions
? actions.map((action) => ({
...action,
uuid: uuidv4(),
}))
: [],
},
};
}
function addLogViewRefToLogThresholdRule(
doc: SavedObjectUnsanitizedDoc<RawRule>
): SavedObjectUnsanitizedDoc<RawRule> {
@ -116,10 +94,5 @@ export const getMigrations870 = (encryptedSavedObjects: EncryptedSavedObjectsPlu
createEsoMigration(
encryptedSavedObjects,
(doc: SavedObjectUnsanitizedDoc<RawRule>): doc is SavedObjectUnsanitizedDoc<RawRule> => true,
pipeMigrations(
addGroupByToEsQueryRule,
addLogViewRefToLogThresholdRule,
addOutcomeOrder,
addActionUuid
)
pipeMigrations(addGroupByToEsQueryRule, addLogViewRefToLogThresholdRule, addOutcomeOrder)
);

View file

@ -0,0 +1,40 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { SavedObjectUnsanitizedDoc } from '@kbn/core-saved-objects-server';
import { EncryptedSavedObjectsPluginSetup } from '@kbn/encrypted-saved-objects-plugin/server';
import { v4 as uuidv4 } from 'uuid';
import { createEsoMigration, pipeMigrations } from '../utils';
import { RawRule } from '../../../types';
function addActionUuid(
doc: SavedObjectUnsanitizedDoc<RawRule>
): SavedObjectUnsanitizedDoc<RawRule> {
const {
attributes: { actions },
} = doc;
return {
...doc,
attributes: {
...doc.attributes,
actions: actions
? actions.map((action) => ({
...action,
uuid: uuidv4(),
}))
: [],
},
};
}
export const getMigrations880 = (encryptedSavedObjects: EncryptedSavedObjectsPluginSetup) =>
createEsoMigration(
encryptedSavedObjects,
(doc: SavedObjectUnsanitizedDoc<RawRule>): doc is SavedObjectUnsanitizedDoc<RawRule> => true,
pipeMigrations(addActionUuid)
);

View file

@ -2628,9 +2628,11 @@ describe('successful migrations', () => {
outcomeOrder: 0,
});
});
});
describe('8.8.0', () => {
test('adds uuid to rule actions', () => {
const migration870 = getMigrations(encryptedSavedObjectsSetup, {}, isPreconfigured)['8.7.0'];
const migration880 = getMigrations(encryptedSavedObjectsSetup, {}, isPreconfigured)['8.8.0'];
const rule = getMockData(
{
params: { foo: true },
@ -2638,9 +2640,9 @@ describe('successful migrations', () => {
},
true
);
const migratedAlert870 = migration870(rule, migrationContext);
const migratedAlert880 = migration880(rule, migrationContext);
expect(migratedAlert870.attributes.actions).toEqual([
expect(migratedAlert880.attributes.actions).toEqual([
{
group: 'default',
actionRef: '1',

View file

@ -30,6 +30,7 @@ import { getMigrations841 } from './8.4';
import { getMigrations850 } from './8.5';
import { getMigrations860 } from './8.6';
import { getMigrations870 } from './8.7';
import { getMigrations880 } from './8.8';
import { AlertLogMeta, AlertMigration } from './types';
import { MINIMUM_SS_MIGRATION_VERSION } from './constants';
import { createEsoMigration, isEsQueryRuleType, pipeMigrations } from './utils';
@ -79,6 +80,7 @@ export function getMigrations(
'8.5.0': executeMigrationWithErrorHandling(getMigrations850(encryptedSavedObjects), '8.5.0'),
'8.6.0': executeMigrationWithErrorHandling(getMigrations860(encryptedSavedObjects), '8.6.0'),
'8.7.0': executeMigrationWithErrorHandling(getMigrations870(encryptedSavedObjects), '8.7.0'),
'8.8.0': executeMigrationWithErrorHandling(getMigrations880(encryptedSavedObjects), '8.8.0'),
},
getSearchSourceMigrations(encryptedSavedObjects, searchSourceMigrations)
);

View file

@ -92,6 +92,7 @@ const rule = {
stateVal: 'My {{state.value}} goes here',
alertVal: 'My {{alertId}} {{alertName}} {{spaceId}} {{tags}} {{alertInstanceId}} goes here',
},
uuid: '111-111',
},
],
} as unknown as SanitizedRule<RuleTypeParams>;
@ -774,7 +775,7 @@ describe('Execution Handler', () => {
await executionHandler.run(
generateAlert({
id: 1,
throttledActions: { 'test:default:1h': { date: new Date(DATE_1970) } },
throttledActions: { '111-111': { date: new Date(DATE_1970) } },
})
);
@ -982,6 +983,7 @@ describe('Execution Handler', () => {
message:
'New: {{alerts.new.count}} Ongoing: {{alerts.ongoing.count}} Recovered: {{alerts.recovered.count}}',
},
uuid: '111-111',
},
],
},
@ -998,8 +1000,8 @@ describe('Execution Handler', () => {
excludedAlertInstanceIds: ['foo'],
});
expect(result).toEqual({
throttledActions: {
'testActionTypeId:summary:1d': {
throttledSummaryActions: {
'111-111': {
date: new Date(),
},
},
@ -1067,13 +1069,14 @@ describe('Execution Handler', () => {
message:
'New: {{alerts.new.count}} Ongoing: {{alerts.ongoing.count}} Recovered: {{alerts.recovered.count}}',
},
uuid: '111-111',
},
],
},
taskInstance: {
state: {
...defaultExecutionParams.taskInstance.state,
summaryActions: { 'testActionTypeId:summary:1d': { date: new Date() } },
summaryActions: { '111-111': { date: new Date() } },
},
} as unknown as ConcreteTaskInstance,
})
@ -1112,6 +1115,7 @@ describe('Execution Handler', () => {
params: {
message: 'New: {{alerts.new.count}}',
},
uuid: '111-111',
},
{
id: '2',
@ -1122,6 +1126,7 @@ describe('Execution Handler', () => {
notifyWhen: 'onThrottleInterval',
throttle: '10d',
},
uuid: '222-222',
},
],
},
@ -1129,9 +1134,9 @@ describe('Execution Handler', () => {
state: {
...defaultExecutionParams.taskInstance.state,
summaryActions: {
'testActionTypeId:summary:1d': { date: new Date() },
'testActionTypeId:summary:10d': { date: new Date() },
'testActionTypeId:summary:10m': { date: new Date() }, // does not exist in the actions list
'111-111': { date: new Date() },
'222-222': { date: new Date() },
'333-333': { date: new Date() }, // does not exist in the actions list
},
},
} as unknown as ConcreteTaskInstance,
@ -1140,11 +1145,11 @@ describe('Execution Handler', () => {
const result = await executionHandler.run({});
expect(result).toEqual({
throttledActions: {
'testActionTypeId:summary:1d': {
throttledSummaryActions: {
'111-111': {
date: new Date(),
},
'testActionTypeId:summary:10d': {
'222-222': {
date: new Date(),
},
},

View file

@ -34,7 +34,7 @@ import {
import {
generateActionHash,
getSummaryActionsFromTaskState,
isSummaryActionOnInterval,
isActionOnInterval,
isSummaryAction,
isSummaryActionThrottled,
isSummaryActionPerRuleRun,
@ -47,7 +47,7 @@ enum Reasons {
}
export interface RunResult {
throttledActions: ThrottledActions;
throttledSummaryActions: ThrottledActions;
}
export class ExecutionHandler<
@ -129,11 +129,11 @@ export class ExecutionHandler<
public async run(
alerts: Record<string, Alert<State, Context, ActionGroupIds | RecoveryActionGroupId>>
): Promise<RunResult> {
const executables = this.generateExecutables(alerts);
const throttledActions: ThrottledActions = getSummaryActionsFromTaskState({
const throttledSummaryActions: ThrottledActions = getSummaryActionsFromTaskState({
actions: this.rule.actions,
summaryActions: this.taskInstance.state?.summaryActions,
});
const executables = this.generateExecutables(alerts, throttledSummaryActions);
if (!!executables.length) {
const {
@ -232,8 +232,8 @@ export class ExecutionHandler<
bulkActions,
});
if (isSummaryActionOnInterval(action)) {
throttledActions[generateActionHash(action)] = { date: new Date() };
if (isActionOnInterval(action)) {
throttledSummaryActions[action.uuid!] = { date: new Date() };
}
logActions.push({
@ -289,10 +289,11 @@ export class ExecutionHandler<
});
if (!this.isRecoveredAlert(actionGroup)) {
if (isSummaryActionOnInterval(action)) {
if (isActionOnInterval(action)) {
executableAlert.updateLastScheduledActions(
action.group as ActionGroupIds,
generateActionHash(action)
generateActionHash(action),
action.uuid
);
} else {
executableAlert.updateLastScheduledActions(action.group as ActionGroupIds);
@ -314,7 +315,7 @@ export class ExecutionHandler<
}
}
}
return { throttledActions };
return { throttledSummaryActions };
}
private hasAlerts(
@ -379,7 +380,8 @@ export class ExecutionHandler<
const throttled = action.frequency?.throttle
? alert.isThrottled({
throttle: action.frequency.throttle ?? null,
actionHash: generateActionHash(action),
actionHash: generateActionHash(action), // generateActionHash must be removed once all the hash identifiers removed from the task state
uuid: action.uuid,
})
: alert.isThrottled({ throttle: rule.throttle ?? null });
@ -462,7 +464,8 @@ export class ExecutionHandler<
}
private generateExecutables(
alerts: Record<string, Alert<State, Context, ActionGroupIds | RecoveryActionGroupId>>
alerts: Record<string, Alert<State, Context, ActionGroupIds | RecoveryActionGroupId>>,
summaryActions: ThrottledActions
) {
const executables = [];
@ -472,7 +475,7 @@ export class ExecutionHandler<
this.canFetchSummarizedAlerts(action) &&
!isSummaryActionThrottled({
action,
summaryActions: this.taskInstance.state?.summaryActions,
summaryActions,
logger: this.logger,
})
) {
@ -525,7 +528,7 @@ export class ExecutionHandler<
}) {
let options;
if (isSummaryActionOnInterval(action)) {
if (isActionOnInterval(action)) {
const throttleMills = parseDuration(action.frequency!.throttle!);
const start = new Date(Date.now() - throttleMills);

View file

@ -10,7 +10,7 @@ import { RuleAction } from '../types';
import {
generateActionHash,
getSummaryActionsFromTaskState,
isSummaryActionOnInterval,
isActionOnInterval,
isSummaryAction,
isSummaryActionThrottled,
} from './rule_action_helper';
@ -46,6 +46,7 @@ const mockSummaryAction: RuleAction = {
notifyWhen: 'onThrottleInterval',
throttle: '1d',
},
uuid: '111-111',
};
describe('rule_action_helper', () => {
@ -63,16 +64,26 @@ describe('rule_action_helper', () => {
const result = isSummaryAction(mockSummaryAction);
expect(result).toBe(true);
});
test('should return false if the action is undefined', () => {
const result = isSummaryAction(undefined);
expect(result).toBe(false);
});
test('should return false if the action is not a proper RuleAction', () => {
const result = isSummaryAction({} as RuleAction);
expect(result).toBe(false);
});
});
describe('isSummaryActionOnInterval', () => {
describe('isActionOnInterval', () => {
test('should return false if the action does not have frequency field', () => {
const result = isSummaryActionOnInterval(mockOldAction);
const result = isActionOnInterval(mockOldAction);
expect(result).toBe(false);
});
test('should return false if notifyWhen is not onThrottleInterval', () => {
const result = isSummaryActionOnInterval({
const result = isActionOnInterval({
...mockAction,
frequency: { ...mockAction.frequency, notifyWhen: 'onActiveAlert' },
} as RuleAction);
@ -80,7 +91,7 @@ describe('rule_action_helper', () => {
});
test('should return false if throttle is not a valid interval string', () => {
const result = isSummaryActionOnInterval({
const result = isActionOnInterval({
...mockAction,
frequency: { ...mockAction.frequency, throttle: null },
} as RuleAction);
@ -88,9 +99,19 @@ describe('rule_action_helper', () => {
});
test('should return true if the action is a throttling action', () => {
const result = isSummaryActionOnInterval(mockSummaryAction);
const result = isActionOnInterval(mockSummaryAction);
expect(result).toBe(true);
});
test('should return false if the action undefined', () => {
const result = isActionOnInterval(undefined);
expect(result).toBe(false);
});
test('should return false if the action is not a proper RuleAction', () => {
const result = isActionOnInterval({} as RuleAction);
expect(result).toBe(false);
});
});
describe('generateActionHash', () => {
@ -108,6 +129,11 @@ describe('rule_action_helper', () => {
const result = generateActionHash(mockSummaryAction);
expect(result).toBe('slack:summary:1d');
});
test('should return a hash for a broken summary action', () => {
const result = generateActionHash(undefined);
expect(result).toBe('no-action-type-id:no-action-group:no-throttling');
});
});
describe('getSummaryActionsFromTaskState', () => {
@ -115,11 +141,21 @@ describe('rule_action_helper', () => {
const result = getSummaryActionsFromTaskState({
actions: [mockSummaryAction],
summaryActions: {
'slack:summary:1d': { date: new Date('01.01.2020') },
'slack:summary:2d': { date: new Date('01.01.2020') },
'111-111': { date: new Date('01.01.2020') },
'222-222': { date: new Date('01.01.2020') },
},
});
expect(result).toEqual({ 'slack:summary:1d': { date: new Date('01.01.2020') } });
expect(result).toEqual({ '111-111': { date: new Date('01.01.2020') } });
});
test('should replace hash with uuid', () => {
const result = getSummaryActionsFromTaskState({
actions: [mockSummaryAction],
summaryActions: {
'slack:summary:1d': { date: new Date('01.01.2020') },
},
});
expect(result).toEqual({ '111-111': { date: new Date('01.01.2020') } });
});
});
@ -131,12 +167,15 @@ describe('rule_action_helper', () => {
beforeEach(() => {
jest.setSystemTime(new Date('2020-01-01T23:00:00.000Z').getTime());
});
afterEach(() => {
jest.clearAllMocks();
});
afterAll(() => {
jest.useRealTimers();
});
const logger = { debug: jest.fn } as unknown as Logger;
const summaryActions = { 'slack:summary:1d': { date: new Date('2020-01-01T00:00:00.000Z') } };
const logger = { debug: jest.fn() } as unknown as Logger;
const summaryActions = { '111-111': { date: new Date('2020-01-01T00:00:00.000Z') } };
test('should return false if the action does not have throttle filed', () => {
const result = isSummaryActionThrottled({
@ -183,7 +222,7 @@ describe('rule_action_helper', () => {
test('should return false if the action is not in the task instance', () => {
const result = isSummaryActionThrottled({
action: mockSummaryAction,
summaryActions: { 'slack:summary:2d': { date: new Date('2020-01-01T00:00:00.000Z') } },
summaryActions: { '123-456': { date: new Date('2020-01-01T00:00:00.000Z') } },
logger,
});
expect(result).toBe(false);
@ -193,7 +232,7 @@ describe('rule_action_helper', () => {
jest.advanceTimersByTime(3600000 * 2);
const result = isSummaryActionThrottled({
action: mockSummaryAction,
summaryActions: { 'slack:summary:1d': { date: new Date('2020-01-01T00:00:00.000Z') } },
summaryActions: { '123-456': { date: new Date('2020-01-01T00:00:00.000Z') } },
logger,
});
expect(result).toBe(false);
@ -207,5 +246,42 @@ describe('rule_action_helper', () => {
});
expect(result).toBe(true);
});
test('should return false if the action is broken', () => {
const result = isSummaryActionThrottled({
action: undefined,
summaryActions,
logger,
});
expect(result).toBe(false);
});
test('should return false if there is no summary action in the state', () => {
const result = isSummaryActionThrottled({
action: mockSummaryAction,
summaryActions: undefined,
logger,
});
expect(result).toBe(false);
});
test('should return false if the actions throttle interval is not valid', () => {
const result = isSummaryActionThrottled({
action: {
...mockSummaryAction,
frequency: {
summary: true,
notifyWhen: 'onThrottleInterval',
throttle: '1',
},
},
summaryActions,
logger,
});
expect(result).toBe(false);
expect(logger.debug).toHaveBeenCalledWith(
"Action'slack:1', has an invalid throttle interval"
);
});
});
});

View file

@ -13,12 +13,12 @@ import {
ThrottledActions,
} from '../../common';
export const isSummaryAction = (action: RuleAction) => {
return action.frequency?.summary || false;
export const isSummaryAction = (action?: RuleAction) => {
return action?.frequency?.summary || false;
};
export const isSummaryActionOnInterval = (action: RuleAction) => {
if (!action.frequency) {
export const isActionOnInterval = (action?: RuleAction) => {
if (!action?.frequency) {
return false;
}
return (
@ -42,36 +42,41 @@ export const isSummaryActionThrottled = ({
summaryActions,
logger,
}: {
action: RuleAction;
action?: RuleAction;
summaryActions?: ThrottledActions;
logger: Logger;
}) => {
if (!isSummaryActionOnInterval(action)) {
if (!isActionOnInterval(action)) {
return false;
}
if (!summaryActions) {
return false;
}
const hash = generateActionHash(action);
const triggeredSummaryAction = summaryActions[hash];
const triggeredSummaryAction = summaryActions[action?.uuid!];
if (!triggeredSummaryAction) {
return false;
}
const throttleMills = parseDuration(action.frequency!.throttle!);
let throttleMills = 0;
try {
throttleMills = parseDuration(action?.frequency!.throttle!);
} catch (e) {
logger.debug(`Action'${action?.actionTypeId}:${action?.id}', has an invalid throttle interval`);
}
const throttled = triggeredSummaryAction.date.getTime() + throttleMills > Date.now();
if (throttled) {
logger.debug(
`skipping scheduling the action '${action.actionTypeId}:${action.id}', summary action is still being throttled`
`skipping scheduling the action '${action?.actionTypeId}:${action?.id}', summary action is still being throttled`
);
}
return throttled;
};
export const generateActionHash = (action: RuleAction) => {
return `${action.actionTypeId}:${action.frequency?.summary ? 'summary' : action.group}:${
action.frequency?.throttle || 'no-throttling'
}`;
export const generateActionHash = (action?: RuleAction) => {
return `${action?.actionTypeId || 'no-action-type-id'}:${
action?.frequency?.summary ? 'summary' : action?.group || 'no-action-group'
}:${action?.frequency?.throttle || 'no-throttling'}`;
};
export const getSummaryActionsFromTaskState = ({
@ -82,11 +87,12 @@ export const getSummaryActionsFromTaskState = ({
summaryActions?: ThrottledActions;
}) => {
return Object.entries(summaryActions).reduce((newObj, [key, val]) => {
const actionExists = actions.some(
(action) => action.frequency?.summary && generateActionHash(action) === key
const actionExists = actions.find(
(action) =>
action.frequency?.summary && (action.uuid === key || generateActionHash(action) === key)
);
if (actionExists) {
return { ...newObj, [key]: val };
return { ...newObj, [actionExists.uuid!]: val }; // replace hash with uuid
} else {
return newObj;
}

View file

@ -1486,7 +1486,7 @@ describe('Task Runner', () => {
generateEnqueueFunctionInput({ isBulk, id: '1', foo: true })
);
expect(result.state.summaryActions).toEqual({
'slack:summary:1h': { date: new Date(DATE_1970) },
'111-111': { date: new Date(DATE_1970) },
});
}
);

View file

@ -446,7 +446,7 @@ export class TaskRunner<
actionsClient: await this.context.actionsPlugin.getActionsClientWithRequest(fakeRequest),
});
let executionHandlerRunResult: RunResult = { throttledActions: {} };
let executionHandlerRunResult: RunResult = { throttledSummaryActions: {} };
await this.timer.runWithTimer(TaskRunnerTimerSpan.TriggerActions, async () => {
await rulesClient.clearExpiredSnoozes({ id: rule.id });
@ -484,7 +484,7 @@ export class TaskRunner<
alertTypeState: updatedRuleTypeState || undefined,
alertInstances: alertsToReturn,
alertRecoveredInstances: recoveredAlertsToReturn,
summaryActions: executionHandlerRunResult.throttledActions,
summaryActions: executionHandlerRunResult.throttledSummaryActions,
};
}