[ML] Data frame analytics: Adds map view #81666

alvarezmelissa87 · 2020-10-26T19:30:11Z

Summary

Meta issue with screenshots: #75295
Please add any feedback to the meta issue. Thank you!

This PR is the first part of the map view addition. Items addressed in this PR are the checked items in the meta issue above. Known issues are also included in the meta issue.

Includes fixes for issues found in original draft PR #76952

Delete action in flyout is temporarily commented out and will be addressed in a followup.

Checklist

Delete any items that are not applicable to this PR.

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
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This was checked for cross-browser compatibility

elasticmachine · 2020-10-26T19:31:03Z

Pinging @elastic/ml-ui (:ml)

peteharverson · 2020-10-30T14:49:53Z

Opening the map off an outlier job created from a transform on the gallery data set, I always see this error:

Response from @alvarezmelissa87 --

@peteharverson I was not able to reproduce this so far. I can add it as a known issue to the meta issue and keep trying.

peteharverson · 2020-10-30T15:30:46Z

Are these arrows correct on here? I expected the gallery_clientip_outliers3 index pattern to come out of the outliers job node.

If I 'fetch related nodes' for the gallery_clientip_outliers2 node, it then rearranges to give the layout I expected:

x-pack/plugins/ml/common/constants/data_frame_analytics.ts

darnautov · 2020-10-29T08:54:19Z

x-pack/plugins/ml/common/util/analytics_utils.ts

+  const keys = Object.keys(analysis || {});
+
+  if (keys.length === 1) {
+    return keys[0] as DataFrameAnalysisConfigType;


Is it worth checking the actual key value instead of typecasting?

...ication/data_frame_analytics/pages/analytics_management/components/action_map/map_button.tsx

...lication/data_frame_analytics/pages/analytics_management/components/analytics_list/common.ts

darnautov · 2020-11-02T09:15:09Z

...ication/data_frame_analytics/pages/analytics_management/components/action_map/map_button.tsx

+}
+
+export const MapButton: FC<MapButtonProps> = ({ item }) => {
+  const toolTipContent = i18n.translate(


should be defined inside of the disabled condition

...ns/ml/public/application/data_frame_analytics/pages/job_map/components/cytoscape_options.tsx

darnautov · 2020-11-02T09:41:38Z

...ns/ml/public/application/data_frame_analytics/pages/job_map/components/cytoscape_options.tsx

+  boxSelectionEnabled: false,
+  maxZoom: 3,
+  minZoom: 0.2,
+  // @ts-ignore


quite some @ts-ignore here 🤔 . is it possible to avoid it? or at least specify the reason

darnautov · 2020-11-02T09:42:04Z

...lugins/ml/public/application/data_frame_analytics/pages/job_map/components/delete_button.tsx

+      // if (isDataFrameAnalyticsFailed(d.stats.state)) {
+      //   await ml.dataFrameAnalytics.stopDataFrameAnalytics(d.config.id, true);
+      // }


commented code

The delete functionality will be added in in a follow up as it requires some refactoring and I'm trying to keep this PR from getting too big.

Makes sense trying to keep the PR concise! But I believe we should not commit commented code. You can always stash it locally. This article provides an explanation of why, and there are plenty of others

darnautov · 2020-11-02T09:43:35Z

...lugins/ml/public/application/data_frame_analytics/pages/job_map/components/delete_button.tsx

+import React, { FC } from 'react';
+import { EuiButton } from '@elastic/eui';
+import { i18n } from '@kbn/i18n';
+import { ml } from '../../../../services/ml_api_service';


please use

const { services: { mlServices: { mlApiServices }, }, } = useMlKibana();

instead of a static import

darnautov · 2020-11-02T09:46:23Z

x-pack/plugins/ml/public/application/data_frame_analytics/pages/job_map/components/legend.tsx

+        </EuiFlexItem>
+        <EuiFlexItem grow={false}>
+          <EuiText size="xs" color="subdued">
+            {'inference model'}


should we provide a translation for this string?

As above, this should probably be 'trained model'. As @darnautov mentioned, do these legend labels need translations?

darnautov · 2020-11-02T09:49:23Z

...ns/ml/public/application/data_frame_analytics/pages/job_map/components/use_ref_dimensions.ts

+import { useRef } from 'react';
+import { useWindowSize } from 'react-use';
+
+export function useRefDimensions() {


useRefDimensions seems like a misleading name for this hook. And I'm not sure if you need it at all.
Have you tried to use EuiResizeObserver instead?

darnautov · 2020-11-02T09:50:06Z

x-pack/plugins/ml/public/application/data_frame_analytics/pages/job_map/job_map.tsx

+import { Cytoscape, Controls, JobMapLegend } from './components';
+import { ml } from '../../../services/ml_api_service';
+// TODO: don't use dep cache - switch to hook
+import { getToastNotifications } from '../../../util/dependency_cache';


Indeed, please use toastNotifications from the hook :)

...ns/ml/public/application/data_frame_analytics/pages/job_map/components/use_ref_dimensions.ts

x-pack/plugins/ml/public/application/data_frame_analytics/pages/job_map/job_map.tsx

darnautov · 2020-11-02T10:09:55Z

x-pack/plugins/ml/server/models/data_frame_analytics/analytics_manager.ts

+      while (complete === false) {
+        count++;
+        if (count >= 100) {
+          break;
+        }


For the fixed amount of iterations, a for loop is used

The problem is we don't know how many iteration we need since we're working backwards from the job/model id. We need to keep going until we've hit the source index/transform. The count is just to protect against infinite loops in case something changes or a new case is introduced that we haven't accounted for yet. The 100 limit is just an arbitrary value right now.

for loop also supports the break statement.

darnautov · 2020-11-02T10:14:49Z

x-pack/plugins/ml/server/models/data_frame_analytics/analytics_manager.ts

+        if (currentElem !== undefined && nextElem !== undefined) {
+          result.elements.push({
+            data: {
+              id: `${currentElem.data.id}~${nextElem.data.id}`,


why do you use ~ as a separator here?

Just a way to denote that the edge connects one node id to another node id. It doesn't have any particular significance.

darnautov · 2020-11-02T10:18:01Z

x-pack/plugins/ml/server/models/data_frame_analytics/types.ts

+export type MapElements = AnalyticsMapNodeElement | AnalyticsMapEdgeElement;
+export interface AnalyticsMapReturnType {
+  elements: MapElements[];
+  details: object; // transform, job, or index details


could you please specify it as a union type instead?

darnautov · 2020-11-02T10:19:12Z

x-pack/plugins/ml/server/routes/data_frame_analytics.ts

+    mlLicense.fullLicenseAPIGuard(async ({ client, request, response }) => {
+      try {
+        const { analyticsId } = request.params;
+        // @ts-ignore


is it possible to avoid this @ts-ignore

x-pack/plugins/ml/server/routes/data_frame_analytics.ts

darnautov · 2020-11-02T10:45:24Z

shall we wrap JSON with https://elastic.github.io/eui/#/editors-syntax/code#code-block ?

darnautov · 2020-11-02T10:50:00Z

Noticed this bug when I can't open the flyout again after I closed it

Response from @alvarezmelissa87 - This has been added to the meta issue.

darnautov · 2020-11-02T11:02:01Z

I've noticed "Refresh" indicator keeps spinning
Should we support a dark theme?
There are 2 scroll bars appear on the page

@darnautov - thanks for taking a look! 👍 The refresh indicator stuck in loading has been fixed. I've removed it as we don't need it for the job map. bc2b0f82f908f3958c99ca90d3e5da46f1356b9e

What browser are you using when you see the double scroll bars?

Yes, dark theme support is a good point - I'll add that as an item in the meta issue.

darnautov · 2020-11-02T11:08:04Z

Regarding the dark theme, check the APM Service Map for inspiration

darnautov · 2020-11-02T12:15:35Z

@alvarezmelissa87 Also worth checking why bundle size has been increased by 1.4MB. webpack-bundle-analyzer is a great tool for that

walterra · 2020-11-02T14:34:27Z

Great work, users can really benefit from getting an overview how all their resources rely to each other!

I noticed this layout problem when's there's lots of jobs based off the same source index:

Could this maybe improved by making the layout horizontal so that the labels don't overlap on the same line?

It also look like there's an issue with the graphs height, it overflows the panels bottom.

walterra · 2020-11-02T14:39:16Z

I'm seeing these warnings in dev console:

@walterra - thanks for taking a look! 😄 This is an item in the meta issue (near the bottom under Known Issues) and will be addressed in a follow up once the default icon is available.

walterra · 2020-11-02T14:47:52Z

Design Nits:

Suggest to tweak the padding of the legend to not stick to the bottom
Or make the legend background white, then the padding should be fine
The nodes of the graph have a white fill, the legend items don't have that

peteharverson · 2020-11-06T11:52:07Z

x-pack/plugins/ml/common/constants/data_frame_analytics.ts

+export const JOB_MAP_NODE_TYPES = {
+  ANALYTICS: 'analytics',
+  TRANSFORM: 'transform',
+  INDEX_PATTERN: 'index-pattern',


Is the map actually looking for Kibana index patterns, or Elasticsearch indexes? Wondering as I deleted the outliers-clientip-03 index pattern here, but the node still shows up on the map. Maybe the legend should be labelled as index rather than index-pattern ?

Oh good catch - it's the index - I've updated it to reflect that 👍 bc2b0f82f908f3958c99ca90d3e5da46f1356b9e

peteharverson · 2020-11-12T16:09:15Z

x-pack/plugins/ml/common/constants/data_frame_analytics.ts

+  ANALYTICS: 'analytics',
+  TRANSFORM: 'transform',
+  INDEX: 'index',
+  INFERENCE_MODEL: 'inferenceModel',


'Trained model' is the term we are generally using now, so should probably rename these to be consistent.

peteharverson · 2020-11-12T16:10:33Z

x-pack/plugins/ml/public/application/data_frame_analytics/pages/job_map/components/_legend.scss

+  display: 'inline-block';
+}
+
+.mlJobMapLegend__inferenceModel {


Might need renaming to trainedModel if you switch 'inference model' node type to 'trained model' - see above.

peteharverson · 2020-11-12T16:11:40Z

x-pack/plugins/ml/public/application/data_frame_analytics/pages/job_map/components/legend.tsx

+        </EuiFlexItem>
+        <EuiFlexItem grow={false}>
+          <EuiText size="xs" color="subdued">
+            {'inference model'}


As above, this should probably be 'trained model'. As @darnautov mentioned, do these legend labels need translations?

peteharverson

Tested latest edits and LGTM.

Happy for the layout issue when the map is a single row of nodes to be done in a follow-up. For now, it is easy enough for the user to drag the selection into the middle of the view.

kibanamachine · 2020-11-12T17:19:13Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 39728e2

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	1242	1558	+316

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`ml`	4.5MB	5.2MB	⚠️ +732.2KB

Distributable file count

id	before	after	diff
`default`	42793	42800	+7

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`ml`	66.0KB	66.8KB	+885.0B

History

💚 Build #87257 succeeded 853be2f7915cc08c1e01db13065f4ae1334b948e
💚 Build #87157 succeeded 05273b8b2d2a2e51bc5ec0dec9c2351b6e8524c7
💚 Build #87152 succeeded bc2b0f82f908f3958c99ca90d3e5da46f1356b9e
💔 Build #86533 failed bf4250c869540c2a11e8d6563b363b838ce271d6
💔 Build #86498 failed 2b98ae971388bcaeb53ff7eda0d0d48c25869d3f

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

darnautov

LGTM

* add analytics map endpoint and server model * add map action to job and models list * wip:fetch models for jobs. Use url generator * get models when extending node. deduplicate elements * add job type icons. disable map action if job not finished. * move shared const to common dir * persist map tab. handle indexPattern from visualizer * use url generator in models list * temporarily disable delete action in flyout * update legend style. make map horizontal * update dfa model to use spaces changes * format creation time * update from indexPattern to index.remove refresh button * handle index patterns with wildcard

* master: [Security Solution][Detections] Adds framework for replacing API schemas (elastic#82462) [Enterprise Search] Share Loading component (elastic#83246) Consolidates Jest configuration files and scripts (elastic#82671) APM header changes (elastic#82870) [Security Solutions] Adds a default for indicator match custom query of *:* (elastic#81727) [Security Solution] Note 10k object paging limit on Endpoint list (elastic#82889) [packerCache] fix gulp usage, don't archive node_modules (elastic#83327) Upgrade Node.js to version 12 (elastic#61587) [Actions] Removing placeholders and updating validation messages on connector forms (elastic#82734) [Fleet] Rename ingest_manager_api_integration tests fleet_api_integration (elastic#83011) [DOCS] Updates Discover docs (elastic#82773) [ML] Data frame analytics: Adds map view (elastic#81666) enables actions scoped within the stack to register at Basic license (elastic#82931)

alvarezmelissa87 requested review from a team as code owners October 26, 2020 19:30

alvarezmelissa87 self-assigned this Oct 26, 2020

alvarezmelissa87 mentioned this pull request Oct 26, 2020

[ML] WIP DF Analytics Map #76952

Closed

7 tasks

alvarezmelissa87 added :ml Feature:Data Frame Analytics ML data frame analytics features release_note:enhancement v7.11.0 labels Oct 26, 2020

alvarezmelissa87 force-pushed the ml-dfanalytics-map-view branch 2 times, most recently from 25ed802 to 5c69dc7 Compare October 29, 2020 16:28

alvarezmelissa87 changed the title ~~WIP: [ML] DF Analytics Map view~~ [ML] DF Analytics Map view: Part one Oct 29, 2020

alvarezmelissa87 requested review from peteharverson, walterra and jgowdyelastic October 29, 2020 17:53

peteharverson mentioned this pull request Oct 30, 2020

[Meta] [ML] DF Analytics map #75295

Closed

48 tasks

darnautov reviewed Nov 2, 2020

View reviewed changes

alvarezmelissa87 force-pushed the ml-dfanalytics-map-view branch 2 times, most recently from 1bafe2e to 725ef83 Compare November 4, 2020 20:13

peteharverson reviewed Nov 6, 2020

View reviewed changes

alvarezmelissa87 force-pushed the ml-dfanalytics-map-view branch 2 times, most recently from bc2b0f8 to 05273b8 Compare November 10, 2020 17:31

alvarezmelissa87 added 14 commits November 12, 2020 08:16

add analytics map endpoint and server model

c8106fd

add map action to job and models list

2533c79

wip:fetch models for jobs. Use url generator

616f02c

get models when extending node. deduplicate elements

3e3099f

add job type icons. disable map action if job not finished.

380dcd7

move shared const to common dir

ded6a59

persist map tab. handle indexPattern from visualizer

1d7341f

use url generator in models list

05b82ec

temporarily disable delete action in flyout

701886a

update legend style. make map horizontal

3dbf6a4

update dfa model to use spaces changes

7475434

format creation time

fd17d54

update from indexPattern to index.remove refresh button

d763ec1

handle index patterns with wildcard

39728e2

alvarezmelissa87 force-pushed the ml-dfanalytics-map-view branch from 853be2f to 39728e2 Compare November 12, 2020 15:31

peteharverson reviewed Nov 12, 2020

View reviewed changes

alvarezmelissa87 changed the title ~~[ML] DF Analytics Map view: Part one~~ [ML] Data frame analytics: Adds map view Nov 12, 2020

peteharverson approved these changes Nov 12, 2020

View reviewed changes

darnautov approved these changes Nov 12, 2020

View reviewed changes

alvarezmelissa87 merged commit 6519b83 into elastic:master Nov 12, 2020

alvarezmelissa87 mentioned this pull request Nov 12, 2020

[7.x] [ML] Data frame analytics: Adds map view (#81666) #83319

Merged

alvarezmelissa87 deleted the ml-dfanalytics-map-view branch November 12, 2020 20:01

This was referenced Nov 18, 2020

[ML] Data frame analytics: Adds functionality to map view #83710

Merged

[ML] Data frame analytics: Adds job deletion and creation to map view #84299

Merged

[ML] Data frame analytics: Adds map view #81666

[ML] Data frame analytics: Adds map view #81666

Conversation

alvarezmelissa87 commented Oct 26, 2020 • edited by peteharverson Loading

Summary

Checklist

elasticmachine commented Oct 26, 2020

peteharverson commented Oct 30, 2020 • edited by alvarezmelissa87 Loading

peteharverson commented Oct 30, 2020

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 Nov 2, 2020 • 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

alvarezmelissa87 Nov 2, 2020 • 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

darnautov commented Nov 2, 2020

darnautov commented Nov 2, 2020 • edited by alvarezmelissa87 Loading

darnautov commented Nov 2, 2020 • edited by alvarezmelissa87 Loading

darnautov commented Nov 2, 2020

darnautov commented Nov 2, 2020

walterra commented Nov 2, 2020 • edited Loading

walterra commented Nov 2, 2020 • edited by alvarezmelissa87 Loading

walterra commented Nov 2, 2020

Choose a reason for hiding this comment

alvarezmelissa87 Nov 6, 2020 • 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

peteharverson left a comment

Choose a reason for hiding this comment

kibanamachine commented Nov 12, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Distributable file count

Page load bundle

History

darnautov left a comment

Choose a reason for hiding this comment

alvarezmelissa87 commented Oct 26, 2020 •

edited by peteharverson

Loading

peteharverson commented Oct 30, 2020 •

edited by alvarezmelissa87

Loading

alvarezmelissa87 Nov 2, 2020 •

edited

Loading

alvarezmelissa87 Nov 2, 2020 •

edited

Loading

darnautov commented Nov 2, 2020 •

edited by alvarezmelissa87

Loading

darnautov commented Nov 2, 2020 •

edited by alvarezmelissa87

Loading

walterra commented Nov 2, 2020 •

edited

Loading

walterra commented Nov 2, 2020 •

edited by alvarezmelissa87

Loading

alvarezmelissa87 Nov 6, 2020 •

edited

Loading