Skip to content

Commit

Permalink
[Response Ops][Alerting] Update stack rules to respect max alert limit (
Browse files Browse the repository at this point in the history
#141000)

* wip

* wip

* Adding bucket selector clauses

* Adding comparator script generator

* Generating all the right queries

* Skip condition check if group agg

* Fixing functional test

* Fixing comparator script

* Fixing tests

* Fixing tests

* Renaming

* Using limit services in es query rule executor

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
ymao1 and kibanamachine authored Oct 6, 2022
1 parent dbbf3ad commit a231f9c
Show file tree
Hide file tree
Showing 23 changed files with 1,918 additions and 148 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ export async function executor(
const currentTimestamp = new Date().toISOString();
const publicBaseUrl = core.http.basePath.publicBaseUrl ?? '';

const alertLimit = alertFactory.alertLimit.getValue();

const compareFn = ComparatorFns.get(params.thresholdComparator);
if (compareFn == null) {
throw new Error(getInvalidComparatorError(params.thresholdComparator));
Expand Down Expand Up @@ -91,6 +93,12 @@ export async function executor(
if (firstValidTimefieldSort) {
latestTimestamp = firstValidTimefieldSort;
}

// we only create one alert if the condition is met, so we would only ever
// reach the alert limit if the limit is less than 1
alertFactory.alertLimit.setLimitReached(alertLimit < 1);
} else {
alertFactory.alertLimit.setLimitReached(false);
}

const { getRecoveredAlerts } = alertFactory.done();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { BaseActionContext, addMessages } from './action_context';
import { ParamsSchema } from './alert_type_params';
import { ParamsSchema } from './rule_type_params';

describe('ActionContext', () => {
it('generates expected properties if aggField is null', async () => {
Expand All @@ -28,10 +28,10 @@ describe('ActionContext', () => {
value: 42,
conditions: 'count greater than 4',
};
const context = addMessages({ name: '[alert-name]' }, base, params);
expect(context.title).toMatchInlineSnapshot(`"alert [alert-name] group [group] met threshold"`);
const context = addMessages({ name: '[rule-name]' }, base, params);
expect(context.title).toMatchInlineSnapshot(`"alert [rule-name] group [group] met threshold"`);
expect(context.message).toEqual(
`alert '[alert-name]' is active for group '[group]':
`alert '[rule-name]' is active for group '[group]':
- Value: 42
- Conditions Met: count greater than 4 over 5m
Expand Down Expand Up @@ -59,10 +59,10 @@ describe('ActionContext', () => {
value: 42,
conditions: 'avg([aggField]) greater than 4.2',
};
const context = addMessages({ name: '[alert-name]' }, base, params);
expect(context.title).toMatchInlineSnapshot(`"alert [alert-name] group [group] met threshold"`);
const context = addMessages({ name: '[rule-name]' }, base, params);
expect(context.title).toMatchInlineSnapshot(`"alert [rule-name] group [group] met threshold"`);
expect(context.message).toEqual(
`alert '[alert-name]' is active for group '[group]':
`alert '[rule-name]' is active for group '[group]':
- Value: 42
- Conditions Met: avg([aggField]) greater than 4.2 over 5m
Expand All @@ -89,10 +89,10 @@ describe('ActionContext', () => {
value: 4,
conditions: 'count between 4 and 5',
};
const context = addMessages({ name: '[alert-name]' }, base, params);
expect(context.title).toMatchInlineSnapshot(`"alert [alert-name] group [group] met threshold"`);
const context = addMessages({ name: '[rule-name]' }, base, params);
expect(context.title).toMatchInlineSnapshot(`"alert [rule-name] group [group] met threshold"`);
expect(context.message).toEqual(
`alert '[alert-name]' is active for group '[group]':
`alert '[rule-name]' is active for group '[group]':
- Value: 4
- Conditions Met: count between 4 and 5 over 5m
Expand All @@ -119,10 +119,10 @@ describe('ActionContext', () => {
value: 'unknown',
conditions: 'count between 4 and 5',
};
const context = addMessages({ name: '[alert-name]' }, base, params);
expect(context.title).toMatchInlineSnapshot(`"alert [alert-name] group [group] met threshold"`);
const context = addMessages({ name: '[rule-name]' }, base, params);
expect(context.title).toMatchInlineSnapshot(`"alert [rule-name] group [group] met threshold"`);
expect(context.message).toEqual(
`alert '[alert-name]' is active for group '[group]':
`alert '[rule-name]' is active for group '[group]':
- Value: unknown
- Conditions Met: count between 4 and 5 over 5m
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@

import { i18n } from '@kbn/i18n';
import { RuleExecutorOptions, AlertInstanceContext } from '@kbn/alerting-plugin/server';
import { Params } from './alert_type_params';
import { Params } from './rule_type_params';

// alert type context provided to actions
// rule type context provided to actions

type RuleInfo = Pick<RuleExecutorOptions, 'name'>;

Expand All @@ -21,10 +21,10 @@ export interface ActionContext extends BaseActionContext {
}

export interface BaseActionContext extends AlertInstanceContext {
// the aggType used in the alert
// the aggType used in the rule
// the value of the aggField, if used, otherwise 'all documents'
group: string;
// the date the alert was run as an ISO date
// the date the rule was run as an ISO date
date: string;
// the value that met the threshold
value: number | string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { Logger } from '@kbn/core/server';
import { AlertingSetup, StackAlertsStartDeps } from '../../types';
import { getAlertType } from './alert_type';
import { getRuleType } from './rule_type';

// future enhancement: make these configurable?
export const MAX_INTERVALS = 1000;
Expand All @@ -22,5 +22,5 @@ interface RegisterParams {

export function register(params: RegisterParams) {
const { logger, data, alerting } = params;
alerting.registerType(getAlertType(logger, data));
alerting.registerType(getRuleType(logger, data));
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,44 @@
*/

import uuid from 'uuid';
import sinon from 'sinon';
import type { Writable } from '@kbn/utility-types';
import { loggingSystemMock } from '@kbn/core/server/mocks';
import { RuleExecutorServices } from '@kbn/alerting-plugin/server';
import { getAlertType, ActionGroupId } from './alert_type';
import { getRuleType, ActionGroupId } from './rule_type';
import { ActionContext } from './action_context';
import { Params } from './alert_type_params';
import { Params } from './rule_type_params';
import { TIME_SERIES_BUCKET_SELECTOR_FIELD } from '@kbn/triggers-actions-ui-plugin/server';
import { RuleExecutorServicesMock, alertsMock } from '@kbn/alerting-plugin/server/mocks';
import { Comparator } from '../../../common/comparator_types';

describe('alertType', () => {
let fakeTimer: sinon.SinonFakeTimers;

describe('ruleType', () => {
const logger = loggingSystemMock.create().get();
const data = {
timeSeriesQuery: jest.fn(),
};
const alertServices: RuleExecutorServicesMock = alertsMock.createRuleExecutorServices();

const alertType = getAlertType(logger, Promise.resolve(data));
const ruleType = getRuleType(logger, Promise.resolve(data));

beforeAll(() => {
fakeTimer = sinon.useFakeTimers();
});

afterEach(() => {
data.timeSeriesQuery.mockReset();
});

it('alert type creation structure is the expected value', async () => {
expect(alertType.id).toBe('.index-threshold');
expect(alertType.name).toBe('Index threshold');
expect(alertType.actionGroups).toEqual([{ id: 'threshold met', name: 'Threshold met' }]);
afterAll(() => fakeTimer.restore());

expect(alertType.actionVariables).toMatchInlineSnapshot(`
it('rule type creation structure is the expected value', async () => {
expect(ruleType.id).toBe('.index-threshold');
expect(ruleType.name).toBe('Index threshold');
expect(ruleType.actionGroups).toEqual([{ id: 'threshold met', name: 'Threshold met' }]);

expect(ruleType.actionVariables).toMatchInlineSnapshot(`
Object {
"context": Array [
Object {
Expand Down Expand Up @@ -123,11 +133,11 @@ describe('alertType', () => {
threshold: [0],
};

expect(alertType.validate?.params?.validate(params)).toBeTruthy();
expect(ruleType.validate?.params?.validate(params)).toBeTruthy();
});

it('validator fails with invalid params', async () => {
const paramsSchema = alertType.validate?.params;
const paramsSchema = ruleType.validate?.params;
if (!paramsSchema) throw new Error('params validator not set');

const params: Partial<Writable<Params>> = {
Expand Down Expand Up @@ -168,7 +178,7 @@ describe('alertType', () => {
threshold: [1],
};

await alertType.executor({
await ruleType.executor({
alertId: uuid.v4(),
executionId: uuid.v4(),
startedAt: new Date(),
Expand Down Expand Up @@ -234,7 +244,7 @@ describe('alertType', () => {
threshold: [1],
};

await alertType.executor({
await ruleType.executor({
alertId: uuid.v4(),
executionId: uuid.v4(),
startedAt: new Date(),
Expand Down Expand Up @@ -300,7 +310,7 @@ describe('alertType', () => {
threshold: [1],
};

await alertType.executor({
await ruleType.executor({
alertId: uuid.v4(),
executionId: uuid.v4(),
startedAt: new Date(),
Expand Down Expand Up @@ -342,4 +352,90 @@ describe('alertType', () => {

expect(customAlertServices.alertFactory.create).not.toHaveBeenCalled();
});

it('should correctly pass comparator script to timeSeriesQuery', async () => {
data.timeSeriesQuery.mockImplementation((...args) => {
return {
results: [
{
group: 'all documents',
metrics: [['2021-07-14T14:49:30.978Z', 0]],
},
],
};
});
const params: Params = {
index: 'index-name',
timeField: 'time-field',
aggType: 'foo',
groupBy: 'all',
timeWindowSize: 5,
timeWindowUnit: 'm',
thresholdComparator: Comparator.LT,
threshold: [1],
};

await ruleType.executor({
alertId: uuid.v4(),
executionId: uuid.v4(),
startedAt: new Date(),
previousStartedAt: new Date(),
services: alertServices as unknown as RuleExecutorServices<
{},
ActionContext,
typeof ActionGroupId
>,
params,
state: {
latestTimestamp: undefined,
},
spaceId: uuid.v4(),
name: uuid.v4(),
tags: [],
createdBy: null,
updatedBy: null,
rule: {
name: uuid.v4(),
tags: [],
consumer: '',
producer: '',
ruleTypeId: '',
ruleTypeName: '',
enabled: true,
schedule: {
interval: '1h',
},
actions: [],
createdBy: null,
updatedBy: null,
createdAt: new Date(),
updatedAt: new Date(),
throttle: null,
notifyWhen: null,
},
});

expect(data.timeSeriesQuery).toHaveBeenCalledWith(
expect.objectContaining({
query: {
aggField: undefined,
aggType: 'foo',
dateEnd: '1970-01-01T00:00:00.000Z',
dateStart: '1970-01-01T00:00:00.000Z',
groupBy: 'all',
index: 'index-name',
interval: undefined,
termField: undefined,
termSize: undefined,
timeField: 'time-field',
timeWindowSize: 5,
timeWindowUnit: 'm',
},
condition: {
conditionScript: `${TIME_SERIES_BUCKET_SELECTOR_FIELD} < 1L`,
resultLimit: 1000,
},
})
);
});
});
Loading

0 comments on commit a231f9c

Please sign in to comment.