Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution][Detections] Preview Rule: Make it possible to configure the time interval and look-back time #137102

Merged
merged 9 commits into from
Jul 26, 2022

Conversation

e40pud
Copy link
Contributor

@e40pud e40pud commented Jul 25, 2022

Summary

These changes add advanced rule preview options which allows power users to have control over the timeframe, rule interval and look-back time.

Fixes https://github.com/elastic/security-team/issues/4362

Screenshot 2022-07-25 at 18 44 05

Screenshot 2022-07-28 at 16 49 00

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space. Low High Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. High Low Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled. Medium High Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

@e40pud e40pud requested a review from a team as a code owner July 25, 2022 16:44
@e40pud e40pud self-assigned this Jul 25, 2022
@e40pud e40pud requested a review from a team as a code owner July 25, 2022 16:44
@e40pud e40pud requested a review from banderror July 25, 2022 16:44
@e40pud e40pud added release_note:feature Makes this part of the condensed release notes v8.4.0 release_note:enhancement Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team Team:Detection Alerts Security Detection Alerts Area Team and removed release_note:feature Makes this part of the condensed release notes labels Jul 25, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just glanced at the changes real quick and skipped a thorough review and testing: I'm not really familiar with the internal implementation of rule preview and I don't want to block this PR.

High-level, the changes LGTM 👍

@e40pud e40pud enabled auto-merge (squash) July 25, 2022 20:59
Copy link
Contributor

@dplumlee dplumlee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all looking great so far! One thing we'll need to update though is the rule run from the backend, specifically right here. The way the logic is currently set up, we just use now (or moment() in this case) to set up the timerange in which the rule runs. Then we use the interval count to step backwards and get to the startedAt time we use in the rule execution runs. With the addition of the advanced rule preview and the arbitrary dates we can now use, we'll have to set this up differently in the case of a user, for instance, running a rule from 20 days to 19 days ago (just a random example). I think the best way to do that is calculate and pass in an optional param alongside invocationCount called finalInvocationStartTime or something and have that essentially be the end time the user specifies. Then we can do something similar to the current logic where we use the invocation count that you've calculated and step backwards to run the rule over the user specified time range, not just based on the relative-to-now math we're currently using

@e40pud e40pud requested a review from dplumlee July 26, 2022 08:03
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 3229 3230 +1

Async chunks

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

id before after diff
securitySolution 5.5MB 5.5MB +4.5KB

History

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

cc @e40pud

Copy link
Contributor

@dplumlee dplumlee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for making those changes!

@marshallmain
Copy link
Contributor

For docs reference, here is the warning that is displayed if the number of preview executions needed for the selected timeframe and rule interval is greater than 200:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:enhancement SecuritySolution:QAAssist Part of QA testing process for release Team:Detection Alerts Security Detection Alerts Area Team Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants