diff --git a/x-pack/plugins/ml/public/explorer/explorer_controller.js b/x-pack/plugins/ml/public/explorer/explorer_controller.js index 2476628187e14..da03387642e7d 100644 --- a/x-pack/plugins/ml/public/explorer/explorer_controller.js +++ b/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 - }); + }).then((resp) => { + $scope.$evalAsync(() => { + const annotationsData = []; + jobIds.forEach((jobId) => { + const jobAnnotations = resp.annotations[jobId]; + if (jobAnnotations !== undefined) { + annotationsData.push(...jobAnnotations); + } + }); - $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; + }); }); - - $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(); }); } } 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 index 94e66606f0af1..1bddcf50d1572 100644 --- 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, @@ -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 += `
Scheduled events:
${marker.scheduledEvents.map(mlEscape).join('
')}`; } - if (mlAnnotationsEnabled && _.has(marker, 'annotation')) { + if (annotationsEnabled && _.has(marker, 'annotation')) { contents = mlEscape(marker.annotation); contents += `
${moment(marker.timestamp).format('MMMM Do YYYY, HH:mm')}`; diff --git a/x-pack/plugins/ml/public/timeseriesexplorer/components/timeseries_chart/timeseries_chart_directive.js b/x-pack/plugins/ml/public/timeseriesexplorer/components/timeseries_chart/timeseries_chart_directive.js index 3e36b799751f0..cb31f1dec82fd 100644 --- a/x-pack/plugins/ml/public/timeseriesexplorer/components/timeseries_chart/timeseries_chart_directive.js +++ b/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: '=', diff --git a/x-pack/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.html b/x-pack/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.html index cb2b92fc7ece9..85c7e40b55b30 100644 --- a/x-pack/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.html +++ b/x-pack/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.html @@ -108,6 +108,7 @@
{ - 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 { diff --git a/x-pack/plugins/ml/server/models/annotation_service/annotation.test.ts b/x-pack/plugins/ml/server/models/annotation_service/annotation.test.ts index 0b7f37137d554..b9874198524a2 100644 --- a/x-pack/plugins/ml/server/models/annotation_service/annotation.test.ts +++ b/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, diff --git a/x-pack/plugins/ml/server/models/annotation_service/annotation.ts b/x-pack/plugins/ml/server/models/annotation_service/annotation.ts index 237f6426dd8d5..2473c4c5c2ff1 100644 --- a/x-pack/plugins/ml/server/models/annotation_service/annotation.ts +++ b/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 (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.`); + } - if (isAnnotations(docs) === false) { - throw Boom.badRequest(`Annotations didn't pass integrity check.`); - } + 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; + }); - docs.forEach((doc: Annotation) => { - const jobId = doc.job_id; - if (typeof obj.annotations[jobId] === 'undefined') { - obj.annotations[jobId] = []; + 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.`); } - obj.annotations[jobId].push(doc); - }); - return obj; + 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) {