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

- 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/explorer/explorer_controller.js

@@ -953,7 +953,7 @@ module.controller('MlExplorerController', function (
     }
   }
 
-  async function loadAnnotationsTableData() {
+  function loadAnnotationsTableData() {
     $scope.annotationsData = [];
 
     const cellData = $scope.cellData;
@@ -962,30 +962,35 @@ module.controller('MlExplorerController', function (
     const timeRange = getSelectionTimeRange(cellData);
 
     if (mlAnnotationsEnabled) {
-      const resp = await ml.annotations.getAnnotations({
+      ml.annotations.getAnnotations({
         jobIds,
         earliestMs: timeRange.earliestMs,
         latestMs: timeRange.latestMs,
         maxAnnotations: ANNOTATIONS_TABLE_DEFAULT_QUERY_SIZE
-      });
-
-      $scope.$evalAsync(() => {
-        const annotationsData = [];
-        jobIds.forEach((jobId) => {
-          const jobAnnotations = resp.annotations[jobId];
-          if (jobAnnotations !== undefined) {
-            annotationsData.push(...jobAnnotations);
-          }
-        });
-
-        $scope.annotationsData = annotationsData
-          .sort((a, b) => {
-            return a.timestamp - b.timestamp;
-          })
-          .map((d, i) => {
-            d.key = String.fromCharCode(65 + i);
-            return d;
+      }).then((resp) => {
+        $scope.$evalAsync(() => {
+          const annotationsData = [];
+          jobIds.forEach((jobId) => {
+            const jobAnnotations = resp.annotations[jobId];
+            if (jobAnnotations !== undefined) {
+              annotationsData.push(...jobAnnotations);
+            }
           });
+
+          $scope.annotationsData = 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.
+        $scope.annotationsData = [];
+        $scope.$applyAsync();
       });
     }
   }

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,
@@ -56,8 +54,6 @@ import {
   unhighlightFocusChartAnnotation
 } from './timeseries_chart_annotations';
 
-const mlAnnotationsEnabled = chrome.getInjected('mlAnnotationsEnabled', false);
-
 const focusZoomPanelHeight = 25;
 const focusChartHeight = 310;
 const focusHeight = focusZoomPanelHeight + focusChartHeight;
@@ -91,6 +87,7 @@ function getSvgHeight() {
 
 export class TimeseriesChart extends React.Component {
   static propTypes = {
+    annotationsEnabled: PropTypes.bool,
     indexAnnotation: PropTypes.func,
     autoZoomDuration: PropTypes.number,
     contextAggregationInterval: PropTypes.object,
@@ -215,6 +212,7 @@ export class TimeseriesChart extends React.Component {
 
   componentDidMount() {
     const {
+      annotationsEnabled,
       svgWidth
     } = this.props;
 
@@ -247,7 +245,7 @@ export class TimeseriesChart extends React.Component {
     this.fieldFormat = undefined;
 
     // Annotations Brush
-    if (mlAnnotationsEnabled) {
+    if (annotationsEnabled) {
       this.annotateBrush = getAnnotationBrush.call(this);
     }
 
@@ -297,6 +295,7 @@ export class TimeseriesChart extends React.Component {
 
   renderChart() {
     const {
+      annotationsEnabled,
       contextChartData,
       contextForecastData,
       detectorIndex,
@@ -398,7 +397,7 @@ export class TimeseriesChart extends React.Component {
       .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')
@@ -474,6 +473,7 @@ export class TimeseriesChart extends React.Component {
     // as we want to re-render the paths and points when the zoom area changes.
 
     const {
+      annotationsEnabled,
       contextForecastData
     } = this.props;
 
@@ -490,7 +490,7 @@ export class TimeseriesChart extends React.Component {
     this.createZoomInfoElements(zoomGroup, fcsWidth);
 
     // Create the elements for annotations
-    if (mlAnnotationsEnabled) {
+    if (annotationsEnabled) {
       const annotateBrush = this.annotateBrush.bind(this);
 
       fcsGroup.append('g')
@@ -572,6 +572,7 @@ export class TimeseriesChart extends React.Component {
 
   renderFocusChart() {
     const {
+      annotationsEnabled,
       focusAggregationInterval,
       focusAnnotationData,
       focusChartData,
@@ -659,7 +660,7 @@ export class TimeseriesChart extends React.Component {
 
       // 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 revisting to be a more robust normalization
@@ -695,7 +696,7 @@ export class TimeseriesChart extends React.Component {
         .classed('hidden', !showModelBounds);
     }
 
-    if (mlAnnotationsEnabled) {
+    if (annotationsEnabled) {
       renderAnnotations(
         focusChart,
         focusAnnotationData,
@@ -709,7 +710,7 @@ export class TimeseriesChart extends React.Component {
 
       // 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')
@@ -1285,6 +1286,7 @@ export class TimeseriesChart extends React.Component {
 
   showFocusChartTooltip(marker, circle) {
     const {
+      annotationsEnabled,
       modelPlotEnabled
     } = this.props;
 
@@ -1350,7 +1352,7 @@ export class TimeseriesChart extends React.Component {
       contents += `<br/><hr/>Scheduled events:<br/>${marker.scheduledEvents.map(mlEscape).join('<br/>')}`;
     }
 
-    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')}`;
 
@@ -1426,12 +1428,13 @@ export class TimeseriesChart extends React.Component {
   }
 
   render() {
+    const { annotationsEnabled } = this.props;
     const { annotation, isDeleteModalVisible, isFlyoutVisible } = this.state;
 
     return (
       <React.Fragment>
         <div className="ml-timeseries-chart-react" ref={this.setRef.bind(this)} />
-        {mlAnnotationsEnabled && isFlyoutVisible &&
+        {annotationsEnabled && isFlyoutVisible &&
           <React.Fragment>
             <AnnotationFlyout
               annotation={annotation}

x-pack/plugins/ml/public/timeseriesexplorer/components/timeseries_chart/timeseries_chart_directive.js

@@ -27,9 +27,6 @@ const module = uiModules.get('apps/ml');
 
 import { ml } from 'plugins/ml/services/ml_api_service';
 
-import chrome from 'ui/chrome';
-const mlAnnotationsEnabled = chrome.getInjected('mlAnnotationsEnabled', false);
-
 module.directive('mlTimeseriesChart', function ($timeout) {
 
   function link(scope, element) {
@@ -45,6 +42,7 @@ module.directive('mlTimeseriesChart', function ($timeout) {
       svgWidth = Math.max(angular.element('.results-container').width(), 0);
 
       const props = {
+        annotationsEnabled: scope.annotationsEnabled,
         indexAnnotation: ml.annotations.indexAnnotation,
         autoZoomDuration: scope.autoZoomDuration,
         contextAggregationInterval: scope.contextAggregationInterval,
@@ -93,7 +91,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);
     }
@@ -118,6 +117,7 @@ module.directive('mlTimeseriesChart', function ($timeout) {
 
   return {
     scope: {
+      annotationsEnabled: '=',
       selectedJob: '=',
       detectorIndex: '=',
       modelPlotEnabled: '=',

x-pack/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.html

@@ -108,6 +108,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

@@ -56,7 +56,7 @@ import {
 
 
 import chrome from 'ui/chrome';
-const mlAnnotationsEnabled = chrome.getInjected('mlAnnotationsEnabled', false);
+let mlAnnotationsEnabled = chrome.getInjected('mlAnnotationsEnabled', false);
 
 uiRoutes
   .when('/timeseriesexplorer/?', {
@@ -392,6 +392,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;
+          }
         });
       }
     }
@@ -466,19 +472,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;
 
@@ -56,8 +58,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,
@@ -74,13 +74,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,
@@ -108,7 +132,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',
@@ -140,8 +163,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

@@ -72,6 +72,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'));
     }
 
@@ -214,27 +215,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) {