From 5bed96e20fad6dafea6833f29962528e3b92d28a Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Mon, 20 Jan 2020 17:01:49 +0100 Subject: [PATCH] [ML] Correctly pass on severity value to anomaly explorer charts. (#55207) (#55329) - Fixes passing on the severity value correctly to anomaly explorer charts. The wrong value of undefined being passed down caused anomaly markers not showing up. - This bug surfaced that the severity value was never applied to filter multi-bucket anomalies which is now also fixed by this PR. - Adds a check if topInfluencers is an array. --- .../ml/public/application/explorer/explorer.d.ts | 1 + .../ml/public/application/explorer/explorer.js | 4 ++-- .../explorer_charts/explorer_chart_single_metric.js | 11 ++++++----- .../ml/public/application/explorer/explorer_utils.js | 12 +++++++----- .../public/application/routing/routes/explorer.tsx | 1 + 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/x-pack/legacy/plugins/ml/public/application/explorer/explorer.d.ts b/x-pack/legacy/plugins/ml/public/application/explorer/explorer.d.ts index b8df021990f584..a85674986c7f7f 100644 --- a/x-pack/legacy/plugins/ml/public/application/explorer/explorer.d.ts +++ b/x-pack/legacy/plugins/ml/public/application/explorer/explorer.d.ts @@ -15,6 +15,7 @@ import { AppStateSelectedCells } from '../explorer/explorer_utils'; declare interface ExplorerProps { explorerState: ExplorerState; + severity: number; showCharts: boolean; setSelectedCells: (swimlaneSelectedCells: AppStateSelectedCells) => void; } diff --git a/x-pack/legacy/plugins/ml/public/application/explorer/explorer.js b/x-pack/legacy/plugins/ml/public/application/explorer/explorer.js index 79071319965781..6a1c5339de1f56 100644 --- a/x-pack/legacy/plugins/ml/public/application/explorer/explorer.js +++ b/x-pack/legacy/plugins/ml/public/application/explorer/explorer.js @@ -96,6 +96,7 @@ export class Explorer extends React.Component { static propTypes = { explorerState: PropTypes.object.isRequired, setSelectedCells: PropTypes.func.isRequired, + severity: PropTypes.number.isRequired, showCharts: PropTypes.bool.isRequired, }; @@ -260,7 +261,7 @@ export class Explorer extends React.Component { }; render() { - const { showCharts } = this.props; + const { showCharts, severity } = this.props; const { annotationsData, @@ -276,7 +277,6 @@ export class Explorer extends React.Component { queryString, selectedCells, selectedJobs, - severity, swimlaneContainerWidth, tableData, tableQueryString, diff --git a/x-pack/legacy/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.js b/x-pack/legacy/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.js index 583375c87007e4..a255b6b0434e4e 100644 --- a/x-pack/legacy/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.js +++ b/x-pack/legacy/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.js @@ -53,7 +53,7 @@ export const ExplorerChartSingleMetric = injectI18n( static propTypes = { tooManyBuckets: PropTypes.bool, seriesConfig: PropTypes.object, - severity: PropTypes.number, + severity: PropTypes.number.isRequired, }; componentDidMount() { @@ -312,13 +312,16 @@ export const ExplorerChartSingleMetric = injectI18n( }) .on('mouseout', () => mlChartTooltipService.hide()); + const isAnomalyVisible = d => + _.has(d, 'anomalyScore') && Number(d.anomalyScore) >= severity; + // Update all dots to new positions. dots .attr('cx', d => lineChartXScale(d.date)) .attr('cy', d => lineChartYScale(d.value)) .attr('class', d => { let markerClass = 'metric-value'; - if (_.has(d, 'anomalyScore') && Number(d.anomalyScore) >= severity) { + if (isAnomalyVisible(d)) { markerClass += ` anomaly-marker ${getSeverityWithLow(d.anomalyScore).id}`; } return markerClass; @@ -328,9 +331,7 @@ export const ExplorerChartSingleMetric = injectI18n( const multiBucketMarkers = lineChartGroup .select('.chart-markers') .selectAll('.multi-bucket') - .data( - data.filter(d => d.anomalyScore !== null && showMultiBucketAnomalyMarker(d) === true) - ); + .data(data.filter(d => isAnomalyVisible(d) && showMultiBucketAnomalyMarker(d) === true)); // Remove multi-bucket markers that are no longer needed multiBucketMarkers.exit().remove(); diff --git a/x-pack/legacy/plugins/ml/public/application/explorer/explorer_utils.js b/x-pack/legacy/plugins/ml/public/application/explorer/explorer_utils.js index 4fb4e7d4df94f2..14d356c0d1c81c 100644 --- a/x-pack/legacy/plugins/ml/public/application/explorer/explorer_utils.js +++ b/x-pack/legacy/plugins/ml/public/application/explorer/explorer_utils.js @@ -280,11 +280,13 @@ export function loadViewByTopFieldValuesForSelectedTime( const topFieldValues = []; const topInfluencers = resp.influencers[viewBySwimlaneFieldName]; - topInfluencers.forEach(influencerData => { - if (influencerData.maxAnomalyScore > 0) { - topFieldValues.push(influencerData.influencerFieldValue); - } - }); + if (Array.isArray(topInfluencers)) { + topInfluencers.forEach(influencerData => { + if (influencerData.maxAnomalyScore > 0) { + topFieldValues.push(influencerData.influencerFieldValue); + } + }); + } resolve(topFieldValues); }); } else { diff --git a/x-pack/legacy/plugins/ml/public/application/routing/routes/explorer.tsx b/x-pack/legacy/plugins/ml/public/application/routing/routes/explorer.tsx index 6aaad5294369b7..633efc2856dac1 100644 --- a/x-pack/legacy/plugins/ml/public/application/routing/routes/explorer.tsx +++ b/x-pack/legacy/plugins/ml/public/application/routing/routes/explorer.tsx @@ -184,6 +184,7 @@ const ExplorerUrlStateManager: FC = ({ jobsWithTim explorerState, setSelectedCells, showCharts, + severity: tableSeverity.val, }} />