Skip to content

Commit

Permalink
[ResponseOps] Update flapping logic order to determine whether an ale…
Browse files Browse the repository at this point in the history
…rt is flapping after it's returned for notification (elastic#151148)

Resolves elastic#151135

## Summary

Moved the logic around to check flapping after an alert is already
returned for notification. I also updated the event log test cases to
match examples in the rfc.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### To Verify
The new test cases capture this example but if you want to test it
locally:
- Create a connector and make sure to save the connector id
```
curl -X POST -u {username}:{password} "http://localhost:5601/api/actions/connector" -H 'kbn-xsrf: true' -H 'Content-Type: application/json' -d'
{
  "connector_type_id": ".server-log",
  "name": "server log",
  "config": {},
  "secrets": {}
}'
```
- Create the rule and let it run
```
curl -X POST -u {username}:{password} "http://localhost:5601/api/alerting/rule/" -H 'kbn-xsrf: true' -H 'Content-Type: application/json' -d'
{
  "rule_type_id": "example.pattern",
  "name": "pattern",
  "schedule": {
    "interval": "5s"
  },
  "actions": [
    { "group": "default", "id": {connector id}, "params": { "message": "{{alert.id}} active on run {{context.runs}} step {{context.patternIndex}} flapping {{alert.flapping}}"}},
    { "group": "recovered", "id": {connector id}, "params": { "message": "{{alert.id}} recovered on run flapping {{alert.flapping}}"}}
  ],
  "consumer": "alerts",
  "tags": [],
  "notify_when": "onActionGroupChange",
  "params": {
    "patterns": {
      "instA": " a - - a - a - a - a - - - - - - - - "
    }
  }
}'

```
- Verify that you see the following, we want to make sure that aren't
missing any notifications

![image](https://user-images.githubusercontent.com/109488926/221615405-48061d10-4b80-4b98-812a-6951da4178da.png)
  • Loading branch information
doakalexi authored and bmorelli25 committed Mar 10, 2023
1 parent c2120a5 commit 1b0074f
Show file tree
Hide file tree
Showing 7 changed files with 276 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { createAlertFactory, getPublicAlertFactory } from '../alert/create_alert
import { Alert } from '../alert/alert';
import { alertingEventLoggerMock } from '../lib/alerting_event_logger/alerting_event_logger.mock';
import { ruleRunMetricsStoreMock } from '../lib/rule_run_metrics_store.mock';
import { getAlertsForNotification, processAlerts, setFlapping } from '../lib';
import { getAlertsForNotification, processAlerts } from '../lib';
import { logAlerts } from '../task_runner/log_alerts';
import { DEFAULT_FLAPPING_SETTINGS } from '../../common/rules_settings';

Expand Down Expand Up @@ -254,19 +254,6 @@ describe('Legacy Alerts Client', () => {
flappingSettings: DEFAULT_FLAPPING_SETTINGS,
});

expect(setFlapping).toHaveBeenCalledWith(
{
enabled: true,
lookBackWindow: 20,
statusChangeThreshold: 4,
},
{
'1': new Alert<AlertInstanceContext, AlertInstanceContext>('1', testAlert1),
'2': new Alert<AlertInstanceContext, AlertInstanceContext>('2', testAlert2),
},
{}
);

expect(getAlertsForNotification).toHaveBeenCalledWith(
{
enabled: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,6 @@ export class LegacyAlertsClient<
flappingSettings,
});

setFlapping<State, Context, ActionGroupIds, RecoveryActionGroupId>(
flappingSettings,
processedAlertsActive,
processedAlertsRecovered
);

const { trimmedAlertsRecovered, earlyRecoveredAlerts } = trimRecoveredAlerts(
this.options.logger,
processedAlertsRecovered,
Expand Down Expand Up @@ -213,4 +207,12 @@ export class LegacyAlertsClient<
public getExecutorServices() {
return getPublicAlertFactory(this.alertFactory!);
}

public setFlapping(flappingSettings: RulesSettingsFlappingProperties) {
setFlapping<State, Context, ActionGroupIds, RecoveryActionGroupId>(
flappingSettings,
this.processedAlerts.active,
this.processedAlerts.recovered
);
}
}
2 changes: 2 additions & 0 deletions x-pack/plugins/alerting/server/task_runner/task_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,8 @@ export class TaskRunner<
}
});

this.legacyAlertsClient.setFlapping(flappingSettings);

let alertsToReturn: Record<string, RawAlertInstance> = {};
let recoveredAlertsToReturn: Record<string, RawAlertInstance> = {};
// Only serialize alerts into task state if we're auto-recovering, otherwise
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,7 @@ describe('createLifecycleExecutor', () => {
return { state };
});

await executor(
const serializedAlerts = await executor(
createDefaultAlertExecutorOptions({
alertId: 'TEST_ALERT_0',
params: {},
Expand Down Expand Up @@ -1061,6 +1061,43 @@ describe('createLifecycleExecutor', () => {
})
);

expect(serializedAlerts.state.trackedAlerts).toEqual({
TEST_ALERT_0: {
alertId: 'TEST_ALERT_0',
alertUuid: 'TEST_ALERT_0_UUID',
flapping: true,
flappingHistory: flapping.slice(1).concat([false]),
pendingRecoveredCount: 0,
started: '2020-01-01T12:00:00.000Z',
},
TEST_ALERT_1: {
alertId: 'TEST_ALERT_1',
alertUuid: 'TEST_ALERT_1_UUID',
flapping: false,
flappingHistory: [false, false, false],
pendingRecoveredCount: 0,
started: '2020-01-02T12:00:00.000Z',
},
TEST_ALERT_2: {
alertId: 'TEST_ALERT_2',
alertUuid: 'TEST_ALERT_2_UUID',
flapping: true,
flappingHistory: flapping.slice(1).concat([false]),
pendingRecoveredCount: 0,
started: '2020-01-01T12:00:00.000Z',
},
TEST_ALERT_3: {
alertId: 'TEST_ALERT_3',
alertUuid: 'TEST_ALERT_3_UUID',
flapping: true,
flappingHistory: [false, false, false],
pendingRecoveredCount: 0,
started: '2020-01-02T12:00:00.000Z',
},
});

expect(serializedAlerts.state.trackedAlertsRecovered).toEqual({});

expect((await ruleDataClientMock.getWriter()).bulk).toHaveBeenCalledWith(
expect.objectContaining({
body: [
Expand All @@ -1070,7 +1107,7 @@ describe('createLifecycleExecutor', () => {
[ALERT_INSTANCE_ID]: 'TEST_ALERT_0',
[ALERT_WORKFLOW_STATUS]: 'closed',
[ALERT_STATUS]: ALERT_STATUS_ACTIVE,
[ALERT_FLAPPING]: true,
[ALERT_FLAPPING]: false,
[EVENT_ACTION]: 'active',
[EVENT_KIND]: 'signal',
}),
Expand Down Expand Up @@ -1182,7 +1219,7 @@ describe('createLifecycleExecutor', () => {
return { state };
});

await executor(
const serializedAlerts = await executor(
createDefaultAlertExecutorOptions({
alertId: 'TEST_ALERT_0',
params: {},
Expand Down Expand Up @@ -1228,17 +1265,55 @@ describe('createLifecycleExecutor', () => {
})
);

expect(serializedAlerts.state.trackedAlerts).toEqual({
TEST_ALERT_2: {
alertId: 'TEST_ALERT_2',
alertUuid: 'TEST_ALERT_2_UUID',
flapping: true,
flappingHistory: [true, true, true],
pendingRecoveredCount: 1,
started: '2020-01-02T12:00:00.000Z',
},
});

expect(serializedAlerts.state.trackedAlertsRecovered).toEqual({
TEST_ALERT_0: {
alertId: 'TEST_ALERT_0',
alertUuid: 'TEST_ALERT_0_UUID',
flapping: true,
flappingHistory: [true, true, true, true, true],
pendingRecoveredCount: 0,
started: '2020-01-01T12:00:00.000Z',
},
TEST_ALERT_1: {
alertId: 'TEST_ALERT_1',
alertUuid: 'TEST_ALERT_1_UUID',
flapping: false,
flappingHistory: notFlapping.slice(0, notFlapping.length - 1).concat([true]),
pendingRecoveredCount: 0,
started: '2020-01-02T12:00:00.000Z',
},
TEST_ALERT_3: {
alertId: 'TEST_ALERT_3',
alertUuid: 'TEST_ALERT_3_UUID',
flapping: false,
flappingHistory: notFlapping.slice(0, notFlapping.length - 1).concat([true]),
pendingRecoveredCount: 0,
started: '2020-01-02T12:00:00.000Z',
},
});

expect((await ruleDataClientMock.getWriter()).bulk).toHaveBeenCalledWith(
expect.objectContaining({
body: expect.arrayContaining([
// alert document
{ index: { _id: 'TEST_ALERT_0_UUID' } },
expect.objectContaining({
[ALERT_INSTANCE_ID]: 'TEST_ALERT_0',
[ALERT_STATUS]: ALERT_STATUS_ACTIVE,
[EVENT_ACTION]: 'active',
[ALERT_STATUS]: ALERT_STATUS_RECOVERED,
[EVENT_ACTION]: 'close',
[EVENT_KIND]: 'signal',
[ALERT_FLAPPING]: true,
[ALERT_FLAPPING]: false,
}),
{ index: { _id: 'TEST_ALERT_1_UUID' } },
expect.objectContaining({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,7 @@ export const createLifecycleExecutor =
trackedAlertRecoveredIds
);

const {
alertUuid,
started,
flapping: isCurrentlyFlapping,
pendingRecoveredCount,
} = !isNew
const { alertUuid, started, flapping, pendingRecoveredCount } = !isNew
? state.trackedAlerts[alertId]
: {
alertUuid: lifecycleAlertServices.getAlertUuid(alertId),
Expand All @@ -294,8 +289,6 @@ export const createLifecycleExecutor =
pendingRecoveredCount: 0,
};

const flapping = isFlapping(flappingSettings, flappingHistory, isCurrentlyFlapping);

const event: ParsedTechnicalFields & ParsedExperimentalFields = {
...alertData?.fields,
...commonRuleFields,
Expand Down Expand Up @@ -368,10 +361,11 @@ export const createLifecycleExecutor =
const nextTrackedAlerts = Object.fromEntries(
allEventsToIndex
.filter(({ event }) => event[ALERT_STATUS] !== ALERT_STATUS_RECOVERED)
.map(({ event, flappingHistory, flapping, pendingRecoveredCount }) => {
.map(({ event, flappingHistory, flapping: isCurrentlyFlapping, pendingRecoveredCount }) => {
const alertId = event[ALERT_INSTANCE_ID]!;
const alertUuid = event[ALERT_UUID]!;
const started = new Date(event[ALERT_START]!).toISOString();
const flapping = isFlapping(flappingSettings, flappingHistory, isCurrentlyFlapping);
return [
alertId,
{ alertId, alertUuid, started, flappingHistory, flapping, pendingRecoveredCount },
Expand All @@ -389,10 +383,11 @@ export const createLifecycleExecutor =
event[ALERT_STATUS] === ALERT_STATUS_RECOVERED &&
(flapping || flappingHistory.filter((f: boolean) => f).length > 0)
)
.map(({ event, flappingHistory, flapping, pendingRecoveredCount }) => {
.map(({ event, flappingHistory, flapping: isCurrentlyFlapping, pendingRecoveredCount }) => {
const alertId = event[ALERT_INSTANCE_ID]!;
const alertUuid = event[ALERT_UUID]!;
const started = new Date(event[ALERT_START]!).toISOString();
const flapping = isFlapping(flappingSettings, flappingHistory, isCurrentlyFlapping);
return [
alertId,
{ alertId, alertUuid, started, flappingHistory, flapping, pendingRecoveredCount },
Expand Down
Loading

0 comments on commit 1b0074f

Please sign in to comment.