[ML] Add option for anomaly charts for metric detector should plot min, mean or max as appropriate #81662

qn895 · 2020-10-26T18:54:45Z

Summary

This PR addresses #65078 by:

Plot the function associated with the record with the highest anomaly high score in the Single Metric Viewer and Anomaly Explorer by default.
Add additional select options so users can choose what function want to view by (e.g. avg, min, or max) where the anomaly markers will only show up if they correspond to the right view.

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
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

elasticmachine · 2020-10-27T13:26:09Z

Pinging @elastic/ml-ui (:ml)

x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js

peteharverson · 2020-10-28T17:58:36Z

As discussed, if feasible, I think it makes sense to filter the table and mini swim lane anomalies for the detector function (min / avg / max) as well as the main chart. That way the view shows information for the single series (detector, partitioning fields, function) being viewed.

x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js

peteharverson · 2020-11-03T12:30:27Z

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

+          mustCriteria.push({
+            term: {
+              function_description:
+                actualPlotFunctionIfMetric === 'avg' ? 'mean' : actualPlotFunctionIfMetric,


Can you use the enums out of common/constants/aggregation_types here?

x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js

peteharverson · 2020-11-03T12:34:29Z

x-pack/plugins/ml/server/models/results_service/results_service.ts

+    if (actualPlotFunction !== undefined) {
+      boolCriteria.push({
+        term: {
+          function_description: actualPlotFunction === 'avg' ? 'mean' : actualPlotFunction,


Can you use the enums out of common/constants/aggregation_types here?

peteharverson · 2020-11-03T12:37:02Z

x-pack/plugins/ml/server/models/results_service/results_service.ts

@@ -57,7 +57,8 @@ export function resultsServiceProvider(client: IScopedClusterClient) {
    dateFormatTz: string,
    maxRecords: number = ANOMALIES_TABLE_DEFAULT_QUERY_SIZE,
    maxExamples: number = DEFAULT_MAX_EXAMPLES,
-    influencersFilterQuery: any
+    influencersFilterQuery?: any,
+    actualPlotFunction?: string | undefined


Maybe this should be renamed to functionDescription to match the name of the property in the records?

Updated here cb2429e

peteharverson · 2020-11-03T12:38:14Z

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

@@ -37,6 +37,7 @@ function getAnomaliesTableData(client: IScopedClusterClient, payload: any) {
    maxRecords,
    maxExamples,
    influencersFilterQuery,
+    actualPlotFunction,


As before, maybe this should be renamed to functionDescription to match the name of the property in the records?

Updated here cb2429e

peteharverson · 2020-11-03T12:38:30Z

x-pack/plugins/ml/server/routes/schemas/results_service_schema.ts

@@ -26,6 +26,7 @@ export const anomaliesTableDataSchema = schema.object({
  maxRecords: schema.number(),
  maxExamples: schema.maybe(schema.number()),
  influencersFilterQuery: schema.maybe(schema.any()),
+  actualPlotFunction: schema.maybe(schema.nullable(schema.string())),


As above, maybe this should be renamed to functionDescription to match the name of the property in the records?

Updated here cb2429e

peteharverson · 2020-11-03T12:38:49Z

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

-    influencersFilterQuery: any
+    maxExamples?: number,
+    influencersFilterQuery?: any,
+    actualPlotFunction?: string | undefined


Maybe this should be renamed to functionDescription to match the name of the property in the records?

Updated here cb2429e

peteharverson · 2020-11-03T12:50:03Z

Coming into the view from the jobs list, I see the function dropdown disappear after selecting a partitioning field:

peteharverson · 2020-11-03T13:12:49Z

Noticed some odd behavior in the chart when using the metric function, which is happening here and also on master. For these anomalies which are in min, the chart data seems to be plotted always with avg. Here the anomaly should be plotted with a value of 53.6:

  "analysis_config": {
    "bucket_span": "15m",
    "detectors": [
      {
        "detector_description": "metric(responsetime) partitionfield=airline",
        "function": "metric",
        "field_name": "responsetime",
        "partition_field_name": "airline",
        "detector_index": 0
      }
    ],
    "influencers": [
      "airline"
    ]
  },
  "analysis_limits": {
    "model_memory_limit": "13mb",
    "categorization_examples_limit": 4
  },
  "data_description": {
    "time_field": "@timestamp",
    "time_format": "epoch_ms"
  },
  "model_plot_config": {
    "enabled": true,
    "annotations_enabled": true
  },

qn895 · 2020-11-03T20:20:44Z

Coming into the view from the jobs list, I see the function dropdown disappear after selecting a partitioning field

@peteharverson I have added a fix for this issue. The dropdown should now work with or without the partitioning and should be updated.

Noticed some odd behavior in the chart when using the metric function, which is happening here and also on master. For these anomalies which are in min, the chart data seems to be plotted always with avg.

I also added a fix for this one in ea8d71f. Now, when model plot is enabled, it will 1) get the anomaly records based on the selected function description, and 2) plot the chart point with the actual data point.

x-pack/plugins/ml/public/application/timeseriesexplorer/get_function_description.ts

darnautov · 2020-11-04T09:18:40Z

.../application/timeseriesexplorer/components/plot_function_controls/plot_function_controls.tsx

+}) => {
+  if (functionDescription === undefined) return null;
+  return (
+    <EuiFlexItem style={{ textAlign: 'right' }} grow={false}>


to avoid inline styles, you can use EuiText with textAlign prop

I realized this style wasn't necessary so I removed it here a7e2482

.../application/timeseriesexplorer/components/plot_function_controls/plot_function_controls.tsx

peteharverson · 2020-11-04T14:09:51Z

Found what looks like an issue with setting the bounds on the focus chart:

Config is:

 "analysis_config": {
    "bucket_span": "15m",
    "detectors": [
      {
        "detector_description": "metric(responsetime) partitionfield=airline",
        "function": "metric",
        "field_name": "responsetime",
        "partition_field_name": "airline",
        "detector_index": 0
      }
    ],
    "influencers": [
      "airline"
    ]
  },

Same job - the bounds on the main chart aren't set correctly, and also the critical anomaly isn't marked in the swim lane.

Same config, but with model plot enabled - again the critical anomalies don't show up in the swim lane:

qn895 · 2020-11-04T17:15:59Z

@peteharverson I have fixed the two issues in my latest changes. The bounds should be now correct, and the mini swimlanes should should up correctly for when function = mean as well.

walterra

Added some questions, one is about risk of render looks or race conditions.

walterra · 2020-11-05T16:49:43Z

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

-    influencersFilterQuery: any
+    maxExamples?: number,
+    influencersFilterQuery?: any,
+    functionDescription?: string | undefined


You should be able to remove | undefined because of the ?.

Updated here 64ecedb

walterra · 2020-11-05T16:55:14Z

.../application/timeseriesexplorer/components/plot_function_controls/plot_function_controls.tsx

+import { EuiFlexItem, EuiFormRow, EuiSelect } from '@elastic/eui';
+import { i18n } from '@kbn/i18n';
+
+const plotByFunctionOptions = [


Is it necessary to translate these since they relate to Elasticsearch aggregations?

Since these are not strict terms that are excluded in the translation guide, I'll keep these translations here.

walterra · 2020-11-05T17:04:34Z

x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js

-    if (currentSelectedJob === undefined) {
-      return;
-    }


Can you explain why this check is no longer necessary?

Thanks for catching that. This was removed by accident. I have revert the change in 64ecedb

walterra · 2020-11-05T17:07:03Z

x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js

+      previousProps.selectedDetectorIndex !== this.props.selectedDetectorIndex ||
+      !isEqual(previousProps.selectedEntities, this.props.selectedEntities)
+    ) {
+      this.getFunctionDescription();


Could we run into problems with a race condition since this is async function?
(Just trying to understand the implications here, since previously only relying on a change previousProps did make sure we check against from changes coming from outside the components - wondering if with including perviousState and then again triggering async state functions if we might run into render loops)

peteharverson

Tested latest edits and LGTM.
Just left some minor comments on the code.

peteharverson · 2020-11-05T17:47:53Z

x-pack/plugins/ml/public/application/services/results_service/result_service_rx.ts

+      earliestMs: number | null,
+      latestMs: number | null,
+      maxResults: number | undefined,
+      functionDescription?: string | undefined


As noted above, you should be able to remove | undefined because of the ?.

Updated here 64ecedb

peteharverson · 2020-11-05T17:55:46Z

x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_config_builder.js

@@ -48,6 +49,10 @@ export function buildConfig(record) {
  // define the metric series to be plotted.
  config.entityFields = getEntityFieldList(record);

+  if (record.function === 'metric') {


Use ML_JOB_AGGREGATION.METRIC from common/constants/aggregation_types

Updated here 64ecedb

peteharverson · 2020-11-05T17:56:39Z

...l/public/application/timeseriesexplorer/timeseriesexplorer_utils/timeseriesexplorer_utils.js

@@ -142,6 +143,8 @@ export function processDataForFocusAnomalies(
    // Look for a chart point with the same time as the record.
    // If none found, find closest time in chartData set.
    const recordTime = record[TIME_FIELD_NAME];
+    if (record.function === 'metric' && record.function_description !== functionDescription) return;


Use ML_JOB_AGGREGATION.METRIC from common/constants/aggregation_types

Updated here 64ecedb

peteharverson · 2020-11-05T17:56:48Z

...l/public/application/timeseriesexplorer/timeseriesexplorer_utils/timeseriesexplorer_utils.js

@@ -160,6 +163,10 @@ export function processDataForFocusAnomalies(
            chartPoint.value = record.actual;
          }

+          if (record.function === 'metric') {


Use ML_JOB_AGGREGATION.METRIC from common/constants/aggregation_types

Updated here 64ecedb

peteharverson · 2020-11-05T17:56:59Z

x-pack/plugins/ml/public/application/timeseriesexplorer/get_function_description.ts

+) => {
+  // if the detector's function is metric, fetch the highest scoring anomaly record
+  // and set to plot the function_description (avg/min/max) of that record by default
+  if (selectedJob?.analysis_config?.detectors[selectedDetectorIndex]?.function !== 'metric') return;


Use ML_JOB_AGGREGATION.METRIC from common/constants/aggregation_types

Updated here 64ecedb

…ption type signature

kibanamachine · 2020-11-09T06:07:27Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: df26340

Metrics [docs]

@kbn/optimizer bundle module count

id	before	after	diff
`ml`	1238	1242	+4

async chunks size

id	before	after	diff
`ml`	6.6MB	6.7MB	+17.3KB

History

💚 Build #85865 succeeded 595b8b3
💚 Build #85650 succeeded 09fe186
💚 Build #85635 succeeded f694868
💚 Build #85403 succeeded 21bb6cb
💚 Build #83747 succeeded 3c8972a

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

darnautov

LGTM

walterra

LGTM, we can think of ways to improve componentDidUpdate() in the single metric viewer component in a follow up.

…n, mean or max as appropriate (elastic#81662)

…lot min, mean or max as appropriate (#81662) (#82978)

* master: (39 commits) Fix ilm navigation (elastic#81664) [Lens] Distinct icons for XY and pie chart value labels toolbar (elastic#82927) [data.search.aggs] Throw an error when trying to create an agg type that doesn't exist. (elastic#81509) Index patterns api - load field list on server (elastic#82629) New events resolver (elastic#82170) [App Search] Misc naming tech debt (elastic#82770) load empty_kibana in test to have clean starting point (elastic#82772) Remove data <--> expressions circular dependencies. (elastic#82685) Update 8.0 breaking change template to gather information on how to programmatically detect it. (elastic#82905) Add alerting as codeowners to related documentation folder (elastic#82777) Add captions to user and space grid pages (elastic#82713) add alternate path for x-pack/Cloud test for Lens (elastic#82634) Uses asCurrentUser in getClusterUuid (elastic#82908) [Alerting][Connectors] Add new executor subaction to get 3rd party case fields (elastic#82519) Fix test import objects (elastic#82767) [ML] Add option for anomaly charts for metric detector should plot min, mean or max as appropriate (elastic#81662) Update alert type selection layout to rows instead of grid (elastic#73665) Prevent Kerberos and PKI providers from initiating a new session for unauthenticated XHR/API requests. (elastic#82817) Update grunt and related packages (elastic#79327) Allow the repository to search across all namespaces (elastic#82863) ...

qn895 added 4 commits October 26, 2020 13:45

[ML] Add extra function info to marker tooltip if metric function

7a4e9a0

[ML] Add dropdown to pick what function to plot by if metric

26aa395

[ML] Plot function associated with highest anomaly score by default

ffce815

[ML] Clean up

3c8972a

qn895 added release_note:enhancement :ml v8.0.0 v7.11.0 labels Oct 26, 2020

qn895 self-assigned this Oct 26, 2020

qn895 requested review from walterra, darnautov and peteharverson October 27, 2020 13:24

qn895 added the Feature:Anomaly Detection ML anomaly detection label Oct 27, 2020

qn895 marked this pull request as ready for review October 27, 2020 13:26

qn895 requested a review from a team as a code owner October 27, 2020 13:26

darnautov reviewed Oct 27, 2020

View reviewed changes

peteharverson reviewed Oct 28, 2020

View reviewed changes

x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js Outdated Show resolved Hide resolved

qn895 added 5 commits October 28, 2020 17:34

Merge remote-tracking branch 'upstream/master' into ml-metric-plotting

5fff206

[ML] Add filter to table

eda2c30

[ML] Metric mini swimlane

d1071a3

Merge remote upstream/master into ml-metric-plotting

85f1526

[ML] Refactor

21bb6cb

peteharverson reviewed Nov 3, 2020

View reviewed changes

qn895 added 3 commits November 3, 2020 08:54

[ML] Delete residual merge code

a583988

Merge remote-tracking branch 'upstream/master' into ml-metric-plotting

e27e433

[ML] Rename actualPlotFunction, fix mean and avg name mismatch

cb2429e

[ML] Fix typescript

f694868

[ML] Remove metricActualPlotFunction because we no longer need it

09fe186

darnautov reviewed Nov 4, 2020

View reviewed changes

x-pack/plugins/ml/public/application/timeseriesexplorer/get_function_description.ts Outdated Show resolved Hide resolved

darnautov reviewed Nov 4, 2020

View reviewed changes

qn895 added 7 commits November 4, 2020 09:02

Merge remote-tracking branch 'upstream/master' into ml-metric-plotting

12b32ad

[ML] Fix bounds issue caused by value in form of array

638ed89

[ML] Fix anomalies not showing up in mini swimlane

3c02f4f

[ML] Use toastNotificationService from kibana context

5350e3d

[ML] Clean up

a7e2482

Merge upstream/master into ml-metric-plotting

2f028cd

[ML] Minor fixes & clean up

595b8b3

walterra reviewed Nov 5, 2020

View reviewed changes

peteharverson approved these changes Nov 5, 2020

View reviewed changes

qn895 added 2 commits November 8, 2020 21:15

[ML] Use constant metric, revert code removal, update function descri…

64ecedb

…ption type signature

Merge remote-tracking branch 'upstream/master' into ml-metric-plotting

df26340

darnautov approved these changes Nov 9, 2020

View reviewed changes

walterra approved these changes Nov 9, 2020

View reviewed changes

qn895 merged commit 9c984f4 into elastic:master Nov 9, 2020

qn895 deleted the ml-metric-plotting branch November 9, 2020 16:41

qn895 mentioned this pull request Nov 9, 2020

[7.x] [ML] Add option for anomaly charts for metric detector should plot min, mean or max as appropriate (#81662) #82978

Merged

qn895 added a commit to qn895/kibana that referenced this pull request Nov 9, 2020

[ML] Add option for anomaly charts for metric detector should plot mi…

3a642e9

…n, mean or max as appropriate (elastic#81662)

qn895 added a commit that referenced this pull request Nov 9, 2020

[7.x] [ML] Add option for anomaly charts for metric detector should p…

d0dd9f8

…lot min, mean or max as appropriate (#81662) (#82978)

qn895 mentioned this pull request Nov 17, 2020

[ML] Persist URL state for Anomaly detection jobs using metric function #83507

Merged

1 task

peteharverson mentioned this pull request Dec 8, 2020

[ML] Anomaly charts for metric detector should plot min, mean or max as appropriate #65078

Closed

qn895 mentioned this pull request Dec 14, 2020

[ML] If there's no anomaly found, auto default to plotting the mean #85866

Merged

2 tasks

[ML] Add option for anomaly charts for metric detector should plot min, mean or max as appropriate #81662

[ML] Add option for anomaly charts for metric detector should plot min, mean or max as appropriate #81662

Conversation

qn895 commented Oct 26, 2020 • edited Loading

Summary

Checklist

elasticmachine commented Oct 27, 2020

peteharverson commented Oct 28, 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

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 commented Nov 3, 2020

peteharverson commented Nov 3, 2020

qn895 commented Nov 3, 2020 • edited Loading

Choose a reason for hiding this comment

qn895 Nov 4, 2020 • edited Loading

Choose a reason for hiding this comment

peteharverson commented Nov 4, 2020 • edited Loading

qn895 commented Nov 4, 2020

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

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

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

kibanamachine commented Nov 9, 2020

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

async chunks size

History

darnautov left a comment

Choose a reason for hiding this comment

walterra left a comment

Choose a reason for hiding this comment

qn895 commented Oct 26, 2020 •

edited

Loading

qn895 commented Nov 3, 2020 •

edited

Loading

qn895 Nov 4, 2020 •

edited

Loading

peteharverson commented Nov 4, 2020 •

edited

Loading