[ML] Overview tab for ML #45864

alvarezmelissa87 · 2019-09-17T04:53:17Z

Summary

Moved and renamed MlInMemoryTable (was AnalyticsTable) to a shared location in public/components. Updated all imports with new name/location.

Adds Overview main tab with Anomaly Detection jobs section and Analytics job section.

This is version one of the overview tab. In place of max anomaly score, the anomaly detection section will have a mini graph/swimlane representing the anomalies for the last 24hrs across jobs for that group.

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-09-17T04:53:19Z

Pinging @elastic/ml-ui

elasticmachine · 2019-09-17T06:28:44Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

sophiec20 · 2019-09-17T14:32:25Z

We rarely show decimal places for anomaly score. Even though it is not the final graphic, probably worth rounding.

elasticmachine · 2019-09-17T18:26:55Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-17T20:45:18Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-17T22:20:09Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

alvarezmelissa87 · 2019-09-18T02:38:13Z

retest

elasticmachine · 2019-09-18T04:06:53Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

walterra

Overall this looks good, the only thing I'd consider a must for this PR are some missing translations.

I made some suggestions for improving types but that could be done in a follow up since I assume the use of some anys stems from consuming/reusing code from the anomaly jobs list which isn't TypeScript yet.

walterra · 2019-09-18T07:57:29Z

...public/data_frame_analytics/pages/analytics_management/components/analytics_list/actions.tsx

@@ -21,31 +21,33 @@ import { stopAnalytics } from '../../services/analytics_service';
 import { StartAction } from './action_start';
 import { DeleteAction } from './action_delete';

+export const ANALYTICS_VIEW_ACTION = {


nit: could be analyticsViewAction, it's not an enum or a simple constant.

walterra · 2019-09-18T08:00:47Z

...public/data_frame_analytics/pages/analytics_management/components/analytics_list/columns.tsx

@@ -58,6 +58,57 @@ export const getTaskStateBadge = (
  );
 };

+export const PROGRESS_COLUMN = {


nit: could be progressColumn, it's not an enum or simple constant.

walterra · 2019-09-18T08:03:55Z

x-pack/legacy/plugins/ml/public/overview/components/analytics_panel/analytics_panel.tsx

+      {isInitialized === true && analytics.length === 0 && (
+        <EuiEmptyPrompt
+          iconType="createAdvancedJob"
+          title={<h2>Create your first analytics job</h2>}


This needs to be translated.

walterra · 2019-09-18T08:04:05Z

x-pack/legacy/plugins/ml/public/overview/components/analytics_panel/analytics_panel.tsx

+          body={
+            <Fragment>
+              <p>
+                Data frame analytics enable you to perform different analyses of your data and


This needs to be translated.

walterra · 2019-09-18T08:04:13Z

x-pack/legacy/plugins/ml/public/overview/components/analytics_panel/analytics_panel.tsx

+          }
+          actions={
+            <EuiButton href="#/data_frame_analytics?" color="primary" fill>
+              Create job


This needs to be translated.

walterra · 2019-09-18T08:55:50Z

x-pack/legacy/plugins/ml/public/overview/components/anomaly_detection_panel/utils.ts

+import { Group, Groups, Jobs, Job } from './anomaly_detection_panel';
+
+export function getGroupsFromJobs(jobs: Jobs): Groups {
+  const groups: any = {


Could this be groups: Groups?

walterra · 2019-09-18T09:50:13Z

...cy/plugins/ml/public/overview/components/anomaly_detection_panel/anomaly_detection_panel.tsx

+  max_anomaly_score: number | null;
+}
+
+export interface Job {


We have MlJob in ml/common/types/jobs.ts could this be used instead here? This would help to get rid of all the anys when working with this type/interface. If we use that, I'd suggest that we remove the Jobs type from this file, using MlJob[] in the consuming code would be more clear IMHO.

That's not quite the type I need since I'm getting jobs from ml.jobsSummary but I went ahead and added a MlSummaryJob and MlSummaryJobs as we'll need those types when we convert all the job management code to typescript.

walterra · 2019-09-18T09:53:04Z

...cy/plugins/ml/public/overview/components/anomaly_detection_panel/anomaly_detection_panel.tsx

+import { mlResultsService } from '../../../services/results_service';
+import { getGroupsFromJobs, getStatsBarData, getJobsWithTimerange } from './utils';
+
+export interface Groups {


nit: could we name this GroupsDictionary and reuse Dictionary from x-pack/legacy/plugins/ml/common/types/common.ts? It's just that looking at the consuming code I assumed Groups was an array (because Jobs was) which was a bit irritating.

walterra · 2019-09-18T09:53:50Z

...cy/plugins/ml/public/overview/components/anomaly_detection_panel/anomaly_detection_panel.tsx

+
+export type Jobs = Job[];
+
+interface MaxScoresByGroup {


This could use Dictionary from x-pack/legacy/plugins/ml/common/types/common.ts

walterra · 2019-09-18T10:08:10Z

x-pack/legacy/plugins/ml/public/overview/components/anomaly_detection_panel/utils.ts

+  }
+
+  // object to keep track of nodes being used by jobs
+  const mlNodes: any = {};


Looking at how this is used later on, could this just be an array of job.nodeNames like const mlNodes: string[] = []; or even just a counter like let activeNodes = 0;?

peteharverson · 2019-09-18T13:25:47Z

x-pack/legacy/plugins/ml/public/overview/components/anomaly_detection_panel/table.tsx

+        defaultMessage: 'Jobs in group',
+      }),
+      render: (jobIds: Group['jobIds']) => jobIds.length,
+      sortable: true,


I am seeing some odd sort behavior here. It doesn't seem to sort numerically:

Added to follow-up meta issue 👍

peteharverson · 2019-09-18T13:31:48Z

x-pack/legacy/plugins/ml/public/overview/components/anomaly_detection_panel/table.tsx

+      name: i18n.translate('xpack.ml.overview.anomalyDetection.tableActionLabel', {
+        defaultMessage: 'Actions',
+      }),
+      render: (group: Group) => <ExplorerLink jobsList={getJobsFromGroup(group, jobsList)} />,


Seen this strange behavior where the count in the table is different to the count in the tooltip on the link to the Anomaly Explorer (16 is the correct number):

Fixed with 59c9808

peteharverson · 2019-09-18T13:35:09Z

x-pack/legacy/plugins/ml/public/overview/components/analytics_panel/analytics_panel.tsx

+        <Fragment>
+          <AnalyticsTable items={analytics} />
+          <EuiSpacer size="m" />
+          <div className="mlOverviewPanel__buttons">


I wonder if the Refresh and Manage buttons could do with a bit more padding between them?

Added to meta issue 👍

peteharverson · 2019-09-18T13:40:29Z

x-pack/legacy/plugins/ml/public/overview/components/anomaly_detection_panel/table.tsx

+    },
+    {
+      name: i18n.translate('xpack.ml.overview.anomalyDetection.tableActionLabel', {
+        defaultMessage: 'Actions',


The two Actions columns have different horizontal alignments. Could they both get the same?

Added to follow-up meta issue

peteharverson · 2019-09-18T14:00:40Z

...cy/plugins/ml/public/overview/components/anomaly_detection_panel/anomaly_detection_panel.tsx

+    <EuiPanel className="mlOverviewPanel">
+      {typeof errorMessage !== 'undefined' && errorDisplay}
+      {isLoading && <EuiLoadingSpinner />}   
+      {isLoading === false && typeof errorMessage === 'undefined' && groupsCount === 0 && (


As previously reported, when there are no Anomaly Detection jobs, rather than the expected EuiEmptyPrompt I am seeing:

Fixed with 59c9808
(ensures removal of ungrouped when there are no ungrouped jobs)

peteharverson · 2019-09-18T14:39:52Z

...cy/plugins/ml/public/overview/components/anomaly_detection_panel/anomaly_detection_panel.tsx

+        const resultsIndex = scores[groupId] && scores[groupId].index;
+        const promiseResult: { success: boolean; results: { [key: string]: number } } =
+          resultsIndex !== undefined && results[resultsIndex];
+        if (promiseResult.success === true && promiseResult.results !== undefined) {


This throws an error if there are no ungrouped jobs, with promiseResult being undefined:

Good catch, @peteharverson. This is occurring because we're passing the empty jobIds array. I've created a fix to remove ungrouped when there are no ungrouped job ids. This also fixes the issue where we weren't seeing the empty prompt when there were no Anomaly Detection jobs and were seeing the table with the loading anomaly score for ungrouped.

darnautov · 2019-09-18T14:11:10Z

x-pack/legacy/plugins/ml/public/overview/breadcrumbs.ts

+import { ML_BREADCRUMB } from '../breadcrumbs';
+
+export function getOverviewBreadcrumbs() {
+  // Whilst top level nav menu with tabs remains,


typo: While*

whilst is a real word - means while

darnautov · 2019-09-18T14:16:07Z

x-pack/legacy/plugins/ml/public/overview/components/analytics_panel/analytics_panel.tsx

+// TODO: panels can be smaller when empty
+export const AnalyticsPanel: FC = () => {
+  const [analytics, setAnalytics] = useState<DataFrameAnalyticsListRow[]>([]);
+  const [errorMessage, setErrorMessage] = useState<any>(undefined);


maybe provide an error interface or just get rid of any?

Will add as a follow up in the meta issue 👍 😄

darnautov · 2019-09-18T14:20:17Z

x-pack/legacy/plugins/ml/public/overview/components/analytics_panel/analytics_panel.tsx

+        color="danger"
+        iconType="alert"
+      >
+        <pre>{JSON.stringify(errorMessage)}</pre>


also can be a dedicated component for rendering error messages which cover this logic

darnautov · 2019-09-18T14:24:28Z

x-pack/legacy/plugins/ml/public/overview/components/analytics_panel/table.tsx

+import { AnalyticsStatsBar } from './analytics_stats_bar';
+
+interface Props {
+  items: any[];


Item interface might be helpful

darnautov · 2019-09-18T14:34:43Z

...cy/plugins/ml/public/overview/components/anomaly_detection_panel/anomaly_detection_panel.tsx

+    const scores = getDefaultAnomalyScores(groupsList);
+
+    try {
+      const promises: any[] = [];


nit: something like

Suggested change

const promises: any[] = [];

const promises = groupsList

.filter(group => group.jobIds.length > 0)

.map((group) => {

return mlResultsService.getOverallBucketScores(

group.jobIds,

1,

group.earliest_timestamp,

group.latest_timestamp

);

});

Yep, great suggestion for preventing the anomaly score fetch error with empty jobIds 👌

darnautov · 2019-09-18T14:35:30Z

...cy/plugins/ml/public/overview/components/anomaly_detection_panel/anomaly_detection_panel.tsx

+      });
+
+      const results = await Promise.all(promises);
+      const tempGroups = Object.assign({}, { ...groupsObject });


Probably should only Object.assign or just spread

darnautov · 2019-09-18T14:42:30Z

x-pack/legacy/plugins/ml/public/overview/components/anomaly_detection_panel/utils.ts

+    },
+  };
+
+  jobs.forEach((job: any) => {


there is a Job interface at x-pack/legacy/plugins/ml/public/jobs/new_job_new/common/job_creator/configs/job.ts

darnautov · 2019-09-18T14:43:04Z

x-pack/legacy/plugins/ml/public/overview/components/anomaly_detection_panel/utils.ts

+    },
+  };
+
+  jobs.forEach((job: any) => {


probably deserves a function

darnautov · 2019-09-18T14:43:18Z

x-pack/legacy/plugins/ml/public/overview/components/anomaly_detection_panel/utils.ts

+  return groups;
+}
+
+export function getStatsBarData(jobsList: any) {


would by nice to replace any

darnautov · 2019-09-18T14:46:35Z

x-pack/legacy/plugins/ml/public/overview/components/anomaly_detection_panel/utils.ts

+}
+
+export function getJobsWithTimerange(jobsList: any) {
+  const jobs: any = {};


nit:

Suggested change

const jobs: any = {};

const jobs = jobsList

.filter(job => jobs[job.id] === undefined && job.earliestTimestampMs !== undefined)

.reduce((acc, job: Job) => {

const { earliestTimestampMs, latestResultsTimestampMs } = job;

acc[job.id] = {

id: job.id,

earliestTimestampMs,

latestResultsTimestampMs,

};

return acc;

}, {});

peteharverson

I am seeing errors from the Transforms wizard with the tables failing to load.

peteharverson · 2019-09-18T15:09:11Z

...ta_frame/pages/data_frame_new_pivot/components/source_index_preview/source_index_preview.tsx

@@ -33,7 +33,7 @@ import {
  MlInMemoryTable,
  SortingPropType,
  SORT_DIRECTION,
-} from '../../../../../../common/types/eui/in_memory_table';
+} from '../../../../../components/ml_in_memory_table';


Can you check if this table works in the Transforms wizard. I am getting errors that the pageIndex property is missing from the pagination part of the EuiBasicTable so the tables fail to load, with this error seen in the console.

Good catch! Fixed with d981ba0

peteharverson · 2019-09-18T15:09:40Z

...ins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/pivot_preview.tsx

@@ -25,7 +25,7 @@ import {
  ColumnType,
  MlInMemoryTable,
  SORT_DIRECTION,
-} from '../../../../../../common/types/eui/in_memory_table';
+} from '../../../../../components/ml_in_memory_table';


As above, this table fails to load for me, due to the missing pageIndex prop.

Fixed in d981ba0

darnautov · 2019-09-18T15:33:04Z

@elasticmachine merge upstream

elasticmachine · 2019-09-18T16:58:42Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

lcawl · 2019-09-18T19:03:22Z

x-pack/legacy/plugins/ml/public/overview/components/analytics_panel/analytics_panel.tsx

+            <Fragment>
+              <p>
+                Data frame analytics enable you to perform different analyses of your data and
+                annotate it with the results. As part of its output, data frame analytics appends


I think the second sentence should be changed to something like this, since otherwise it seems to imply that the source index is altered: "The analytics job stores the annotated data, as well as a copy of the source data, in a new index."

elasticmachine · 2019-09-18T23:07:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-23T04:54:32Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

pheyos · 2019-09-23T07:03:48Z

x-pack/legacy/plugins/ml/public/components/navigation_menu/main_tabs.tsx

@@ -66,7 +66,7 @@ interface TabData {
 }

 const TAB_DATA: Record<TabId, TabData> = {
-  // overview: { testSubject: 'mlTabOverview', pathId: 'overview' },
+  overview: { testSubject: 'mlTabOverview', pathId: 'overview' },


The test subject naming has slightly changed with #45793. Would be good to have mlMainTab overview here to match the other tab's attributes.

pheyos · 2019-09-23T07:07:02Z

x-pack/test/functional/services/machine_learning/navigation.ts

@@ -50,6 +50,10 @@ export function MachineLearningNavigationProvider({
      ]);
    },

+    async navigateToOverview() {
+      await this.navigateToArea('mlTabOverview', 'mlPageOverview');


If you follow my suggestion from above, also change the tab data subject to mlMainTab overview here.

pheyos

The analytics summary doesn't count the running job (Started: 0):

pheyos

Clicking the Explore action for an anomaly detection group, puts all jobs of that group into the job selection of the anomaly explorer. This could be a large number of jobs, maybe it's worth to think about just adding the group to the job selection?

pheyos

The refresh of the analytics table works as expected, but clicking the Refresh button in the anomaly detection table, removes the whole table for a second and displays it again. Is this intended?

lcawl

Text LGTM

elasticmachine · 2019-09-23T16:16:48Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

peteharverson

Tested and LGTM. Happy for remaining enhancements and fixes listed in #46357 to be done in follow-ups.

walterra

Latest changes LGTM

elasticmachine · 2019-09-23T17:46:13Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* show overview tab with main tabs * wip:add overview dir with initial components * convert and move AnalyticsTable and types to MlInMemoryTable components dir * create analytics table for overview * add stats bar to analytics panel * wip: adds anomaly detection table * adds actions column to anomaly detection table * add stats bar to anomaly detection panel * add max anomaly score to table * update score display. * add refresh button to panels * create scss files for styles * update functional nav tests * fix functional test failure * fix anomalyDetection for when there are no jobs * add translations and update jobList types * add maxAnomalyScore endpoint * fix types and update tab testSub * fix transforms use of inMemoryTable

alvarezmelissa87 added release_note:enhancement :ml v8.0.0 v7.5.0 labels Sep 17, 2019

alvarezmelissa87 requested review from a team as code owners September 17, 2019 04:53

alvarezmelissa87 force-pushed the ml-overview-tab branch from eeafa38 to b5f65d4 Compare September 17, 2019 19:24

alvarezmelissa87 changed the title ~~WIP [ML] Overview tab for ML~~ [ML] Overview tab for ML Sep 17, 2019

alvarezmelissa87 requested review from jgowdyelastic, walterra, peteharverson and darnautov September 17, 2019 19:25

walterra reviewed Sep 18, 2019

View reviewed changes

peteharverson reviewed Sep 18, 2019

View reviewed changes

darnautov reviewed Sep 18, 2019

View reviewed changes

peteharverson reviewed Sep 18, 2019

View reviewed changes

lcawl reviewed Sep 18, 2019

View reviewed changes

alvarezmelissa87 added 10 commits September 22, 2019 20:59

adds actions column to anomaly detection table

26afc88

add stats bar to anomaly detection panel

bc2800c

add max anomaly score to table

8ebf1fa

update score display.

14da9e6

add refresh button to panels

dfc1e5b

create scss files for styles

aa7b38e

update functional nav tests

1e899ea

fix functional test failure

e682a0c

fix anomalyDetection for when there are no jobs

59c9808

add translations and update jobList types

ce6b3fa

alvarezmelissa87 force-pushed the ml-overview-tab branch from 51974cd to ce6b3fa Compare September 23, 2019 03:12

pheyos reviewed Sep 23, 2019

View reviewed changes

alvarezmelissa87 added 3 commits September 23, 2019 07:59

add maxAnomalyScore endpoint

640f802

fix types and update tab testSub

a9dfe1c

fix transforms use of inMemoryTable

d981ba0

lcawl approved these changes Sep 23, 2019

View reviewed changes

alvarezmelissa87 mentioned this pull request Sep 23, 2019

[Meta] [ML] Overview page #46357

Open

9 tasks

peteharverson approved these changes Sep 23, 2019

View reviewed changes

walterra approved these changes Sep 23, 2019

View reviewed changes

alvarezmelissa87 merged commit dce008b into elastic:master Sep 23, 2019

alvarezmelissa87 mentioned this pull request Sep 23, 2019

[7.x] [ML] Overview tab for ML (#45864) #46372

Merged

alvarezmelissa87 deleted the ml-overview-tab branch September 23, 2019 19:40

-      const promises: any[] = [];
+const promises = groupsList
+  .filter(group => group.jobIds.length > 0)
+  .map((group) => {
+    return mlResultsService.getOverallBucketScores(
+      group.jobIds,
+,
+      group.earliest_timestamp,
+      group.latest_timestamp
+    );
+});

-  const jobs: any = {};
+  const jobs = jobsList
+  .filter(job => jobs[job.id] === undefined && job.earliestTimestampMs !== undefined)
+  .reduce((acc, job: Job) => {
+      const { earliestTimestampMs, latestResultsTimestampMs } = job;
+      acc[job.id] = {
+        id: job.id,
+        earliestTimestampMs,
+        latestResultsTimestampMs,
+      };
+      return acc;
+}, {});

[ML] Overview tab for ML #45864

[ML] Overview tab for ML #45864

Conversation

alvarezmelissa87 commented Sep 17, 2019 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Sep 17, 2019

elasticmachine commented Sep 17, 2019

💔 Build Failed

sophiec20 commented Sep 17, 2019 • edited Loading

elasticmachine commented Sep 17, 2019

💚 Build Succeeded

elasticmachine commented Sep 17, 2019

💔 Build Failed

elasticmachine commented Sep 17, 2019

💔 Build Failed

alvarezmelissa87 commented Sep 18, 2019

elasticmachine commented Sep 18, 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

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

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

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

Choose a reason for hiding this comment

alvarezmelissa87 Sep 18, 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peteharverson 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

Choose a reason for hiding this comment

darnautov commented Sep 18, 2019

elasticmachine commented Sep 18, 2019

💔 Build Failed

Choose a reason for hiding this comment

elasticmachine commented Sep 18, 2019

💚 Build Succeeded

elasticmachine commented Sep 23, 2019

💚 Build Succeeded

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pheyos left a comment

Choose a reason for hiding this comment

pheyos left a comment

Choose a reason for hiding this comment

pheyos left a comment

alvarezmelissa87 commented Sep 17, 2019 •

edited

Loading

sophiec20 commented Sep 17, 2019 •

edited

Loading

alvarezmelissa87 Sep 18, 2019 •

edited

Loading