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

- 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.

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;
     }
   }
 

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(

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,7 +210,7 @@ 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]));
     }
@@ -220,6 +218,7 @@ const TimeseriesChartIntl = injectI18n(class TimeseriesChart extends React.Compo
 
   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')}`;
 

x-pack/plugins/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: '=',

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"

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 {

x-pack/plugins/ml/server/models/annotation_service/annotation.test.ts

@@ -16,6 +16,8 @@ import { annotationServiceProvider } from './index';
 
 const acknowledgedResponseMock = { acknowledged: true };
 
+const jobIdMock = 'jobIdMock';
+
 describe('annotation_service', () => {
   let callWithRequestSpy: jest.Mock;
 
@@ -55,8 +57,6 @@ describe('annotation_service', () => {
     it('should get annotations for specific job', async done => {
       const { getAnnotations } = annotationServiceProvider(callWithRequestSpy);
 
-      const jobIdMock = 'jobIdMock';
-
       const indexAnnotationArgsMock: IndexAnnotationArgs = {
         jobIds: [jobIdMock],
         earliestMs: 1454804100000,
@@ -73,13 +73,37 @@ describe('annotation_service', () => {
       expect(isAnnotations(response.annotations[jobIdMock])).toBeTruthy();
       done();
     });
+
+    it('should throw and catch an error', async () => {
+      const mockEsError = {
+        statusCode: 404,
+        error: 'Not Found',
+        message: 'mock error message',
+      };
+
+      const callWithRequestSpyError = jest.fn(() => {
+        return Promise.resolve(mockEsError);
+      });
+
+      const { getAnnotations } = annotationServiceProvider(callWithRequestSpyError);
+
+      const indexAnnotationArgsMock: IndexAnnotationArgs = {
+        jobIds: [jobIdMock],
+        earliestMs: 1454804100000,
+        latestMs: 1455233399999,
+        maxAnnotations: 500,
+      };
+
+      await expect(getAnnotations(indexAnnotationArgsMock)).rejects.toEqual(
+        Error(`Annotations couldn't be retrieved from Elasticsearch.`)
+      );
+    });
   });
 
   describe('indexAnnotation()', () => {
     it('should index annotation', async done => {
       const { indexAnnotation } = annotationServiceProvider(callWithRequestSpy);
 
-      const jobIdMock = 'jobIdMock';
       const annotationMock: Annotation = {
         annotation: 'Annotation text',
         job_id: jobIdMock,
@@ -107,7 +131,6 @@ describe('annotation_service', () => {
     it('should remove ._id and .key before updating annotation', async done => {
       const { indexAnnotation } = annotationServiceProvider(callWithRequestSpy);
 
-      const jobIdMock = 'jobIdMock';
       const annotationMock: Annotation = {
         _id: 'mockId',
         annotation: 'Updated annotation text',
@@ -139,8 +162,6 @@ describe('annotation_service', () => {
     it('should update annotation text and the username for modified_username', async done => {
       const { getAnnotations, indexAnnotation } = annotationServiceProvider(callWithRequestSpy);
 
-      const jobIdMock = 'jobIdMock';
-
       const indexAnnotationArgsMock: IndexAnnotationArgs = {
         jobIds: [jobIdMock],
         earliestMs: 1454804100000,

x-pack/plugins/ml/server/models/annotation_service/annotation.ts

@@ -70,6 +70,7 @@ export type callWithRequestType = (
 export function annotationProvider(callWithRequest: callWithRequestType) {
   async function indexAnnotation(annotation: Annotation, username: string) {
     if (isAnnotation(annotation) === false) {
+      // No need to translate, this will not be exposed in the UI.
       return Promise.reject(new Error('invalid annotation format'));
     }
 
@@ -211,27 +212,37 @@ export function annotationProvider(callWithRequest: callWithRequestType) {
       },
     };
 
-    const resp = await callWithRequest('search', params);
+    try {
+      const resp = await callWithRequest('search', params);
 
-    const docs: Annotations = _.get(resp, ['hits', 'hits'], []).map((d: EsResult) => {
-      // get the original source document and the document id, we need it
-      // to identify the annotation when editing/deleting it.
-      return { ...d._source, _id: d._id } as Annotation;
-    });
-
-    if (isAnnotations(docs) === false) {
-      throw Boom.badRequest(`Annotations didn't pass integrity check.`);
-    }
-
-    docs.forEach((doc: Annotation) => {
-      const jobId = doc.job_id;
-      if (typeof obj.annotations[jobId] === 'undefined') {
-        obj.annotations[jobId] = [];
+      if (resp.error !== undefined && resp.message !== undefined) {
+        // No need to translate, this will not be exposed in the UI.
+        throw new Error(`Annotations couldn't be retrieved from Elasticsearch.`);
       }
-      obj.annotations[jobId].push(doc);
-    });
 
-    return obj;
+      const docs: Annotations = _.get(resp, ['hits', 'hits'], []).map((d: EsResult) => {
+        // get the original source document and the document id, we need it
+        // to identify the annotation when editing/deleting it.
+        return { ...d._source, _id: d._id } as Annotation;
+      });
+
+      if (isAnnotations(docs) === false) {
+        // No need to translate, this will not be exposed in the UI.
+        throw new Error(`Annotations didn't pass integrity check.`);
+      }
+
+      docs.forEach((doc: Annotation) => {
+        const jobId = doc.job_id;
+        if (typeof obj.annotations[jobId] === 'undefined') {
+          obj.annotations[jobId] = [];
+        }
+        obj.annotations[jobId].push(doc);
+      });
+
+      return obj;
+    } catch (error) {
+      throw Boom.badRequest(error);
+    }
   }
 
   async function deleteAnnotation(id: string) {