[Maps][ML] Integration part 1: Create anomalies layer in maps #122862

alvarezmelissa87 · 2022-01-12T18:07:38Z

Summary

Related draft PR: #113857

Extends and updates #90497
Incorporates latest maps changes (#112465, #112333) into initial draft PR (#111228)

Nice to have:

Anytime a job is "mappable", should provide a hyperlink in the ML-app, something like "Explore in maps".
hyperlink for anomaly explorer page ([Maps][ML] Integration part 1: Create anomalies layer in maps #122862 (comment))
Would it be useful to add a switch to turn on/off "time-awareness".
get the partitioning field names and values in the tooltip too

Screenshots

Anomalies layer card in maps:

Job selection for jobs that contain supported geo data:

Layers available:

Tooltip:

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)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

thomasneirynck

This looks really great from Maps-perspective.

It will be really useful to users to easily map results from spatial anomaly detection.

Below some UX suggestions mostly, but nothing blocking AFAIC.

thomasneirynck · 2022-01-18T21:00:41Z

x-pack/plugins/ml/public/maps/anomaly_layer_wizard_factory.tsx

+          style: {
+            type: 'VECTOR',
+            properties: {
+              fillColor: {


consider setting this same property for lineColor . That way, when users select "connected", they see the same symbology:

properties: { fillColor: { type: STYLE_TYPE.DYNAMIC, options: { customColorRamp: SEVERITY_COLOR_RAMP, field: { name: 'record_score', origin: FIELD_ORIGIN.SOURCE, }, useCustomColorRamp: true, }, }, lineColor: { type: STYLE_TYPE.DYNAMIC, options: { customColorRamp: SEVERITY_COLOR_RAMP, field: { name: 'record_score', origin: FIELD_ORIGIN.SOURCE, }, useCustomColorRamp: true, }, }, } as unknown as VectorStylePropertiesDescriptor,

Added in e121d67

thomasneirynck · 2022-01-18T21:03:31Z

x-pack/plugins/ml/public/maps/anomaly_layer_wizard.tsx

+  }),
+  icon: 'outlierDetectionJob',
+  getIsDisabled: () => {
+    // return false by default


Consider having this check for enterprise license. For an example, see the track layer wizard.

kibana/x-pack/plugins/maps/public/classes/sources/es_geo_line_source/layer_wizard.tsx

Lines 25 to 29 in 0675a6a

disabledReason: REQUIRES_GOLD_LICENSE_MSG,

icon: TracksLayerIcon,

getIsDisabled: () => {

return !getIsGoldPlus();

},

This is checked in the plugin setup and passed through to the layer wizard https://github.com/elastic/kibana/pull/122862/files#diff-9658f57fcb39c79f75a675c515053ccb46c515a7e9f9eac7ecb200d21c3af09aR172

This default is just a placeholder.

ah makes sense, thx! (I missed that)

fwiw: you could still register the card, but in "disabled" state. That way the card is still visible (but in disabled grayed-out state) . The advantage is that this feature would still be advertized for non-enterprise users.

thomasneirynck · 2022-01-18T21:06:34Z

x-pack/plugins/ml/public/maps/anomaly_job_selector.tsx

+    return (
+      <EuiFormRow
+        label={i18n.translate('xpack.ml.maps.jobIdLabel', {
+          defaultMessage: 'JobId',


nit: consider Job Id.

extra nit - we tend to use Job ID in ML

Updated in e121d67

thomasneirynck · 2022-01-18T21:08:03Z

x-pack/plugins/ml/public/maps/anomaly_job_selector.tsx

+          selectedOptions={
+            this.state.jobId ? [{ value: this.state.jobId, label: this.state.jobId }] : []
+          }
+        />


nit: consider adding some explanatory text.

e.g. a little help box about what this is.

e.g.: something along the lines of:
Select anomaly detection job using spatial outlier detection. These use the lon_lat_detector function. LinkToDoc, LinkToMLApp.

Added to follow-ups as I'll need to craft a good explanation message. 👍

thomasneirynck · 2022-01-18T21:11:26Z

x-pack/plugins/ml/public/maps/anomaly_layer_wizard.tsx

+import type { LayerWizard } from '../../../maps/public';
+
+export const anomalyLayerWizard: Partial<LayerWizard> = {
+  categories: [LAYER_WIZARD_CATEGORY.SOLUTIONS],


I'd also add the Elasticsearch-category, so it shows up in that facet as well

categories: [LAYER_WIZARD_CATEGORY.SOLUTIONS, LAYER_WIZARD_CATEGORY.ELASTICSEARCH],

Added in e121d67

thomasneirynck · 2022-01-18T21:17:21Z

x-pack/plugins/ml/public/maps/update_anomaly_source_editor.tsx

+            typicalActual={this.props.typicalActual}
+          />
+        </EuiPanel>
+        <EuiSpacer size="s" />


Would it be useful to add a switch here to turn on/off "time-awareness".

E.g. something along the lines of:

E.g. add another property applyGlobalTime to the source-descriptor.

export interface AnomalySourceDescriptor extends AbstractSourceDescriptor { jobId: string; typicalActual: MlAnomalyLayers; applyGlobalTime: boolean; }

The switch controls the value of applyGlobalTime.

Then implement AnomalySource#isTimeAware as:

async isTimeAware(): Promise<boolean> { return this._descriptor.applyGlobalTime; }

This way you can turn on/off whether users will see all the data (regardless of time-selection), or only a cut of the data.

Sounds like a good one to discuss for a follow-up 👍

thomasneirynck · 2022-01-18T21:26:25Z

x-pack/plugins/ml/public/maps/anomaly_source.tsx

+          defaultMessage: 'Job Id',
+        }),
+        value: this._descriptor.jobId,
+      },


Consider adding second object with hyperlink to the anomaly-explorer page in ML based on the job-Id.

e.g. this is some sample code.

kibana/x-pack/plugins/maps/public/classes/sources/ems_file_source/ems_file_source.tsx

Lines 156 to 162 in 6a3d194

{

label: i18n.translate('xpack.maps.source.emsFile.layerLabel', {

defaultMessage: `Layer`,

}),

value: this._descriptor.id,

link: emsLink,

},

So for ML, it'd be something like:

return [ { label: i18n.translate('xpack.ml.maps.anomalySourcePropLabel', { defaultMessage: 'Job Id', }), value: this._descriptor.jobId, link: `http://localhost:5601/hen/app/ml/explorer?_g={jobIds:[%22${this._jobId}%22]` // todo: this link should be constructed with `core.http.basePath.prepend()` and utilities there are on ML-side to derive this URL },

Added for a follow up 👍

elasticmachine · 2022-01-18T22:06:55Z

Pinging @elastic/kibana-gis (Team:Geo)

elasticmachine · 2022-01-18T22:06:55Z

Pinging @elastic/ml-ui (:ml)

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

x-pack/plugins/ml/public/application/services/ml_api_service/jobs.ts

x-pack/plugins/ml/public/maps/anomaly_job_selector.tsx

x-pack/plugins/ml/public/maps/anomaly_layer_wizard_factory.tsx

darnautov · 2022-01-19T09:30:43Z

x-pack/plugins/ml/public/maps/anomaly_layer_wizard_factory.tsx

+    private getStartServices: StartServicesAccessor<MlStartDependencies, MlPluginStart>,
+    private canGetJobs: boolean
+  ) {
+    this.canGetJobs = canGetJobs;


this line seems redundant

Suggested change

this.canGetJobs = canGetJobs;

x-pack/plugins/ml/public/maps/util.ts

darnautov · 2022-01-19T09:52:03Z

x-pack/plugins/ml/public/plugin.ts

@@ -113,6 +114,7 @@ export class MlPlugin implements Plugin<MlPluginSetup, MlPluginStart> {
            licenseManagement: pluginsSetup.licenseManagement,
            home: pluginsSetup.home,
            embeddable: { ...pluginsSetup.embeddable, ...pluginsStart.embeddable },
+            // @ts-ignore


can we remove this ts-ignore?

darnautov · 2022-01-19T09:52:52Z

x-pack/plugins/ml/public/plugin.ts

+
+      if (pluginsSetup.maps) {
+        // Pass capabilites.ml.canGetJobs as minimum permission to show anomalies card in maps layers
+        const canGetJobs = capabilities.ml?.canGetJobs === true || false;


nit:

Suggested change

const canGetJobs = capabilities.ml?.canGetJobs === true || false;

const canGetJobs = !!capabilities.ml?.canGetJobs;

x-pack/plugins/ml/server/models/job_service/jobs.ts

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

peteharverson · 2022-01-19T17:28:37Z

x-pack/plugins/ml/public/maps/layer_selector.tsx

+          onChange={this.onSelect}
+          options={[
+            { value: 'typical', label: 'typical' },
+            { value: 'actual', label: 'actual' },


I wonder if it makes more sense for actual to be the top, default option?

Good call - updated in c88762c

peteharverson · 2022-01-19T17:35:15Z

x-pack/plugins/ml/public/maps/anomaly_source_field.ts

+import { ITooltipProperty } from '../../../maps/public';
+import { Filter } from '../../../../../src/plugins/data/public';
+
+export const ANOMALY_SOURCE_FIELDS: Record<string, Record<string, string>> = {


Are these strings used for the tooltip text? If so, I think they need to be internationalized.

Updated in c88762c

peteharverson · 2022-01-19T17:36:40Z

x-pack/plugins/ml/public/maps/anomaly_source_field.ts

+  timestamp: { label: 'Timestamp', type: 'string' },
+  fieldName: { label: 'Field label', type: 'string' },
+  functionDescription: { label: 'Function description', type: 'string' },
+  actual: { label: 'Actual', type: 'string' },


We should use the same labels for these fields as we use in the anomalies table:

using Time, Field name and Function

Can we limit the number of decimal places used for the actual and typical positions in the tooltip?

Updated in c88762c

peteharverson · 2022-01-19T17:40:58Z

x-pack/plugins/ml/public/maps/anomaly_source_field.ts

+import { ITooltipProperty } from '../../../maps/public';
+import { Filter } from '../../../../../src/plugins/data/public';
+
+export const ANOMALY_SOURCE_FIELDS: Record<string, Record<string, string>> = {


Could we get the partitioning field names and values in the tooltip too?

Added to follow up 👍

peteharverson · 2022-01-19T17:52:19Z

x-pack/plugins/ml/public/maps/anomaly_layer_wizard.tsx

+export const anomalyLayerWizard: Partial<LayerWizard> = {
+  categories: [LAYER_WIZARD_CATEGORY.SOLUTIONS],
+  description: i18n.translate('xpack.ml.maps.anomalyLayerDescription', {
+    defaultMessage: 'Create anomalies layers',


This message needs editing I think - something like Display anomalies from a machine learning job

Updated in c88762c

peteharverson · 2022-01-19T17:52:58Z

x-pack/plugins/ml/public/maps/anomaly_layer_wizard.tsx

+  }),
+  disabledReason: i18n.translate('xpack.ml.maps.anomalyLayerUnavailableMessage', {
+    defaultMessage:
+      'Whatever reason the user cannot see ML card (likely because no enterprise license or no ML privileges)',


This message needs some work

Updated in c88762c

peteharverson · 2022-01-19T18:00:28Z

x-pack/plugins/ml/public/maps/layer_selector.tsx

+    const typicalActual: MlAnomalyLayers = selectedOptions[0].value! as
+      | 'typical'
+      | 'actual'
+      | 'connected';


I assume this shows a line from the typical position to the actual? If so, I'm not sure connected is the most intuitive label. Can't think of an alternative though!

It is - I also couldn't think of anything more intuitive but am super open to suggestions 🙏

nreese

gis code owners changes LGTM
code review

thomasneirynck

🚀 This is a great integration. It makes the geo-anomaly-detection capabilities of ML more visible in Maps, and it allows "mashing-up" of these anomaly-layers with other contextual data. Also adds embeddability of maps with anomaly-layers, something which was not possible before It also sets the stage for better cross-linking of Maps/ML (e.g. open in Maps-button from the ML-UX).

darnautov

LGTM

kibana-ci · 2022-01-20T17:53:03Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	1596	1607	+11

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`maps`	213	214	+1
`ml`	289	290	+1
total			+2

Async chunks

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

id	before	after	diff
`ml`	3.5MB	3.5MB	+17.8KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`maps`	28	27	-1

Page load bundle

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

id	before	after	diff
`ml`	38.5KB	38.9KB	+341.0B

Unknown metric groups

API count

id	before	after	diff
`maps`	214	215	+1
`ml`	293	294	+1
total			+2

async chunk count

id	before	after	diff
`ml`	25	27	+2

ESLint disabled line counts

id	before	after	diff
`ml`	95	98	+3

References to deprecated APIs

id	before	after	diff
`ml`	67	73	+6

Total ESLint disabled count

id	before	after	diff
`ml`	98	101	+3

History

💛 Build #18585 was flaky c88762c
💚 Build #18516 succeeded e121d67
💚 Build #18204 succeeded a52c840
💚 Build #18163 succeeded b4e42b0
💛 Build #18047 was flaky 2273e4d
💔 Build #17380 failed 3c5fbc9

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

cc @alvarezmelissa87

alvarezmelissa87 · 2022-01-20T18:08:53Z

Follow ups tracked in meta issue #123492

alvarezmelissa87 requested a review from thomasneirynck January 12, 2022 18:07

alvarezmelissa87 self-assigned this Jan 12, 2022

alvarezmelissa87 changed the title ~~wip: create anomalies layer in maps~~ [Maps][ML] Integration part 1: Create anomalies layer in maps Jan 12, 2022

alvarezmelissa87 mentioned this pull request Jan 12, 2022

[Maps][ML] Integration: part 1 POC extended #113857

Closed

9 tasks

This was referenced Jan 12, 2022

[Maps] alvarezmelissa87/kibana#2

Draft

[Maps] Add ordering to layer wizard registration API #122999

Merged

alvarezmelissa87 force-pushed the ml-maps-part-1 branch 2 times, most recently from 3c5fbc9 to 2273e4d Compare January 18, 2022 15:53

thomasneirynck reviewed Jan 18, 2022

View reviewed changes

alvarezmelissa87 marked this pull request as ready for review January 18, 2022 22:00

alvarezmelissa87 requested a review from a team as a code owner January 18, 2022 22:00

alvarezmelissa87 requested review from peteharverson and jgowdyelastic January 18, 2022 22:01

alvarezmelissa87 added v8.1.0 release_note:enhancement :ml [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation labels Jan 18, 2022

darnautov reviewed Jan 19, 2022

View reviewed changes

peteharverson reviewed Jan 19, 2022

View reviewed changes

alvarezmelissa87 requested a review from a team as a code owner January 19, 2022 19:37

nreese approved these changes Jan 19, 2022

View reviewed changes

thomasneirynck approved these changes Jan 20, 2022

View reviewed changes

alvarezmelissa87 added 6 commits January 20, 2022 09:31

wip: create anomalies layer in maps

8f98895

ensure anomaly search respects time filter

b45b690

add custom color ramp and connected layer

fc23831

update ml api service types

814cbcb

add tooltip

d956df2

update naming and types

c22e6dd

alvarezmelissa87 added 2 commits January 20, 2022 09:31

update warning messages. make actual layer default

6425009

remove any

f66f8a6

alvarezmelissa87 force-pushed the ml-maps-part-1 branch from 7cd7abb to f66f8a6 Compare January 20, 2022 16:32

darnautov approved these changes Jan 20, 2022

View reviewed changes

alvarezmelissa87 merged commit 9368aa4 into elastic:main Jan 20, 2022

kibanamachine added the backport:skip This commit does not require backporting label Jan 20, 2022

alvarezmelissa87 mentioned this pull request Jan 20, 2022

[Maps][ML] Integration meta issue #123492

Closed

8 tasks

peteharverson added release_note:feature Makes this part of the condensed release notes and removed release_note:enhancement labels Feb 2, 2022

This was referenced Feb 24, 2022

[DOCS] Add example of anomalies layer in Maps elastic/stack-docs#2067

Merged

[DOCS] Add anomaly layer in Maps #126397

Merged

	disabledReason: REQUIRES_GOLD_LICENSE_MSG,
	icon: TracksLayerIcon,
	getIsDisabled: () => {
	return !getIsGoldPlus();
	},

	{
	label: i18n.translate('xpack.maps.source.emsFile.layerLabel', {
	defaultMessage: `Layer`,
	}),
	value: this._descriptor.id,
	link: emsLink,
	},

	const canGetJobs = capabilities.ml?.canGetJobs === true \|\| false;
	const canGetJobs = !!capabilities.ml?.canGetJobs;

[Maps][ML] Integration part 1: Create anomalies layer in maps #122862

[Maps][ML] Integration part 1: Create anomalies layer in maps #122862

Conversation

alvarezmelissa87 commented Jan 12, 2022 • edited Loading

Summary

Screenshots

Checklist

thomasneirynck 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

elasticmachine commented Jan 18, 2022

elasticmachine commented Jan 18, 2022

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

nreese left a comment

Choose a reason for hiding this comment

thomasneirynck left a comment

Choose a reason for hiding this comment

darnautov left a comment

Choose a reason for hiding this comment

kibana-ci commented Jan 20, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

async chunk count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

alvarezmelissa87 commented Jan 20, 2022

alvarezmelissa87 commented Jan 12, 2022 •

edited

Loading