From c656f3661bca5e0b598ecddf387f3ff367ac710a Mon Sep 17 00:00:00 2001 From: "Quynh Nguyen (Quinn)" <43350163+qn895@users.noreply.github.com> Date: Fri, 9 Feb 2024 13:47:00 -0600 Subject: [PATCH] [ML] Fix Single Metric Viewer not showing chart for metric functions and mismatch function in tooltip (#176354) ## Summary This PR fixes https://github.com/elastic/kibana/issues/176301. Fixes include: 1) The schema for `getAnomalyRecordsSchema` was updated, but missed the additional parameter `functionDescription `. So the API call was throwing error. 2) The tooltip to showcase the underlying function (min, max, or avg) for metric detector that yields the highest anomaly score was showing the incorrect fallback value. 3) Fix label in Single Metric Viewer to match with the min/max/avg used: After https://github.com/elastic/kibana/assets/43350163/03d8e10b-c4cd-4c28-a28c-7c6a7e108065 Screenshot 2024-02-06 at 17 11 15 ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### Risk Matrix Delete this section if it is not applicable to this PR. Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release. When forming the risk matrix, consider some of the following examples and how they may potentially impact the change: | Risk | Probability | Severity | Mitigation/Notes | |---------------------------|-------------|----------|-------------------------| | Multiple Spaces—unexpected behavior in non-default Kibana Space. | Low | High | Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces. | | Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. | High | Low | Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure. | | Code should gracefully handle cases when feature X or plugin Y are disabled. | Medium | High | Unit tests will verify that any feature flag or plugin combination still results in our service operational. | | [See more potential risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) | ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../timeseries_search_service.ts | 41 ++++++++++++++----- .../timeseriesexplorer/timeseriesexplorer.js | 3 +- .../application/util/chart_config_builder.ts | 19 +++++++-- .../models/results_service/anomaly_charts.ts | 4 +- .../routes/schemas/results_service_schema.ts | 1 + 5 files changed, 51 insertions(+), 17 deletions(-) diff --git a/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseries_search_service.ts b/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseries_search_service.ts index 044d166ca5efa18..05adc2355ef3c6f 100644 --- a/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseries_search_service.ts +++ b/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseries_search_service.ts @@ -6,6 +6,7 @@ */ import { each, find, get, filter } from 'lodash'; +import type { ES_AGGREGATION } from '@kbn/ml-anomaly-utils'; import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; @@ -15,7 +16,6 @@ import { isModelPlotChartableForDetector, isModelPlotEnabled, } from '../../../common/util/job_utils'; -// @ts-ignore import { buildConfigFromDetector } from '../util/chart_config_builder'; import { mlResultsService } from '../services/results_service'; import { ModelPlotOutput } from '../services/results_service/result_service_rx'; @@ -113,29 +113,48 @@ function getMetricData( } } -// Builds chart detail information (charting function description and entity counts) used -// in the title area of the time series chart. -// Queries Elasticsearch if necessary to obtain the distinct count of entities -// for which data is being plotted. +interface TimeSeriesExplorerChartDetails { + success: boolean; + results: { + functionLabel: string | null; + entityData: { count?: number; entities: Array<{ fieldName: string; cardinality?: number }> }; + }; +} + +/** + * Builds chart detail information (charting function description and entity counts) used + * in the title area of the time series chart. + * Queries Elasticsearch if necessary to obtain the distinct count of entities + * for which data is being plotted. + * @param job Job config info + * @param detectorIndex The index of the detector in the job config + * @param entityFields Array of field name - field value pairs + * @param earliestMs Earliest timestamp in milliseconds + * @param latestMs Latest timestamp in milliseconds + * @param metricFunctionDescription The underlying function (min, max, avg) for "metric" detector type + * @returns chart data to plot for Single Metric Viewer/Time series explorer + */ function getChartDetails( job: Job, detectorIndex: number, - entityFields: any[], + entityFields: MlEntityField[], earliestMs: number, - latestMs: number + latestMs: number, + metricFunctionDescription?: ES_AGGREGATION ) { return new Promise((resolve, reject) => { - const obj: any = { + const obj: TimeSeriesExplorerChartDetails = { success: true, results: { functionLabel: '', entityData: { entities: [] } }, }; - const chartConfig = buildConfigFromDetector(job, detectorIndex); + const chartConfig = buildConfigFromDetector(job, detectorIndex, metricFunctionDescription); + let functionLabel: string | null = chartConfig.metricFunction; if (chartConfig.metricFieldName !== undefined) { - functionLabel += ' '; - functionLabel += chartConfig.metricFieldName; + functionLabel += ` ${chartConfig.metricFieldName}`; } + obj.results.functionLabel = functionLabel; const blankEntityFields = filter(entityFields, (entity) => { diff --git a/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js b/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js index ad3f71e5df22d53..76c0ffb0389711d 100644 --- a/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js +++ b/x-pack/plugins/ml/public/application/timeseriesexplorer/timeseriesexplorer.js @@ -568,7 +568,8 @@ export class TimeSeriesExplorer extends React.Component { detectorIndex, entityControls, searchBounds.min.valueOf(), - searchBounds.max.valueOf() + searchBounds.max.valueOf(), + this.props.functionDescription ) .then((resp) => { stateUpdate.chartDetails = resp.results; diff --git a/x-pack/plugins/ml/public/application/util/chart_config_builder.ts b/x-pack/plugins/ml/public/application/util/chart_config_builder.ts index e9c322d30a6e566..c1057fc335e82c0 100644 --- a/x-pack/plugins/ml/public/application/util/chart_config_builder.ts +++ b/x-pack/plugins/ml/public/application/util/chart_config_builder.ts @@ -19,9 +19,19 @@ import { Job } from '../../../common/types/anomaly_detection_jobs'; import { mlFunctionToESAggregation } from '../../../common/util/job_utils'; -// Builds the basic configuration to plot a chart of the source data -// analyzed by the the detector at the given index from the specified ML job. -export function buildConfigFromDetector(job: Job, detectorIndex: number) { +/** + * Builds the basic configuration to plot a chart of the source data + * analyzed by the the detector at the given index from the specified ML job. + * @param job Job config info + * @param detectorIndex The index of the detector in the job config + * @param metricFunctionDescription The underlying function (min, max, avg) for "metric" detector type + * @returns + */ +export function buildConfigFromDetector( + job: Job, + detectorIndex: number, + metricFunctionDescription?: ES_AGGREGATION +) { const analysisConfig = job.analysis_config; const detector = analysisConfig.detectors[detectorIndex]; @@ -38,6 +48,9 @@ export function buildConfigFromDetector(job: Job, detectorIndex: number) { datafeedConfig: job.datafeed_config!, summaryCountFieldName: job.analysis_config.summary_count_field_name, }; + if (detector.function === ML_JOB_AGGREGATION.METRIC && metricFunctionDescription !== undefined) { + config.metricFunction = metricFunctionDescription; + } if (detector.field_name !== undefined) { config.metricFieldName = detector.field_name; diff --git a/x-pack/plugins/ml/server/models/results_service/anomaly_charts.ts b/x-pack/plugins/ml/server/models/results_service/anomaly_charts.ts index 707a594d5eff347..345b6cf6054af38 100644 --- a/x-pack/plugins/ml/server/models/results_service/anomaly_charts.ts +++ b/x-pack/plugins/ml/server/models/results_service/anomaly_charts.ts @@ -766,12 +766,12 @@ export function anomalyChartsDataProvider(mlClient: MlClient, client: IScopedClu } // Build the tooltip data for the chart info icon, showing further details on what is being plotted. - let functionLabel = `${config.metricFunction}`; + let functionLabel = `${fullSeriesConfig.metricFunction ?? config.metricFunction}`; if ( fullSeriesConfig.metricFieldName !== undefined && fullSeriesConfig.metricFieldName !== null ) { - functionLabel += ` ${fullSeriesConfig.metricFieldName}`; + functionLabel += `(${fullSeriesConfig.metricFieldName})`; } fullSeriesConfig.infoTooltip = { diff --git a/x-pack/plugins/ml/server/routes/schemas/results_service_schema.ts b/x-pack/plugins/ml/server/routes/schemas/results_service_schema.ts index 70fc5098fe3b549..656007b3e1e1c05 100644 --- a/x-pack/plugins/ml/server/routes/schemas/results_service_schema.ts +++ b/x-pack/plugins/ml/server/routes/schemas/results_service_schema.ts @@ -144,4 +144,5 @@ export const getAnomalyRecordsSchema = schema.object({ latestMs: schema.number(), criteriaFields: schema.arrayOf(schema.any()), interval: schema.string(), + functionDescription: schema.maybe(schema.nullable(schema.string())), });