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

[SIEM] ML Rules Details #61182

Merged
merged 15 commits into from
Mar 25, 2020
Merged

[SIEM] ML Rules Details #61182

merged 15 commits into from
Mar 25, 2020

Conversation

rylnd
Copy link
Contributor

@rylnd rylnd commented Mar 24, 2020

Summary

  • Adds the following to Rule Description view (shown both on readonly form view(edit, create) and Rule Details:
    • Link to ML Jobs page
    • Badge representing started/stopped status (reuses logic from MLPopover)
    • Audit message, if present
    • Humanizes rule type on Rule Description (e.g. machine_learning -> Machine Learning)
  • Only ML Admins can create rules
  • Adds placeholder option to ML Jobs dropdown ("Select a job")

bf58cdfa-cc2b-4a04-bfd4-36aa25def613_-_Kibana

Fullscreen_3_24_20__6_42_PM

Checklist

Delete any items that are not applicable to this PR.

For maintainers

rylnd added 13 commits March 24, 2020 16:53
Don't display the row as fullwidth, lest the help text wrap across the
entire page. It only looks okay now because it was a short sentence;
adding the ML Job select with its wrapped text caused some visual
weirdness, so this at least makes it consistent.
This is displayed both on the readonly form view, and the Rule Details
page.
This is a base hook that we can combine with our permissions helpers.
If we're auto-activating jobs on their behalf, they'll need to be an
admin.
This adds the auditMessage as well as a link to ML; actual status is
next
Also simplifies the layout of these job details.
UseSiemJobs uses uiSettings, so we need to use our kibana mocks here.
@rylnd rylnd added Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Mar 24, 2020
@rylnd rylnd requested a review from spong March 24, 2020 22:42
@rylnd rylnd self-assigned this Mar 24, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@rylnd rylnd marked this pull request as ready for review March 24, 2020 22:49
@rylnd rylnd requested a review from a team as a code owner March 24, 2020 22:49
[]
);
const listItems = keys.reduce((acc: ListItems[], key: string) => {
if (key === 'machineLearningJobId') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this branch here because I didn't want to add yet another optional argument to buildListItems that would only be used in one case.


import { isJobStarted, isJobLoading, isJobFailed } from './';

describe('isJobStarted', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Yaaay! Tests for verifying job states -- thanks @rylnd! 🎉🥇

@rylnd rylnd mentioned this pull request Mar 24, 2020
11 tasks
@@ -101,6 +102,7 @@ export interface MlSetupArgs {
* Representation of an ML Job as returned from the `ml/jobs/jobs_summary` API
*/
export interface JobSummary {
auditMessage?: AuditMessageBase;
Copy link
Member

Choose a reason for hiding this comment

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

Nice that we can use the explicit ML types! ++ 🎉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was a little wary of this one... I'm getting no linter complaints, and that file is just types, so I think we should be fine.

The long job names were causing the panel to overflow.
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, tested locally, and performed code review -- LGTM! Thanks for the added cleanup around MlCapabilitiesContext with useMlCapabilities as well. Nice clean code as always @rylnd! 🙂 👍

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@rylnd rylnd merged commit 683bf3a into elastic:master Mar 25, 2020
@rylnd rylnd deleted the annotate_jobs_with_status branch March 25, 2020 02:39
rylnd added a commit that referenced this pull request Mar 25, 2020
* Add basic help text to ML Job dropdown on Rule form

* Use EUI's preferred layout for form fields

* Add a link to ML in the Job select help text

* Restrict timeline picker to EUI guidelines

Don't display the row as fullwidth, lest the help text wrap across the
entire page. It only looks okay now because it was a short sentence;
adding the ML Job select with its wrapped text caused some visual
weirdness, so this at least makes it consistent.

* Add placeholder option to ML Job dropdown

* Humanize rule type on Rule Description component

This is displayed both on the readonly form view, and the Rule Details
page.

* Add useMlCapabilities hook

This is a base hook that we can combine with our permissions helpers.

* Restrict ML Rule creation to ML Admins

If we're auto-activating jobs on their behalf, they'll need to be an
admin.

* Extract ML Job status helpers to separate file

* WIP: Enrich Rule Description with ML Job Data

This adds the auditMessage as well as a link to ML; actual status is
next

* Display job status as a badge on Rule Details

Also simplifies the layout of these job details.

* Port helper tests to new location

* Fix DescriptionStep tests now that they use useSiemJobs

UseSiemJobs uses uiSettings, so we need to use our kibana mocks here.

* Fix responsiveness of ML Rule Details

The long job names were causing the panel to overflow.
@@ -266,3 +268,27 @@ export const buildNoteDescription = (label: string, note: string): ListItems[] =
}
return [];
};

export const buildRuleTypeDescription = (label: string, ruleType: RuleType): ListItems[] => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MadameSheema here's the terminal point of those Rule Details changes if you're interested. I haven't addressed the sliders discrepancy but I'll ping you on that when it's resolved!

@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
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 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants