Skip to content

Commit

Permalink
[8.8] Don't trigger summary actions when there are no alerts to report (
Browse files Browse the repository at this point in the history
#156421) (#156617)

# Backport

This will backport the following commits from `main` to `8.8`:
- [Don't trigger summary actions when there are no alerts to report
(#156421)](#156421)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Ersin
Erdal","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-05-03T19:15:01Z","message":"Don't
trigger summary actions when there are no alerts to report
(#156421)\n\nResolves: #155708\r\n\r\nCurrently we always trigger
summary actions on custom interval even if\r\nthere are no alerts to
report.\r\nThis PR changes this behaviour to skip summary actions when
there are no\r\nalerts.\r\n\r\n## To verify\r\nCreate a Security Rule
with a summary action that is on custom interval\r\n(`Summary of alerts`
-> `Custom Frequency`)\r\nAdd an alerts filter to filter out all the
alerts (e.g. by using host\r\nname that doesn't exist)\r\nExpect the
summary action not to be
triggered.","sha":"506806fee7f374205d98995a2754a0d296fbebb0","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:ResponseOps","v8.8.0","v8.9.0"],"number":156421,"url":"https://github.com/elastic/kibana/pull/156421","mergeCommit":{"message":"Don't
trigger summary actions when there are no alerts to report
(#156421)\n\nResolves: #155708\r\n\r\nCurrently we always trigger
summary actions on custom interval even if\r\nthere are no alerts to
report.\r\nThis PR changes this behaviour to skip summary actions when
there are no\r\nalerts.\r\n\r\n## To verify\r\nCreate a Security Rule
with a summary action that is on custom interval\r\n(`Summary of alerts`
-> `Custom Frequency`)\r\nAdd an alerts filter to filter out all the
alerts (e.g. by using host\r\nname that doesn't exist)\r\nExpect the
summary action not to be
triggered.","sha":"506806fee7f374205d98995a2754a0d296fbebb0"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/156421","number":156421,"mergeCommit":{"message":"Don't
trigger summary actions when there are no alerts to report
(#156421)\n\nResolves: #155708\r\n\r\nCurrently we always trigger
summary actions on custom interval even if\r\nthere are no alerts to
report.\r\nThis PR changes this behaviour to skip summary actions when
there are no\r\nalerts.\r\n\r\n## To verify\r\nCreate a Security Rule
with a summary action that is on custom interval\r\n(`Summary of alerts`
-> `Custom Frequency`)\r\nAdd an alerts filter to filter out all the
alerts (e.g. by using host\r\nname that doesn't exist)\r\nExpect the
summary action not to be
triggered.","sha":"506806fee7f374205d98995a2754a0d296fbebb0"}}]}]
BACKPORT-->

Co-authored-by: Ersin Erdal <[email protected]>
  • Loading branch information
kibanamachine and ersin-erdal authored May 3, 2023
1 parent cde3abf commit c403354
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 c403354

Please sign in to comment.