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.

Sign up for GitHub

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

Jump to bottom

[SIEM] Mark rule run as failure if there was an error #62383

Merged
merged 1 commit into from
Apr 3, 2020
Merged

[SIEM] Mark rule run as failure if there was an error #62383

merged 1 commit into from
Apr 3, 2020

Conversation

rylnd
Copy link
Contributor

@rylnd rylnd commented Apr 2, 2020 •
edited
Loading

Summary

Because of the binary nature of Rule Statuses (success/fail), we need to collapse multiple Rule execution states. We currently have two recoverable errors that may occur during rule execution: a gap error, and a "ML Job not running" error.

While we have always been recording these errors and displaying them in the "Last Failures" tab, we were marking those executions as successful as they could still have generated signals and notifications (assuming they did not encounter another unrecoverable error).

However, marking the execution as successful meant that the user could easily miss the error, as there would be no CTA on the Rule Details, nor would the error be shown on the Monitoring page.

For now, until we revisit Rule execution/statuses, we're going to mark any rule execution that encounters an error as a failure, with the goal of increasing the error's visibility to the user. NB that a failed execution still has the potential to generate signals/notifications, depending on the error.

Checklist

For maintainers

@rylnd
Mark rule run as failure if there was an error
1e40f25 
While we still let the rule execute in the case of gap errors and
stopped ML jobs, we now mark that execution as a failure instead of a
success.
@rylnd rylnd added Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v7.8.0 labels Apr 2, 2020
@rylnd rylnd requested a review from spong April 2, 2020 22:05
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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

@rylnd rylnd marked this pull request as ready for review April 3, 2020 00:28
@rylnd rylnd requested a review from a team as a code owner April 3, 2020 00:28
@spong spong mentioned this pull request Apr 3, 2020
Merged
3 tasks
dhurley14
dhurley14 reviewed Apr 3, 2020
View reviewed changes
x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/signal_rule_alert_type.ts
@@ -55,6 +55,7 @@ export const signalRulesAlertType = ({
index,
filters,
language,
maxSignals,
Copy link
Contributor

Choose a reason for hiding this comment

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

good idea to just pull this out of params instead of having params.maxSignals.

dhurley14
dhurley14 approved these changes Apr 3, 2020
View reviewed changes
Copy link
Contributor

@dhurley14 dhurley14 left a comment

Choose a reason for hiding this comment

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

LGTM!

@rylnd
Copy link
Contributor Author

rylnd commented Apr 3, 2020 •
edited
Loading

Updating with some screenshots:

Details View when ML Rule's job is not running:
Detections_-_Kibana

Details View when Rule has gap error:
Detections_-_Kibana

Details View, Errors tab for Rule that had a gap, then found that its ML Job was not running:
Detections_-_Kibana

Monitoring view with both Gap and "Job not started" failures:
Detections_-_Kibana

spong
spong approved these changes Apr 3, 2020
View reviewed changes
Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Checked out and tested locally -- verified correct state of ML Rule when the corresponding ML Job is enabled/disabled, and also verified the job would switch between those two states when enabling/disabling the job while rule is activated and all looks good. Thanks for this fix @rylnd! LGTM 👍

@rylnd rylnd merged commit 30afc9d into elastic:master Apr 3, 2020
@rylnd rylnd deleted the fail_rule_on_error branch April 3, 2020 20:22
This was referenced Apr 3, 2020
Merged
Merged
rylnd added a commit to rylnd/kibana that referenced this pull request Apr 3, 2020
@rylnd
Mark rule run as failure if there was an error (elastic#62383)
cb5c510 
While we still let the rule execute in the case of gap errors and
stopped ML jobs, we now mark that execution as a failure instead of a
success.
rylnd added a commit that referenced this pull request Apr 3, 2020
@rylnd
Mark rule run as failure if there was an error (#62383) (#62513)
d42c08f 
While we still let the rule execute in the case of gap errors and
stopped ML jobs, we now mark that execution as a failure instead of a
success.
rylnd added a commit that referenced this pull request Apr 3, 2020
@rylnd
Mark rule run as failure if there was an error (#62383) (#62514)
d3d8ea8 
While we still let the rule execute in the case of gap errors and
stopped ML jobs, we now mark that execution as a failure instead of a
success.
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 6, 2020
@gmmorris
Merge branch 'event-log/query-support' of github.com:gmmorris/kibana …
503bea6 
…into event-log/query-support

* 'event-log/query-support' of github.com:gmmorris/kibana: (41 commits)
  [jenkins] refer to sizes in most pipeline code (elastic#62082)
  skip flaky suite (elastic#60470)
  [Discover] Fix flaky FT in field visualize (elastic#62418)
  [ML] Data Frame Analytics: Fix feature importance (elastic#61761)
  [Reporting] Use a shim for server config (elastic#62086)
  [Reporting] Fix reporting for non-default spaces (elastic#62226)
  Fix bug that coerced empty scaled float value to 0 (elastic#62251)
  [SIEM] [Detection Engine] Remove has manage api keys requireme… (elastic#62446)
  [Maps] Safely handle empty string and invalid strings from EuiColorPicker (elastic#62507)
  Reporting/bug more blacklisted headers (elastic#62389)
  [SIEM] Prevent undefined behavior in our ML popover (elastic#62498)
  [SIEM] [Detection Engine] remove all unknowns from all rules t… (elastic#62327)
  base changes for active/current node styling (elastic#62007)
  [kbn/ui-shared-deps] expand and split (elastic#62364)
  [ML] DF Analytics - ensure destination index pattern created (elastic#62450)
  Mark rule run as failure if there was an error (elastic#62383)
  Add docs for metric explorer alerts (elastic#62314)
  skip flaky suite (elastic#62281)
  [SIEM][Detection Engine] Fixes export of single rule and the icons
  fixes flakiness (elastic#62406)
  ...
spong added a commit that referenced this pull request Apr 6, 2020
@spong
[SIEM] Fixes UX issues around prebuilt ML Rules (#62396)
0bdcda8 
## Summary

This PR fixes a number of UX issues around the new prebuilt `machine_learning` rules when the user does not have the necessary permissions to manage the backing ML Job. Along with #62383, this ensures there is adequate information for the user determine if a rule is not working because the backing job is not running (and helping to prevent this from occurring). This also includes some requested copy changes, including:

* Renames `Anomaly Detection`  dropdown to `ML job settings`
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320279-57c5a880-7526-11ea-8350-647cbba263a4.png" />
</p>

* Updates copy in `ML job settings` dropdown
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320473-cc98e280-7526-11ea-8871-e97661ff5f78.png" />
</p>

* Only shows `ML job settings` UI when on `/detections/` routes 
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320401-922f4580-7526-11ea-9f97-0ec06526b273.png" />
</p>


### All Rules Changes

* Disables the `activate switch` if user does not have permission to enable/disable jobs
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320892-d3742500-7527-11ea-90bb-91fd203480bd.png" />
</p>

* Adds warning toast when attempting to activate via bulk actions (if user does not have permission to enable/disable jobs)
<p align="center">
  <img width="300" src="https://user-images.githubusercontent.com/2946766/78321015-1a621a80-7528-11ea-8ab0-f9fef19240f7.png" />
</p>

### Rule Details Changes
* `Machine Learning job` link now links to ML App with table filtered to the relevant job

* Disables the `activate switch` if user does not have permission to enable/disable jobs
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78321277-c277e380-7528-11ea-99e9-034970a5054e.png" />
</p>

### Create/Edit Rule Changes

* If the job selected _is not running_, a warning will be displayed to remind the user to enable the job before running the rule. cc @benskelker @MikePaquette -- this okay copy here?
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78321498-63ff3500-7529-11ea-9b09-a87186cbe0ce.png" />
</p>

Resolves elastic/siem-team#575
Resolves elastic/siem-team#519

### Checklist

Delete any items that are not applicable to this PR.

- [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials 
  - Scheduled time with @benskelker to update docs
- [X] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
spong added a commit to spong/kibana that referenced this pull request Apr 6, 2020
@spong
[SIEM] Fixes UX issues around prebuilt ML Rules (elastic#62396)
d7ce199 
## Summary

This PR fixes a number of UX issues around the new prebuilt `machine_learning` rules when the user does not have the necessary permissions to manage the backing ML Job. Along with elastic#62383, this ensures there is adequate information for the user determine if a rule is not working because the backing job is not running (and helping to prevent this from occurring). This also includes some requested copy changes, including:

* Renames `Anomaly Detection`  dropdown to `ML job settings`
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320279-57c5a880-7526-11ea-8350-647cbba263a4.png" />
</p>

* Updates copy in `ML job settings` dropdown
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320473-cc98e280-7526-11ea-8871-e97661ff5f78.png" />
</p>

* Only shows `ML job settings` UI when on `/detections/` routes 
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320401-922f4580-7526-11ea-9f97-0ec06526b273.png" />
</p>


### All Rules Changes

* Disables the `activate switch` if user does not have permission to enable/disable jobs
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320892-d3742500-7527-11ea-90bb-91fd203480bd.png" />
</p>

* Adds warning toast when attempting to activate via bulk actions (if user does not have permission to enable/disable jobs)
<p align="center">
  <img width="300" src="https://user-images.githubusercontent.com/2946766/78321015-1a621a80-7528-11ea-8ab0-f9fef19240f7.png" />
</p>

### Rule Details Changes
* `Machine Learning job` link now links to ML App with table filtered to the relevant job

* Disables the `activate switch` if user does not have permission to enable/disable jobs
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78321277-c277e380-7528-11ea-99e9-034970a5054e.png" />
</p>

### Create/Edit Rule Changes

* If the job selected _is not running_, a warning will be displayed to remind the user to enable the job before running the rule. cc @benskelker @MikePaquette -- this okay copy here?
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78321498-63ff3500-7529-11ea-9b09-a87186cbe0ce.png" />
</p>

Resolves elastic/siem-team#575
Resolves elastic/siem-team#519

### Checklist

Delete any items that are not applicable to this PR.

- [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials 
  - Scheduled time with @benskelker to update docs
- [X] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
spong added a commit to spong/kibana that referenced this pull request Apr 6, 2020
@spong
[SIEM] Fixes UX issues around prebuilt ML Rules (elastic#62396)
c2ca44d 
## Summary

This PR fixes a number of UX issues around the new prebuilt `machine_learning` rules when the user does not have the necessary permissions to manage the backing ML Job. Along with elastic#62383, this ensures there is adequate information for the user determine if a rule is not working because the backing job is not running (and helping to prevent this from occurring). This also includes some requested copy changes, including:

* Renames `Anomaly Detection`  dropdown to `ML job settings`
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320279-57c5a880-7526-11ea-8350-647cbba263a4.png" />
</p>

* Updates copy in `ML job settings` dropdown
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320473-cc98e280-7526-11ea-8871-e97661ff5f78.png" />
</p>

* Only shows `ML job settings` UI when on `/detections/` routes 
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320401-922f4580-7526-11ea-9f97-0ec06526b273.png" />
</p>


### All Rules Changes

* Disables the `activate switch` if user does not have permission to enable/disable jobs
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320892-d3742500-7527-11ea-90bb-91fd203480bd.png" />
</p>

* Adds warning toast when attempting to activate via bulk actions (if user does not have permission to enable/disable jobs)
<p align="center">
  <img width="300" src="https://user-images.githubusercontent.com/2946766/78321015-1a621a80-7528-11ea-8ab0-f9fef19240f7.png" />
</p>

### Rule Details Changes
* `Machine Learning job` link now links to ML App with table filtered to the relevant job

* Disables the `activate switch` if user does not have permission to enable/disable jobs
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78321277-c277e380-7528-11ea-99e9-034970a5054e.png" />
</p>

### Create/Edit Rule Changes

* If the job selected _is not running_, a warning will be displayed to remind the user to enable the job before running the rule. cc @benskelker @MikePaquette -- this okay copy here?
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78321498-63ff3500-7529-11ea-9b09-a87186cbe0ce.png" />
</p>

Resolves elastic/siem-team#575
Resolves elastic/siem-team#519

### Checklist

Delete any items that are not applicable to this PR.

- [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials 
  - Scheduled time with @benskelker to update docs
- [X] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
spong added a commit that referenced this pull request Apr 6, 2020
@spong
[SIEM] Fixes UX issues around prebuilt ML Rules (#62396) (#62691)
b5e3a79 
## Summary

This PR fixes a number of UX issues around the new prebuilt `machine_learning` rules when the user does not have the necessary permissions to manage the backing ML Job. Along with #62383, this ensures there is adequate information for the user determine if a rule is not working because the backing job is not running (and helping to prevent this from occurring). This also includes some requested copy changes, including:

* Renames `Anomaly Detection`  dropdown to `ML job settings`
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320279-57c5a880-7526-11ea-8350-647cbba263a4.png" />
</p>

* Updates copy in `ML job settings` dropdown
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320473-cc98e280-7526-11ea-8871-e97661ff5f78.png" />
</p>

* Only shows `ML job settings` UI when on `/detections/` routes 
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320401-922f4580-7526-11ea-9f97-0ec06526b273.png" />
</p>


### All Rules Changes

* Disables the `activate switch` if user does not have permission to enable/disable jobs
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320892-d3742500-7527-11ea-90bb-91fd203480bd.png" />
</p>

* Adds warning toast when attempting to activate via bulk actions (if user does not have permission to enable/disable jobs)
<p align="center">
  <img width="300" src="https://user-images.githubusercontent.com/2946766/78321015-1a621a80-7528-11ea-8ab0-f9fef19240f7.png" />
</p>

### Rule Details Changes
* `Machine Learning job` link now links to ML App with table filtered to the relevant job

* Disables the `activate switch` if user does not have permission to enable/disable jobs
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78321277-c277e380-7528-11ea-99e9-034970a5054e.png" />
</p>

### Create/Edit Rule Changes

* If the job selected _is not running_, a warning will be displayed to remind the user to enable the job before running the rule. cc @benskelker @MikePaquette -- this okay copy here?
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78321498-63ff3500-7529-11ea-9b09-a87186cbe0ce.png" />
</p>

Resolves elastic/siem-team#575
Resolves elastic/siem-team#519

### Checklist

Delete any items that are not applicable to this PR.

- [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials 
  - Scheduled time with @benskelker to update docs
- [X] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
spong added a commit that referenced this pull request Apr 6, 2020
@spong
[SIEM] Fixes UX issues around prebuilt ML Rules (#62396) (#62692)
b86ed72 
## Summary

This PR fixes a number of UX issues around the new prebuilt `machine_learning` rules when the user does not have the necessary permissions to manage the backing ML Job. Along with #62383, this ensures there is adequate information for the user determine if a rule is not working because the backing job is not running (and helping to prevent this from occurring). This also includes some requested copy changes, including:

* Renames `Anomaly Detection`  dropdown to `ML job settings`
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320279-57c5a880-7526-11ea-8350-647cbba263a4.png" />
</p>

* Updates copy in `ML job settings` dropdown
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320473-cc98e280-7526-11ea-8871-e97661ff5f78.png" />
</p>

* Only shows `ML job settings` UI when on `/detections/` routes 
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320401-922f4580-7526-11ea-9f97-0ec06526b273.png" />
</p>


### All Rules Changes

* Disables the `activate switch` if user does not have permission to enable/disable jobs
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78320892-d3742500-7527-11ea-90bb-91fd203480bd.png" />
</p>

* Adds warning toast when attempting to activate via bulk actions (if user does not have permission to enable/disable jobs)
<p align="center">
  <img width="300" src="https://user-images.githubusercontent.com/2946766/78321015-1a621a80-7528-11ea-8ab0-f9fef19240f7.png" />
</p>

### Rule Details Changes
* `Machine Learning job` link now links to ML App with table filtered to the relevant job

* Disables the `activate switch` if user does not have permission to enable/disable jobs
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78321277-c277e380-7528-11ea-99e9-034970a5054e.png" />
</p>

### Create/Edit Rule Changes

* If the job selected _is not running_, a warning will be displayed to remind the user to enable the job before running the rule. cc @benskelker @MikePaquette -- this okay copy here?
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/78321498-63ff3500-7529-11ea-9b09-a87186cbe0ce.png" />
</p>

Resolves elastic/siem-team#575
Resolves elastic/siem-team#519

### Checklist

Delete any items that are not applicable to this PR.

- [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials 
  - Scheduled time with @benskelker to update docs
- [X] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
@spong spong mentioned this pull request Apr 15, 2020
Open
@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021
@elasticmachine
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spong spong spong approved these changes

@dhurley14 dhurley14 dhurley14 approved these changes

Assignees

@rylnd rylnd

Labels
release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM v7.7.0 v7.8.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@rylnd @elasticmachine @kibanamachine @dhurley14 @spong @MindyRS