From d145f25ba032c2732aa0450dcee54af144a237c6 Mon Sep 17 00:00:00 2001 From: Mike Cote Date: Fri, 26 Jul 2019 10:33:13 -0400 Subject: [PATCH] Leverage startedAt from task manager --- x-pack/legacy/plugins/alerting/README.md | 12 +- .../alerting/server/alerts_client.test.ts | 432 +++++++++--------- .../plugins/alerting/server/alerts_client.ts | 7 +- .../get_create_task_runner_function.test.ts | 82 ++-- .../lib/get_create_task_runner_function.ts | 14 +- .../legacy/plugins/alerting/server/types.ts | 4 +- 6 files changed, 276 insertions(+), 275 deletions(-) diff --git a/x-pack/legacy/plugins/alerting/README.md b/x-pack/legacy/plugins/alerting/README.md index 091d5a9da0115a5..18034807b7ac752 100644 --- a/x-pack/legacy/plugins/alerting/README.md +++ b/x-pack/legacy/plugins/alerting/README.md @@ -52,8 +52,8 @@ This is the primary function for an alert type. Whenever the alert needs to exec |services.callCluster(path, opts)|Use this to do Elasticsearch queries on the cluster Kibana connects to. This function is the same as any other `callCluster` in Kibana.

**NOTE**: This currently authenticates as the Kibana internal user, but will change in a future PR.| |services.savedObjectsClient|This is an instance of the saved objects client. This provides the ability to do CRUD on any saved objects within the same space the alert lives in.

**NOTE**: This currently only works when security is disabled. A future PR will add support for enabled security using Elasticsearch API tokens.| |services.log(tags, [data], [timestamp])|Use this to create server logs. (This is the same function as server.log)| -|scheduledRunAt|The date and time the alert type execution was scheduled to be called.| -|previousScheduledRunAt|The previous date and time the alert type was scheduled to be called.| +|startedAt|The date and time the alert type started execution.| +|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.| @@ -74,8 +74,8 @@ server.plugins.alerting.registerType({ }), }, async executor({ - scheduledRunAt, - previousScheduledRunAt, + startedAt, + previousStartedAt, services, params, state, @@ -131,8 +131,8 @@ server.plugins.alerting.registerType({ }), }, async executor({ - scheduledRunAt, - previousScheduledRunAt, + startedAt, + previousStartedAt, services, params, state, diff --git a/x-pack/legacy/plugins/alerting/server/alerts_client.test.ts b/x-pack/legacy/plugins/alerting/server/alerts_client.test.ts index 4421ea7435c9827..8a6ffe7ed45064c 100644 --- a/x-pack/legacy/plugins/alerting/server/alerts_client.test.ts +++ b/x-pack/legacy/plugins/alerting/server/alerts_client.test.ts @@ -119,99 +119,98 @@ describe('create()', () => { }); const result = await alertsClient.create({ data }); expect(result).toMatchInlineSnapshot(` - Object { - "actions": Array [ - Object { - "group": "default", - "id": "1", - "params": Object { - "foo": true, - }, - }, - ], - "alertTypeId": "123", - "alertTypeParams": Object { - "bar": true, - }, - "id": "1", - "interval": "10s", - "scheduledTaskId": "task-123", - } - `); + Object { + "actions": Array [ + Object { + "group": "default", + "id": "1", + "params": Object { + "foo": true, + }, + }, + ], + "alertTypeId": "123", + "alertTypeParams": Object { + "bar": true, + }, + "id": "1", + "interval": "10s", + "scheduledTaskId": "task-123", + } + `); expect(savedObjectsClient.create).toHaveBeenCalledTimes(1); expect(savedObjectsClient.create.mock.calls[0]).toHaveLength(3); expect(savedObjectsClient.create.mock.calls[0][0]).toEqual('alert'); expect(savedObjectsClient.create.mock.calls[0][1]).toMatchInlineSnapshot(` - Object { - "actions": Array [ - Object { - "actionRef": "action_0", - "group": "default", - "params": Object { - "foo": true, - }, - }, - ], - "alertTypeId": "123", - "alertTypeParams": Object { - "bar": true, - }, - "enabled": true, - "interval": "10s", - } - `); + Object { + "actions": Array [ + Object { + "actionRef": "action_0", + "group": "default", + "params": Object { + "foo": true, + }, + }, + ], + "alertTypeId": "123", + "alertTypeParams": Object { + "bar": true, + }, + "enabled": true, + "interval": "10s", + } + `); expect(savedObjectsClient.create.mock.calls[0][2]).toMatchInlineSnapshot(` - Object { - "references": Array [ - Object { - "id": "1", - "name": "action_0", - "type": "action", - }, - ], - } - `); + Object { + "references": Array [ + Object { + "id": "1", + "name": "action_0", + "type": "action", + }, + ], + } + `); expect(taskManager.schedule).toHaveBeenCalledTimes(1); expect(taskManager.schedule.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - Object { - "params": Object { - "alertId": "1", - "basePath": "/s/default", - }, - "scope": Array [ - "alerting", - ], - "state": Object { - "alertInstances": Object {}, - "alertTypeState": Object {}, - "previousScheduledRunAt": null, - "scheduledRunAt": 2019-02-12T21:01:22.479Z, - }, - "taskType": "alerting:123", - }, - ] - `); + Array [ + Object { + "params": Object { + "alertId": "1", + "basePath": "/s/default", + }, + "scope": Array [ + "alerting", + ], + "state": Object { + "alertInstances": Object {}, + "alertTypeState": Object {}, + "previousStartedAt": null, + }, + "taskType": "alerting:123", + }, + ] + `); expect(savedObjectsClient.update).toHaveBeenCalledTimes(1); expect(savedObjectsClient.update.mock.calls[0]).toHaveLength(4); expect(savedObjectsClient.update.mock.calls[0][0]).toEqual('alert'); expect(savedObjectsClient.update.mock.calls[0][1]).toEqual('1'); expect(savedObjectsClient.update.mock.calls[0][2]).toMatchInlineSnapshot(` - Object { - "scheduledTaskId": "task-123", - } - `); + Object { + "scheduledTaskId": "task-123", + } + `); expect(savedObjectsClient.update.mock.calls[0][3]).toMatchInlineSnapshot(` - Object { - "references": Array [ - Object { - "id": "1", - "name": "action_0", - "type": "action", - }, - ], - } - `); + Object { + "references": Array [ + Object { + "id": "1", + "name": "action_0", + "type": "action", + }, + ], + } + `); }); test('creates a disabled alert', async () => { @@ -252,25 +251,25 @@ describe('create()', () => { }); const result = await alertsClient.create({ data }); expect(result).toMatchInlineSnapshot(` - Object { - "actions": Array [ - Object { - "group": "default", - "id": "1", - "params": Object { - "foo": true, - }, - }, - ], - "alertTypeId": "123", - "alertTypeParams": Object { - "bar": true, - }, - "enabled": false, - "id": "1", - "interval": 10000, - } - `); + Object { + "actions": Array [ + Object { + "group": "default", + "id": "1", + "params": Object { + "foo": true, + }, + }, + ], + "alertTypeId": "123", + "alertTypeParams": Object { + "bar": true, + }, + "enabled": false, + "id": "1", + "interval": 10000, + } + `); expect(savedObjectsClient.create).toHaveBeenCalledTimes(1); expect(taskManager.schedule).toHaveBeenCalledTimes(0); }); @@ -351,11 +350,11 @@ describe('create()', () => { ); expect(savedObjectsClient.delete).toHaveBeenCalledTimes(1); expect(savedObjectsClient.delete.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - "alert", - "1", - ] - `); + Array [ + "alert", + "1", + ] + `); }); test('returns task manager error if cleanup fails, logs to console', async () => { @@ -400,14 +399,14 @@ describe('create()', () => { ); expect(alertsClientParams.log).toHaveBeenCalledTimes(1); expect(alertsClientParams.log.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - Array [ - "alerting", - "error", - ], - "Failed to cleanup alert \\"1\\" after scheduling task failed. Error: Saved object delete error", - ] - `); + Array [ + Array [ + "alerting", + "error", + ], + "Failed to cleanup alert \\"1\\" after scheduling task failed. Error: Saved object delete error", + ] + `); }); test('throws an error if alert type not registerd', async () => { @@ -469,8 +468,7 @@ describe('enable()', () => { state: { alertInstances: {}, alertTypeState: {}, - previousScheduledRunAt: null, - scheduledRunAt: mockedDate, + previousStartedAt: null, }, scope: ['alerting'], }); @@ -577,31 +575,31 @@ describe('get()', () => { }); const result = await alertsClient.get({ id: '1' }); expect(result).toMatchInlineSnapshot(` - Object { - "actions": Array [ - Object { - "group": "default", - "id": "1", - "params": Object { - "foo": true, - }, - }, - ], - "alertTypeId": "123", - "alertTypeParams": Object { - "bar": true, - }, - "id": "1", - "interval": "10s", - } - `); + Object { + "actions": Array [ + Object { + "group": "default", + "id": "1", + "params": Object { + "foo": true, + }, + }, + ], + "alertTypeId": "123", + "alertTypeParams": Object { + "bar": true, + }, + "id": "1", + "interval": "10s", + } + `); expect(savedObjectsClient.get).toHaveBeenCalledTimes(1); expect(savedObjectsClient.get.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - "alert", - "1", - ] - `); + Array [ + "alert", + "1", + ] + `); }); test(`throws an error when references aren't found`, async () => { @@ -672,34 +670,34 @@ describe('find()', () => { }); const result = await alertsClient.find(); expect(result).toMatchInlineSnapshot(` - Array [ - Object { - "actions": Array [ - Object { - "group": "default", - "id": "1", - "params": Object { - "foo": true, + Array [ + Object { + "actions": Array [ + Object { + "group": "default", + "id": "1", + "params": Object { + "foo": true, + }, + }, + ], + "alertTypeId": "123", + "alertTypeParams": Object { + "bar": true, + }, + "id": "1", + "interval": "10s", }, - }, - ], - "alertTypeId": "123", - "alertTypeParams": Object { - "bar": true, - }, - "id": "1", - "interval": "10s", - }, - ] - `); + ] + `); expect(savedObjectsClient.find).toHaveBeenCalledTimes(1); expect(savedObjectsClient.find.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - Object { - "type": "alert", - }, - ] - `); + Array [ + Object { + "type": "alert", + }, + ] + `); }); }); @@ -741,17 +739,17 @@ describe('delete()', () => { expect(result).toEqual({ success: true }); expect(savedObjectsClient.delete).toHaveBeenCalledTimes(1); expect(savedObjectsClient.delete.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - "alert", - "1", - ] - `); + Array [ + "alert", + "1", + ] + `); expect(taskManager.remove).toHaveBeenCalledTimes(1); expect(taskManager.remove.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - "task-123", - ] - `); + Array [ + "task-123", + ] + `); }); }); @@ -823,58 +821,58 @@ describe('update()', () => { }, }); expect(result).toMatchInlineSnapshot(` - Object { - "actions": Array [ - Object { - "group": "default", - "id": "1", - "params": Object { - "foo": true, - }, - }, - ], - "alertTypeParams": Object { - "bar": true, - }, - "enabled": true, - "id": "1", - "interval": "10s", - "scheduledTaskId": "task-123", - } - `); + Object { + "actions": Array [ + Object { + "group": "default", + "id": "1", + "params": Object { + "foo": true, + }, + }, + ], + "alertTypeParams": Object { + "bar": true, + }, + "enabled": true, + "id": "1", + "interval": "10s", + "scheduledTaskId": "task-123", + } + `); expect(savedObjectsClient.update).toHaveBeenCalledTimes(1); expect(savedObjectsClient.update.mock.calls[0]).toHaveLength(4); expect(savedObjectsClient.update.mock.calls[0][0]).toEqual('alert'); expect(savedObjectsClient.update.mock.calls[0][1]).toEqual('1'); expect(savedObjectsClient.update.mock.calls[0][2]).toMatchInlineSnapshot(` - Object { - "actions": Array [ - Object { - "actionRef": "action_0", - "group": "default", - "params": Object { - "foo": true, - }, - }, - ], - "alertTypeParams": Object { - "bar": true, - }, - "interval": "10s", - } - `); + Object { + "actions": Array [ + Object { + "actionRef": "action_0", + "group": "default", + "params": Object { + "foo": true, + }, + }, + ], + "alertTypeParams": Object { + "bar": true, + }, + "interval": "10s", + } + `); expect(savedObjectsClient.update.mock.calls[0][3]).toMatchInlineSnapshot(` - Object { - "references": Array [ - Object { - "id": "1", - "name": "action_0", - "type": "action", - }, - ], - "version": "123", - } - `); + Object { + "references": Array [ + Object { + "id": "1", + "name": "action_0", + "type": "action", + }, + ], + "version": "123", + } + `); }); it('should validate alertTypeParams', async () => { diff --git a/x-pack/legacy/plugins/alerting/server/alerts_client.ts b/x-pack/legacy/plugins/alerting/server/alerts_client.ts index c4f87270d5b7e7d..4cfee6f4ab1f56e 100644 --- a/x-pack/legacy/plugins/alerting/server/alerts_client.ts +++ b/x-pack/legacy/plugins/alerting/server/alerts_client.ts @@ -8,7 +8,7 @@ import { omit } from 'lodash'; import { SavedObjectsClientContract, SavedObjectReference } from 'src/core/server'; import { Alert, RawAlert, AlertTypeRegistry, AlertAction, Log } from './types'; import { TaskManager } from '../../task_manager'; -import { validateAlertTypeParams, parseDuration } from './lib'; +import { validateAlertTypeParams } from './lib'; interface ConstructorOptions { log: Log; @@ -213,10 +213,7 @@ export class AlertsClient { basePath, }, state: { - // This is here because we can't rely on the task manager's internal runAt. - // It changes it for timeout, etc when a task is running. - scheduledRunAt: new Date(Date.now() + parseDuration(interval)), - previousScheduledRunAt: null, + previousStartedAt: null, alertTypeState: {}, alertInstances: {}, }, diff --git a/x-pack/legacy/plugins/alerting/server/lib/get_create_task_runner_function.test.ts b/x-pack/legacy/plugins/alerting/server/lib/get_create_task_runner_function.test.ts index c4a992773e2e51a..f12751e2288dd85 100644 --- a/x-pack/legacy/plugins/alerting/server/lib/get_create_task_runner_function.test.ts +++ b/x-pack/legacy/plugins/alerting/server/lib/get_create_task_runner_function.test.ts @@ -4,18 +4,38 @@ * you may not use this file except in compliance with the Elastic License. */ +import sinon from 'sinon'; import { schema } from '@kbn/config-schema'; import { AlertExecutorOptions } from '../types'; +import { ConcreteTaskInstance } from '../../../task_manager'; import { SavedObjectsClientMock } from '../../../../../../src/core/server/mocks'; import { getCreateTaskRunnerFunction } from './get_create_task_runner_function'; -const mockedNow = new Date('2019-06-03T18:55:25.982Z'); -const mockedLastRunAt = new Date('2019-06-03T18:55:20.982Z'); -(global as any).Date = class Date extends global.Date { - static now() { - return mockedNow.getTime(); - } -}; +let fakeTimer: sinon.SinonFakeTimers; +let mockedTaskInstance: ConcreteTaskInstance; + +beforeAll(() => { + fakeTimer = sinon.useFakeTimers(); + mockedTaskInstance = { + id: '', + attempts: 0, + status: 'running', + version: '123', + runAt: new Date(), + scheduledAt: new Date(), + startedAt: new Date(), + retryAt: new Date(Date.now() + 5 * 60 * 1000), + state: { + startedAt: new Date(Date.now() - 5 * 60 * 1000), + }, + taskType: 'alerting:test', + params: { + alertId: '1', + }, + }; +}); + +afterAll(() => fakeTimer.restore()); const savedObjectsClient = SavedObjectsClientMock.create(); @@ -36,17 +56,6 @@ const getCreateTaskRunnerFunctionParams = { internalSavedObjectsRepository: savedObjectsClient, }; -const mockedTaskInstance = { - runAt: mockedLastRunAt, - state: { - scheduledRunAt: mockedLastRunAt, - }, - taskType: 'alerting:test', - params: { - alertId: '1', - }, -}; - const mockedAlertTypeSavedObject = { id: '1', type: 'alert', @@ -85,23 +94,22 @@ test('successfully executes the task', async () => { const runnerResult = await runner.run(); expect(runnerResult).toMatchInlineSnapshot(` Object { - "runAt": 2019-06-03T18:55:30.982Z, + "runAt": 1970-01-01T00:00:10.000Z, "state": Object { "alertInstances": Object {}, "alertTypeState": undefined, - "previousScheduledRunAt": 2019-06-03T18:55:20.982Z, - "scheduledRunAt": 2019-06-03T18:55:30.982Z, + "previousStartedAt": 1970-01-01T00:00:00.000Z, }, } `); expect(getCreateTaskRunnerFunctionParams.alertType.executor).toHaveBeenCalledTimes(1); const call = getCreateTaskRunnerFunctionParams.alertType.executor.mock.calls[0][0]; expect(call.params).toMatchInlineSnapshot(` - Object { - "bar": true, - } - `); - expect(call.scheduledRunAt).toMatchInlineSnapshot(`2019-06-03T18:55:20.982Z`); + Object { + "bar": true, + } + `); + expect(call.startedAt).toMatchInlineSnapshot(`1970-01-01T00:00:00.000Z`); expect(call.state).toMatchInlineSnapshot(`Object {}`); expect(call.services.alertInstanceFactory).toBeTruthy(); expect(call.services.callCluster).toBeTruthy(); @@ -120,16 +128,16 @@ test('fireAction is called per alert instance that fired', async () => { await runner.run(); expect(getCreateTaskRunnerFunctionParams.fireAction).toHaveBeenCalledTimes(1); expect(getCreateTaskRunnerFunctionParams.fireAction.mock.calls[0]).toMatchInlineSnapshot(` - Array [ - Object { - "basePath": undefined, - "id": "1", - "params": Object { - "foo": true, - }, - }, - ] - `); + Array [ + Object { + "basePath": undefined, + "id": "1", + "params": Object { + "foo": true, + }, + }, + ] + `); }); test('persists alertInstances passed in from state, only if they fire', async () => { @@ -157,7 +165,7 @@ test('persists alertInstances passed in from state, only if they fire', async () Object { "1": Object { "meta": Object { - "lastFired": 1559588125982, + "lastFired": 0, }, "state": Object { "bar": false, diff --git a/x-pack/legacy/plugins/alerting/server/lib/get_create_task_runner_function.ts b/x-pack/legacy/plugins/alerting/server/lib/get_create_task_runner_function.ts index c21ddbe7ed98637..5591b63188b35b3 100644 --- a/x-pack/legacy/plugins/alerting/server/lib/get_create_task_runner_function.ts +++ b/x-pack/legacy/plugins/alerting/server/lib/get_create_task_runner_function.ts @@ -7,7 +7,7 @@ import { SavedObjectsClientContract } from 'src/core/server'; import { ActionsPlugin } from '../../../actions'; import { AlertType, Services, AlertServices } from '../types'; -import { TaskInstance } from '../../../task_manager'; +import { ConcreteTaskInstance } from '../../../task_manager'; import { createFireHandler } from './create_fire_handler'; import { createAlertInstanceFactory } from './create_alert_instance_factory'; import { AlertInstance } from './alert_instance'; @@ -22,7 +22,7 @@ interface CreateTaskRunnerFunctionOptions { } interface TaskRunnerOptions { - taskInstance: TaskInstance; + taskInstance: ConcreteTaskInstance; } export function getCreateTaskRunnerFunction({ @@ -66,8 +66,8 @@ export function getCreateTaskRunnerFunction({ services: alertTypeServices, params: validatedAlertTypeParams, state: taskInstance.state.alertTypeState || {}, - scheduledRunAt: taskInstance.state.scheduledRunAt, - previousScheduledRunAt: taskInstance.state.previousScheduledRunAt, + startedAt: taskInstance.startedAt!, + previousStartedAt: taskInstance.state.previousStartedAt, }); await Promise.all( @@ -88,7 +88,7 @@ export function getCreateTaskRunnerFunction({ ); const nextRunAt = getNextRunAt( - new Date(taskInstance.state.scheduledRunAt), + new Date(taskInstance.startedAt!), alertSavedObject.attributes.interval ); @@ -96,9 +96,7 @@ export function getCreateTaskRunnerFunction({ state: { alertTypeState, alertInstances, - // We store nextRunAt ourselves since task manager changes runAt when executing a task - scheduledRunAt: nextRunAt, - previousScheduledRunAt: taskInstance.state.scheduledRunAt, + previousStartedAt: taskInstance.startedAt!, }, runAt: nextRunAt, }; diff --git a/x-pack/legacy/plugins/alerting/server/types.ts b/x-pack/legacy/plugins/alerting/server/types.ts index 50fbba498226f52..b1e268431e40e94 100644 --- a/x-pack/legacy/plugins/alerting/server/types.ts +++ b/x-pack/legacy/plugins/alerting/server/types.ts @@ -29,8 +29,8 @@ export interface AlertServices extends Services { } export interface AlertExecutorOptions { - scheduledRunAt: Date; - previousScheduledRunAt?: Date; + startedAt: Date; + previousStartedAt?: Date; services: AlertServices; params: Record; state: State;