Skip to content

Commit

Permalink
[alerting] adds an optional name to associate with an instance
Browse files Browse the repository at this point in the history
resolves elastic#64268

Instances can now be retrieved/created via

    alertInstanceFactory('instance-id', { name: 'a nice name for this instance' });

Alert instances can use `getName()` to return the name, if set.

Adds new context variables for all alerts with names `alertInstanceName` and
`alertInstanceNameOrId`.  The former may be `undefined`.  The latter is
`alertInstance.name || alertInstanceId`.

----

However, from existing usage as documented in that issue, I don't think we
need this quite **yet**. I think a lot of the usages of alertIds in those
instance ids aren't required or desired.
  • Loading branch information
pmuellr committed Jul 7, 2020
1 parent 67c70e7 commit c114080
Show file tree
Hide file tree
Showing 9 changed files with 122 additions and 18 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/alerts/common/alert_instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const stateSchema = t.record(t.string, t.unknown);
export type AlertInstanceState = t.TypeOf<typeof stateSchema>;

export const rawAlertInstance = t.partial({
name: t.string,
state: stateSchema,
meta: metaSchema,
});
Expand Down
43 changes: 43 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 @@ -109,6 +109,18 @@ describe('getState()', () => {
});
});

describe('getName()', () => {
test('returns undefined if no name provided', () => {
const alertInstance = new AlertInstance();
expect(alertInstance.getName()).toEqual(undefined);
});

test('returns name if provided', () => {
const alertInstance = new AlertInstance({ name: 'foo' });
expect(alertInstance.getName()).toEqual('foo');
});
});

describe('scheduleActions()', () => {
test('makes hasScheduledActions() return true', () => {
const alertInstance = new AlertInstance({
Expand Down Expand Up @@ -215,6 +227,22 @@ describe('toJSON', () => {
'{"state":{"foo":true},"meta":{"lastScheduledActions":{"date":"1970-01-01T00:00:00.000Z","group":"default"}}}'
);
});

test('serializes state, meta and name', () => {
const alertInstance = new AlertInstance({
name: 'foo',
state: { foo: true },
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
},
},
});
expect(JSON.stringify(alertInstance)).toEqual(
'{"name":"foo","state":{"foo":true},"meta":{"lastScheduledActions":{"date":"1970-01-01T00:00:00.000Z","group":"default"}}}'
);
});
});

describe('toRaw', () => {
Expand All @@ -231,4 +259,19 @@ describe('toRaw', () => {
const alertInstance = new AlertInstance(raw);
expect(alertInstance.toRaw()).toEqual(raw);
});

test('returns unserialised underlying state, meta and name', () => {
const raw = {
name: 'foo',
state: { foo: true },
meta: {
lastScheduledActions: {
date: new Date(),
group: 'default',
},
},
};
const alertInstance = new AlertInstance(raw);
expect(alertInstance.toRaw()).toEqual(raw);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ interface ScheduledExecutionOptions {
export type AlertInstances = Record<string, AlertInstance>;
export class AlertInstance {
private scheduledExecutionOptions?: ScheduledExecutionOptions;
private name?: string;
private meta: AlertInstanceMeta;
private state: AlertInstanceState;

constructor({ state = {}, meta = {} }: RawAlertInstance = {}) {
constructor({ state = {}, meta = {}, name }: RawAlertInstance = {}) {
this.name = name;
this.state = state;
this.meta = meta;
}
Expand Down Expand Up @@ -62,6 +64,10 @@ export class AlertInstance {
return this.state;
}

getName() {
return this.name;
}

scheduleActions(actionGroup: string, context: Context = {}) {
if (this.hasScheduledActions()) {
throw new Error('Alert instance execution has already been scheduled, cannot schedule twice');
Expand All @@ -88,6 +94,7 @@ export class AlertInstance {

toRaw(): RawAlertInstance {
return {
name: this.name,
state: this.state,
meta: this.meta,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ test('creates new instances for ones not passed in', () => {
const alertInstanceFactory = createAlertInstanceFactory({});
const result = alertInstanceFactory('1');
expect(result).toMatchInlineSnapshot(`
Object {
"meta": Object {},
"state": Object {},
}
`);
Object {
"meta": Object {},
"name": undefined,
"state": Object {},
}
`);
});

test('reuses existing instances', () => {
Expand All @@ -44,6 +45,7 @@ test('reuses existing instances', () => {
"group": "default",
},
},
"name": undefined,
"state": Object {
"foo": true,
},
Expand All @@ -56,11 +58,27 @@ test('mutates given instances', () => {
const alertInstanceFactory = createAlertInstanceFactory(alertInstances);
alertInstanceFactory('1');
expect(alertInstances).toMatchInlineSnapshot(`
Object {
"1": Object {
"meta": Object {},
"state": Object {},
},
}
`);
Object {
"1": Object {
"meta": Object {},
"name": undefined,
"state": Object {},
},
}
`);
});

test('creates an instance with a name', () => {
const alertInstances = {};
const alertInstanceFactory = createAlertInstanceFactory(alertInstances);
alertInstanceFactory('1', { name: 'one' });
expect(alertInstances).toMatchInlineSnapshot(`
Object {
"1": Object {
"meta": Object {},
"name": "one",
"state": Object {},
},
}
`);
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@

import { AlertInstance } from './alert_instance';

export interface AlertInstanceOptions {
name?: string;
}

export function createAlertInstanceFactory(alertInstances: Record<string, AlertInstance>) {
return (id: string): AlertInstance => {
return (id: string, options: AlertInstanceOptions = {}): AlertInstance => {
if (!alertInstances[id]) {
alertInstances[id] = new AlertInstance();
alertInstances[id] = new AlertInstance(options);
}

return alertInstances[id];
Expand Down
7 changes: 5 additions & 2 deletions x-pack/plugins/alerts/server/task_runner/task_runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,9 @@ describe('Task Runner', () => {
test('persists alertInstances passed in from state, only if they are scheduled for execution', async () => {
alertType.executor.mockImplementation(
({ services: executorServices }: AlertExecutorOptions) => {
executorServices.alertInstanceFactory('1').scheduleActions('default');
executorServices
.alertInstanceFactory('1', { name: 'ignored-name' })
.scheduleActions('default');
}
);
const taskRunner = new TaskRunner(
Expand All @@ -432,7 +434,7 @@ describe('Task Runner', () => {
state: {
...mockedTaskInstance.state,
alertInstances: {
'1': { meta: {}, state: { bar: false } },
'1': { name: 'one', meta: {}, state: { bar: false } },
'2': { meta: {}, state: { bar: false } },
},
},
Expand All @@ -458,6 +460,7 @@ describe('Task Runner', () => {
"group": "default",
},
},
"name": "one",
"state": Object {
"bar": false,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ interface TransformActionParamsOptions {
spaceId: string;
tags?: string[];
alertInstanceId: string;
alertInstanceName: string;
actionParams: AlertActionParams;
state: State;
context: Context;
Expand All @@ -25,6 +26,7 @@ export function transformActionParams({
spaceId,
tags,
alertInstanceId,
alertInstanceName,
context,
actionParams,
state,
Expand All @@ -35,12 +37,15 @@ export function transformActionParams({
// when the list of variables we pass in here changes,
// the UI will need to be updated as well; see:
// x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.ts
const alertInstanceNameOrId = alertInstanceName ?? alertInstanceId;
const variables = {
alertId,
alertName,
spaceId,
tags,
alertInstanceId,
alertInstanceName,
alertInstanceNameOrId,
context,
state,
};
Expand Down
6 changes: 5 additions & 1 deletion x-pack/plugins/alerts/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,12 @@ export interface Services {
): ILegacyScopedClusterClient['callAsCurrentUser'];
}

interface AlertInstanceFactoryOptions {
name?: string;
}

export interface AlertServices extends Services {
alertInstanceFactory: (id: string) => AlertInstance;
alertInstanceFactory: (id: string, options?: AlertInstanceFactoryOptions) => AlertInstance;
}

export interface AlertExecutorOptions {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,24 @@ function getAlwaysProvidedActionVariables(): ActionVariable[] {
}),
});

result.push({
name: 'alertInstanceName',
description: i18n.translate('xpack.triggersActionsUI.actionVariables.alertInstanceNameLabel', {
defaultMessage:
'The name of the alert instance that scheduled actions for the alert, if provided.',
}),
});

result.push({
name: 'alertInstanceNameOrId',
description: i18n.translate(
'xpack.triggersActionsUI.actionVariables.alertInstanceNameOrIdLabel',
{
defaultMessage:
'The name of the alert instance that scheduled actions for the alert, if provided, otherwise the id.',
}
),
});

return result;
}

0 comments on commit c114080

Please sign in to comment.