Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Commit

Permalink
Simplify historical detector failure states (#368)
Browse files Browse the repository at this point in the history
  • Loading branch information
ohltyler authored Feb 3, 2021
1 parent f9c50c5 commit 34d1567
Show file tree
Hide file tree
Showing 11 changed files with 141 additions and 36 deletions.
1 change: 1 addition & 0 deletions public/models/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export type Detector = {
detectionDateRange?: DetectionDateRange;
taskId?: string;
taskProgress?: number;
taskError?: string;
};

export type DetectorListItem = {
Expand Down
2 changes: 1 addition & 1 deletion public/pages/DetectorsList/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ export const HistoricalDetectorDetail = (
backgroundColor: '#FFF',
};

const callout = getCallout(detector, isStoppingDetector, handleEditAction);
const callout = getCallout(detector, isStoppingDetector);

return (
<React.Fragment>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -169,17 +173,34 @@ describe('<HistoricalDetectorDetail /> 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);
});
});
32 changes: 17 additions & 15 deletions public/pages/HistoricalDetectorDetail/utils/helpers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,14 @@ import {
EuiText,
EuiFlexItem,
EuiProgress,
EuiButton,
} from '@elastic/eui';
import { Detector } from '../../../models/interfaces';
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;
}
Expand Down Expand Up @@ -125,23 +120,30 @@ export const getCallout = (
color="primary"
/>
);
case DETECTOR_STATE.NO_DATA:
case DETECTOR_STATE.FAILED:
return (
<EuiCallOut
title={<EuiText>{detector.taskError}</EuiText>}
iconType="alert"
color="danger"
>
<EuiText size="s">
Try editing the configuration and restarting the detector.
</EuiText>
</EuiCallOut>
);
case DETECTOR_STATE.UNEXPECTED_FAILURE:
return (
<EuiCallOut
title={
<EuiText>
<p>
No data available in the selected date range for the detector.
</p>
The historical detector has failed unexpectedly. Try restarting
the detector.
</EuiText>
}
iconType="alert"
color="danger"
>
<EuiButton color="danger" onClick={() => onEdit()}>
Edit historical detector
</EuiButton>
</EuiCallOut>
></EuiCallOut>
);
default:
return null;
Expand Down
4 changes: 2 additions & 2 deletions public/pages/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<DETECTOR_STATE, DETECTOR_STATE_COLOR>()
Expand All @@ -43,7 +43,7 @@ export const stateToColorMap = new Map<DETECTOR_STATE, DETECTOR_STATE_COLOR>()
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 = [];
Expand Down
1 change: 1 addition & 0 deletions server/models/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export type Detector = {
detectionDateRange?: DetectionDateRange;
taskId?: string;
taskProgress?: number;
taskError?: string;
};

export type Monitor = {
Expand Down
5 changes: 5 additions & 0 deletions server/routes/ad.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import {
appendTaskInfo,
getDetectorResults,
getHistoricalDetectorState,
processTaskError,
} from './utils/adHelpers';
import { isNumber, set } from 'lodash';
import {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -315,6 +319,7 @@ export default class AdService {
? {
taskId: taskId,
taskProgress: taskProgress,
taskError: taskError,
}
: {}),
};
Expand Down
67 changes: 66 additions & 1 deletion server/routes/utils/__tests__/adHelpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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.'
);
});
});
});
27 changes: 18 additions & 9 deletions server/routes/utils/adHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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'
Expand All @@ -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 + '.';
};
5 changes: 3 additions & 2 deletions server/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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: ';

0 comments on commit 34d1567

Please sign in to comment.