Skip to content

Commit

Permalink
pass more alert info into alert executor (#54035)
Browse files Browse the repository at this point in the history
resolves #50522

The alert executor function is now passed these additional alert-specific
properties as parameters:

- spaceId
- namespace
- name
- tags
- createdBy
- updatedBy
  • Loading branch information
pmuellr committed Jan 9, 2020
1 parent 32e6159 commit 5853360
Show file tree
Hide file tree
Showing 15 changed files with 281 additions and 54 deletions.
7 changes: 7 additions & 0 deletions x-pack/legacy/plugins/alerting/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ This is the primary function for an alert type. Whenever the alert needs to exec
|previousStartedAt|The previous date and time the alert type started a successful execution.|
|params|Parameters for the execution. This is where the parameters you require will be passed in. (example threshold). Use alert type validation to ensure values are set before execution.|
|state|State returned from previous execution. This is the alert level state. What the executor returns will be serialized and provided here at the next execution.|
|alertId|The id of this alert.|
|spaceId|The id of the space of this alert.|
|namespace|The namespace of the space of this alert; same as spaceId, unless spaceId === 'default', then namespace = undefined.|
|name|The name of this alert.|
|tags|The tags associated with this alert.|
|createdBy|The userid that created this alert.|
|updatedBy|The userid that last updated this alert.|

### Example

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ describe('Task Runner', () => {
enabled: true,
alertTypeId: '123',
schedule: { interval: '10s' },
name: 'alert-name',
tags: ['alert-', '-tags'],
createdBy: 'alert-creator',
updatedBy: 'alert-updater',
mutedInstanceIds: [],
params: {
bar: true,
Expand Down Expand Up @@ -138,6 +142,10 @@ describe('Task Runner', () => {
`);
expect(call.startedAt).toMatchInlineSnapshot(`1970-01-01T00:00:00.000Z`);
expect(call.state).toMatchInlineSnapshot(`Object {}`);
expect(call.name).toBe('alert-name');
expect(call.tags).toEqual(['alert-', '-tags']);
expect(call.createdBy).toBe('alert-creator');
expect(call.updatedBy).toBe('alert-updater');
expect(call.services.alertInstanceFactory).toBeTruthy();
expect(call.services.callCluster).toBeTruthy();
expect(call.services).toBeTruthy();
Expand Down
33 changes: 28 additions & 5 deletions x-pack/legacy/plugins/alerting/server/task_runner/task_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { createExecutionHandler } from './create_execution_handler';
import { AlertInstance, createAlertInstanceFactory } from '../alert_instance';
import { getNextRunAt } from './get_next_run_at';
import { validateAlertTypeParams } from '../lib';
import { AlertType, RawAlert, IntervalSchedule, Services, State } from '../types';
import { AlertType, RawAlert, IntervalSchedule, Services, State, AlertInfoParams } from '../types';
import { promiseResult, map } from '../lib/result_type';

type AlertInstances = Record<string, AlertInstance>;
Expand Down Expand Up @@ -118,13 +118,25 @@ export class TaskRunner {

async executeAlertInstances(
services: Services,
{ params, throttle, muteAll, mutedInstanceIds }: SavedObject['attributes'],
executionHandler: ReturnType<typeof createExecutionHandler>
alertInfoParams: AlertInfoParams,
executionHandler: ReturnType<typeof createExecutionHandler>,
spaceId: string
): Promise<State> {
const {
params,
throttle,
muteAll,
mutedInstanceIds,
name,
tags,
createdBy,
updatedBy,
} = alertInfoParams;
const {
params: { alertId },
state: { alertInstances: alertRawInstances = {}, alertTypeState = {}, previousStartedAt },
} = this.taskInstance;
const namespace = this.context.spaceIdToNamespace(spaceId);

const alertInstances = mapValues<AlertInstances>(
alertRawInstances,
Expand All @@ -141,6 +153,12 @@ export class TaskRunner {
state: alertTypeState,
startedAt: this.taskInstance.startedAt!,
previousStartedAt,
spaceId,
namespace,
name,
tags,
createdBy,
updatedBy,
});

// Cleanup alert instances that are no longer scheduling actions to avoid over populating the alertInstances object
Expand Down Expand Up @@ -175,7 +193,7 @@ export class TaskRunner {
async validateAndRunAlert(
services: Services,
apiKey: string | null,
attributes: SavedObject['attributes'],
attributes: RawAlert,
references: SavedObject['references']
) {
const {
Expand All @@ -191,7 +209,12 @@ export class TaskRunner {
attributes.actions,
references
);
return this.executeAlertInstances(services, { ...attributes, params }, executionHandler);
return this.executeAlertInstances(
services,
{ ...attributes, params },
executionHandler,
spaceId
);
}

async run() {
Expand Down
18 changes: 18 additions & 0 deletions x-pack/legacy/plugins/alerting/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ export interface AlertExecutorOptions {
services: AlertServices;
params: Record<string, any>;
state: State;
spaceId: string;
namespace?: string;
name: string;
tags: string[];
createdBy: string | null;
updatedBy: string | null;
}

export interface AlertType {
Expand Down Expand Up @@ -108,6 +114,18 @@ export interface RawAlert extends SavedObjectAttributes {
mutedInstanceIds: string[];
}

export type AlertInfoParams = Pick<
RawAlert,
| 'params'
| 'throttle'
| 'muteAll'
| 'mutedInstanceIds'
| 'name'
| 'tags'
| 'createdBy'
| 'updatedBy'
>;

export interface AlertingPlugin {
setup: PluginSetupContract;
start: PluginStartContract;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,21 @@ export default function(kibana: any) {
id: 'test.always-firing',
name: 'Test: Always Firing',
actionGroups: ['default', 'other'],
async executor({ services, params, state }: AlertExecutorOptions) {
async executor(alertExecutorOptions: AlertExecutorOptions) {
const {
services,
params,
state,
alertId,
spaceId,
namespace,
name,
tags,
createdBy,
updatedBy,
} = alertExecutorOptions;
let group = 'default';
const alertInfo = { alertId, spaceId, namespace, name, tags, createdBy, updatedBy };

if (params.groupsToScheduleActionsInSeries) {
const index = state.groupInSeriesIndex || 0;
Expand All @@ -226,6 +239,7 @@ export default function(kibana: any) {
params,
reference: params.reference,
source: 'alert:test.always-firing',
alertInfo,
},
});
return {
Expand Down
91 changes: 65 additions & 26 deletions x-pack/test/alerting_api_integration/common/lib/alert_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ export interface CreateAlertWithActionOpts {
reference: string;
}

interface UpdateAlwaysFiringAction {
alertId: string;
actionId: string | undefined;
reference: string;
user: User;
overwrites: Record<string, any>;
}

export class AlertUtils {
private referenceCounter = 1;
private readonly user?: User;
Expand Down Expand Up @@ -176,38 +184,41 @@ export class AlertUtils {
if (this.user) {
request = request.auth(this.user.username, this.user.password);
}
const response = await request.send({
enabled: true,
name: 'abc',
schedule: { interval: '1m' },
throttle: '1m',
tags: [],
alertTypeId: 'test.always-firing',
consumer: 'bar',
params: {
index: ES_TEST_INDEX_NAME,
reference,
},
actions: [
{
group: 'default',
id: this.indexRecordActionId,
params: {
index: ES_TEST_INDEX_NAME,
reference,
message:
'instanceContextValue: {{context.instanceContextValue}}, instanceStateValue: {{state.instanceStateValue}}',
},
},
],
...overwrites,
});
const alertBody = getDefaultAlwaysFiringAlertData(reference, actionId);
const response = await request.send({ ...alertBody, ...overwrites });
if (response.statusCode === 200) {
objRemover.add(this.space.id, response.body.id, 'alert');
}
return response;
}

public async updateAlwaysFiringAction({
alertId,
actionId,
reference,
user,
overwrites = {},
}: UpdateAlwaysFiringAction) {
actionId = actionId || this.indexRecordActionId;

if (!actionId) {
throw new Error('actionId is required ');
}

const request = this.supertestWithoutAuth
.put(`${getUrlPrefix(this.space.id)}/api/alert/${alertId}`)
.set('kbn-xsrf', 'foo')
.auth(user.username, user.password);

const alertBody = getDefaultAlwaysFiringAlertData(reference, actionId);
delete alertBody.alertTypeId;
delete alertBody.enabled;
delete alertBody.consumer;

const response = await request.send({ ...alertBody, ...overwrites });
return response;
}

public async createAlwaysFailingAction({
objectRemover,
overwrites = {},
Expand Down Expand Up @@ -251,3 +262,31 @@ export class AlertUtils {
return response;
}
}

function getDefaultAlwaysFiringAlertData(reference: string, actionId: string) {
return {
enabled: true,
name: 'abc',
schedule: { interval: '1m' },
throttle: '1m',
tags: [],
alertTypeId: 'test.always-firing',
consumer: 'bar',
params: {
index: ES_TEST_INDEX_NAME,
reference,
},
actions: [
{
group: 'default',
id: actionId,
params: {
index: ES_TEST_INDEX_NAME,
reference,
message:
'instanceContextValue: {{context.instanceContextValue}}, instanceStateValue: {{state.instanceStateValue}}',
},
},
],
};
}
1 change: 1 addition & 0 deletions x-pack/test/alerting_api_integration/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export interface User {

export interface Space {
id: string;
namespace?: string;
name: string;
disabledFeatures: string[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const NoKibanaPrivileges: User = {
},
};

const Superuser: User = {
export const Superuser: User = {
username: 'superuser',
fullName: 'superuser',
password: 'superuser-password',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import expect from '@kbn/expect';
import { UserAtSpaceScenarios } from '../../scenarios';
import { UserAtSpaceScenarios, Superuser } from '../../scenarios';
import { FtrProviderContext } from '../../../common/ftr_provider_context';
import {
ESTestIndexTool,
Expand Down Expand Up @@ -96,7 +96,9 @@ export default function alertTests({ getService }: FtrProviderContext) {

// Wait for the action to index a document before disabling the alert and waiting for tasks to finish
await esTestIndexTool.waitForDocs('action:test.index-record', reference);
await alertUtils.disable(response.body.id);

const alertId = response.body.id;
await alertUtils.disable(alertId);
await taskManagerUtils.waitForIdle(testStart);

// Ensure only 1 alert executed with proper params
Expand All @@ -113,6 +115,15 @@ export default function alertTests({ getService }: FtrProviderContext) {
index: ES_TEST_INDEX_NAME,
reference,
},
alertInfo: {
alertId,
spaceId: space.id,
namespace: space.id,
name: 'abc',
tags: [],
createdBy: user.fullName,
updatedBy: user.fullName,
},
});

// Ensure only 1 action executed with proper params
Expand Down Expand Up @@ -142,6 +153,56 @@ export default function alertTests({ getService }: FtrProviderContext) {
}
});

it('should pass updated alert params to executor', async () => {
// create an alert
const reference = alertUtils.generateReference();
const overwrites = {
throttle: '1s',
schedule: { interval: '1s' },
};
const response = await alertUtils.createAlwaysFiringAction({ reference, overwrites });

// only need to test creation success paths
if (response.statusCode !== 200) return;

// update the alert with super user
const alertId = response.body.id;
const reference2 = alertUtils.generateReference();
const response2 = await alertUtils.updateAlwaysFiringAction({
alertId,
actionId: indexRecordActionId,
user: Superuser,
reference: reference2,
overwrites: {
name: 'def',
tags: ['fee', 'fi', 'fo'],
throttle: '1s',
schedule: { interval: '1s' },
},
});

expect(response2.statusCode).to.eql(200);

// make sure alert info passed to executor is correct
await esTestIndexTool.waitForDocs('alert:test.always-firing', reference2);
await alertUtils.disable(alertId);
const alertSearchResult = await esTestIndexTool.search(
'alert:test.always-firing',
reference2
);

expect(alertSearchResult.hits.total.value).to.be.greaterThan(0);
expect(alertSearchResult.hits.hits[0]._source.alertInfo).to.eql({
alertId,
spaceId: space.id,
namespace: space.id,
name: 'def',
tags: ['fee', 'fi', 'fo'],
createdBy: user.fullName,
updatedBy: Superuser.fullName,
});
});

it('should handle custom retry logic when appropriate', async () => {
const testStart = new Date();
// We have to provide the test.rate-limit the next runAt, for testing purposes
Expand Down
Loading

0 comments on commit 5853360

Please sign in to comment.