[Security Solution][Alerts] Alert suppression time window #148868

marshallmain · 2023-01-13T02:32:52Z

Summary

Adds ability to specify a time window with alert suppression on Query rules. If more alerts are detected with the same value in the "group by" field in subsequent rule executions, the existing alert will be updated to reflect the new doc count and suppression end time rather than creating a new alert.

Create Rule

Rule Details

…on-time-window

…-fix'

elasticmachine · 2023-01-17T19:47:54Z

Pinging @elastic/security-solution (Team: SecuritySolution)

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

x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts

x-pack/test/detection_engine_api_integration/security_and_spaces/rule_execution_logic/query.ts

mikecote

Thanks for putting up the PR for rule registry changes! I left a question before continuing my review.

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

spong · 2023-01-24T22:22:30Z

...rity_solution/server/lib/detection_engine/signals/alert_suppression/group_and_bulk_create.ts

+      const suppressionDuration = runOpts.completeRule.ruleParams.alertSuppression?.duration;
+
+      if (suppressionDuration) {
+        const suppressionWindow = `now-${suppressionDuration.value}${suppressionDuration.unit}`;


Curious about the case where a rule not configured with suppression is run, it creates some alerts, then is disabled, edited to have a suppression time period covering the previous execution, and is re-enabled. In this situation the next execution (as triggered by the re-enable) will run with suppression enabled and will suppress any new alerts that would match those alerts from the previous execution, correct?

The suppression across rule runs depends on existing alerts having the kibana.alert.instance.id field populated. If the first rule execution has no suppression configured at all, then the initial alerts won't populate an instance ID. When suppression is configured, with or without a duration, the instance ID does get populated (the instance ID calculation depends on the field chosen for suppression and the value in that field, so we'd have to do a runtime computation of each alert's instance ID if the suppression field is allowed to be chosen after the alert is created).

In the scenario you described, after suppression is configured for the first time the next rule execution will still create new alerts even if the previous execution created alerts within the suppression duration with the same host name or whatever field is chosen as the suppression field.

Alternatively, if per rule execution is chosen initially and then the duration is added (without changing the suppression field) then the existing alerts from the per rule execution run would be updated, assuming they're still within the suppression duration.

Perfect, thank you for the clarification here!

…on-time-window

spong · 2023-01-25T21:15:17Z

x-pack/plugins/security_solution/public/detections/components/rules/step_define_rule/index.tsx

+                  isDisabled={
+                    groupByFields?.length === 0 ||
+                    groupByRadioSelection.value !== GroupByOptions.PerTimePeriod


Whoops -- one even more corner case here with regards to being on basic license and then duplicating a prebuilt rule that has alert_suppression:

Suggested change

isDisabled={

groupByFields?.length === 0 ||

groupByRadioSelection.value !== GroupByOptions.PerTimePeriod

isDisabled={

!license.isAtLeast(minimumLicenseForSuppression) ||

groupByFields?.length === 0 ||

groupByRadioSelection.value !== GroupByOptions.PerTimePeriod

spong

Checked out, tested locally, and code reviewed -- rules area changes LGTM! 👍

Performed a bunch of different tests, from testing locally hosted prebuilt rules with alert_suppression configured, license changes, and general functionality of the time window feature during rule execution.

Thank you @marshallmain for replying to all my comments and providing further context/test scenarios -- really appreciate it! 🙂 🚀

spong · 2023-01-25T21:31:05Z

One last UX thought I had with regards to Alert Details -- would be nice if we included the suppression terms and duration under Insights for quickly grokking why these alerts were suppressed:

e40pud

LGTM

mikecote

I reviewed the changes in the rule registry, looking good! A few questions and comments but after this we should be good to go! 🚀

mikecote · 2023-01-27T13:00:49Z

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

@@ -48,6 +48,7 @@ const ALERT_BUILDING_BLOCK_TYPE = `${ALERT_NAMESPACE}.building_block_type` as co
 const ALERT_EVALUATION_THRESHOLD = `${ALERT_NAMESPACE}.evaluation.threshold` as const;
 const ALERT_EVALUATION_VALUE = `${ALERT_NAMESPACE}.evaluation.value` as const;
 const ALERT_INSTANCE_ID = `${ALERT_NAMESPACE}.instance.id` as const;
+const ALERT_LAST_DETECTED = `${ALERT_NAMESPACE}.last_detected_at` as const;


Can we move this field to the default_alerts_as_data.ts file to make it available for all alerts and have it renamed to last_detected for consistency with start and end?

mikecote · 2023-01-27T13:44:19Z

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

+    return {
+      ...alert,
+      _source: {
+        [ALERT_LAST_DETECTED]: includeLastDetected ? currentTimeOverride ?? new Date() : undefined,


Can we always set ALERT_LAST_DETECTED? The framework plans to rely on this field to know when an alert was last detected, even if it was just once (persistent alerts).

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

mikecote · 2023-01-27T14:31:49Z

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

+                  throw new Error('Failed to parse suppression window');
+                }
+
+                const suppressionAlertSearchRequest = {


Can we make this query work with ALERT_START instead of TIMESTAMP? It will make it easier for the framework to enable suppression for O11y and stack alerts too (given timestamp behaves differently in O11y / stack vs security solution) we need to look at the first detected time.

We will need to set ALERT_START for newly detected alerts within augmentAlerts to make this work.

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

…on-time-window

mikecote

Changes LGTM! Just one place left to use ALERT_START 🚀

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

…ule_type_wrapper.ts Co-authored-by: Mike Côté <[email protected]>

kibana-ci · 2023-01-30T20:08:24Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 1ca4f87

Failed CI Steps

Security Solution Tests #3

Test Failures

[job] [logs] Security Solution Tests #3 / Inspect Network stats and tables "after each" hook for "inspects the Top DNS Domains Table"
[job] [logs] Security Solution Tests #3 / Inspect Network stats and tables inspects the Top DNS Domains Table
[job] [logs] Security Solution Tests #3 / Timelines Creates a timeline by clicking untitled timeline from bottom bar can be added notes

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	3563	3565	+2

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/rule-data-utils`	96	97	+1
`ruleRegistry`	213	223	+10
total			+11

Async chunks

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

id	before	after	diff
`securitySolution`	12.8MB	12.9MB	+20.0KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`ruleRegistry`	10	11	+1

Unknown metric groups

API count

id	before	after	diff
`@kbn/rule-data-utils`	99	100	+1
`ruleRegistry`	241	251	+10
total			+11

ESLint disabled in files

id	before	after	diff
`securitySolution`	76	77	+1

Total ESLint disabled count

id	before	after	diff
`securitySolution`	502	503	+1

History

💔 Build #103528 failed 487f56b
💔 Build #103483 failed 3f1ecb2
💔 Build #103447 failed 6052cc9
💚 Build #103178 succeeded 969253f
💛 Build #102820 was flaky 6e6c572

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

…8868) ## Summary Adds ability to specify a time window with alert suppression on Query rules. If more alerts are detected with the same value in the "group by" field in subsequent rule executions, the existing alert will be updated to reflect the new doc count and suppression end time rather than creating a new alert. ### Create Rule ![image](https://user-images.githubusercontent.com/55718608/212997145-cee96a7d-fc3b-4b08-8845-5a9c7876fa0a.png) ### Rule Details ![image](https://user-images.githubusercontent.com/55718608/212997293-69d93392-f74e-4e4e-925a-befbee531659.png) --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Mike Côté <[email protected]>

marshallmain added 6 commits December 21, 2022 08:38

WIP

d48454d

Merge branch 'main' of github.com:elastic/kibana into alert-suppressi…

51776bc

…on-time-window

Add radio button to rule create, implement alert update logic

382f0c3

Add explicit index to bulk updates

bf6ec62

Fix deduplication bug with null values, add tests

b210b3b

Cleanup

5d52a88

marshallmain added Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Feature:Detection Alerts Security Solution Detection Alerts Feature Team:Detection Alerts Security Detection Alerts Area Team v8.7.0 release_note:enhancement labels Jan 13, 2023

kibanamachine and others added 9 commits January 13, 2023 03:06

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

6743a5d

…-fix'

Fix types

899d160

Merge branch 'main' into alert-suppression-time-window

3a6d854

Fix more types

f39021f

Fix unit tests

7d7b504

Fix test data

60b8490

Merge branch 'main' into alert-suppression-time-window

bea5a14

Add real rule execution test for suppression

14037fd

Merge branch 'main' into alert-suppression-time-window

57cb8cf

marshallmain marked this pull request as ready for review January 17, 2023 19:47

marshallmain requested review from a team as code owners January 17, 2023 19:47

marshallmain requested a review from spong January 17, 2023 19:47

Merge branch 'main' into alert-suppression-time-window

b2d2df2

spong reviewed Jan 20, 2023

View reviewed changes

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

spong reviewed Jan 20, 2023

View reviewed changes

x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts Outdated Show resolved Hide resolved

spong reviewed Jan 20, 2023

View reviewed changes

x-pack/test/detection_engine_api_integration/security_and_spaces/rule_execution_logic/query.ts Show resolved Hide resolved

mikecote reviewed Jan 23, 2023

View reviewed changes

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

Apply Garrett's fix

79e81a9

spong reviewed Jan 24, 2023

View reviewed changes

marshallmain and others added 5 commits January 24, 2023 15:44

Add timestamp override and max_signals tests, move schema

d2c586b

Merge branch 'main' of github.com:elastic/kibana into alert-suppressi…

d654699

…on-time-window

Fix import

0c0b157

Fix technical field names post merge

ce3a624

Merge branch 'main' into alert-suppression-time-window

424d74b

spong reviewed Jan 25, 2023

View reviewed changes

spong approved these changes Jan 25, 2023

View reviewed changes

Helper funcs in persistence wrapper, fix license check in UI

6e6c572

e40pud approved these changes Jan 26, 2023

View reviewed changes

Exclude closed alerts from suppression update logic

969253f

mikecote reviewed Jan 27, 2023

View reviewed changes

marshallmain added 2 commits January 27, 2023 08:42

PR comments

9b98237

Merge branch 'main' of github.com:elastic/kibana into alert-suppressi…

6052cc9

…on-time-window

mikecote approved these changes Jan 27, 2023

View reviewed changes

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

marshallmain and others added 4 commits January 27, 2023 10:24

Update x-pack/plugins/rule_registry/server/utils/create_persistence_r…

3f1ecb2

…ule_type_wrapper.ts Co-authored-by: Mike Côté <[email protected]>

Modify tests to account for alert start and enable rule timestamp bug

487f56b

Fix tests

5650d6f

Merge branch 'main' into alert-suppression-time-window

1ca4f87

marshallmain merged commit 4d353f0 into elastic:main Jan 30, 2023

kibanamachine added the backport:skip This commit does not require backporting label Jan 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Alerts] Alert suppression time window #148868

[Security Solution][Alerts] Alert suppression time window #148868

marshallmain commented Jan 13, 2023 •

edited

Loading

elasticmachine commented Jan 17, 2023

mikecote left a comment

spong Jan 24, 2023

marshallmain Jan 24, 2023

spong Jan 25, 2023

spong Jan 25, 2023

spong left a comment

spong commented Jan 25, 2023

e40pud left a comment

mikecote left a comment

mikecote Jan 27, 2023

mikecote Jan 27, 2023

mikecote Jan 27, 2023

mikecote left a comment

kibana-ci commented Jan 30, 2023

API count

ESLint disabled in files

Total ESLint disabled count

[Security Solution][Alerts] Alert suppression time window #148868

[Security Solution][Alerts] Alert suppression time window #148868

Conversation

marshallmain commented Jan 13, 2023 • edited Loading

Summary

Create Rule

Rule Details

elasticmachine commented Jan 17, 2023

mikecote left a comment

Choose a reason for hiding this comment

spong Jan 24, 2023

Choose a reason for hiding this comment

marshallmain Jan 24, 2023

Choose a reason for hiding this comment

spong Jan 25, 2023

Choose a reason for hiding this comment

spong Jan 25, 2023

Choose a reason for hiding this comment

spong left a comment

Choose a reason for hiding this comment

spong commented Jan 25, 2023

e40pud left a comment

Choose a reason for hiding this comment

mikecote left a comment

Choose a reason for hiding this comment

mikecote Jan 27, 2023

Choose a reason for hiding this comment

mikecote Jan 27, 2023

Choose a reason for hiding this comment

mikecote Jan 27, 2023

Choose a reason for hiding this comment

mikecote left a comment

Choose a reason for hiding this comment

kibana-ci commented Jan 30, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

API count

ESLint disabled in files

Total ESLint disabled count

History

marshallmain commented Jan 13, 2023 •

edited

Loading