From 34d1567cc8abe661fd5696436924e7553a4dc730 Mon Sep 17 00:00:00 2001 From: Tyler Ohlsen Date: Wed, 3 Feb 2021 13:26:45 -0800 Subject: [PATCH] Simplify historical detector failure states (#368) --- public/models/interfaces.ts | 1 + public/pages/DetectorsList/utils/helpers.ts | 2 +- .../HistoricalDetectorDetail.tsx | 2 +- .../HistoricalDetectorDetail.test.tsx | 31 +++++++-- .../utils/helpers.tsx | 32 ++++----- public/pages/utils/constants.ts | 4 +- server/models/types.ts | 1 + server/routes/ad.ts | 5 ++ .../routes/utils/__tests__/adHelpers.test.ts | 67 ++++++++++++++++++- server/routes/utils/adHelpers.ts | 27 +++++--- server/utils/constants.ts | 5 +- 11 files changed, 141 insertions(+), 36 deletions(-) diff --git a/public/models/interfaces.ts b/public/models/interfaces.ts index c65cc6e3..c95ad7fa 100644 --- a/public/models/interfaces.ts +++ b/public/models/interfaces.ts @@ -113,6 +113,7 @@ export type Detector = { detectionDateRange?: DetectionDateRange; taskId?: string; taskProgress?: number; + taskError?: string; }; export type DetectorListItem = { diff --git a/public/pages/DetectorsList/utils/helpers.ts b/public/pages/DetectorsList/utils/helpers.ts index 07bf41db..d4268773 100644 --- a/public/pages/DetectorsList/utils/helpers.ts +++ b/public/pages/DetectorsList/utils/helpers.ts @@ -63,7 +63,7 @@ export const getDetectorStateOptions = () => { .filter( (option) => option.label !== DETECTOR_STATE.FINISHED && - option.label !== DETECTOR_STATE.NO_DATA + option.label !== DETECTOR_STATE.FAILED ); }; diff --git a/public/pages/HistoricalDetectorDetail/containers/HistoricalDetectorDetail/HistoricalDetectorDetail.tsx b/public/pages/HistoricalDetectorDetail/containers/HistoricalDetectorDetail/HistoricalDetectorDetail.tsx index f4c359af..7dc1030f 100644 --- a/public/pages/HistoricalDetectorDetail/containers/HistoricalDetectorDetail/HistoricalDetectorDetail.tsx +++ b/public/pages/HistoricalDetectorDetail/containers/HistoricalDetectorDetail/HistoricalDetectorDetail.tsx @@ -298,7 +298,7 @@ export const HistoricalDetectorDetail = ( backgroundColor: '#FFF', }; - const callout = getCallout(detector, isStoppingDetector, handleEditAction); + const callout = getCallout(detector, isStoppingDetector); return ( diff --git a/public/pages/HistoricalDetectorDetail/containers/HistoricalDetectorDetail/__tests__/HistoricalDetectorDetail.test.tsx b/public/pages/HistoricalDetectorDetail/containers/HistoricalDetectorDetail/__tests__/HistoricalDetectorDetail.test.tsx index 01697cb5..fb30d614 100644 --- a/public/pages/HistoricalDetectorDetail/containers/HistoricalDetectorDetail/__tests__/HistoricalDetectorDetail.test.tsx +++ b/public/pages/HistoricalDetectorDetail/containers/HistoricalDetectorDetail/__tests__/HistoricalDetectorDetail.test.tsx @@ -35,8 +35,12 @@ const ACTIONS_BUTTON_TEXT = 'Actions'; const STOPPED_CALLOUT_TEXT = 'The historical detector is stopped'; const INIT_CALLOUT_TEXT = 'Initializing the historical detector'; const RUNNING_CALLOUT_TEXT = 'Running the historical detector'; -const NO_DATA_CALLOUT_TEXT = - 'No data available in the selected date range for the detector.'; +const FAILED_CALLOUT_TEXT = 'Some failure.'; +const FAILED_CALLOUT_DETAIL_TEXT = + 'Try editing the configuration and restarting the detector.'; +const UNEXPECTED_FAILURE_CALLOUT_TEXT = + 'The historical detector has failed unexpectedly. Try restarting the detector.'; +const FAILURE_STACK_TRACE = `at some.stack.trace(SomeFile.java:50)`; const START_DETECTOR_BUTTON_TEXT = 'Start historical detector'; const STOP_DETECTOR_BUTTON_TEXT = 'Stop historical detector'; @@ -169,17 +173,34 @@ describe(' spec', () => { getByText('Finished'); getByText(START_DETECTOR_BUTTON_TEXT); }); - test('shows correct callout when detector has no data', async () => { + test('shows correct callout when detector failed', async () => { httpClientMock.get = jest.fn().mockResolvedValue({ ok: true, response: { ...TEST_DETECTOR, - curState: DETECTOR_STATE.NO_DATA, + curState: DETECTOR_STATE.FAILED, + taskError: FAILED_CALLOUT_TEXT, }, }); const { getByText } = renderWithRouter(); await wait(); - getByText(NO_DATA_CALLOUT_TEXT); + getByText(FAILED_CALLOUT_TEXT); + getByText(FAILED_CALLOUT_DETAIL_TEXT); + getByText(START_DETECTOR_BUTTON_TEXT); + }); + test('shows correct callout when detector unexpectedly failed', async () => { + httpClientMock.get = jest.fn().mockResolvedValue({ + ok: true, + response: { + ...TEST_DETECTOR, + curState: DETECTOR_STATE.UNEXPECTED_FAILURE, + taskError: FAILURE_STACK_TRACE, + }, + }); + const { getByText, queryByText } = renderWithRouter(); + await wait(); + expect(queryByText(FAILURE_STACK_TRACE)).toBeNull(); + getByText(UNEXPECTED_FAILURE_CALLOUT_TEXT); getByText(START_DETECTOR_BUTTON_TEXT); }); }); diff --git a/public/pages/HistoricalDetectorDetail/utils/helpers.tsx b/public/pages/HistoricalDetectorDetail/utils/helpers.tsx index 428e8fcd..4d27ee21 100644 --- a/public/pages/HistoricalDetectorDetail/utils/helpers.tsx +++ b/public/pages/HistoricalDetectorDetail/utils/helpers.tsx @@ -23,7 +23,6 @@ import { EuiText, EuiFlexItem, EuiProgress, - EuiButton, } from '@elastic/eui'; import { Detector } from '../../../models/interfaces'; import { DETECTOR_STATE } from '../../../../server/utils/constants'; @@ -31,11 +30,7 @@ import { DETECTOR_STATE } from '../../../../server/utils/constants'; export const waitForMs = (ms: number) => new Promise((res) => setTimeout(res, ms)); -export const getCallout = ( - detector: Detector, - isStoppingDetector: boolean, - onEdit: () => any -) => { +export const getCallout = (detector: Detector, isStoppingDetector: boolean) => { if (!detector || !detector.curState) { return null; } @@ -125,23 +120,30 @@ export const getCallout = ( color="primary" /> ); - case DETECTOR_STATE.NO_DATA: + case DETECTOR_STATE.FAILED: + return ( + {detector.taskError}} + iconType="alert" + color="danger" + > + + Try editing the configuration and restarting the detector. + + + ); + case DETECTOR_STATE.UNEXPECTED_FAILURE: return ( -

- No data available in the selected date range for the detector. -

+ The historical detector has failed unexpectedly. Try restarting + the detector. } iconType="alert" color="danger" - > - onEdit()}> - Edit historical detector - - + > ); default: return null; diff --git a/public/pages/utils/constants.ts b/public/pages/utils/constants.ts index 208e561b..dbc739a0 100644 --- a/public/pages/utils/constants.ts +++ b/public/pages/utils/constants.ts @@ -29,7 +29,7 @@ export enum DETECTOR_STATE_COLOR { FEATURE_REQUIRED = '#98A2B3', INIT_FAILURE = '#F66', UNEXPECTED_FAILURE = '#F66', - NO_DATA = '#F66', + FAILED = '#F66', } export const stateToColorMap = new Map() @@ -43,7 +43,7 @@ export const stateToColorMap = new Map() DETECTOR_STATE.UNEXPECTED_FAILURE, DETECTOR_STATE_COLOR.UNEXPECTED_FAILURE ) - .set(DETECTOR_STATE.NO_DATA, DETECTOR_STATE_COLOR.NO_DATA); + .set(DETECTOR_STATE.FAILED, DETECTOR_STATE_COLOR.FAILED); export const ALL_DETECTOR_STATES = []; export const ALL_INDICES = []; diff --git a/server/models/types.ts b/server/models/types.ts index 25fe8545..f35463df 100644 --- a/server/models/types.ts +++ b/server/models/types.ts @@ -71,6 +71,7 @@ export type Detector = { detectionDateRange?: DetectionDateRange; taskId?: string; taskProgress?: number; + taskError?: string; }; export type Monitor = { diff --git a/server/routes/ad.ts b/server/routes/ad.ts index 2ed2c486..bba0aa75 100644 --- a/server/routes/ad.ts +++ b/server/routes/ad.ts @@ -57,6 +57,7 @@ import { appendTaskInfo, getDetectorResults, getHistoricalDetectorState, + processTaskError, } from './utils/adHelpers'; import { isNumber, set } from 'lodash'; import { @@ -248,6 +249,9 @@ export default class AdService { 'anomaly_detection_task.task_progress', null ); + const taskError = processTaskError( + get(response, 'anomaly_detection_task.error', '') + ); // Getting detector state, depending on realtime or historical let detectorState; @@ -315,6 +319,7 @@ export default class AdService { ? { taskId: taskId, taskProgress: taskProgress, + taskError: taskError, } : {}), }; diff --git a/server/routes/utils/__tests__/adHelpers.test.ts b/server/routes/utils/__tests__/adHelpers.test.ts index ed12e69b..003412df 100644 --- a/server/routes/utils/__tests__/adHelpers.test.ts +++ b/server/routes/utils/__tests__/adHelpers.test.ts @@ -13,12 +13,18 @@ * permissions and limitations under the License. */ -import { SORT_DIRECTION } from '../../../utils/constants'; +import { + SORT_DIRECTION, + ES_EXCEPTION_PREFIX, + DETECTOR_STATE, +} from '../../../utils/constants'; import { convertDetectorKeysToCamelCase, convertDetectorKeysToSnakeCase, getResultAggregationQuery, convertPreviewInputKeysToSnakeCase, + processTaskError, + getHistoricalDetectorState, } from '../adHelpers'; describe('adHelpers', () => { @@ -573,4 +579,63 @@ describe('adHelpers', () => { }); }); }); + describe('getHistoricalDetectorState', () => { + test('should convert to disabled if no task', () => { + const task = null; + expect(getHistoricalDetectorState(task)).toEqual(DETECTOR_STATE.DISABLED); + }); + test('should convert to unexpected failure if failed and error message is stack trace', () => { + const task = { + state: 'FAILED', + error: `at some.stack.trace(SomeFile.java:50)`, + }; + expect(getHistoricalDetectorState(task)).toEqual( + DETECTOR_STATE.UNEXPECTED_FAILURE + ); + }); + test('should convert to failed if failed and error message is not stack trace', () => { + const task = { + state: 'FAILED', + error: 'Some regular error message', + }; + expect(getHistoricalDetectorState(task)).toEqual(DETECTOR_STATE.FAILED); + }); + test('should convert to initializing if in created state', () => { + const task = { + state: 'CREATED', + }; + expect(getHistoricalDetectorState(task)).toEqual(DETECTOR_STATE.INIT); + }); + test('should convert to disabled if in stopped state', () => { + const task = { + state: 'STOPPED', + }; + expect(getHistoricalDetectorState(task)).toEqual(DETECTOR_STATE.DISABLED); + }); + test('should not convert if in running state', () => { + const task = { + state: 'RUNNING', + }; + expect(getHistoricalDetectorState(task)).toEqual(DETECTOR_STATE.RUNNING); + }); + test('should not convert if in finished state', () => { + const task = { + state: 'FINISHED', + }; + expect(getHistoricalDetectorState(task)).toEqual(DETECTOR_STATE.FINISHED); + }); + }); + describe('processTaskError', () => { + test('should add punctuation if none exists', () => { + expect(processTaskError('Some failure')).toEqual('Some failure.'); + }); + test('should not add punctuation if it exists', () => { + expect(processTaskError('Some failure.')).toEqual('Some failure.'); + }); + test('should remove ES exception prefix if it exists', () => { + expect(processTaskError(ES_EXCEPTION_PREFIX + 'Some failure.')).toEqual( + 'Some failure.' + ); + }); + }); }); diff --git a/server/routes/utils/adHelpers.ts b/server/routes/utils/adHelpers.ts index 1f23f561..c4269008 100644 --- a/server/routes/utils/adHelpers.ts +++ b/server/routes/utils/adHelpers.ts @@ -17,7 +17,11 @@ import { get, omit, cloneDeep, isEmpty } from 'lodash'; import { AnomalyResults } from '../../models/interfaces'; import { GetDetectorsQueryParams, Detector } from '../../models/types'; import { mapKeysDeep, toCamel, toSnake } from '../../utils/helpers'; -import { DETECTOR_STATE, NO_DATA_ERROR_MSG } from '../../utils/constants'; +import { + DETECTOR_STATE, + STACK_TRACE_PATTERN, + ES_EXCEPTION_PREFIX, +} from '../../utils/constants'; import { InitProgress } from '../../models/interfaces'; export const convertDetectorKeysToSnakeCase = (payload: any) => { @@ -326,17 +330,15 @@ export const appendTaskInfo = ( }; // Following checks/transformations need to be made here: -// (1) set to DISABLED if there is no existing task for this detector -// (2) set to NO_DATA if the task failed with NO_DATA_ERROR_MSG -// (3) set to UNEXPECTED_FAILURE if the task is in a FAILED state to stay consistent -// (4) set to INIT if the task is in a CREATED state -// (5) set to DISABLED if the task is in a STOPPED state +// - set to DISABLED if there is no existing task for this detector +// - set to UNEXPECTED_FAILURE if the task is in a FAILED state & the error message is unreadable / is a stack trace +// - set to INIT if the task is in a CREATED state +// - set to DISABLED if the task is in a STOPPED state export const getHistoricalDetectorState = (task: any) => { const state = get(task, 'state', 'DISABLED'); + const errorMessage = processTaskError(get(task, 'error', '')); const updatedState = - state === 'FAILED' && task.error === NO_DATA_ERROR_MSG - ? 'NO_DATA' - : state === 'FAILED' + state === 'FAILED' && errorMessage.includes(STACK_TRACE_PATTERN) ? 'UNEXPECTED_FAILURE' : state === 'CREATED' ? 'INIT' @@ -346,3 +348,10 @@ export const getHistoricalDetectorState = (task: any) => { //@ts-ignore return DETECTOR_STATE[updatedState]; }; + +export const processTaskError = (error: string) => { + const errorWithPrefixRemoved = error.replace(ES_EXCEPTION_PREFIX, ''); + return errorWithPrefixRemoved.endsWith('.') + ? errorWithPrefixRemoved + : errorWithPrefixRemoved + '.'; +}; diff --git a/server/utils/constants.ts b/server/utils/constants.ts index 19d8dc2f..71ac7a33 100644 --- a/server/utils/constants.ts +++ b/server/utils/constants.ts @@ -69,7 +69,7 @@ export enum DETECTOR_STATE { FEATURE_REQUIRED = 'Feature required', INIT_FAILURE = 'Initialization failure', UNEXPECTED_FAILURE = 'Unexpected failure', - NO_DATA = 'No data', + FAILED = 'Failed', } export enum SAMPLE_TYPE { @@ -85,4 +85,5 @@ export const ENTITY_NAME_PATH_FIELD = 'entity.name'; export const DOC_COUNT_FIELD = 'doc_count'; export const KEY_FIELD = 'key'; -export const NO_DATA_ERROR_MSG = 'There is no data in the detection date range'; +export const STACK_TRACE_PATTERN = '.java:'; +export const ES_EXCEPTION_PREFIX = 'org.elasticsearch.ElasticsearchException: ';