Skip to content

Commit

Permalink
Don't trigger summary actions when there are no alerts to report (ela…
Browse files Browse the repository at this point in the history
…stic#156421)

Resolves: elastic#155708

Currently we always trigger summary actions on custom interval even if
there are no alerts to report.
This PR changes this behaviour to skip summary actions when there are no
alerts.

## To verify
Create a Security Rule with a summary action that is on custom interval
(`Summary of alerts` -> `Custom Frequency`)
Add an alerts filter to filter out all the alerts (e.g. by using host
name that doesn't exist)
Expect the summary action not to be triggered.

(cherry picked from commit 506806f)
  • Loading branch information
ersin-erdal committed May 3, 2023
1 parent 7ae26ec commit 7859834
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1299,7 +1299,7 @@ describe('Execution Handler', () => {
`);
});

test('does not schedule actions for the summarized alerts that are filtered out', async () => {
test('does not schedule actions for the summarized alerts that are filtered out (for each alert)', async () => {
getSummarizedAlertsMock.mockResolvedValue({
new: {
count: 0,
Expand Down Expand Up @@ -1362,6 +1362,66 @@ describe('Execution Handler', () => {
);
});

test('does not schedule actions for the summarized alerts that are filtered out (summary of alerts onThrottleInterval)', async () => {
getSummarizedAlertsMock.mockResolvedValue({
new: {
count: 0,
data: [],
},
ongoing: {
count: 0,
data: [],
},
recovered: { count: 0, data: [] },
});
const executionHandler = new ExecutionHandler(
generateExecutionParams({
rule: {
...defaultExecutionParams.rule,
mutedInstanceIds: ['foo'],
actions: [
{
id: '1',
uuid: '111',
group: null,
actionTypeId: 'testActionTypeId',
frequency: {
summary: true,
notifyWhen: 'onThrottleInterval',
throttle: '1h',
},
params: {
message:
'New: {{alerts.new.count}} Ongoing: {{alerts.ongoing.count}} Recovered: {{alerts.recovered.count}}',
},
alertsFilter: {
query: { kql: 'kibana.alert.rule.name:foo', dsl: '{}', filters: [] },
},
},
],
},
})
);

await executionHandler.run({
...generateAlert({ id: 1 }),
...generateAlert({ id: 2 }),
});

expect(getSummarizedAlertsMock).toHaveBeenCalledWith({
ruleId: '1',
spaceId: 'test1',
end: new Date('1970-01-01T00:01:30.000Z'),
start: new Date('1969-12-31T23:01:30.000Z'),
excludedAlertInstanceIds: ['foo'],
alertsFilter: {
query: { kql: 'kibana.alert.rule.name:foo', dsl: '{}', filters: [] },
},
});
expect(actionsClient.bulkEnqueueExecution).not.toHaveBeenCalled();
expect(alertingEventLogger.logAction).not.toHaveBeenCalled();
});

test('does not schedule actions for the for-each type alerts that are filtered out', async () => {
getSummarizedAlertsMock.mockResolvedValue({
new: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import {
isActionOnInterval,
isSummaryAction,
isSummaryActionOnInterval,
isSummaryActionPerRuleRun,
isSummaryActionThrottled,
} from './rule_action_helper';

Expand Down Expand Up @@ -511,10 +510,7 @@ export class ExecutionHandler<
}

if (isSummaryAction(action)) {
if (summarizedAlerts) {
if (isSummaryActionPerRuleRun(action) && summarizedAlerts.all.count === 0) {
continue;
}
if (summarizedAlerts && summarizedAlerts.all.count !== 0) {
executables.push({ action, summarizedAlerts });
}
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,6 @@ export const isActionOnInterval = (action?: RuleAction) => {
);
};

export const isSummaryActionPerRuleRun = (action: RuleAction) => {
if (!action.frequency) {
return false;
}
return action.frequency.notifyWhen === RuleNotifyWhenTypeValues[1] && action.frequency.summary;
};

export const isSummaryActionOnInterval = (action: RuleAction) => {
return isActionOnInterval(action) && action.frequency?.summary;
};
Expand Down

0 comments on commit 7859834

Please sign in to comment.