[RAC] Populate common rule fields in alert helpers #108679

weltenwort · 2021-08-16T12:12:00Z

📝 Summary

This marks some technical alert-as-data fields as required and changes the alert status values from open/closed to active/recovered. It also updates the lifecycle and persistent rule type helpers to consistently populate several common alert fields with values from the alerting framework.

closes #108161

🕵️ Review notes

Since the persistent rule type helper almost exclusively passes through very loosely typed documents I was unable to adapt any downstream-changes that might be required due to it. @elastic/security-threat-hunting would you be able to adapt to it in your code-base in a follow-up or should I remove that change from the PR?

weltenwort · 2021-08-16T14:29:06Z

@elastic/security-threat-hunting these changes might impact your persistent alert type code, but I was unable to determine the exact changes required. Please let me know what you think of this. 😇

marshallmain · 2021-08-16T17:16:24Z

x-pack/plugins/rule_registry/server/utils/get_common_alert_fields.ts

+    [ALERT_RULE_NAME]: options.rule.name,
+    [ALERT_RULE_PRODUCER]: options.rule.producer,
+    [ALERT_RULE_TYPE_ID]: options.rule.ruleTypeId,
+    [ALERT_RULE_UUID]: options.alertId,


We'll probably want to change this to ALERT_RULE_ID and remove ALERT_RULE_UUID entirely throughout the codebase to align with the schema

Ok, I'll check if it's used anywhere. (probably not)

Per slack conversation, let's stick with ALERT_RULE_UUID here and plan to remove ALERT_RULE_ID elsewhere.

awesome, thanks for following up

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

mgiota · 2021-08-23T23:35:31Z

x-pack/test/apm_api_integration/tests/alerts/rule_registry.ts

@@ -391,7 +391,7 @@ export default function ApiTest({ getService }: FtrProviderContext) {
              "apm.transaction_error_rate",
            ],
            "kibana.alert.status": Array [


Probably not part of this PR, but shouldn't we use constants for the alert fields here instead of hardcoding them?

This is a jest inline snapshot. Manually interpolating the constants here would render the auto-updating of the snapshot mute, wouldn't it?

x-pack/plugins/rule_registry/server/routes/get_alert_by_id.test.ts

mgiota

LGTM! I left a few comments.

Let me know if it makes sense to import the common alert fields from the get_common_alert_fields you created instead of '@kbn/rule-data-utils'. Isn't it the reason why this file was created? Do I miss something here? This would require quite a few changes though.

I tested it locally and it works fine. Notice in my screenshot the empty cells. They are empty because these entries still have the open value. This wouldn't happen to our users, but maybe we could fall back to an empty dash ?

weltenwort · 2021-08-24T14:47:20Z

Thanks for taking a look at this PR.

Notice in my screenshot the empty cells. They are empty because these entries still have the open value.

I think we don't need to be backwards-compatible to the previous unfinished schema.

mgiota

LGTM

x-pack/plugins/rule_registry/server/utils/get_common_alert_fields.ts

marshallmain

Security solution and rule registry changes LGTM

kibanamachine · 2021-08-26T13:19:49Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`apm`	1622	1623	+1
`infra`	970	971	+1
`observability`	340	341	+1
`securitySolution`	2392	2393	+1
`timelines`	328	329	+1
`uptime`	668	669	+1
total			+6

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
`ruleRegistry`	118	109	-9

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`apm`	4.4MB	4.4MB	+624.0B
`infra`	1.7MB	1.7MB	+10.0B
`observability`	553.4KB	554.2KB	+842.0B
`securitySolution`	6.5MB	6.5MB	+576.0B
`timelines`	424.3KB	426.6KB	+2.3KB
`uptime`	1.0MB	1.0MB	+3.0B
total			+4.3KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`infra`	149.8KB	150.3KB	+574.0B
`uptime`	36.2KB	36.7KB	+574.0B
total			+1.1KB

Unknown metric groups

API count

id	before	after	diff
`ruleRegistry`	141	132	-9

History

💚 Build #148281 succeeded 4a18143
💚 Build #148243 succeeded 64405c1
💔 Build #147971 failed a8ccfae
💔 Build #147955 failed 8bf9f17
💔 Build #146780 failed 3e615cb

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

cc @weltenwort

Co-authored-by: mgiota <panagiota.mitsopoulou@elastic.co>

kibanamachine · 2021-08-26T13:23:09Z

💚 Backport successful

Status	Branch	Result
✅	7.15
✅	7.x

The backport PRs will be merged automatically after passing CI.

Co-authored-by: mgiota <panagiota.mitsopoulou@elastic.co> Co-authored-by: Felix Stürmer <weltenwort@users.noreply.github.com> Co-authored-by: mgiota <panagiota.mitsopoulou@elastic.co>

Populate common rule fields in lifecycle helper

b06c451

weltenwort self-assigned this Aug 16, 2021

weltenwort mentioned this pull request Aug 16, 2021

[RAC] Preserve lifecycle alert document changes for active alerts #108670

Closed

Kerry350 mentioned this pull request Aug 16, 2021

[RAC] Update alert status column in alerts table #108695

Merged

weltenwort added 8 commits August 16, 2021 15:33

Fix unit tests

2a8bba9

Fix more unit tests

d4ab514

Use shared constant for alert status in t-grid

074e9eb

Use shared alert status constant in browser

dfa1885

Revert t-grid status filter changes

743f16c

Use new alert status terminology in api tests

25ec60b

Merge branch 'master' into rac-populate-common-alert-fields

64be12c

Populate common rule fields in persistent rule helper

b11d2cf

Merge branch 'master' into rac-populate-common-alert-fields

5a28f91

weltenwort mentioned this pull request Aug 16, 2021

[RAC] RuleDataClient writer should handle inserting required fields into new alert documents #107980

Closed

marshallmain reviewed Aug 16, 2021

View reviewed changes

weltenwort added 3 commits August 16, 2021 20:30

Clean up symbol names and remove unused code

396612e

Regenerate api docs

bf180ce

Merge branch 'master' into rac-populate-common-alert-fields

e9cfce1

mistic removed the v7.15.0 label Aug 18, 2021

weltenwort added the v7.15.0 label Aug 18, 2021

mgiota marked this pull request as ready for review August 23, 2021 23:24

mgiota reviewed Aug 23, 2021

View reviewed changes

packages/kbn-rule-data-utils/src/alerts_as_data_rbac.ts Outdated Show resolved Hide resolved

mgiota reviewed Aug 23, 2021

View reviewed changes

x-pack/plugins/rule_registry/server/routes/get_alert_by_id.test.ts Show resolved Hide resolved

mgiota reviewed Aug 23, 2021

View reviewed changes

mgiota marked this pull request as draft August 24, 2021 05:56

weltenwort added 5 commits August 24, 2021 17:18

Merge branch 'master' into rac-populate-common-alert-fields

9f6f3a5

Move alert status constants to separate file.

f822a74

Merge branch 'master' into rac-populate-common-alert-fields

0f1222d

Pass required fields in security rule type tests mocks

64405c1

Merge branch 'master' into rac-populate-common-alert-fields

4a18143

mgiota approved these changes Aug 24, 2021

View reviewed changes

mgiota marked this pull request as ready for review August 24, 2021 19:23

mgiota requested a review from marshallmain August 24, 2021 19:23

mgiota mentioned this pull request Aug 24, 2021

[RAC] Replace usages of kibana.alert.status: open with active #109033

Merged

9 tasks

marshallmain reviewed Aug 24, 2021

View reviewed changes

x-pack/plugins/rule_registry/server/utils/get_common_alert_fields.ts Show resolved Hide resolved

weltenwort enabled auto-merge (squash) August 25, 2021 15:49

marshallmain approved these changes Aug 25, 2021

View reviewed changes

dgieselaar approved these changes Aug 26, 2021

View reviewed changes

resolve conflicts with master

6536ba8

weltenwort merged commit 137c182 into elastic:master Aug 26, 2021

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 26, 2021

[RAC] Populate common rule fields in alert helpers (elastic#108679)

3ab15b6

Co-authored-by: mgiota <panagiota.mitsopoulou@elastic.co>

kibanamachine mentioned this pull request Aug 26, 2021

[7.15] [RAC] Populate common rule fields in alert helpers (#108679) #110231

Merged

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 26, 2021

[RAC] Populate common rule fields in alert helpers (elastic#108679)

446d75d

Co-authored-by: mgiota <panagiota.mitsopoulou@elastic.co>

kibanamachine mentioned this pull request Aug 26, 2021

[7.x] [RAC] Populate common rule fields in alert helpers (#108679) #110232

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RAC] Populate common rule fields in alert helpers #108679

[RAC] Populate common rule fields in alert helpers #108679

weltenwort commented Aug 16, 2021 •

edited

Loading

weltenwort commented Aug 16, 2021

marshallmain Aug 16, 2021 •

edited

Loading

weltenwort Aug 16, 2021

marshallmain Aug 17, 2021

weltenwort Aug 17, 2021

mgiota Aug 23, 2021

weltenwort Aug 24, 2021

mgiota left a comment

weltenwort commented Aug 24, 2021 •

edited

Loading

mgiota left a comment

marshallmain left a comment

kibanamachine commented Aug 26, 2021

API count

kibanamachine commented Aug 26, 2021

[RAC] Populate common rule fields in alert helpers #108679

[RAC] Populate common rule fields in alert helpers #108679

Conversation

weltenwort commented Aug 16, 2021 • edited Loading

📝 Summary

🕵️ Review notes

weltenwort commented Aug 16, 2021

marshallmain Aug 16, 2021 • edited Loading

Choose a reason for hiding this comment

weltenwort Aug 16, 2021

Choose a reason for hiding this comment

marshallmain Aug 17, 2021

Choose a reason for hiding this comment

weltenwort Aug 17, 2021

Choose a reason for hiding this comment

mgiota Aug 23, 2021

Choose a reason for hiding this comment

weltenwort Aug 24, 2021

Choose a reason for hiding this comment

mgiota left a comment

Choose a reason for hiding this comment

weltenwort commented Aug 24, 2021 • edited Loading

mgiota left a comment

Choose a reason for hiding this comment

marshallmain left a comment

Choose a reason for hiding this comment

kibanamachine commented Aug 26, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

kibanamachine commented Aug 26, 2021

💚 Backport successful

weltenwort commented Aug 16, 2021 •

edited

Loading

marshallmain Aug 16, 2021 •

edited

Loading

weltenwort commented Aug 24, 2021 •

edited

Loading