[SIEM] Add toaster logic for Machine Learning #41401

FrankHassanabad · 2019-07-17T19:14:33Z

Summary

Fixes two blockers:

Anomaly table would spin forever if you zero jobs configured on a fresh install.
If you get Network or ML errors this will report the error to the user in the form of the global toaster.

Two examples of Error Toaster before the "See the full error(s)" is clicked:

Example Error Toasters from start job expanded and collapsed:

Example Network Error Toaster:

Example API Error if you send in something bad such as a bad payload:

Example Anomalies Table Error if you have a network issue:

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

x-pack/legacy/plugins/siem/public/components/ml/api/throw_if_not_ok.ts

elasticmachine · 2019-07-17T21:03:32Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

x-pack/legacy/plugins/siem/public/components/ml/anomaly/use_anomalies_table_data.ts

spong · 2019-07-17T21:11:38Z

x-pack/legacy/plugins/siem/public/components/ml/api/error_to_toaster.ts

+import { ToasterErrorsType, ToasterErrors } from './throw_if_not_ok';
+
+export type ErrorToToasterArgs = Partial<AppToast> & {
+  error: unknown;


++ on the unknown here! And thanks for the reading material -- good stuff!

x-pack/legacy/plugins/siem/public/components/ml/api/throw_if_not_ok.ts

spong · 2019-07-17T21:51:52Z

x-pack/legacy/plugins/siem/public/components/ml_popover/api.tsx

+  });
+
+  await throwIfNotOk(closeJobsResponse);
+  return [stopDatafeedsResponseJson, await closeJobsResponse.json()];


This file cleaned up real well with the way you've structured you're error reporting -- very nice! 👏

x-pack/legacy/plugins/siem/public/components/ml_popover/translations.ts

elasticmachine · 2019-07-17T21:59:45Z

Pinging @elastic/secops

spong · 2019-07-17T22:13:45Z

Looks like there's some title overflow in IE11, but this appears to be pre-existing, so no change necessary.

It's got the super skinny, but it works! 🎉

spong

Performed code review, checked out, tested locally against an Elastic Cloud 7.3.0 BC4 deployment in all browsers (Chrome, FF, Safari & IE11), and was getting errors everywhere expected. Did additional testing with multiple ml_nodes, disabling nodes while jobs were running, and so on and so forth and everything is looked good. LGTM 👍

elasticmachine · 2019-07-17T23:01:04Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* Initial toaster logic * Updated toaster error handling * Cleanups of code not used * Added i18n support * Added missing i18n keys and fixed the name spaces of all of them * Added unit test * Added the rest of the unit tests * Updated from feedback from the PR review * Fix capitalization issue

## Summary Fixes two blockers: 1) Anomaly table would spin forever if you zero jobs configured on a fresh install. 2) If you get Network or ML errors this will report the error to the user in the form of the global toaster. Two examples of Error Toaster before the "See the full error(s)" is clicked: ![toast](https://user-images.githubusercontent.com/1151048/61415060-f2089200-a8ac-11e9-8293-3dfcdbbe069e.png) <img width="348" alt="Screen Shot 2019-07-17 at 4 04 30 PM" src="https://user-images.githubusercontent.com/1151048/61414971-ac4bc980-a8ac-11e9-9d15-28ab1229922f.png"> Example Error Toasters from start job expanded and collapsed: <img width="800" alt="Screen Shot 2019-07-17 at 12 15 04 PM" src="https://user-images.githubusercontent.com/1151048/61414610-9e497900-a8ab-11e9-97cd-ec68e77a555c.png"> <img width="808" alt="Screen Shot 2019-07-17 at 12 14 58 PM" src="https://user-images.githubusercontent.com/1151048/61414628-aacdd180-a8ab-11e9-8ad8-edc67deb8874.png"> Example Network Error Toaster: <img width="437" alt="Screen Shot 2019-07-17 at 12 21 10 PM" src="https://user-images.githubusercontent.com/1151048/61414635-b3260c80-a8ab-11e9-8e50-a35a05fc6d2b.png"> Example API Error if you send in something bad such as a bad payload: <img width="442" alt="Screen Shot 2019-07-17 at 12 34 04 PM" src="https://user-images.githubusercontent.com/1151048/61414658-c0db9200-a8ab-11e9-88ed-e6634f17103a.png"> Example Anomalies Table Error if you have a network issue: <img width="464" alt="Screen Shot 2019-07-17 at 12 39 57 PM" src="https://user-images.githubusercontent.com/1151048/61414691-cf29ae00-a8ab-11e9-882c-3cc770776f2f.png"> ### Checklist Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR. - [x] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) - [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 - [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 - [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist) ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process) - [x] This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)

XavierM · 2019-07-18T07:35:55Z

x-pack/legacy/plugins/siem/public/components/ml/anomaly/use_anomalies_table_data.ts

@@ -75,6 +79,7 @@ export const useAnomaliesTableData = ({
  const config = useContext(KibanaConfigContext);
  const capabilities = useContext(MlCapabilitiesContext);
  const userPermissions = hasMlUserPermissions(capabilities);
+  const [, dispatchToaster] = useStateToaster();


nit -I am used to see it the other way around like const dispatchToaster = useStateToaster()[1];

Nothing wrong with that way since it is valid ts/js, but in the all the videos for React Hooks, including the introduction of them, they use array destructoring which is the reasoning for my choice.

Let me know if I'm not seeing something with the tutorials or others that have hooks which use array indexes over destructoring as I am curious.

XavierM · 2019-07-18T07:42:10Z

x-pack/legacy/plugins/siem/public/components/ml/api/error_to_toaster.ts

+  iconType = 'alert',
+  dispatchToaster,
+}: ErrorToToasterArgs) => {
+  if (isToasterError(error)) {


nit, I think that you can simplify it like that

const toast: AppToast = { id, title, color, iconType, errors: ['Network Error'], }; if (isToasterError(error)) { toast.errors = error.messages; } else if (isAnError(error)) { toast.errors = [error.message]; } dispatchToaster({ type: 'addToaster', toast, });

Appreciate it, I will try to work it into a follow up PR 🤞 here soon.

Thank you for looking at my PR Xavier and giving me feedback.

FrankHassanabad added 5 commits July 16, 2019 17:04

Initial toaster logic

6383e68

Updated toaster error handling

f82abe5

Merge branch 'master' into add-toaster-logic

7d2e6bd

Cleanups of code not used

6fb1cf1

Added i18n support

6e0c1eb

FrankHassanabad commented Jul 17, 2019

View reviewed changes

x-pack/legacy/plugins/siem/public/components/ml/api/throw_if_not_ok.ts Outdated Show resolved Hide resolved

FrankHassanabad commented Jul 17, 2019

View reviewed changes

x-pack/legacy/plugins/siem/public/components/ml/api/throw_if_not_ok.ts Outdated Show resolved Hide resolved

FrankHassanabad commented Jul 17, 2019

View reviewed changes

x-pack/legacy/plugins/siem/public/components/ml/api/throw_if_not_ok.ts Outdated Show resolved Hide resolved

FrankHassanabad added 2 commits July 17, 2019 14:05

Added missing i18n keys and fixed the name spaces of all of them

27ce944

Added unit test

0f501b8

spong reviewed Jul 17, 2019

View reviewed changes

x-pack/legacy/plugins/siem/public/components/ml/anomaly/use_anomalies_table_data.ts Show resolved Hide resolved

spong reviewed Jul 17, 2019

View reviewed changes

x-pack/legacy/plugins/siem/public/components/ml/api/throw_if_not_ok.ts Outdated Show resolved Hide resolved

Added the rest of the unit tests

54a0f70

FrankHassanabad changed the title ~~Add toaster logic~~ [SIEM] Add toaster logic for Machine Learning Jul 17, 2019

Updated from feedback from the PR review

d991a22

spong reviewed Jul 17, 2019

View reviewed changes

FrankHassanabad marked this pull request as ready for review July 17, 2019 21:55

FrankHassanabad added release_note:skip Skip the PR/issue when compiling release notes v8.0.0 v7.3.0 v7.4.0 labels Jul 17, 2019

spong reviewed Jul 17, 2019

View reviewed changes

x-pack/legacy/plugins/siem/public/components/ml_popover/translations.ts Outdated Show resolved Hide resolved

spong added the Team:SIEM label Jul 17, 2019

spong assigned FrankHassanabad Jul 17, 2019

Fix capitalization issue

b191778

spong approved these changes Jul 17, 2019

View reviewed changes

FrankHassanabad merged commit 1f8c215 into elastic:master Jul 17, 2019

FrankHassanabad deleted the add-toaster-logic branch July 17, 2019 23:45

FrankHassanabad mentioned this pull request Jul 17, 2019

[7.x] [SIEM] Add toaster logic for Machine Learning (#41401) #41420

Merged

FrankHassanabad mentioned this pull request Jul 17, 2019

[7.3] [SIEM] Add toaster logic for Machine Learning (#41401) #41421

Merged

XavierM reviewed Jul 18, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SIEM] Add toaster logic for Machine Learning #41401

[SIEM] Add toaster logic for Machine Learning #41401

FrankHassanabad commented Jul 17, 2019 •

edited

Loading

elasticmachine commented Jul 17, 2019

spong Jul 17, 2019

spong Jul 17, 2019

elasticmachine commented Jul 17, 2019

spong commented Jul 17, 2019 •

edited

Loading

spong left a comment

elasticmachine commented Jul 17, 2019

XavierM Jul 18, 2019

FrankHassanabad Jul 19, 2019

XavierM Jul 18, 2019

FrankHassanabad Jul 19, 2019

[SIEM] Add toaster logic for Machine Learning #41401

[SIEM] Add toaster logic for Machine Learning #41401

Conversation

FrankHassanabad commented Jul 17, 2019 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Jul 17, 2019

💚 Build Succeeded

spong Jul 17, 2019

Choose a reason for hiding this comment

spong Jul 17, 2019

Choose a reason for hiding this comment

elasticmachine commented Jul 17, 2019

spong commented Jul 17, 2019 • edited Loading

spong left a comment

Choose a reason for hiding this comment

elasticmachine commented Jul 17, 2019

💚 Build Succeeded

XavierM Jul 18, 2019

Choose a reason for hiding this comment

FrankHassanabad Jul 19, 2019

Choose a reason for hiding this comment

XavierM Jul 18, 2019

Choose a reason for hiding this comment

FrankHassanabad Jul 19, 2019

Choose a reason for hiding this comment

FrankHassanabad commented Jul 17, 2019 •

edited

Loading

spong commented Jul 17, 2019 •

edited

Loading