Skip to content

Commit

Permalink
[8.6] [Alert Summaries] [BE] Move “Notify When” and throttle from rul…
Browse files Browse the repository at this point in the history
…e to action (elastic#144130) (elastic#145469)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[Alert Summaries] [BE] Move “Notify When” and throttle from rule to
action (elastic#144130)](elastic#144130)

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

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

<!--BACKPORT [{"author":{"name":"Zacqary Adam
Xeper","email":"Zacqary@users.noreply.github.com"},"sourceCommit":{"committedDate":"2022-11-16T22:02:36Z","message":"[Alert
Summaries] [BE] Move “Notify When” and throttle from rule to action
(elastic#144130)\n\n* Add frequency props to actions and validate them on
edit/create\r\n\r\n* Nullify undefined throttle\r\n\r\n* Update schema
to allow for frequency param on actions\r\n\r\n* [CI] Auto-commit
changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD
--fix'\r\n\r\n* Commit missing file\r\n\r\n* Fix types\r\n\r\n* [CI]
Auto-commit changed files from 'node scripts/eslint --no-cache
--fix'\r\n\r\n* Fix types\r\n\r\n* Fix jest\r\n\r\n* Fix validating
global freq params\r\n\r\n* Add tests for create and edit\r\n\r\n* Reset
legacy api\r\n\r\n* Make notify and throttle optional in route
schemas\r\n\r\n* Fix tests\r\n\r\n* Split missing frequency test
cases\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"0c864fe479e332aa92bf189f0123661cd52262ae","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:ResponseOps","Feature:Alerting/RulesManagement","v8.6.0","v8.7.0"],"number":144130,"url":"https://github.com/elastic/kibana/pull/144130","mergeCommit":{"message":"[Alert
Summaries] [BE] Move “Notify When” and throttle from rule to action
(elastic#144130)\n\n* Add frequency props to actions and validate them on
edit/create\r\n\r\n* Nullify undefined throttle\r\n\r\n* Update schema
to allow for frequency param on actions\r\n\r\n* [CI] Auto-commit
changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD
--fix'\r\n\r\n* Commit missing file\r\n\r\n* Fix types\r\n\r\n* [CI]
Auto-commit changed files from 'node scripts/eslint --no-cache
--fix'\r\n\r\n* Fix types\r\n\r\n* Fix jest\r\n\r\n* Fix validating
global freq params\r\n\r\n* Add tests for create and edit\r\n\r\n* Reset
legacy api\r\n\r\n* Make notify and throttle optional in route
schemas\r\n\r\n* Fix tests\r\n\r\n* Split missing frequency test
cases\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"0c864fe479e332aa92bf189f0123661cd52262ae"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/144130","number":144130,"mergeCommit":{"message":"[Alert
Summaries] [BE] Move “Notify When” and throttle from rule to action
(elastic#144130)\n\n* Add frequency props to actions and validate them on
edit/create\r\n\r\n* Nullify undefined throttle\r\n\r\n* Update schema
to allow for frequency param on actions\r\n\r\n* [CI] Auto-commit
changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD
--fix'\r\n\r\n* Commit missing file\r\n\r\n* Fix types\r\n\r\n* [CI]
Auto-commit changed files from 'node scripts/eslint --no-cache
--fix'\r\n\r\n* Fix types\r\n\r\n* Fix jest\r\n\r\n* Fix validating
global freq params\r\n\r\n* Add tests for create and edit\r\n\r\n* Reset
legacy api\r\n\r\n* Make notify and throttle optional in route
schemas\r\n\r\n* Fix tests\r\n\r\n* Split missing frequency test
cases\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"0c864fe479e332aa92bf189f0123661cd52262ae"}}]}]
BACKPORT-->

Co-authored-by: Zacqary Adam Xeper <Zacqary@users.noreply.github.com>
  • Loading branch information
kibanamachine and Zacqary committed Nov 16, 2022
1 parent 14b5e30 commit be37c7d
Show file tree
Hide file tree
Showing 19 changed files with 533 additions and 45 deletions.
2 changes: 1 addition & 1 deletion x-pack/plugins/alerting/common/alert_summary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export interface AlertSummary {
ruleTypeId: string;
consumer: string;
muteAll: boolean;
throttle: string | null;
throttle?: string | null;
enabled: boolean;
statusStartDate: string;
statusEndDate: string;
Expand Down
9 changes: 7 additions & 2 deletions x-pack/plugins/alerting/common/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ export interface RuleAction {
id: string;
actionTypeId: string;
params: RuleActionParams;
frequency?: {
summary: boolean;
notifyWhen: RuleNotifyWhenType;
throttle: string | null;
};
}

export interface RuleAggregations {
Expand Down Expand Up @@ -123,9 +128,9 @@ export interface Rule<Params extends RuleTypeParams = never> {
updatedAt: Date;
apiKey: string | null;
apiKeyOwner: string | null;
throttle: string | null;
throttle?: string | null;
muteAll: boolean;
notifyWhen: RuleNotifyWhenType | null;
notifyWhen?: RuleNotifyWhenType | null;
mutedInstanceIds: string[];
executionStatus: RuleExecutionStatus;
monitoring?: RuleMonitoring;
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/alerting/common/rule_notify_when_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

const RuleNotifyWhenTypeValues = [
export const RuleNotifyWhenTypeValues = [
'onActionGroupChange',
'onActiveAlert',
'onThrottleInterval',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export function alertSummaryFromEventLog(params: AlertSummaryFromEventLogParams)
statusEndDate: dateEnd,
status: 'OK',
muteAll: rule.muteAll,
throttle: rule.throttle,
throttle: rule.throttle ?? null,
enabled: rule.enabled,
lastRun: undefined,
errorMessages: [],
Expand Down
17 changes: 7 additions & 10 deletions x-pack/plugins/alerting/server/routes/create_rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ import { CreateOptions } from '../rules_client';
import {
RewriteRequestCase,
RewriteResponseCase,
rewriteActions,
handleDisabledApiKeysError,
verifyAccessAndContext,
countUsageOfPredefinedIds,
actionsSchema,
rewriteRuleLastRun,
} from './lib';
import {
Expand All @@ -31,20 +33,13 @@ export const bodySchema = schema.object({
enabled: schema.boolean({ defaultValue: true }),
consumer: schema.string(),
tags: schema.arrayOf(schema.string(), { defaultValue: [] }),
throttle: schema.nullable(schema.string({ validate: validateDurationSchema })),
throttle: schema.nullable(schema.maybe(schema.string({ validate: validateDurationSchema }))),
params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
schedule: schema.object({
interval: schema.string({ validate: validateDurationSchema }),
}),
actions: schema.arrayOf(
schema.object({
group: schema.string(),
id: schema.string(),
params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
}),
{ defaultValue: [] }
),
notify_when: schema.string({ validate: validateNotifyWhenType }),
actions: actionsSchema,
notify_when: schema.maybe(schema.string({ validate: validateNotifyWhenType })),
});

const rewriteBodyReq: RewriteRequestCase<CreateOptions<RuleTypeParams>['data']> = ({
Expand All @@ -56,6 +51,7 @@ const rewriteBodyReq: RewriteRequestCase<CreateOptions<RuleTypeParams>['data']>
alertTypeId,
notifyWhen,
});

const rewriteBodyRes: RewriteResponseCase<SanitizedRule<RuleTypeParams>> = ({
actions,
alertTypeId,
Expand Down Expand Up @@ -132,6 +128,7 @@ export const createRuleRoute = ({ router, licenseState, usageCounter }: RouteOpt
await rulesClient.create<RuleTypeParams>({
data: rewriteBodyReq({
...rule,
actions: rewriteActions(rule.actions),
notify_when: rule.notify_when as RuleNotifyWhenType,
}),
options: { id: params?.id },
Expand Down
29 changes: 29 additions & 0 deletions x-pack/plugins/alerting/server/routes/lib/actions_schema.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { schema } from '@kbn/config-schema';
import { validateDurationSchema } from '../../lib';

export const actionsSchema = schema.arrayOf(
schema.object({
group: schema.string(),
id: schema.string(),
params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
frequency: schema.maybe(
schema.object({
summary: schema.boolean(),
notify_when: schema.oneOf([
schema.literal('onActionGroupChange'),
schema.literal('onActiveAlert'),
schema.literal('onThrottleInterval'),
]),
throttle: schema.nullable(schema.string({ validate: validateDurationSchema })),
})
),
}),
{ defaultValue: [] }
);
2 changes: 2 additions & 0 deletions x-pack/plugins/alerting/server/routes/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,7 @@ export type {
} from './rewrite_request_case';
export { verifyAccessAndContext } from './verify_access_and_context';
export { countUsageOfPredefinedIds } from './count_usage_of_predefined_ids';
export { rewriteActions } from './rewrite_actions';
export { actionsSchema } from './actions_schema';
export { rewriteRule, rewriteRuleLastRun } from './rewrite_rule';
export { rewriteNamespaces } from './rewrite_namespaces';
32 changes: 32 additions & 0 deletions x-pack/plugins/alerting/server/routes/lib/rewrite_actions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { CamelToSnake, RewriteRequestCase } from './rewrite_request_case';
import { RuleAction } from '../../types';

type ReqRuleAction = Omit<RuleAction, 'actionTypeId' | 'frequency'> & {
frequency?: {
[K in keyof NonNullable<RuleAction['frequency']> as CamelToSnake<K>]: NonNullable<
RuleAction['frequency']
>[K];
};
};
export const rewriteActions: (
actions?: ReqRuleAction[]
) => Array<Omit<RuleAction, 'actionTypeId'>> = (actions) => {
const rewriteFrequency: RewriteRequestCase<NonNullable<RuleAction['frequency']>> = ({
notify_when: notifyWhen,
...rest
}) => ({ ...rest, notifyWhen });
if (!actions) return [];
return actions.map(
(action) =>
({
...action,
...(action.frequency ? { frequency: rewriteFrequency(action.frequency) } : {}),
} as RuleAction)
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export type RewriteResponseCase<T> = (
*
* For more details see this PR comment: https://github.com/microsoft/TypeScript/pull/40336#issuecomment-686723087
*/
type CamelToSnake<T extends string> = string extends T
export type CamelToSnake<T extends string> = string extends T
? string
: T extends `${infer C0}${infer C1}${infer R}`
? `${C0 extends Uppercase<C0> ? '_' : ''}${Lowercase<C0>}${C1 extends Uppercase<C1>
Expand Down
10 changes: 9 additions & 1 deletion x-pack/plugins/alerting/server/routes/lib/rewrite_rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,19 @@ export const rewriteRule = ({
last_execution_date: executionStatus.lastExecutionDate,
last_duration: executionStatus.lastDuration,
},
actions: actions.map(({ group, id, actionTypeId, params }) => ({
actions: actions.map(({ group, id, actionTypeId, params, frequency }) => ({
group,
id,
params,
connector_type_id: actionTypeId,
...(frequency
? {
frequency: {
...frequency,
notify_when: frequency.notifyWhen,
},
}
: {}),
})),
...(lastRun ? { last_run: rewriteRuleLastRun(lastRun) } : {}),
...(nextRun ? { next_run: nextRun } : {}),
Expand Down
16 changes: 6 additions & 10 deletions x-pack/plugins/alerting/server/routes/update_rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
RewriteResponseCase,
RewriteRequestCase,
handleDisabledApiKeysError,
rewriteActions,
actionsSchema,
rewriteRuleLastRun,
} from './lib';
import {
Expand All @@ -35,17 +37,10 @@ const bodySchema = schema.object({
schedule: schema.object({
interval: schema.string({ validate: validateDurationSchema }),
}),
throttle: schema.nullable(schema.string({ validate: validateDurationSchema })),
throttle: schema.nullable(schema.maybe(schema.string({ validate: validateDurationSchema }))),
params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
actions: schema.arrayOf(
schema.object({
group: schema.string(),
id: schema.string(),
params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
}),
{ defaultValue: [] }
),
notify_when: schema.string({ validate: validateNotifyWhenType }),
actions: actionsSchema,
notify_when: schema.maybe(schema.string({ validate: validateNotifyWhenType })),
});

const rewriteBodyReq: RewriteRequestCase<UpdateOptions<RuleTypeParams>> = (result) => {
Expand Down Expand Up @@ -137,6 +132,7 @@ export const updateRuleRoute = (
id,
data: {
...rule,
actions: rewriteActions(rule.actions),
notify_when: rule.notify_when as RuleNotifyWhenType,
},
})
Expand Down
63 changes: 54 additions & 9 deletions x-pack/plugins/alerting/server/rules_client/rules_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,8 @@ export interface UpdateOptions<Params extends RuleTypeParams> {
schedule: IntervalSchedule;
actions: NormalizedAlertAction[];
params: Params;
throttle: string | null;
notifyWhen: RuleNotifyWhenType | null;
throttle?: string | null;
notifyWhen?: RuleNotifyWhenType | null;
};
}

Expand Down Expand Up @@ -691,7 +691,7 @@ export class RulesClient {
throw Boom.badRequest(`Error creating rule: could not create API key - ${error.message}`);
}

await this.validateActions(ruleType, data.actions);
await this.validateActions(ruleType, data);

// Throw error if schedule interval is less than the minimum and we are enforcing it
const intervalInMs = parseDuration(data.schedule.interval);
Expand All @@ -711,7 +711,8 @@ export class RulesClient {
const createTime = Date.now();
const lastRunTimestamp = new Date();
const legacyId = Semver.lt(this.kibanaVersion, '8.0.0') ? id : null;
const notifyWhen = getRuleNotifyWhenType(data.notifyWhen, data.throttle);
const notifyWhen = getRuleNotifyWhenType(data.notifyWhen ?? null, data.throttle ?? null);
const throttle = data.throttle ?? null;

const rawRule: RawRule = {
...data,
Expand All @@ -727,6 +728,7 @@ export class RulesClient {
muteAll: false,
mutedInstanceIds: [],
notifyWhen,
throttle,
executionStatus: getRuleExecutionStatusPending(lastRunTimestamp.toISOString()),
monitoring: getDefaultMonitoring(lastRunTimestamp.toISOString()),
};
Expand Down Expand Up @@ -1870,7 +1872,7 @@ export class RulesClient {

// Validate
const validatedAlertTypeParams = validateRuleTypeParams(data.params, ruleType.validate?.params);
await this.validateActions(ruleType, data.actions);
await this.validateActions(ruleType, data);

// Throw error if schedule interval is less than the minimum and we are enforcing it
const intervalInMs = parseDuration(data.schedule.interval);
Expand Down Expand Up @@ -1899,7 +1901,7 @@ export class RulesClient {
}

const apiKeyAttributes = this.apiKeyAsAlertAttributes(createdAPIKey, username);
const notifyWhen = getRuleNotifyWhenType(data.notifyWhen, data.throttle);
const notifyWhen = getRuleNotifyWhenType(data.notifyWhen ?? null, data.throttle ?? null);

let updatedObject: SavedObject<RawRule>;
const createAttributes = this.updateMeta({
Expand Down Expand Up @@ -2429,7 +2431,7 @@ export class RulesClient {
for (const operation of operations) {
switch (operation.field) {
case 'actions':
await this.validateActions(ruleType, operation.value);
await this.validateActions(ruleType, { ...attributes, actions: operation.value });
ruleActions = applyBulkEditOperation(operation, ruleActions);
break;
case 'snoozeSchedule':
Expand Down Expand Up @@ -2539,7 +2541,7 @@ export class RulesClient {

// get notifyWhen
const notifyWhen = getRuleNotifyWhenType(
attributes.notifyWhen,
attributes.notifyWhen ?? null,
attributes.throttle ?? null
);

Expand Down Expand Up @@ -3904,8 +3906,23 @@ export class RulesClient {

private async validateActions(
alertType: UntypedNormalizedRuleType,
actions: NormalizedAlertAction[]
data: Pick<RawRule, 'notifyWhen' | 'throttle'> & { actions: NormalizedAlertAction[] }
): Promise<void> {
const { actions, notifyWhen, throttle } = data;
const hasNotifyWhen = typeof notifyWhen !== 'undefined';
const hasThrottle = typeof throttle !== 'undefined';
let usesRuleLevelFreqParams;
if (hasNotifyWhen && hasThrottle) usesRuleLevelFreqParams = true;
else if (!hasNotifyWhen && !hasThrottle) usesRuleLevelFreqParams = false;
else {
throw Boom.badRequest(
i18n.translate('xpack.alerting.rulesClient.usesValidGlobalFreqParams.oneUndefined', {
defaultMessage:
'Rule-level notifyWhen and throttle must both be defined or both be undefined',
})
);
}

if (actions.length === 0) {
return;
}
Expand Down Expand Up @@ -3948,6 +3965,34 @@ export class RulesClient {
})
);
}

// check for actions using frequency params if the rule has rule-level frequency params defined
if (usesRuleLevelFreqParams) {
const actionsWithFrequency = actions.filter((action) => Boolean(action.frequency));
if (actionsWithFrequency.length) {
throw Boom.badRequest(
i18n.translate('xpack.alerting.rulesClient.validateActions.mixAndMatchFreqParams', {
defaultMessage:
'Cannot specify per-action frequency params when notify_when and throttle are defined at the rule level: {groups}',
values: {
groups: actionsWithFrequency.map((a) => a.group).join(', '),
},
})
);
}
} else {
const actionsWithoutFrequency = actions.filter((action) => !action.frequency);
if (actionsWithoutFrequency.length) {
throw Boom.badRequest(
i18n.translate('xpack.alerting.rulesClient.validateActions.notAllActionsWithFreq', {
defaultMessage: 'Actions missing frequency parameters: {groups}',
values: {
groups: actionsWithoutFrequency.map((a) => a.group).join(', '),
},
})
);
}
}
}

private async extractReferences<
Expand Down
Loading

0 comments on commit be37c7d

Please sign in to comment.