Skip to content

Commit

Permalink
[Actions] Notify only on action group change (#82969) (#85642)
Browse files Browse the repository at this point in the history
* plugged Task Manager lifecycle into status reactively

* fixed tests

* Revert "fixed tests"

This reverts commit e9f2cd0.

* made action group fields optional

* revert deletion

* again

* extracted action type for mto its own component

* extracted more sections of the action form to their own components

* updated icon

* added docs

* fixed always firing alert

* fixed export of components

* fixed react warning

* Adding flag for notifying on state change

* Updating logic in task runner

* Starting to update tests

* Adding tests

* Fixing types check

* Tests and types

* Tests

* Tests

* Tests

* Tests

* Tests

* Renaming field to a more descriptive name. Adding migrations

* Renaming field to a more descriptive name. Adding migrations

* Fixing tests

* Type check and tests

* Moving schedule and notify interval to bottom of flyout. Implementing dropdown from mockup in new component

* Changing boolean flag to enum type and updating in triggers_actions_ui

* Changing boolean flag to enum type and updating in alerts plugin

* Fixing types check

* Fixing monitoring jest tests

* Changing last references to old variable names

* Moving form inputs back to the top

* Renaming to alert_notify_when

* Updating functional tests

* Adding new functional test for notifyWhen onActionGroupChange

* Updating wording

* Incorporating action subgroups into logic

* PR fixes

* Updating functional test

* Fixing types check

* Changing default throttle interval to hour

* Fixing types check

Co-authored-by: Gidi Meir Morris <github@gidi.io>

Co-authored-by: Gidi Meir Morris <github@gidi.io>
  • Loading branch information
ymao1 and gmmorris committed Dec 11, 2020
1 parent 1ff75db commit d433e6e
Show file tree
Hide file tree
Showing 72 changed files with 1,721 additions and 124 deletions.
2 changes: 2 additions & 0 deletions x-pack/plugins/alerts/common/alert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import { SavedObjectAttribute, SavedObjectAttributes } from 'kibana/server';
import { AlertNotifyWhenType } from './alert_notify_when_type';

// eslint-disable-next-line @typescript-eslint/no-explicit-any
export type AlertTypeState = Record<string, any>;
Expand Down Expand Up @@ -68,6 +69,7 @@ export interface Alert {
apiKey: string | null;
apiKeyOwner: string | null;
throttle: string | null;
notifyWhen: AlertNotifyWhenType | null;
muteAll: boolean;
mutedInstanceIds: string[];
executionStatus: AlertExecutionStatus;
Expand Down
18 changes: 18 additions & 0 deletions x-pack/plugins/alerts/common/alert_notify_when_type.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { validateNotifyWhenType } from './alert_notify_when_type';

test('validates valid notify when type', () => {
expect(validateNotifyWhenType('onActionGroupChange')).toBeUndefined();
expect(validateNotifyWhenType('onActiveAlert')).toBeUndefined();
expect(validateNotifyWhenType('onThrottleInterval')).toBeUndefined();
});
test('returns error string if input is not valid notify when type', () => {
expect(validateNotifyWhenType('randomString')).toEqual(
`string is not a valid AlertNotifyWhenType: randomString`
);
});
19 changes: 19 additions & 0 deletions x-pack/plugins/alerts/common/alert_notify_when_type.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

const AlertNotifyWhenTypeValues = [
'onActionGroupChange',
'onActiveAlert',
'onThrottleInterval',
] as const;
export type AlertNotifyWhenType = typeof AlertNotifyWhenTypeValues[number];

export function validateNotifyWhenType(notifyWhen: string) {
if (AlertNotifyWhenTypeValues.includes(notifyWhen as AlertNotifyWhenType)) {
return;
}
return `string is not a valid AlertNotifyWhenType: ${notifyWhen}`;
}
1 change: 1 addition & 0 deletions x-pack/plugins/alerts/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export * from './alert_navigation';
export * from './alert_instance_summary';
export * from './builtin_action_groups';
export * from './disabled_action_groups';
export * from './alert_notify_when_type';

export interface AlertingFrameworkHealth {
isSufficientlySecure: boolean;
Expand Down
108 changes: 108 additions & 0 deletions x-pack/plugins/alerts/server/alert_instance/alert_instance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,114 @@ describe('isThrottled', () => {
});
});

describe('scheduledActionGroupOrSubgroupHasChanged()', () => {
test('should be false if no last scheduled and nothing scheduled', () => {
const alertInstance = new AlertInstance();
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(false);
});

test('should be false if group does not change', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
},
},
});
alertInstance.scheduleActions('default');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(false);
});

test('should be false if group and subgroup does not change', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
subgroup: 'subgroup',
},
},
});
alertInstance.scheduleActionsWithSubGroup('default', 'subgroup');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(false);
});

test('should be false if group does not change and subgroup goes from undefined to defined', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
},
},
});
alertInstance.scheduleActionsWithSubGroup('default', 'subgroup');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(false);
});

test('should be false if group does not change and subgroup goes from defined to undefined', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
subgroup: 'subgroup',
},
},
});
alertInstance.scheduleActions('default');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(false);
});

test('should be true if no last scheduled and has scheduled action', () => {
const alertInstance = new AlertInstance();
alertInstance.scheduleActions('default');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(true);
});

test('should be true if group does change', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
},
},
});
alertInstance.scheduleActions('penguin');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(true);
});

test('should be true if group does change and subgroup does change', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
subgroup: 'subgroup',
},
},
});
alertInstance.scheduleActionsWithSubGroup('penguin', 'fish');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(true);
});

test('should be true if group does not change and subgroup does change', () => {
const alertInstance = new AlertInstance({
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
subgroup: 'subgroup',
},
},
});
alertInstance.scheduleActionsWithSubGroup('default', 'fish');
expect(alertInstance.scheduledActionGroupOrSubgroupHasChanged()).toEqual(true);
});
});

describe('getScheduledActionOptions()', () => {
test('defaults to undefined', () => {
const alertInstance = new AlertInstance();
Expand Down
25 changes: 25 additions & 0 deletions x-pack/plugins/alerts/server/alert_instance/alert_instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,31 @@ export class AlertInstance<
return false;
}

scheduledActionGroupOrSubgroupHasChanged(): boolean {
if (!this.meta.lastScheduledActions && this.scheduledExecutionOptions) {
// it is considered a change when there are no previous scheduled actions
// and new scheduled actions
return true;
}

if (this.meta.lastScheduledActions && this.scheduledExecutionOptions) {
// compare previous and new scheduled actions if both exist
return (
!this.scheduledActionGroupIsUnchanged(
this.meta.lastScheduledActions,
this.scheduledExecutionOptions
) ||
!this.scheduledActionSubgroupIsUnchanged(
this.meta.lastScheduledActions,
this.scheduledExecutionOptions
)
);
}

// no previous and no new scheduled actions
return false;
}

private scheduledActionGroupIsUnchanged(
lastScheduledActions: NonNullable<AlertInstanceMeta['lastScheduledActions']>,
scheduledExecutionOptions: ScheduledExecutionOptions<State, Context>
Expand Down
16 changes: 14 additions & 2 deletions x-pack/plugins/alerts/server/alerts_client/alerts_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,13 @@ import {
AlertTaskState,
AlertInstanceSummary,
AlertExecutionStatusValues,
AlertNotifyWhenType,
} from '../types';
import { validateAlertTypeParams, alertExecutionStatusFromRaw } from '../lib';
import {
validateAlertTypeParams,
alertExecutionStatusFromRaw,
getAlertNotifyWhenType,
} from '../lib';
import {
GrantAPIKeyResult as SecurityPluginGrantAPIKeyResult,
InvalidateAPIKeyResult as SecurityPluginInvalidateAPIKeyResult,
Expand Down Expand Up @@ -157,6 +162,7 @@ interface UpdateOptions {
actions: NormalizedAlertAction[];
params: Record<string, unknown>;
throttle: string | null;
notifyWhen: AlertNotifyWhenType | null;
};
}

Expand Down Expand Up @@ -251,6 +257,8 @@ export class AlertsClient {
const createTime = Date.now();
const { references, actions } = await this.denormalizeActions(data.actions);

const notifyWhen = getAlertNotifyWhenType(data.notifyWhen, data.throttle);

const rawAlert: RawAlert = {
...data,
...this.apiKeyAsAlertAttributes(createdAPIKey, username),
Expand All @@ -262,6 +270,7 @@ export class AlertsClient {
params: validatedAlertTypeParams as RawAlert['params'],
muteAll: false,
mutedInstanceIds: [],
notifyWhen,
executionStatus: {
status: 'pending',
lastExecutionDate: new Date().toISOString(),
Expand Down Expand Up @@ -694,6 +703,7 @@ export class AlertsClient {
? await this.createAPIKey(this.generateAPIKeyName(alertType.id, data.name))
: null;
const apiKeyAttributes = this.apiKeyAsAlertAttributes(createdAPIKey, username);
const notifyWhen = getAlertNotifyWhenType(data.notifyWhen, data.throttle);

let updatedObject: SavedObject<RawAlert>;
const createAttributes = this.updateMeta({
Expand All @@ -702,6 +712,7 @@ export class AlertsClient {
...apiKeyAttributes,
params: validatedAlertTypeParams as RawAlert['params'],
actions,
notifyWhen,
updatedBy: username,
updatedAt: new Date().toISOString(),
});
Expand Down Expand Up @@ -1326,7 +1337,7 @@ export class AlertsClient {

private getPartialAlertFromRaw(
id: string,
{ createdAt, updatedAt, meta, scheduledTaskId, ...rawAlert }: Partial<RawAlert>,
{ createdAt, updatedAt, meta, notifyWhen, scheduledTaskId, ...rawAlert }: Partial<RawAlert>,
references: SavedObjectReference[] | undefined
): PartialAlert {
// Not the prettiest code here, but if we want to use most of the
Expand All @@ -1341,6 +1352,7 @@ export class AlertsClient {
const executionStatus = alertExecutionStatusFromRaw(this.logger, id, rawAlert.executionStatus);
return {
id,
notifyWhen,
...rawAlertWithoutExecutionStatus,
// we currently only support the Interval Schedule type
// Once we support additional types, this type signature will likely change
Expand Down
Loading

0 comments on commit d433e6e

Please sign in to comment.