[ML] Adds missing error handling to annotations request. (#32384) (#3…

…2487) - This adds more thorough error handling to retrieving annotations from the server to avoid breaking Anomaly Explorer when there's a problem retrieving annotations. The UI will silently fail and hide the annotations table if for whatever reasons annotations cannot be retrieved for the selected jobs (this could be permissions or setup related if e.g. the annotations feature index/aliases are not present). This is now the same behavior like in Single Metric Viewer. The browser console will still output an error message. This fixes an error scenario where Anomaly Explorer would fail to load if a user upgraded to 6.6 because the backend task wasn't run yet to create the annotation index/aliases. - Disables annotations feature in Single Metric Viewer if annotations fail to load. - Use componentDidUpdate and an update check instead of deprecated componentWillUpdate for updating the annotations table in jobs management.
elastic · Mar 5, 2019 · 062270b · 062270b
1 parent 5a23351
commit 062270b
Show file tree

Hide file tree

Showing 8 changed files with 133 additions and 74 deletions.
diff --git a/x-pack/plugins/ml/public/components/annotations/annotations_table/annotations_table.js b/x-pack/plugins/ml/public/components/annotations/annotations_table/annotations_table.js
@@ -130,14 +130,17 @@ const AnnotationsTable = injectI18n(class AnnotationsTable extends Component {
     }
   }
 
-  componentWillUpdate() {
+  previousJobId = undefined;
+  componentDidUpdate() {
     if (
+      Array.isArray(this.props.jobs) && this.props.jobs.length > 0 &&
+      this.previousJobId !== this.props.jobs[0].job_id &&
       this.props.annotations === undefined &&
       this.state.isLoading === false &&
-      Array.isArray(this.props.jobs) && this.props.jobs.length > 0 &&
       this.state.jobId !== this.props.jobs[0].job_id
     ) {
       annotationsRefresh$.next(true);
+      this.previousJobId = this.props.jobs[0].job_id;
     }
   }
 

diff --git a/x-pack/plugins/ml/public/explorer/explorer_utils.js b/x-pack/plugins/ml/public/explorer/explorer_utils.js
@@ -379,7 +379,7 @@ export function processViewByResults(
   return dataset;
 }
 
-export async function loadAnnotationsTableData(selectedCells, selectedJobs, interval, bounds) {
+export function loadAnnotationsTableData(selectedCells, selectedJobs, interval, bounds) {
   const jobIds = (selectedCells !== null && selectedCells.viewByFieldName === VIEW_BY_JOB_LABEL) ?
     selectedCells.lanes : selectedJobs.map(d => d.id);
   const timeRange = getSelectionTimeRange(selectedCells, interval, bounds);
@@ -388,31 +388,41 @@ export async function loadAnnotationsTableData(selectedCells, selectedJobs, inte
     return Promise.resolve([]);
   }
 
-  const resp = await ml.annotations.getAnnotations({
-    jobIds,
-    earliestMs: timeRange.earliestMs,
-    latestMs: timeRange.latestMs,
-    maxAnnotations: ANNOTATIONS_TABLE_DEFAULT_QUERY_SIZE
-  });
+  return new Promise((resolve) => {
+    ml.annotations.getAnnotations({
+      jobIds,
+      earliestMs: timeRange.earliestMs,
+      latestMs: timeRange.latestMs,
+      maxAnnotations: ANNOTATIONS_TABLE_DEFAULT_QUERY_SIZE
+    }).then((resp) => {
+      if (resp.error !== undefined || resp.annotations === undefined) {
+        return resolve([]);
+      }
 
-  const annotationsData = [];
-  jobIds.forEach((jobId) => {
-    const jobAnnotations = resp.annotations[jobId];
-    if (jobAnnotations !== undefined) {
-      annotationsData.push(...jobAnnotations);
-    }
-  });
+      const annotationsData = [];
+      jobIds.forEach((jobId) => {
+        const jobAnnotations = resp.annotations[jobId];
+        if (jobAnnotations !== undefined) {
+          annotationsData.push(...jobAnnotations);
+        }
+      });
 
-  return Promise.resolve(
-    annotationsData
-      .sort((a, b) => {
-        return a.timestamp - b.timestamp;
-      })
-      .map((d, i) => {
-        d.key = String.fromCharCode(65 + i);
-        return d;
-      })
-  );
+      return resolve(
+        annotationsData
+          .sort((a, b) => {
+            return a.timestamp - b.timestamp;
+          })
+          .map((d, i) => {
+            d.key = String.fromCharCode(65 + i);
+            return d;
+          })
+      );
+    }).catch((resp) => {
+      console.log('Error loading list of annotations for jobs list:', resp);
+      // Silently fail and just return an empty array for annotations to not break the UI.
+      return resolve([]);
+    });
+  });
 }
 
 export async function loadAnomaliesTableData(

diff --git a/x-pack/plugins/ml/public/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js b/x-pack/plugins/ml/public/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js
@@ -19,8 +19,6 @@ import _ from 'lodash';
 import d3 from 'd3';
 import moment from 'moment';
 
-import chrome from 'ui/chrome';
-
 import {
   getSeverityWithLow,
   getMultiBucketImpactLabel,
@@ -58,8 +56,6 @@ import {
 
 import { injectI18n } from '@kbn/i18n/react';
 
-const mlAnnotationsEnabled = chrome.getInjected('mlAnnotationsEnabled', false);
-
 const focusZoomPanelHeight = 25;
 const focusChartHeight = 310;
 const focusHeight = focusZoomPanelHeight + focusChartHeight;
@@ -93,6 +89,7 @@ function getSvgHeight() {
 
 const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Component {
   static propTypes = {
+    annotationsEnabled: PropTypes.bool,
     annotation: PropTypes.object,
     autoZoomDuration: PropTypes.number,
     contextAggregationInterval: PropTypes.object,
@@ -133,6 +130,7 @@ const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Compo
 
   componentDidMount() {
     const {
+      annotationsEnabled,
       svgWidth
     } = this.props;
 
@@ -165,7 +163,7 @@ const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Compo
     this.fieldFormat = undefined;
 
     // Annotations Brush
-    if (mlAnnotationsEnabled) {
+    if (annotationsEnabled) {
       this.annotateBrush = getAnnotationBrush.call(this);
     }
 
@@ -212,14 +210,15 @@ const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Compo
 
     this.renderFocusChart();
 
-    if (mlAnnotationsEnabled && this.props.annotation === null) {
+    if (this.props.annotationsEnabled && this.props.annotation === null) {
       const chartElement = d3.select(this.rootNode);
       chartElement.select('g.mlAnnotationBrush').call(this.annotateBrush.extent([0, 0]));
     }
   }
 
   renderChart() {
     const {
+      annotationsEnabled,
       contextChartData,
       contextForecastData,
       detectorIndex,
@@ -321,7 +320,7 @@ const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Compo
       .attr('transform', 'translate(' + margin.left + ',' + (focusHeight + margin.top + chartSpacing) + ')');
 
     // Mask to hide annotations overflow
-    if (mlAnnotationsEnabled) {
+    if (annotationsEnabled) {
       const annotationsMask = svg
         .append('defs')
         .append('mask')
@@ -397,6 +396,7 @@ const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Compo
     // as we want to re-render the paths and points when the zoom area changes.
 
     const {
+      annotationsEnabled,
       contextForecastData
     } = this.props;
 
@@ -413,7 +413,7 @@ const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Compo
     this.createZoomInfoElements(zoomGroup, fcsWidth);
 
     // Create the elements for annotations
-    if (mlAnnotationsEnabled) {
+    if (annotationsEnabled) {
       const annotateBrush = this.annotateBrush.bind(this);
 
       fcsGroup.append('g')
@@ -495,6 +495,7 @@ const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Compo
 
   renderFocusChart() {
     const {
+      annotationsEnabled,
       focusAggregationInterval,
       focusAnnotationData,
       focusChartData,
@@ -596,7 +597,7 @@ const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Compo
 
       // if annotations are present, we extend yMax to avoid overlap
       // between annotation labels, chart lines and anomalies.
-      if (mlAnnotationsEnabled && focusAnnotationData && focusAnnotationData.length > 0) {
+      if (annotationsEnabled && focusAnnotationData && focusAnnotationData.length > 0) {
         const levels = getAnnotationLevels(focusAnnotationData);
         const maxLevel = d3.max(Object.keys(levels).map(key => levels[key]));
         // TODO needs revisiting to be a more robust normalization
@@ -632,7 +633,7 @@ const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Compo
         .classed('hidden', !showModelBounds);
     }
 
-    if (mlAnnotationsEnabled) {
+    if (annotationsEnabled) {
       renderAnnotations(
         focusChart,
         focusAnnotationData,
@@ -645,7 +646,7 @@ const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Compo
 
       // disable brushing (creation of annotations) when annotations aren't shown
       focusChart.select('.mlAnnotationBrush')
-        .style('pointer-events', (showAnnotations) ? 'all' : 'none');
+        .style('display', (showAnnotations) ? null : 'none');
     }
 
     focusChart.select('.values-line')
@@ -1236,6 +1237,7 @@ const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Compo
 
   showFocusChartTooltip(marker, circle) {
     const {
+      annotationsEnabled,
       modelPlotEnabled,
       intl
     } = this.props;
@@ -1376,7 +1378,7 @@ const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Compo
       });
     }
 
-    if (mlAnnotationsEnabled && _.has(marker, 'annotation')) {
+    if (annotationsEnabled && _.has(marker, 'annotation')) {
       contents = mlEscape(marker.annotation);
       contents += `<br />${moment(marker.timestamp).format('MMMM Do YYYY, HH:mm')}`;
 

diff --git a/...ns/ml/public/timeseriesexplorer/components/timeseries_chart/timeseries_chart_directive.js b/...ns/ml/public/timeseriesexplorer/components/timeseries_chart/timeseries_chart_directive.js
@@ -26,9 +26,6 @@ const module = uiModules.get('apps/ml');
 
 import { I18nContext } from 'ui/i18n';
 
-import chrome from 'ui/chrome';
-const mlAnnotationsEnabled = chrome.getInjected('mlAnnotationsEnabled', false);
-
 module.directive('mlTimeseriesChart', function ($timeout) {
 
   function link(scope, element) {
@@ -44,6 +41,7 @@ module.directive('mlTimeseriesChart', function ($timeout) {
       svgWidth = Math.max(angular.element('.results-container').width(), 0);
 
       const props = {
+        annotationsEnabled: scope.annotationsEnabled,
         autoZoomDuration: scope.autoZoomDuration,
         contextAggregationInterval: scope.contextAggregationInterval,
         contextChartData: scope.contextChartData,
@@ -91,7 +89,8 @@ module.directive('mlTimeseriesChart', function ($timeout) {
     scope.$watchCollection('focusForecastData', renderFocusChart);
     scope.$watchCollection('focusChartData', renderFocusChart);
     scope.$watchGroup(['showModelBounds', 'showForecast'], renderFocusChart);
-    if (mlAnnotationsEnabled) {
+    scope.$watch('annotationsEnabled', renderReactComponent);
+    if (scope.annotationsEnabled) {
       scope.$watchCollection('focusAnnotationData', renderFocusChart);
       scope.$watch('showAnnotations', renderFocusChart);
     }
@@ -116,6 +115,7 @@ module.directive('mlTimeseriesChart', function ($timeout) {
 
   return {
     scope: {
+      annotationsEnabled: '=',
       selectedJob: '=',
       detectorIndex: '=',
       modelPlotEnabled: '=',

diff --git a/x-pack/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.html b/x-pack/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.html
@@ -161,6 +161,7 @@
       <div class="ml-timeseries-chart">
 
         <ml-timeseries-chart style="width: 1200px; height: 400px;"
+          annotations-enabled="showAnnotationsCheckbox"
           selected-job="selectedJob"
           detector-index="detectorId"
           model-plot-enabled="modelPlotEnabled"

diff --git a/x-pack/plugins/ml/public/timeseriesexplorer/timeseriesexplorer_controller.js b/x-pack/plugins/ml/public/timeseriesexplorer/timeseriesexplorer_controller.js
@@ -61,7 +61,7 @@ import { severity$ } from '../components/controls/select_severity/select_severit
 
 
 import chrome from 'ui/chrome';
-const mlAnnotationsEnabled = chrome.getInjected('mlAnnotationsEnabled', false);
+let mlAnnotationsEnabled = chrome.getInjected('mlAnnotationsEnabled', false);
 
 uiRoutes
   .when('/timeseriesexplorer/?', {
@@ -416,6 +416,12 @@ module.controller('MlTimeSeriesExplorerController', function (
           console.log('Time series explorer focus chart data set:', $scope.focusChartData);
 
           $scope.loading = false;
+
+          // If the annotations failed to load and the feature flag is set to `false`,
+          // make sure the checkbox toggle gets hidden.
+          if (mlAnnotationsEnabled === false) {
+            $scope.showAnnotationsCheckbox = false;
+          }
         });
       }
     }
@@ -490,19 +496,24 @@ module.controller('MlTimeSeriesExplorerController', function (
         latestMs: searchBounds.max.valueOf(),
         maxAnnotations: ANNOTATIONS_TABLE_DEFAULT_QUERY_SIZE
       }).then((resp) => {
-        refreshFocusData.focusAnnotationData = resp.annotations[$scope.selectedJob.job_id]
-          .sort((a, b) => {
-            return a.timestamp - b.timestamp;
-          })
-          .map((d, i) => {
-            d.key = String.fromCharCode(65 + i);
-            return d;
-          });
+        refreshFocusData.focusAnnotationData = [];
+
+        if (Array.isArray(resp.annotations[$scope.selectedJob.job_id])) {
+          refreshFocusData.focusAnnotationData = resp.annotations[$scope.selectedJob.job_id]
+            .sort((a, b) => {
+              return a.timestamp - b.timestamp;
+            })
+            .map((d, i) => {
+              d.key = String.fromCharCode(65 + i);
+              return d;
+            });
+        }
 
         finish();
       }).catch(() => {
-        // silent fail
+        // silently fail and disable annotations feature if loading annotations fails.
         refreshFocusData.focusAnnotationData = [];
+        mlAnnotationsEnabled = false;
         finish();
       });
     } else {