Skip to content

Commit fc28db3

Browse files
authored
Fix Warning Message About Custom Result Index on Production Clusters Despite Existing Indices (#759)
Customers were receiving a warning message about the detectors we have created on a production cluster. The message incorrectly warned that the result index for the detector is custom and is not present on the cluster, and would be recreated when real-time or historical detection starts for the detector. However, real-time detection had already been started for these detectors, and the result index did exist. Customers could consistently reproduce the bug, although I only succeeded once when creating a detector immediately after a cluster started. The issue may arise from a potential race condition in the code when the statement finishes before the cat indices call completes (see this code segment https://github.com/opensearch-project/anomaly-detection-dashboards-plugin/blob/main/public/pages/DetectorDetail/containers/DetectorDetail.tsx#L136-L140). This PR adds a check to ensure the cat indices call has finished before concluding that the index does not exist. Additionally, we check if the visible indices are empty. If they are, it likely indicates an issue retrieving existing indices. To be cautious, we choose not to show the error message and consider the result index as not missing. This PR also resolves a warning message encountered during test runs due to the failure to mock HTMLCanvasElement. This was addressed by following the solution provided here: https://stackoverflow.com/questions/48828759/unit-test-raises-error-because-of-getcontext-is-not-implemented console.error Error: Not implemented: HTMLCanvasElement.prototype.getContext (without installing the canvas npm package) at module.exports (/Users/kaituo/code/github/OpenSearch-Dashboards/node_modules/jsdom/lib/jsdom/browser/not-implemented.js:9:17) at HTMLCanvasElementImpl.getContext (/Users/kaituo/code/github/OpenSearch-Dashboards/node_modules/jsdom/lib/jsdom/living/nodes/HTMLCanvasElement-impl.js:42:5) at HTMLCanvasElement.getContext (/Users/kaituo/code/github/OpenSearch-Dashboards/node_modules/jsdom/lib/jsdom/living/generated/HTMLCanvasElement.js:131:58) at Object.23352 (/Users/kaituo/code/github/OpenSearch-Dashboards/plugins/anomaly-detection-dashboards-plugin/node_modules/plotly.js-dist/plotly.js:201984:42) Testing Done: * Conducted end-to-end testing to confirm there is no regression due to these changes. * Added unit tests. Signed-off-by: Kaituo Li <kaituo@amazon.com>
1 parent 9cb2b00 commit fc28db3

File tree

6 files changed

+313
-4
lines changed

6 files changed

+313
-4
lines changed

package.json

+1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
"babel-polyfill": "^6.26.0",
3030
"eslint-plugin-no-unsanitized": "^3.0.2",
3131
"eslint-plugin-prefer-object-spread": "^1.2.1",
32+
"jest-canvas-mock": "^2.5.2",
3233
"lint-staged": "^9.2.0",
3334
"moment": "^2.24.0",
3435
"redux-mock-store": "^1.5.4",

public/pages/DetectorDetail/containers/DetectorDetail.tsx

+27-2
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,34 @@ export const DetectorDetail = (props: DetectorDetailProps) => {
133133
const visibleIndices = useSelector(
134134
(state: AppState) => state.opensearch.indices
135135
) as CatIndex[];
136+
const isCatIndicesRequesting = useSelector(
137+
(state: AppState) => state.opensearch.requesting
138+
) as boolean;
139+
140+
/*
141+
Determine if the result index is missing based on several conditions:
142+
- If the detector is still loading, the result index is not missing.
143+
- If the result index retrieved from the detector is empty, it is not missing.
144+
- If cat indices are being requested, the result index is not missing.
145+
- If visible indices are empty, it is likely there is an issue retrieving existing indices.
146+
To be safe, we'd rather not show the error message and consider the result index not missing.
147+
- If the result index is not found in the visible indices, then it is missing.
148+
*/
136149
const isResultIndexMissing = isLoadingDetector
137150
? false
138151
: isEmpty(get(detector, 'resultIndex', ''))
139152
? false
153+
: isCatIndicesRequesting
154+
? false
155+
: isEmpty(visibleIndices)
156+
? false
140157
: !containsIndex(get(detector, 'resultIndex', ''), visibleIndices);
141158

159+
// debug message: prints visibleIndices if isResultIndexMissing is true
160+
if (isResultIndexMissing) {
161+
console.log(`isResultIndexMissing is true, visibleIndices: ${visibleIndices}, detector result index: ${get(detector, 'resultIndex', '')}`);
162+
}
163+
142164
// String to set in the modal if the realtime detector and/or historical analysis
143165
// are running when the user tries to edit the detector details or model config
144166
const isRTJobRunning = get(detector, 'enabled');
@@ -179,10 +201,12 @@ export const DetectorDetail = (props: DetectorDetailProps) => {
179201
// detector starts, result index recreated or user switches tabs to re-fetch detector)
180202
useEffect(() => {
181203
const getInitialIndices = async () => {
182-
await dispatch(getIndices('', dataSourceId)).catch((error: any) => {
204+
try {
205+
await dispatch(getIndices('', dataSourceId));
206+
} catch (error) {
183207
console.error(error);
184208
core.notifications.toasts.addDanger('Error getting all indices');
185-
});
209+
}
186210
};
187211
// only need to check if indices exist after detector finishes loading
188212
if (!isLoadingDetector) {
@@ -464,6 +488,7 @@ export const DetectorDetail = (props: DetectorDetailProps) => {
464488
)}', but is not found in the cluster. The index will be recreated when you start a real-time or historical job.`}
465489
color="danger"
466490
iconType="alert"
491+
data-test-subj="missingResultIndexCallOut"
467492
></EuiCallOut>
468493
) : null}
469494

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,275 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*
8+
* Modifications Copyright OpenSearch Contributors. See
9+
* GitHub history for details.
10+
*/
11+
12+
import React from 'react';
13+
import { render, screen } from '@testing-library/react';
14+
import { DetectorDetail, DetectorRouterProps } from '../DetectorDetail';
15+
import { Provider } from 'react-redux';
16+
import {
17+
HashRouter as Router,
18+
RouteComponentProps,
19+
Route,
20+
Switch,
21+
Redirect,
22+
} from 'react-router-dom';
23+
import configureMockStore from 'redux-mock-store';
24+
import thunk from 'redux-thunk';
25+
import { httpClientMock, coreServicesMock } from '../../../../../test/mocks';
26+
import { CoreServicesContext } from '../../../../components/CoreServices/CoreServices';
27+
import { getRandomDetector } from '../../../../redux/reducers/__tests__/utils';
28+
import { useFetchDetectorInfo } from '../../../CreateDetectorSteps/hooks/useFetchDetectorInfo';
29+
30+
jest.mock('../../hooks/useFetchMonitorInfo');
31+
32+
//jest.mock('../../../CreateDetectorSteps/hooks/useFetchDetectorInfo');
33+
jest.mock('../../../CreateDetectorSteps/hooks/useFetchDetectorInfo', () => ({
34+
// The jest.mock function is used at the top level of the test file to mock the entire module.
35+
// Within each test, the mock implementation for useFetchDetectorInfo is set using jest.Mock.
36+
// This ensures that the hook returns the desired values for each test case.
37+
useFetchDetectorInfo: jest.fn(),
38+
}));
39+
40+
jest.mock('../../../../services', () => ({
41+
...jest.requireActual('../../../../services'),
42+
43+
getDataSourceEnabled: () => ({
44+
enabled: false,
45+
}),
46+
}));
47+
48+
const detectorId = '4QY4YHEB5W9C7vlb3Mou';
49+
50+
// Configure the mock store
51+
const middlewares = [thunk];
52+
const mockStore = configureMockStore(middlewares);
53+
54+
const renderWithRouter = (detectorId: string, initialState: any) => ({
55+
...render(
56+
<Provider store={mockStore(initialState)}>
57+
<Router>
58+
<Switch>
59+
<Route
60+
path={`/detectors/${detectorId}/results`}
61+
render={(props: RouteComponentProps<DetectorRouterProps>) => {
62+
const testProps = {
63+
...props,
64+
match: {
65+
params: { detectorId: detectorId },
66+
isExact: false,
67+
path: '',
68+
url: '',
69+
},
70+
};
71+
return (
72+
<CoreServicesContext.Provider value={coreServicesMock}>
73+
<DetectorDetail {...testProps} />
74+
</CoreServicesContext.Provider>
75+
);
76+
}}
77+
/>
78+
<Redirect from="/" to={`/detectors/${detectorId}/results`} />
79+
</Switch>
80+
</Router>
81+
</Provider>
82+
),
83+
});
84+
85+
const resultIndex = 'opensearch-ad-plugin-result-test-query2';
86+
87+
describe('detector detail', () => {
88+
beforeEach(() => {
89+
jest.clearAllMocks();
90+
});
91+
92+
test('detector info still loading', () => {
93+
const detectorInfo = {
94+
detector: getRandomDetector(true, resultIndex),
95+
hasError: false,
96+
isLoadingDetector: true,
97+
errorMessage: undefined,
98+
};
99+
100+
(useFetchDetectorInfo as jest.Mock).mockImplementation(() => detectorInfo);
101+
102+
const initialState = {
103+
opensearch: {
104+
indices: [{ health: 'green', index: resultIndex }],
105+
requesting: false,
106+
},
107+
ad: {
108+
detectors: {},
109+
},
110+
alerting: {
111+
monitors: {},
112+
},
113+
};
114+
115+
renderWithRouter(detectorId, initialState);
116+
const element = screen.queryByTestId('missingResultIndexCallOut');
117+
118+
// Assert that the element is not in the document
119+
expect(element).toBeNull();
120+
});
121+
122+
test('detector has no result index', () => {
123+
const detectorInfo = {
124+
detector: getRandomDetector(true, undefined),
125+
hasError: false,
126+
isLoadingDetector: true,
127+
errorMessage: undefined,
128+
};
129+
130+
(useFetchDetectorInfo as jest.Mock).mockImplementation(() => detectorInfo);
131+
132+
const initialState = {
133+
opensearch: {
134+
indices: [{ health: 'green', index: resultIndex }],
135+
requesting: false,
136+
},
137+
ad: {
138+
detectors: {},
139+
},
140+
alerting: {
141+
monitors: {},
142+
},
143+
};
144+
145+
renderWithRouter(detectorId, initialState);
146+
const element = screen.queryByTestId('missingResultIndexCallOut');
147+
148+
// Assert that the element is not in the document
149+
expect(element).toBeNull();
150+
});
151+
152+
test('cat indices are being requested', () => {
153+
const detectorInfo = {
154+
detector: getRandomDetector(true, resultIndex),
155+
hasError: false,
156+
isLoadingDetector: false,
157+
errorMessage: undefined,
158+
};
159+
160+
(useFetchDetectorInfo as jest.Mock).mockImplementation(() => detectorInfo);
161+
162+
const initialState = {
163+
opensearch: {
164+
indices: [],
165+
requesting: true,
166+
},
167+
ad: {
168+
detectors: {},
169+
},
170+
alerting: {
171+
monitors: {},
172+
},
173+
};
174+
175+
renderWithRouter(detectorId, initialState);
176+
const element = screen.queryByTestId('missingResultIndexCallOut');
177+
178+
// Assert that the element is not in the document
179+
expect(element).toBeNull();
180+
});
181+
182+
test('visible indices are empty', () => {
183+
const detectorInfo = {
184+
detector: getRandomDetector(true, resultIndex),
185+
hasError: false,
186+
isLoadingDetector: false,
187+
errorMessage: undefined,
188+
};
189+
190+
(useFetchDetectorInfo as jest.Mock).mockImplementation(() => detectorInfo);
191+
192+
const initialState = {
193+
opensearch: {
194+
indices: [],
195+
requesting: false,
196+
},
197+
ad: {
198+
detectors: {},
199+
},
200+
alerting: {
201+
monitors: {},
202+
},
203+
};
204+
205+
renderWithRouter(detectorId, initialState);
206+
const element = screen.queryByTestId('missingResultIndexCallOut');
207+
208+
// Assert that the element is not in the document
209+
expect(element).toBeNull();
210+
});
211+
212+
test('the result index is not found in the visible indices', () => {
213+
const detectorInfo = {
214+
detector: getRandomDetector(true, resultIndex),
215+
hasError: false,
216+
isLoadingDetector: false,
217+
errorMessage: undefined,
218+
};
219+
220+
(useFetchDetectorInfo as jest.Mock).mockImplementation(() => detectorInfo);
221+
222+
const initialState = {
223+
opensearch: {
224+
indices: [{ health: 'green', index: '.kibana_-962704462_v992471_1' }],
225+
requesting: false,
226+
},
227+
ad: {
228+
detectors: {},
229+
},
230+
alerting: {
231+
monitors: {},
232+
},
233+
};
234+
235+
renderWithRouter(detectorId, initialState);
236+
const element = screen.queryByTestId('missingResultIndexCallOut');
237+
238+
// Assert that the element is in the document
239+
expect(element).not.toBeNull();
240+
});
241+
242+
test('the result index is found in the visible indices', () => {
243+
const detector = getRandomDetector(true, resultIndex);
244+
245+
// Set up the mock implementation for useFetchDetectorInfo
246+
(useFetchDetectorInfo as jest.Mock).mockImplementation(() => ({
247+
detector: detector,
248+
hasError: false,
249+
isLoadingDetector: false,
250+
errorMessage: undefined,
251+
}));
252+
253+
const initialState = {
254+
opensearch: {
255+
indices: [
256+
{ health: 'green', index: '.kibana_-962704462_v992471_1' },
257+
{ health: 'green', index: resultIndex },
258+
],
259+
requesting: false,
260+
},
261+
ad: {
262+
detectors: {},
263+
},
264+
alerting: {
265+
monitors: {},
266+
},
267+
};
268+
269+
renderWithRouter(detectorId, initialState);
270+
const element = screen.queryByTestId('missingResultIndexCallOut');
271+
272+
// Assert that the element is not in the document
273+
expect(element).toBeNull();
274+
});
275+
});

public/redux/reducers/__tests__/utils.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*/
1111

1212
import chance from 'chance';
13-
import { snakeCase } from 'lodash';
13+
import { isEmpty, snakeCase } from 'lodash';
1414
import {
1515
Detector,
1616
FeatureAttributes,
@@ -82,7 +82,10 @@ const getUIMetadata = (features: FeatureAttributes[]) => {
8282
} as UiMetaData;
8383
};
8484

85-
export const getRandomDetector = (isCreate: boolean = true): Detector => {
85+
export const getRandomDetector = (
86+
isCreate: boolean = true,
87+
customResultIndex: string = ''
88+
): Detector => {
8689
const features = new Array(detectorFaker.natural({ min: 1, max: 5 }))
8790
.fill(null)
8891
.map(() => getRandomFeature(isCreate ? false : true));
@@ -116,6 +119,7 @@ export const getRandomDetector = (isCreate: boolean = true): Detector => {
116119
curState: DETECTOR_STATE.INIT,
117120
stateError: '',
118121
shingleSize: DEFAULT_SHINGLE_SIZE,
122+
resultIndex: isEmpty(customResultIndex) ? undefined : customResultIndex
119123
};
120124
};
121125

test/jest.config.js

+3
Original file line numberDiff line numberDiff line change
@@ -54,4 +54,7 @@ module.exports = {
5454
'^.+\\.svg$': '<rootDir>/test/mocks/transformMock.ts',
5555
'^.+\\.html$': '<rootDir>/test/mocks/transformMock.ts',
5656
},
57+
setupFiles: [
58+
"jest-canvas-mock"
59+
]
5760
};

test/setupTests.ts

+1
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@
1010
*/
1111

1212
require('babel-polyfill');
13+
import 'jest-canvas-mock';

0 commit comments

Comments
 (0)