Skip to content

Commit

Permalink
Skip scheduling actions for the alerts without scheduledActions (elas…
Browse files Browse the repository at this point in the history
…tic#195948)

Resolves: elastic#190258

As a result of elastic#190258, we have found out that the odd behaviour happens
when an existing alert is pushed above the max alerts limit by a new
alert.

Scenario:

1. The rule type detects 4 alerts (`alert-1`, `alert-2`, `alert-3`,
`alert-4`),
But reports only the first 3 as the max alerts limit is 3.

2. `alert-2` becomes recovered, therefore the rule type reports 3 active
(`alert-1`, `alert-3`, `alert-4`), 1 recovered (`alert-2`) alert.

3. Alerts `alert-1`,  `alert-3`, `alert-4` are saved in the task state.

4. `alert-2` becomes active again (the others are still active)

5. Rule type reports 3 active alerts (`alert-1`,  `alert-2`, `alert-3`)

6. As a result, the action scheduler tries to schedule actions for
`alert-1`, `alert-3`, `alert-4` as they are the existing alerts.
But, since the rule type didn't report the `alert-4` it has no scheduled
actions, therefore the action scheduler assumes it is recovered and
tries to schedule a recovery action.

This PR changes the actionScheduler to handle active and recovered
alerts separately.
With this change, no action would be scheduled for an alert from
previous run (exists in the task state) and isn't reported by the
ruleTypeExecutor due to max-alerts-limit but it would be kept in the
task state.
  • Loading branch information
ersin-erdal authored Oct 15, 2024
1 parent c448593 commit dd25bf8
Show file tree
Hide file tree
Showing 12 changed files with 542 additions and 196 deletions.
7 changes: 5 additions & 2 deletions x-pack/plugins/alerting/server/alerts_client/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,11 @@ export interface IAlertsClient<
processAlerts(opts: ProcessAlertsOpts): void;
logAlerts(opts: LogAlertsOpts): void;
getProcessedAlerts(
type: 'new' | 'active' | 'activeCurrent' | 'recovered' | 'recoveredCurrent'
): Record<string, LegacyAlert<State, Context, ActionGroupIds | RecoveryActionGroupId>>;
type: 'new' | 'active' | 'activeCurrent'
): Record<string, LegacyAlert<State, Context, ActionGroupIds>> | {};
getProcessedAlerts(
type: 'recovered' | 'recoveredCurrent'
): Record<string, LegacyAlert<State, Context, RecoveryActionGroupId>> | {};
persistAlerts(): Promise<{ alertIds: string[]; maintenanceWindowIds: string[] } | null>;
isTrackedAlert(id: string): boolean;
getSummarizedAlerts?(params: GetSummarizedAlertsParams): Promise<SummarizedAlerts>;
Expand Down
Loading

0 comments on commit dd25bf8

Please sign in to comment.