[ML] Add query bar to Anomaly Explorer for filtering of anomaly results for one or more influencers #31763

alvarezmelissa87 · 2019-02-21T23:22:05Z

Summary

Related issue: #18262

Adds a query bar component to the Anomaly Explorer which allows filtering of anomaly results for one or more influencers. The query bar autocomplete functionality suggests influencer field names and values based on the selected job's influencers.

Follow up PR:

Ability to add filter via icon in Anomalies table
Ability to remove filter via icon in Anomalies table
Add a filter from the Top Influencers list
Remove a filter from the Top Influencers list
Do not retain filter if job switched and filter no longer relevant
console warning for click outside
Overall swimlane click should retain filter in view by swimlane update
Remove warning icon from no influencers found message

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~~

elasticmachine · 2019-02-21T23:22:49Z

Pinging @elastic/ml-ui

elasticmachine · 2019-02-22T00:05:26Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-02-23T04:14:37Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-02-26T00:51:16Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-02-26T01:41:39Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

walterra

Overall this is great! Did a first round of review and added a few questions.

...ml/public/explorer/components/explorer_no_influencers_found/explorer_no_influencers_found.js

x-pack/plugins/ml/public/explorer/explorer.js

x-pack/plugins/ml/public/explorer/explorer_controller.js

walterra · 2019-02-26T11:51:28Z

x-pack/plugins/ml/public/explorer/explorer_swimlane.js

+ .html(label => {
+ const showFilterContext = filterActive === true && label === 'Overall';
+ if (showFilterContext) {
+ return `${label} (unfiltered)`;


Do we still need to use mlEscape here? Like

return `${mlEscape(label)} (unfiltered)`;

Additionally, "(unfiltered)" needs to become a translated message.

yes, i think so

This conversation was marked as resolved, but the suggested change hasn't been made.
is there a reason why we don't need to use mlEscape here?

My mistake - fixed with: a861b7d

walterra · 2019-02-26T11:55:55Z

x-pack/plugins/ml/public/explorer/kuery_filter_bar/click_outside/click_outside.test.js

+ const wrapper = shallow(<ClickOutside onClickOutside={() => {}} />);
+ expect(wrapper).toMatchSnapshot();
+ });
+


Nit suggestion: Would be great to have a test here that tests the actual click event.

x-pack/plugins/ml/public/explorer/_explorer.scss

elasticmachine · 2019-02-26T22:40:43Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-02-27T02:00:50Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

jgowdyelastic · 2019-02-27T10:23:53Z

I'm seeing this error in the browser's console when anomaly explorer loads:

x-pack/plugins/ml/public/explorer/explorer.js

jgowdyelastic · 2019-02-27T12:22:25Z

x-pack/plugins/ml/public/explorer/explorer.js

@@ -666,14 +715,21 @@ export const Explorer = injectI18n(injectObservablesAsProps(
 { viewByLoadedForTimeFormatted: formatHumanReadableDateTime(timerange.earliestMs) }
 );
 } else {
+ let tempInfluencersFilterQuery = influencersFilterQuery;


nit, this could be const tempInfluencersFilterQuery = (maskAll === true) ? undefined : influencersFilterQuery

Updated in: a861b7d

jgowdyelastic · 2019-02-27T12:35:19Z

x-pack/plugins/ml/public/explorer/explorer.js

+ if (influencersFilterQuery.match_all && Object.keys(influencersFilterQuery.match_all).length === 0) {
+ this.props.appStateHandler(APP_STATE_ACTION.CLEAR_INFLUENCER_FILTER_SETTINGS);
+ const stateUpdate = {
+ ...{


this spread operator isn't necessary

👍 Fixed here: a861b7d

jgowdyelastic · 2019-02-27T14:11:04Z

x-pack/plugins/ml/public/explorer/explorer_swimlane.js

+ .html(label => {
+ const showFilterContext = filterActive === true && label === 'Overall';
+ if (showFilterContext) {
+ return `${label} (unfiltered)`;


This conversation was marked as resolved, but the suggested change hasn't been made.
is there a reason why we don't need to use mlEscape here?

jgowdyelastic · 2019-02-27T14:21:40Z

x-pack/plugins/ml/public/explorer/explorer_utils.js

+ return new Promise((resolve) => {
+ const influencers = [];
+ if (noInfluencersConfigured !== true) {
+ ml.getMultipleJobs(


mlJobService already has all the jobs loaded from the server so doesn't need to request them again.
the function selectedJobsHaveInfluencers (in this file) uses this list and so a new function called getInfluencers could be added here which is very similar to selectedJobsHaveInfluencers.
This would remove the need for getMultipleJobs in the ml lib.

Also, noInfluencersConfigured is already based on the results of selectedJobsHaveInfluencers and so isn't needed at all here or in getIndexPattern

IMO, an even better change would be to remove selectedJobsHaveInfluencers entirely from this file and add getInfluencers to mlJobService.
the places where selectedJobsHaveInfluencers is set could then be based on mlJobService.getInfluencers(selectedJobs).length > 0

Really great thoughts here, @jgowdyelastic. I think removing getMultipleJobs is the right call here since I can essentially access the already loaded jobs in this file via the mlJobService as you mentioned.

I've gone ahead and created a getInfluencers function in this file (explorer_utils.js) that is similar to selectedJobsHaveInfluencers but returns an array of influencer name strings. This way I can use it similarly to how you mentioned - by grabbing all the influencers I need in getIndexPattern and also using noInfluencersConfigured: (getInfluencers(selectedJobs).length === 0) in lieu of selectedJobsHaveInfluencers

Since that's the only context selectedJobsHaveInfluencers is being used (and only in explorer.js) I've removed that function since it's no longer needed. This solution also means I can remove loadInfluencerFields from explorer_utils. Let me know what you think.

Updated in: a861b7d

jgowdyelastic · 2019-02-27T15:18:07Z

x-pack/plugins/ml/public/explorer/kuery_filter_bar/kuery_filter_bar.js

+ this.setState({ suggestions, isLoadingSuggestions: false });
+ } catch (e) {
+ console.error('Error while fetching suggestions', e);
+ this.setState({ isLoadingSuggestions: false, error: (e.message ? e.message : 'Error while fetching suggestions') });


should the errors displayed in the callout be translated?

Good call - updated with localized default error messages. 👍

jgowdyelastic · 2019-02-27T15:20:13Z

x-pack/plugins/ml/public/explorer/kuery_filter_bar/kuery_filter_bar.js

+import { uniqueId } from 'lodash';
+import { FilterBar } from './filter_bar';
+import { EuiCallOut } from '@elastic/eui';
+import { fromKueryExpression, toElasticsearchQuery } from '@kbn/es-query'; // luceneStringToDsl


accidentally left in comment?

Yep - missed it! Removed in: a861b7d

jgowdyelastic · 2019-02-27T15:54:15Z

x-pack/plugins/ml/public/explorer/kuery_filter_bar/kuery_filter_bar.js

+ try {
+ const ast = fromKueryExpression(inputValue);
+ const isAndOperator = (ast.function === 'and');
+ const query = convertKueryToEsQuery(inputValue, indexPattern);


kibana's own buildEsQuery function could be used here i believe:

const q = { query: inputValue, language: 'kuery', }; const query = buildEsQuery(indexPattern, [q]);

also, indexPattern is undefined here as it should come from props not state, this made me realise that it's not needed at all and so null or undefined could be passed to convertKueryToEsQuery or buildEsQuery (if you decide to use it).
e.g.
const query = buildEsQuery(null, [q]);

I've decided to keep as is since this conversion function as been used in APM as well so I think it's best to keep things consistent.

jgowdyelastic · 2019-02-27T16:05:46Z

x-pack/plugins/ml/public/explorer/kuery_filter_bar/suggestions/suggestions.js

+ }
+
+ const suggestions = this.props.suggestions.map((suggestion, index) => {
+ const key = suggestion + '_' + index;


nit, a template literal could be used here

jgowdyelastic · 2019-02-27T16:08:24Z

x-pack/plugins/ml/public/explorer/kuery_filter_bar/suggestions/suggestions.js

+ }
+
+ render() {
+ if (!this.props.show || isEmpty(this.props.suggestions)) {


nit, this.props.suggestions.length === 0 could be used instead here

Good call - updated 👍

cchaos

Thx for the SASS change

x-pack/plugins/ml/public/services/results_service.js

peteharverson · 2019-02-27T18:07:30Z

x-pack/plugins/ml/public/services/ml_api_service/index.js

@@ -27,6 +27,12 @@ export const ml = {
 });
 },

+ getMultipleJobs(jobIds) {


Looks like this new function might not be needed if https://github.com/elastic/kibana/pull/31763/files#r260769619 can be implemented.

Removed in a861b7d

elasticmachine · 2019-02-28T20:33:37Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

x-pack/plugins/apm/public/components/shared/KueryBar/Typeahead/Suggestions.js

elasticmachine · 2019-03-01T15:38:42Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

peteharverson

Two quick thoughts:

the kibana query language is now being referred to as 'KQL' through the UI, so I wonder if the folder and filenames should be renamed at this stage from e.g. kuery_filter_bar to kql_filter_bar.
is the query bar component generic enough to be used outside the Anomaly Explorer view? If so, it might be better to move it to the x-pack/plugins/ml/public/components folder. In the future, it would be great for example to use this in the Data Visualizer view.

elasticmachine · 2019-03-04T17:07:43Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-03-04T18:06:46Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

peteharverson

Tested latest changes and LGTM. Happy for the additional work to go into follow-up PRs.

…ts for one or more influencers (elastic#31763) * wip: add autocomplete kueryFilterBar.update endpoints with query. * Create indexPattern on job change. Value is autocompleted * Mask unrelated swimlanes on filter apply * Only show relevant viewBy swimlane * reload viewBy swimlane when view By change is relevant to filter * handle various query types when building filteredFields list * remove deprecated code from old filter bar * Show error callout when invalid filter syntax * remove dependency on deprecated import var * remove all left over apm dependencies * persist filter on refresh * Create initial filter bar placeholder dynamically * add description text when filter active * switch to relevant viewBy fieldname on filter * recalculate placeholder on job change * Create tests for all components * View by dropdown only contains relevant fields for filter * fix localization message noInfluencers component * Move indexPattern to state and viewBy option filtering to utils * remove unnecessary setState when setting viewBy for filter * Use preloaded jobs from mlJobService to get influencers * Only filter viewByOptions if valid field * use kql and move to components dir * move filter bar to public/components/

…ts for one or more influencers (#31763) (#32518) * wip: add autocomplete kueryFilterBar.update endpoints with query. * Create indexPattern on job change. Value is autocompleted * Mask unrelated swimlanes on filter apply * Only show relevant viewBy swimlane * reload viewBy swimlane when view By change is relevant to filter * handle various query types when building filteredFields list * remove deprecated code from old filter bar * Show error callout when invalid filter syntax * remove dependency on deprecated import var * remove all left over apm dependencies * persist filter on refresh * Create initial filter bar placeholder dynamically * add description text when filter active * switch to relevant viewBy fieldname on filter * recalculate placeholder on job change * Create tests for all components * View by dropdown only contains relevant fields for filter * fix localization message noInfluencers component * Move indexPattern to state and viewBy option filtering to utils * remove unnecessary setState when setting viewBy for filter * Use preloaded jobs from mlJobService to get influencers * Only filter viewByOptions if valid field * use kql and move to components dir * move filter bar to public/components/

walterra · 2019-03-06T09:07:31Z

LGTM!

jgowdyelastic · 2019-03-06T09:09:56Z

LGTM

alvarezmelissa87 requested review from a team as code owners February 21, 2019 23:22

alvarezmelissa87 added release_note:enhancement :ml Feature:ml-results legacy - do not use v7.2.0 labels Feb 21, 2019

alvarezmelissa87 force-pushed the ml-anomaly-explorer-autocomplete-filter branch from ba2f052 to e882a70 Compare February 26, 2019 00:11

alvarezmelissa87 changed the title ~~WIP: [ML] Add query bar to Anomaly Explorer for filtering of anomaly results for one or more influencers~~ [ML] Add query bar to Anomaly Explorer for filtering of anomaly results for one or more influencers Feb 26, 2019

alvarezmelissa87 requested review from peteharverson, jgowdyelastic and walterra February 26, 2019 01:04

walterra reviewed Feb 26, 2019

View reviewed changes

jgowdyelastic added the v8.0.0 label Feb 26, 2019

cchaos reviewed Feb 26, 2019

View reviewed changes

x-pack/plugins/ml/public/explorer/_explorer.scss Outdated Show resolved Hide resolved

jgowdyelastic reviewed Feb 27, 2019

View reviewed changes

cchaos approved these changes Feb 27, 2019

View reviewed changes

peteharverson reviewed Feb 27, 2019

View reviewed changes

alvarezmelissa87 requested a review from a team as a code owner March 1, 2019 14:56

sorenlouv reviewed Mar 1, 2019

View reviewed changes

x-pack/plugins/apm/public/components/shared/KueryBar/Typeahead/Suggestions.js Outdated Show resolved Hide resolved

peteharverson reviewed Mar 1, 2019

View reviewed changes

alvarezmelissa87 added 14 commits March 4, 2019 10:22

remove all left over apm dependencies

963823a

persist filter on refresh

2f6b475

Create initial filter bar placeholder dynamically

19c2b35

add description text when filter active

80a6250

switch to relevant viewBy fieldname on filter

daba463

recalculate placeholder on job change

2a804a6

Create tests for all components

7c57a51

View by dropdown only contains relevant fields for filter

2b0ad89

fix localization message noInfluencers component

41c28c2

Move indexPattern to state and viewBy option filtering to utils

d2dee17

remove unnecessary setState when setting viewBy for filter

bb11a34

Use preloaded jobs from mlJobService to get influencers

6c3c8c9

Only filter viewByOptions if valid field

80cf018

use kql and move to components dir

a83bfc8

alvarezmelissa87 force-pushed the ml-anomaly-explorer-autocomplete-filter branch from 2e091d6 to a83bfc8 Compare March 4, 2019 16:22

move filter bar to public/components/

d917054

alvarezmelissa87 requested a review from lukasolson March 4, 2019 21:13

peteharverson approved these changes Mar 5, 2019

View reviewed changes

alvarezmelissa87 mentioned this pull request Mar 5, 2019

[ML] Anomaly Explorer Filtering follow-up tasks #32505

Closed

11 tasks

alvarezmelissa87 merged commit 576c90a into elastic:master Mar 5, 2019

alvarezmelissa87 mentioned this pull request Mar 5, 2019

[7.x] [ML] Add query bar to Anomaly Explorer for filtering of anomaly results for one or more influencers (#31763) #32518

Merged

alvarezmelissa87 deleted the ml-anomaly-explorer-autocomplete-filter branch March 5, 2019 21:15

walterra mentioned this pull request Mar 6, 2019

[ML] Anomaly Explorer Enhancements #18553

Open

42 tasks

sophiec20 added the Feature:Anomaly Detection ML anomaly detection label Jun 19, 2019

[ML] Add query bar to Anomaly Explorer for filtering of anomaly results for one or more influencers #31763

[ML] Add query bar to Anomaly Explorer for filtering of anomaly results for one or more influencers #31763

Conversation

alvarezmelissa87 commented Feb 21, 2019 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Feb 21, 2019

elasticmachine commented Feb 22, 2019

💔 Build Failed

elasticmachine commented Feb 23, 2019

💔 Build Failed

elasticmachine commented Feb 26, 2019

💔 Build Failed

elasticmachine commented Feb 26, 2019

💔 Build Failed

walterra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alvarezmelissa87 Feb 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Feb 26, 2019

💚 Build Succeeded

elasticmachine commented Feb 27, 2019

💚 Build Succeeded

jgowdyelastic commented Feb 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alvarezmelissa87 Feb 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alvarezmelissa87 Feb 28, 2019 • edited Loading

Choose a reason for hiding this comment

jgowdyelastic Feb 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cchaos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alvarezmelissa87 Feb 28, 2019 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Feb 28, 2019

💚 Build Succeeded

elasticmachine commented Mar 1, 2019

💚 Build Succeeded

peteharverson left a comment

Choose a reason for hiding this comment

elasticmachine commented Mar 4, 2019

💚 Build Succeeded

elasticmachine commented Mar 4, 2019

💚 Build Succeeded

peteharverson left a comment

Choose a reason for hiding this comment

walterra commented Mar 6, 2019

jgowdyelastic commented Mar 6, 2019

alvarezmelissa87 commented Feb 21, 2019 •

edited

Loading

alvarezmelissa87 Feb 28, 2019 •

edited

Loading

alvarezmelissa87 Feb 28, 2019 •

edited

Loading

alvarezmelissa87 Feb 28, 2019 •

edited

Loading

jgowdyelastic Feb 27, 2019 •

edited

Loading

alvarezmelissa87 Feb 28, 2019 •

edited

Loading