Save ES Query Rule type alerts in alert-as-data index #161685

ersin-erdal · 2023-07-11T16:45:36Z

This PR replaces AlertFactory in ES Query rule type with AlertsClient so the alerts are persistent in an alert-as-data index.

…fix'

…ta' into 159493-es-query-rule-alert-as-data

ersin-erdal · 2023-07-27T11:16:05Z

x-pack/plugins/stack_alerts/server/rule_types/es_query/executor.ts

+        [ALERT_STATE_DATE_START]: dateStart,
+        [ALERT_STATE_DATE_END]: dateEnd,
+      },
+    });


ES Query Rule used to pass context and state params to the legacy alertsClient. Now we pass them in the payload to save in alert-as-data index.

Fields in the state object: dateEnd, dateStart, and latestTimestamp.
They are grouped under state namespace, like: kibana.alert.state.latestTimestamp

Fields in the context object: hits, link, conditions, value, date, title, message.
I didn't use any namespace for the context fields as they already make sense under the alert namespace.

Two field names has been created for the hits param.
kibana.alert.hits.count to pass the total number of the hits (in case of we want to cap the number of the hits)
kibana.alert.hits.hits to pass the hits (capped or all)

ALERT_URL (kibana.alert.url) is used from the default alert-as-data for the link param.

ALERT_CONDITIONS_MET_VALUE (kibana.alert.conditions_met_value) has been created for the value param in the context obj.

Do these field names and namespaces look OK to you? Should we group context params as well?
Is creating a separate hits.count field name for the hits OK?

cc: @kobelb @shanisagiv1 @mikecote @ymao1

I would put actionContext.message into kibana.alert.reason and look into the ALERT_EVALUATION_THRESHOLD and ALERT_EVALUATION_VALUES fields that are currently used by observability alerts to see if we could use them to store actionContext.conditions and actionContext.value. If they are suitable, we could promote those to framework level fields and encourage rule types to use them to store their evaluation thresholds.

For this first iteration, I would leave out actionContext.hits until we settle on how this data should be properly stored. I believe @kobelb had the idea of maybe copying out all ECS source fields

The other thing to think about is whether we want to use the kibana.alert namespace directly or namespace these fields even more to be for stack alerts, so kibana.alert.stack_context.latest_timestamp for example

+1 to what @ymao1 mentioned. Thank you for kicking off this discussion on field mappings. For the proposal, I've also been thinking the following:

ALERT_HITS_COUNT

There may be more hits than provided within actionContext.hits.length. Should we instead adapt fetchEsQuery and fetchSearchSourceQuery to return result.hits.total via numMatches and use that as the value? For example, setting size: 0 in the query will cause no hits, but ALERT_HITS_COUNT could contain result.hits.total from the query?

ALERT_STATE_LAST_TIMESTAMP, ALERT_STATE_DATE_START, ALERT_STATE_DATE_END

I wonder if we leave these out for now? They are used for internal query building (and I guess mustache) but I'm not sure if they provide value in the alert document itself. The values would update constantly until the last time the alert is detected (but we also store kibana.alert.start and kibana.alert.end at the framework level).

Current fields are:

[ALERT_RULE_NAME]: name, [ALERT_URL]: actionContext.link, [ALERT_HITS_COUNT]: actionContext.hits.length, [ALERT_HITS_HITS]: actionContext.hits, [ALERT_MESSAGE]: actionContext.message, [ALERT_TITLE]: actionContext.title, [ALERT_CONDITIONS]: actionContext.conditions, [ALERT_CONDITIONS_MET_VALUE]: actionContext.value, [ALERT_STATE_LAST_TIMESTAMP]: latestTimestamp, [ALERT_STATE_DATE_START]: dateStart, [ALERT_STATE_DATE_END]: dateEnd,

I realised that ALERT_RULE_NAME is not in the current context, so i'll remove it.

I'll remove ALERT_HITS_HITS for now, as Ying suggested.

Will replace ALERT_MESSAGE with ALERT_REASON

ALERT_EVALUATION_THRESHOLD is used as a number, but actionContext.conditions in Es Query rule is the whole condition string. I'd rather adding ALERT_EVALUATION_CONDITIONS so it wold be compatible with the other ALERT_EVALUATION_*

I'll use ALERT_EVALUATION_VALUES instead of ALERT_CONDITIONS_MET_VALUE (CONDITIONS and VALUES will be under the same namespace)

I also realised that ALERT_HITS_COUNT is the same thing as ALERT_EVALUATION_VALUES, so i'll remove it too.

And will remove all the state fields as Mike suggests.

So the new payload will be:

[ALERT_URL]: actionContext.link, [ALERT_REASON]: actionContext.message, [ALERT_TITLE]: actionContext.title, [ALERT_EVALUATION_CONDITIONS]: actionContext.conditions, [ALERT_EVALUATION_VALUE]: actionContext.value,

And ALERT_TITLE will be the only ES Query specific field. Maybe we can move it to framework level as well, as it sounds like a generic field name.

Can you elaborate on what the conditions field will store? The type just has it being a string, is it a human readable string?

Additionally, can you elaborate on the alert title? How does this differ from the reason?

Otherwise, the revised fields look good to me.

Yes, conditions field is a human readable string that is generated in the rule type out of the params entered by the user.
e.g. Number of matching documents is greater than 1

Alert title is also a string generated in the rule type by using the rule name.
e.g. rule 'test' matched query

Reason on the other hand is the "message" field in the rule, that we use mustasche to parse.

Gotcha, thanks Ersin. These look good to me.

…ta' into 159493-es-query-rule-alert-as-data

ersin-erdal · 2023-08-03T10:04:04Z

packages/kbn-rule-data-utils/src/technical_field_names.ts

@@ -85,6 +85,7 @@ const EVENT_MODULE = 'event.module' as const;
 // Fields pertaining to the alert
 const ALERT_BUILDING_BLOCK_TYPE = `${ALERT_NAMESPACE}.building_block_type` as const;
 const ALERT_EVALUATION_THRESHOLD = `${ALERT_NAMESPACE}.evaluation.threshold` as const;
+const ALERT_EVALUATION_CONDITIONS = `${ALERT_NAMESPACE}.evaluation.conditions` as const;
 const ALERT_EVALUATION_VALUE = `${ALERT_NAMESPACE}.evaluation.value` as const;


Maybe we can move all the ALERT_EVALUATION_* fields to default_alerts_as_data.

We can, if we want them to apply to all alerts. For any of them that we want to move to apply to all alerts, we'll need to get the security solution team's approval. I'd prefer we just move the one we need for the time being to keep the scope small.

…ta' into 159493-es-query-rule-alert-as-data

…fix'

…e-alert-as-data

ymao1 · 2023-08-11T12:46:16Z

x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type.ts

+      fieldMap: {
+        [ALERT_URL]: { type: 'text', array: false, required: false },
+        [ALERT_TITLE]: { type: 'keyword', array: false, required: false },
+        [ALERT_REASON]: { type: 'keyword', array: false, required: false },


REASON should be in the framework fields so we shouldn't need to redefine it here

ymao1 · 2023-08-11T12:46:47Z

x-pack/plugins/stack_alerts/server/rule_types/es_query/rule_type.ts

+    context: STACK_AAD_INDEX_NAME,
+    mappings: {
+      fieldMap: {
+        [ALERT_URL]: { type: 'text', array: false, required: false },


URL should be in the framework fields so we shouldn't need to redefine it here

kibana-ci · 2023-08-14T12:06:28Z

💚 Build Succeeded

Buildkite Build
Commit: 38e036e

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/alerts-as-data-utils`	24	25	+1

Unknown metric groups

API count

id	before	after	diff
`@kbn/alerts-as-data-utils`	24	25	+1

References to deprecated APIs

id	before	after	diff
`stackAlerts`	64	63	-1

History

💚 Build #149642 succeeded 222814c
💚 Build #149567 succeeded f667175
💚 Build #149538 succeeded 8990186
💔 Build #149501 failed 4b6c8de
💔 Build #149410 failed 7de9b55
💔 Build #149331 failed da59cc1

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

cc @ersin-erdal

ersin-erdal added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.10.0 labels Jul 11, 2023

ersin-erdal self-assigned this Jul 11, 2023

Save ES Query Rule type alerts in alert-as-data index

323c18c

ersin-erdal force-pushed the 159493-es-query-rule-alert-as-data branch from ed9c4a4 to 323c18c Compare July 11, 2023 16:47

kibanamachine and others added 8 commits July 11, 2023 16:52

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

518cd45

…fix'

fix functional tests

3a4aebf

Merge remote-tracking branch 'origin/159493-es-query-rule-alert-as-da…

7d27d5f

…ta' into 159493-es-query-rule-alert-as-data

get state from the alert

40e522e

set field mappings

7a8e548

merge main

363e470

fix check type

cfcf291

add tests and refactoring

204c3a8

ersin-erdal commented Jul 27, 2023

View reviewed changes

ersin-erdal and others added 8 commits July 27, 2023 14:53

fix jest test

a9b29b2

Merge branch 'main' into 159493-es-query-rule-alert-as-data

a31c100

fix test

4314951

Merge remote-tracking branch 'origin/159493-es-query-rule-alert-as-da…

da9a0db

…ta' into 159493-es-query-rule-alert-as-data

fix latestTimestamp

5a37a96

Merge branch 'main' into 159493-es-query-rule-alert-as-data

c7000cf

refactor fields

7794b6d

refactor fields

0c0078f

ersin-erdal commented Aug 3, 2023

View reviewed changes

ersin-erdal added 3 commits August 3, 2023 14:56

merge main

b46ec27

fix checkType

e2bfd3d

move ALERT_EVALUATION_CONDITIONS to stack_alerts

302a8d6

ersin-erdal force-pushed the 159493-es-query-rule-alert-as-data branch from b23894d to 302a8d6 Compare August 3, 2023 18:14

ersin-erdal marked this pull request as ready for review August 3, 2023 19:27

ersin-erdal requested a review from a team as a code owner August 3, 2023 19:27

ersin-erdal removed request for a team August 10, 2023 16:37

ersin-erdal and others added 7 commits August 10, 2023 23:50

extend functional tests to test alert-as-data fields

89351a1

Merge remote-tracking branch 'origin/159493-es-query-rule-alert-as-da…

30e2415

…ta' into 159493-es-query-rule-alert-as-data

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

7de9b55

…fix'

Merge branch 'o11y-rbac-rule-feature-branch' into 159493-es-query-rul…

ea78649

…e-alert-as-data

resolve conflict

4b6c8de

rename stack.esquery as stack.threshold

8990186

rename stack.threshold as stack

f667175

ymao1 reviewed Aug 11, 2023

View reviewed changes

remove ALERT_REASON, ALERT_URL

222814c

ersin-erdal changed the base branch from o11y-rbac-rule-feature-branch to main August 14, 2023 11:00

Merge branch 'main' into 159493-es-query-rule-alert-as-data

38e036e

ersin-erdal merged commit 458c67e into elastic:main Aug 14, 2023

kibanamachine added the backport:skip This commit does not require backporting label Aug 14, 2023

ersin-erdal deleted the 159493-es-query-rule-alert-as-data branch August 14, 2023 12:26

mikecote mentioned this pull request Aug 14, 2023

FAAD: Prevent rule types from defining framework fields in their fieldMap #163801

Open

lcawl mentioned this pull request Aug 22, 2023

[OAS] Add more Elasticsearch query rule examples #164386

Merged

mikecote mentioned this pull request Aug 25, 2023

Onboard Index Threshold Rule Type to use FAAD #164222

Closed

mikecote mentioned this pull request Sep 13, 2023

[Response Ops][Alerting] Update stack rule types to framework alerts-as-data #145108

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Save ES Query Rule type alerts in alert-as-data index #161685

Save ES Query Rule type alerts in alert-as-data index #161685

ersin-erdal commented Jul 11, 2023

ersin-erdal Jul 27, 2023

ymao1 Jul 27, 2023 •

edited

Loading

mikecote Jul 27, 2023 •

edited

Loading

ersin-erdal Aug 2, 2023 •

edited

Loading

kobelb Aug 3, 2023 •

edited

Loading

ersin-erdal Aug 3, 2023

kobelb Aug 7, 2023

ersin-erdal Aug 3, 2023

kobelb Aug 3, 2023

ymao1 Aug 11, 2023

ymao1 Aug 11, 2023

kibana-ci commented Aug 14, 2023

API count

References to deprecated APIs

Save ES Query Rule type alerts in alert-as-data index #161685

Save ES Query Rule type alerts in alert-as-data index #161685

Conversation

ersin-erdal commented Jul 11, 2023

ersin-erdal Jul 27, 2023

Choose a reason for hiding this comment

ymao1 Jul 27, 2023 • edited Loading

Choose a reason for hiding this comment

mikecote Jul 27, 2023 • edited Loading

Choose a reason for hiding this comment

ALERT_HITS_COUNT

ALERT_STATE_LAST_TIMESTAMP, ALERT_STATE_DATE_START, ALERT_STATE_DATE_END

ersin-erdal Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

kobelb Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

ersin-erdal Aug 3, 2023

Choose a reason for hiding this comment

kobelb Aug 7, 2023

Choose a reason for hiding this comment

ersin-erdal Aug 3, 2023

Choose a reason for hiding this comment

kobelb Aug 3, 2023

Choose a reason for hiding this comment

ymao1 Aug 11, 2023

Choose a reason for hiding this comment

ymao1 Aug 11, 2023

Choose a reason for hiding this comment

kibana-ci commented Aug 14, 2023

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

API count

References to deprecated APIs

History

ymao1 Jul 27, 2023 •

edited

Loading

mikecote Jul 27, 2023 •

edited

Loading

ersin-erdal Aug 2, 2023 •

edited

Loading

kobelb Aug 3, 2023 •

edited

Loading