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 index 9e51f4cc5cc72..842efa7bf5aa2 100644 --- 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 index 65a62989cf1b9..8e609599fbda2 100644 --- 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 index dc34522fc5872..91c6da49f02e6 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, @@ -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 += `
${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 f097a55a58477..e4bfe294f91e3 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 @@ -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 index d73d5ab07ba0d..c475a86247b11 100644 --- a/x-pack/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.html +++ b/x-pack/plugins/ml/public/timeseriesexplorer/timeseriesexplorer.html @@ -161,6 +161,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 b8ea8e49964c2..ea16eb8870014 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; @@ -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, 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 aaea6d08c80a5..addcdcb376b93 100644 --- a/x-pack/plugins/ml/server/models/annotation_service/annotation.ts +++ b/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 (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) {