[ML] Anomaly Detection alert type#89286
Merged
darnautov merged 68 commits intoelastic:masterfrom Feb 11, 2021
Merged
Conversation
# Conflicts: # x-pack/plugins/ml/kibana.json # x-pack/plugins/ml/public/plugin.ts
Member
|
LGTM 🎉 . Just a few minor comments. |
| \\{\\{/context.topRecords\\}\\} | ||
|
|
||
| \\{\\{! Replace kibanaBaseUrl if not configured in Kibana \\}\\} | ||
| [Open in Anomaly Explorer](\\{\\{\\{context.kibanaBaseUrl\\}\\}\\}\\{\\{\\{context.anomalyExplorerUrl\\}\\}\\}) |
Contributor
There was a problem hiding this comment.
For an influencer type, I am getting an error on opening the Anomaly Explorer from the link (testing the email action)
The link in the email is:
https://localhost:5601/abc/app/ml/explorer/?_g=(ml%3A(jobIds%3A!(ecommerce_high_sum_total_sales))%2Ctime%3A(from%3A%272021-02-10T08%3A00%3A00.000Z%27%2Cmode%3Aabsolute%2Cto%3A%272021-02-11T04%3A00%3A00.000Z%27))&_a=(explorer%3A(mlExplorerFilter%3A(filterActive%3A!t%2CfilteredFields%3A!(category.keyword%2C%27Women!%27s%20Clothing%2CWomen!%27s%20Shoes%27)%2CinfluencersFilterQuery%3A(bool%3A(minimum_should_match%3A1%2Cshould%3A!((match_phrase%3A(category.keyword%3A%27Women!%27s%20Clothing%2CWomen!%27s%20Shoes%27))))))%2CmlExplorerSwimlane%3A(selectedLanes%3A!(%27Women!%27s%20Clothing%2CWomen!%27s%20Shoes%27)%2CselectedTimes%3A1612980000%2CselectedType%3AviewBy%2CviewByFieldName%3Acategory.keyword)))
Contributor
There was a problem hiding this comment.
Testing 657b1fe the Anomaly Explorer is not loading correctly when clicking on a link in an email for a bucket alert. Link is:
https://localhost:5601/abc/app/ml/explorer/?_g=(ml%3A(jobIds%3A!(ecommerce_high_sum_total_sales))%2Ctime%3A(from%3A%272021-02-11T03%3A00%3A00.000Z%27%2Cmode%3Aabsolute%2Cto%3A%272021-02-11T23%3A00%3A00.000Z%27))&_a=(explorer%3A(mlExplorerFilter%3A()%2CmlExplorerSwimlane%3A(selectedLanes%3A!(Overall)%2CselectedTimes%3A1613048400%2CselectedType%3Aoverall)))
pheyos
reviewed
Feb 11, 2021
Member
pheyos
left a comment
There was a problem hiding this comment.
Functional tests are looking good overall, great to have them in as part of this PR! 🎉
Just a few suggestions:
| await testSubjects.click('saveAlertButton'); | ||
| const isConfirmationModalVisible = await testSubjects.isDisplayed('confirmAlertSaveModal'); | ||
| expect(isConfirmationModalVisible).to.eql(true, 'Expect confirmation modal to be visible'); | ||
| await testSubjects.click('confirmModalConfirmButton'); |
Member
There was a problem hiding this comment.
Would be good to have two more assertions here after the click. I'd suggest to check that
- the modal is not displayed anymore
- the alert creation flyout is closed (probably needs some waiting - maybe implement in a separate method?)
| await browser.pressKeys(browser.keys.ESCAPE); | ||
| }, | ||
|
|
||
| async setSliderValue(testDataSubj: string, value: number) { |
Member
There was a problem hiding this comment.
Great to have this available here so it's re-usable! Could we let the data_frame_analytics_creation service methods setTrainingPercent and assertTrainingPercentValue use this implementation, so we avoid code duplication?
Member
There was a problem hiding this comment.
Could we do the same for data_frame_analytics_creation.assertTrainingPercentValue -> common_ui.assertSliderValue? This is not only used to check that setTrainingPercent worked correctly, but also during clone to check that the slider value is carried over correctly.
|
|
||
| Top records: | ||
| \\{\\{#context.topRecords\\}\\} | ||
| \\{\\{function\\}\\}(\\{\\{field_name\\}\\}) \\{\\{by_field_value\\}\\} \\{\\{over_field_value\\}\\} \\{\\{partition_field_value\\}\\} [\\{\\{score\\}\\}] |
Contributor
There was a problem hiding this comment.
We need to floor the scores displayed in the preview and the alert, to match what we do in the Anomaly Explorer, including < 1 if possible for scores less than 1. Can you use getFormattedSeverityScore from anomaly_utils.ts?
peteharverson
approved these changes
Feb 11, 2021
Contributor
peteharverson
left a comment
There was a problem hiding this comment.
Tested latest edits and LGTM.
Would be good to use getFormattedSeverityScore for formatting scores less than 0 as < 1 but that can be done in a follow-up, as can a fix for the Anomaly Explorer to select a range of cell(s) from the link in the alert.
Contributor
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / Chrome UI Functional Tests.test/functional/apps/dashboard/dashboard_unsaved_state·ts.dashboard app using current data dashboard unsaved panels retains unsaved panel count after navigating to listing page and backStandard OutStack TraceMetrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
darnautov
added a commit
to darnautov/kibana
that referenced
this pull request
Feb 11, 2021
* [ML] init ML alerts * [ML] job selector * [ML] move schema server-side * [ML] fix type 🤦 * [ML] severity selector * [ML] add alerting capabilities * [ML] add alerting capabilities * [ML] result type selector * [ML] time range selector * [ML] init alert preview endpoint * [ML] update SeveritySelector component * [ML] adjust the form * [ML] adjust the form * [ML] server-side, preview component * [ML] update defaultMessage * [ML] Anomaly explorer URL * [ML] validate preview interval * [ML] rename alert type * [ML] fix i18n * [ML] fix TS and mocks * [ML] update licence headers * [ML] add ts config references * [ML] init functional tests * [ML] functional test for creating anomaly detection alert * [ML] adjust bucket results query * [ML] fix messages * [ML] resolve functional tests related issues * [ML] fix result check * [ML] change preview layout * [ML] extend ml client types * [ML] add missing types, remove unused client variable * [ML] change to import type * [ML] handle preview error * [ML] move error callout * [ML] better error handling * [ML] add client-side validation * [ML] move fake request to the executor * [ML] revert ml client type changes, set response type manually * [ML] documentationUrl * [ML] add extra sentence for interim results * [ML] use publicBaseUrl * [ML] adjust the query * [ML] fix anomaly explorer url * [ML] adjust the alert params schema * [ML] remove default severity threshold for records and influencers * [ML] fix query with filter block * [ML] fix functional tests * [ML] remove isInterim check * [ML] remove redundant fragment * [ML] fix selected cells hook * [ML] set query string * [ML] support sample size by the preview endpoint * [ML] update counter * [ML] add check for the bucket span * [ML] fix effects * [ML] disable mlExplorerSwimlane * [ML] refactor functional tests to use setSliderValue * [ML] add assertTestIntervalValue * [ML] floor scores
darnautov
added a commit
that referenced
this pull request
Feb 12, 2021
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
14 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Related issue #88940
This PR introduces a new alert type - ML Anomaly detection alert.
List of features covered so far:
By default, the action group is "Anomaly score matched the condition"
At the moment there is only one default message allowed for all action types (limitation of the framework).
The list of available context variable:
timestamptimestampIso8601jobIdsisInterimscoretopRecordstopInfluencersanomalyExplorerUrlBear in mind that Kibana public URL isn't always available server-side of Kibana (the user has to specify it), so it's required to complete the
anomalyExplorerUrlin the Action Message field with the hostname.Example of the Slack message action:
Checklist