[EventLog] Populate alert instances view with event log data #68437

pmuellr · 2020-06-05T19:18:41Z

resolves #57446

Summary

Adds a new API (AlertClient and HTTP endpoint) getAlertStatus() which returns
data calculated from the event log.

The data returned in this PR is fairly minimal - just enough to replace the
current instance details view data. In the future, we can add the following
sorts of things:

alert execution errors
counts of alert execution
sequences of active instance time spans
if we can get the alert SO into the action execution, we could also
provide action execution errors

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

resolves elastic#57446 Adds a new API (AlertClient and HTTP endpoint) `getAlertStatus()` which returns data calculated from the event log. The data returned in this PR is fairly minimal - just enough to replace the current instance details view data. In the future, we can add the following sorts of things: - alert execution errors - counts of alert execution - sequences of active instance time spans - if we can get the alert SO into the action execution, we could also provide action execution errors

mikecote

LGTM! This is great, the activity information is much better and precise. I added a question and a bunch of nits that can be skipped.

mikecote · 2020-08-10T17:50:18Z

x-pack/plugins/alerts/README.md

+
+|Property|Description|Type|
+|---|---|---|
+|id|The id of the alert whose state you're trying to get.|string|


mikecote · 2020-08-10T17:55:41Z

x-pack/plugins/alerts/server/alerts_client.ts

@@ -269,6 +284,72 @@ export class AlertsClient {
    }
  }

+  public async getAlertStatus({ id, dateStart }: GetAlertStatusParams): Promise<AlertStatus> {
+    const eventLogClient = await this.getEventLogClient();


super nit: This could be move below authorization checks to avoid loading when authorization fails.

mikecote · 2020-08-10T17:56:18Z

x-pack/plugins/alerts/server/alerts_client.ts

+  public async getAlertStatus({ id, dateStart }: GetAlertStatusParams): Promise<AlertStatus> {
+    const eventLogClient = await this.getEventLogClient();
+
+    this.logger.debug('getAlertStatus(): getting the alert');


nit: more context

Suggested change

this.logger.debug('getAlertStatus(): getting the alert');

this.logger.debug(`getAlertStatus(): getting alert ${id}`);

mikecote · 2020-08-10T17:57:46Z

x-pack/plugins/alerts/server/alerts_client.ts

+    const defaultDateStart = new Date(dateNow.valueOf() - durationMillis);
+    const parsedDateStart = parseDate(dateStart, 'dateStart', defaultDateStart);
+
+    this.logger.debug('getAlertStatus(): search the event log');


nit: more context

Suggested change

this.logger.debug('getAlertStatus(): search the event log');

this.logger.debug(`getAlertStatus(): search the event log for alert ${id}`);

mikecote · 2020-08-10T17:58:16Z

x-pack/plugins/alerts/server/alerts_client.ts

+      });
+      events = queryResults.data;
+    } catch (err) {
+      this.logger.debug(`alertsClient.getAlertStatus(): error searching event log: ${err.message}`);


nit: more context

Suggested change

this.logger.debug(`alertsClient.getAlertStatus(): error searching event log: ${err.message}`);

this.logger.debug(`alertsClient.getAlertStatus(): error searching event log: ${err.message} for alert ${id}`);

mikecote · 2020-08-10T17:59:31Z

x-pack/plugins/alerts/server/alerts_client.ts

+      dateEnd: dateNow.toISOString(),
+    });
+
+    function parseDate(


super nit: Function can be moved out of getAlertStatus to avoid being recreated on every call.

mikecote · 2020-08-10T18:00:30Z

x-pack/plugins/alerts/server/alerts_client.ts

+    await this.authorization.ensureAuthorized(
+      alert.alertTypeId,
+      alert.consumer,
+      ReadOperations.GetAlertStatus
+    );


I will defer to @gmmorris to review the access control bits.

I copied the bits from getAlertState for getAlertStatus, creating a new operation, etc. Hopefully I got everything - took a bit to find all the places this got hit, including something in security for this new operation.

Looks right, but that's just from a cursory look at the code.
I'll try running it soon, just trying to get something done locally before I can pull it down.

mikecote · 2020-08-10T18:08:59Z

x-pack/plugins/alerts/server/task_runner/task_runner.ts

+  const { eventLogger, alertId, namespace } = params;
  const { currentAlertInstanceIds, originalAlertInstanceIds } = params;


nit: these two lines can merge

I suspect I did this on purpose - a split like this seems silly, but ends up resulting in 2 lines of code - the merged version requires the props to be split one-per-line ala prettier, and so turns in to 7 lines. :-)

But if it's odd enough for someone to comment on, worth paying the extra 5 lines for it!

Ah, didn't think of that! :-)

mikecote · 2020-08-10T19:57:41Z

x-pack/plugins/alerts/server/alerts_client.ts

+    try {
+      const queryResults = await eventLogClient.findEventsBySavedObject('alert', id, {
+        page: 1,
+        per_page: 10000,


I wonder if we should open a tech debt issue to expand this limitation?

I'm sure you've put more thought than I have. I'm wondering how likely this number would be reached on alerts that run frequently or have a massive number of instances? Ex: 100 always firing instances * 100 executions would limit the events returned.

This is a bit of a funky API, since you can only provide a start date and not an end date, since the end date is always NOW because the data returned also takes into current data (eg, muted instance ids).

I'm not opposed to creating a tech debt issue, especially since it's possible that we could "miss" some data. I guess the idea would be that we should page through the events till we get to the start date. I just opened issue #74860 for this.

jportner

Code review only -- Security changes in x-pack/plugins/security/server/authorization/privileges/feature_privilege_builder/alerting.ts and x-pack/plugins/security/server/authorization/privileges/feature_privilege_builder/alerting.test.ts LGTM!

YulNaumenko

LGTM

elasticmachine · 2020-08-12T13:59:29Z

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

kibanamachine · 2020-08-12T16:44:13Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: fb3a7a9

Build metrics

@kbn/optimizer bundle module count

id	value	diff	baseline
alerts	26	+1	25

async chunks size

id	value	diff	baseline
triggers_actions_ui	736.6KB	+882.0B	735.7KB

page load bundle size

id	value	diff	baseline
alerts	89.1KB	+602.0B	88.5KB

History

💚 Build #67701 succeeded ab427f5
💚 Build #67284 succeeded 0733286
💚 Build #67276 succeeded c10f66f
💚 Build #67261 succeeded 742af94357cbc89120603fe6b0a79c96afa0466a
💚 Build #66617 succeeded d3ec56f411e358505ef840ae2a7b0104cd3669a2

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…#68437) resolves elastic#57446 Adds a new API (AlertClient and HTTP endpoint) `getAlertStatus()` which returns alert data calculated from the event log.

* master: Fix bug on TopN weird behavior with zero values (elastic#74942) [Lens] Fix table sorting bug (elastic#74902) [SECURITY_SOLUTION] Retry on ingest setup (elastic#75000) [file upload] lazy load to reduce page load size (elastic#74967) Drilldowns for TSVB / Vega / Timelion (elastic#74848) [EventLog] Populate alert instances view with event log data (elastic#68437) [UiActions] pass trigger into action execution context (elastic#74363)

…#75036) resolves #57446 Adds a new API (AlertClient and HTTP endpoint) `getAlertStatus()` which returns alert data calculated from the event log.

pmuellr mentioned this pull request Jun 5, 2020

[Alerting] use event log to populate alert details view #66351

Closed

7 tasks

pmuellr force-pushed the alerting/details-via-el-2 branch from 2ceae3f to 46257d9 Compare June 9, 2020 13:24

pmuellr changed the title ~~[Alerting] enhance alert instances view with event log data~~ [EventLog] Populate alert instances view with event log data Jun 9, 2020

pmuellr force-pushed the alerting/details-via-el-2 branch 3 times, most recently from ab786db to bac3a8a Compare June 16, 2020 16:24

pmuellr force-pushed the alerting/details-via-el-2 branch 3 times, most recently from 0a8bd81 to 70570ef Compare June 25, 2020 21:04

pmuellr mentioned this pull request Jun 26, 2020

[event log] can't find events since action/alert saved object are now hidden #70086

Closed

pmuellr force-pushed the alerting/details-via-el-2 branch 2 times, most recently from df65c14 to c2dd065 Compare June 30, 2020 14:12

pmuellr force-pushed the alerting/details-via-el-2 branch 4 times, most recently from a32dda4 to acb9f42 Compare July 23, 2020 00:55

pmuellr force-pushed the alerting/details-via-el-2 branch 2 times, most recently from caa38d0 to 2e04a44 Compare August 5, 2020 00:00

pmuellr force-pushed the alerting/details-via-el-2 branch from 742af94 to c10f66f Compare August 10, 2020 15:08

add short doc on the GET status API to the readme

0733286

pmuellr marked this pull request as ready for review August 10, 2020 15:34

pmuellr requested review from a team as code owners August 10, 2020 15:34

mikecote self-requested a review August 10, 2020 17:50

mikecote approved these changes Aug 10, 2020

View reviewed changes

jportner approved these changes Aug 10, 2020

View reviewed changes

YulNaumenko approved these changes Aug 11, 2020

View reviewed changes

merge master and fix conflict

ab427f5

pmuellr added the Feature:Alerting label Aug 12, 2020

pmuellr added the Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) label Aug 12, 2020

pmuellr added v7.10.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Aug 12, 2020

pmuellr mentioned this pull request Aug 12, 2020

[Alerting] getAlertStatus() should exhaustively search through event log #74860

Open

resolve PR comments

fb3a7a9

pmuellr merged commit 67e28ac into elastic:master Aug 14, 2020

pmuellr mentioned this pull request Aug 14, 2020

[7.x] [EventLog] Populate alert instances view with event log data (#68437) #75036

Merged

pmuellr added the backported label Aug 14, 2020

pmuellr mentioned this pull request Aug 18, 2020

[Alerting] rename alert client getStatus API #75332

Closed

mikecote mentioned this pull request Nov 25, 2020

[Alerting] Index threshold: Actions not fired as expected #84335

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EventLog] Populate alert instances view with event log data #68437

[EventLog] Populate alert instances view with event log data #68437

pmuellr commented Jun 5, 2020 •

edited

Loading

mikecote left a comment

mikecote Aug 10, 2020

mikecote Aug 10, 2020

mikecote Aug 10, 2020

mikecote Aug 10, 2020

mikecote Aug 10, 2020

mikecote Aug 10, 2020

mikecote Aug 10, 2020

pmuellr Aug 12, 2020

gmmorris Aug 12, 2020

mikecote Aug 10, 2020

pmuellr Aug 12, 2020

mikecote Aug 12, 2020

mikecote Aug 10, 2020 •

edited

Loading

pmuellr Aug 12, 2020

jportner left a comment

YulNaumenko left a comment

elasticmachine commented Aug 12, 2020

kibanamachine commented Aug 12, 2020

	\|id\|The id of the alert whose state you're trying to get.\|string\|
	\|id\|The id of the alert whose status you're trying to get.\|string\|

	this.logger.debug('getAlertStatus(): getting the alert');
	this.logger.debug(`getAlertStatus(): getting alert ${id}`);

	this.logger.debug('getAlertStatus(): search the event log');
	this.logger.debug(`getAlertStatus(): search the event log for alert ${id}`);

	this.logger.debug(`alertsClient.getAlertStatus(): error searching event log: ${err.message}`);
	this.logger.debug(`alertsClient.getAlertStatus(): error searching event log: ${err.message} for alert ${id}`);

		const { eventLogger, alertId, namespace } = params;
		const { currentAlertInstanceIds, originalAlertInstanceIds } = params;

[EventLog] Populate alert instances view with event log data #68437

[EventLog] Populate alert instances view with event log data #68437

Conversation

pmuellr commented Jun 5, 2020 • edited Loading

Summary

Checklist

mikecote left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikecote Aug 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jportner left a comment

Choose a reason for hiding this comment

YulNaumenko left a comment

Choose a reason for hiding this comment

elasticmachine commented Aug 12, 2020

kibanamachine commented Aug 12, 2020

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

async chunks size

page load bundle size

History

pmuellr commented Jun 5, 2020 •

edited

Loading

mikecote Aug 10, 2020 •

edited

Loading