From 6064ca8783cf512c94ae3f137396dcd0c435e3ac Mon Sep 17 00:00:00 2001 From: Kaituo Li Date: Thu, 16 May 2024 14:04:10 -0700 Subject: [PATCH] Fix Warning Message About Custom Result Index on Production Clusters Despite Existing Indices 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 --- package.json | 1 + .../containers/DetectorDetail.tsx | 23 ++ .../__tests__/CustomIndexErrorMsg.test.tsx | 275 ++++++++++++++++++ public/redux/reducers/__tests__/utils.ts | 8 +- test/jest.config.js | 3 + test/setupTests.ts | 1 + 6 files changed, 309 insertions(+), 2 deletions(-) create mode 100644 public/pages/DetectorDetail/containers/__tests__/CustomIndexErrorMsg.test.tsx diff --git a/package.json b/package.json index be4f3767..d09c1a22 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "babel-polyfill": "^6.26.0", "eslint-plugin-no-unsanitized": "^3.0.2", "eslint-plugin-prefer-object-spread": "^1.2.1", + "jest-canvas-mock": "^2.5.2", "lint-staged": "^9.2.0", "moment": "^2.24.0", "redux-mock-store": "^1.5.4", diff --git a/public/pages/DetectorDetail/containers/DetectorDetail.tsx b/public/pages/DetectorDetail/containers/DetectorDetail.tsx index f2184e15..0c7a0e3e 100644 --- a/public/pages/DetectorDetail/containers/DetectorDetail.tsx +++ b/public/pages/DetectorDetail/containers/DetectorDetail.tsx @@ -133,12 +133,34 @@ export const DetectorDetail = (props: DetectorDetailProps) => { const visibleIndices = useSelector( (state: AppState) => state.opensearch.indices ) as CatIndex[]; + const isCatIndicesRequesting = useSelector( + (state: AppState) => state.opensearch.requesting + ) as boolean; + + /* + Determine if the result index is missing based on several conditions: + - If the detector is still loading, the result index is not missing. + - If the result index retrieved from the detector is empty, it is not missing. + - If cat indices are being requested, the result index is not missing. + - If visible indices are empty, it is likely there is an issue retrieving existing indices. + To be safe, we'd rather not show the error message and consider the result index not missing. + - If the result index is not found in the visible indices, then it is missing. + */ const isResultIndexMissing = isLoadingDetector ? false : isEmpty(get(detector, 'resultIndex', '')) ? false + : isCatIndicesRequesting + ? false + : isEmpty(visibleIndices) + ? false : !containsIndex(get(detector, 'resultIndex', ''), visibleIndices); + // debug message: prints visibleIndices if isResultIndexMissing is true + if (isResultIndexMissing) { + console.log(`isResultIndexMissing is true, visibleIndices: ${visibleIndices}, detector result index: ${get(detector, 'resultIndex', '')}`); + } + // String to set in the modal if the realtime detector and/or historical analysis // are running when the user tries to edit the detector details or model config const isRTJobRunning = get(detector, 'enabled'); @@ -464,6 +486,7 @@ export const DetectorDetail = (props: DetectorDetailProps) => { )}', but is not found in the cluster. The index will be recreated when you start a real-time or historical job.`} color="danger" iconType="alert" + data-test-subj="missingResultIndexCallOut" > ) : null} diff --git a/public/pages/DetectorDetail/containers/__tests__/CustomIndexErrorMsg.test.tsx b/public/pages/DetectorDetail/containers/__tests__/CustomIndexErrorMsg.test.tsx new file mode 100644 index 00000000..3848fa2c --- /dev/null +++ b/public/pages/DetectorDetail/containers/__tests__/CustomIndexErrorMsg.test.tsx @@ -0,0 +1,275 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import { DetectorDetail, DetectorRouterProps } from '../DetectorDetail'; +import { Provider } from 'react-redux'; +import { + HashRouter as Router, + RouteComponentProps, + Route, + Switch, + Redirect, +} from 'react-router-dom'; +import configureMockStore from 'redux-mock-store'; +import thunk from 'redux-thunk'; +import { httpClientMock, coreServicesMock } from '../../../../../test/mocks'; +import { CoreServicesContext } from '../../../../components/CoreServices/CoreServices'; +import { getRandomDetector } from '../../../../redux/reducers/__tests__/utils'; +import { useFetchDetectorInfo } from '../../../CreateDetectorSteps/hooks/useFetchDetectorInfo'; + +jest.mock('../../hooks/useFetchMonitorInfo'); + +//jest.mock('../../../CreateDetectorSteps/hooks/useFetchDetectorInfo'); +jest.mock('../../../CreateDetectorSteps/hooks/useFetchDetectorInfo', () => ({ + // The jest.mock function is used at the top level of the test file to mock the entire module. + // Within each test, the mock implementation for useFetchDetectorInfo is set using jest.Mock. + // This ensures that the hook returns the desired values for each test case. + useFetchDetectorInfo: jest.fn(), +})); + +jest.mock('../../../../services', () => ({ + ...jest.requireActual('../../../../services'), + + getDataSourceEnabled: () => ({ + enabled: false, + }), +})); + +const detectorId = '4QY4YHEB5W9C7vlb3Mou'; + +// Configure the mock store +const middlewares = [thunk]; +const mockStore = configureMockStore(middlewares); + +const renderWithRouter = (detectorId: string, initialState: any) => ({ + ...render( + + + + ) => { + const testProps = { + ...props, + match: { + params: { detectorId: detectorId }, + isExact: false, + path: '', + url: '', + }, + }; + return ( + + + + ); + }} + /> + + + + + ), +}); + +const resultIndex = 'opensearch-ad-plugin-result-test-query2'; + +describe('detector detail', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('detector info still loading', () => { + const detectorInfo = { + detector: getRandomDetector(true, resultIndex), + hasError: false, + isLoadingDetector: true, + errorMessage: undefined, + }; + + (useFetchDetectorInfo as jest.Mock).mockImplementation(() => detectorInfo); + + const initialState = { + opensearch: { + indices: [{ health: 'green', index: resultIndex }], + requesting: false, + }, + ad: { + detectors: {}, + }, + alerting: { + monitors: {}, + }, + }; + + renderWithRouter(detectorId, initialState); + const element = screen.queryByTestId('missingResultIndexCallOut'); + + // Assert that the element is not in the document + expect(element).toBeNull(); + }); + + test('detector has no result index', () => { + const detectorInfo = { + detector: getRandomDetector(true, undefined), + hasError: false, + isLoadingDetector: true, + errorMessage: undefined, + }; + + (useFetchDetectorInfo as jest.Mock).mockImplementation(() => detectorInfo); + + const initialState = { + opensearch: { + indices: [{ health: 'green', index: resultIndex }], + requesting: false, + }, + ad: { + detectors: {}, + }, + alerting: { + monitors: {}, + }, + }; + + renderWithRouter(detectorId, initialState); + const element = screen.queryByTestId('missingResultIndexCallOut'); + + // Assert that the element is not in the document + expect(element).toBeNull(); + }); + + test('cat indices are being requested', () => { + const detectorInfo = { + detector: getRandomDetector(true, resultIndex), + hasError: false, + isLoadingDetector: false, + errorMessage: undefined, + }; + + (useFetchDetectorInfo as jest.Mock).mockImplementation(() => detectorInfo); + + const initialState = { + opensearch: { + indices: [], + requesting: true, + }, + ad: { + detectors: {}, + }, + alerting: { + monitors: {}, + }, + }; + + renderWithRouter(detectorId, initialState); + const element = screen.queryByTestId('missingResultIndexCallOut'); + + // Assert that the element is not in the document + expect(element).toBeNull(); + }); + + test('visible indices are empty', () => { + const detectorInfo = { + detector: getRandomDetector(true, resultIndex), + hasError: false, + isLoadingDetector: false, + errorMessage: undefined, + }; + + (useFetchDetectorInfo as jest.Mock).mockImplementation(() => detectorInfo); + + const initialState = { + opensearch: { + indices: [], + requesting: false, + }, + ad: { + detectors: {}, + }, + alerting: { + monitors: {}, + }, + }; + + renderWithRouter(detectorId, initialState); + const element = screen.queryByTestId('missingResultIndexCallOut'); + + // Assert that the element is not in the document + expect(element).toBeNull(); + }); + + test('the result index is not found in the visible indices', () => { + const detectorInfo = { + detector: getRandomDetector(true, resultIndex), + hasError: false, + isLoadingDetector: false, + errorMessage: undefined, + }; + + (useFetchDetectorInfo as jest.Mock).mockImplementation(() => detectorInfo); + + const initialState = { + opensearch: { + indices: [{ health: 'green', index: '.kibana_-962704462_v992471_1' }], + requesting: false, + }, + ad: { + detectors: {}, + }, + alerting: { + monitors: {}, + }, + }; + + renderWithRouter(detectorId, initialState); + const element = screen.queryByTestId('missingResultIndexCallOut'); + + // Assert that the element is in the document + expect(element).not.toBeNull(); + }); + + test('the result index is found in the visible indices', () => { + const detector = getRandomDetector(true, resultIndex); + + // Set up the mock implementation for useFetchDetectorInfo + (useFetchDetectorInfo as jest.Mock).mockImplementation(() => ({ + detector: detector, + hasError: false, + isLoadingDetector: false, + errorMessage: undefined, + })); + + const initialState = { + opensearch: { + indices: [ + { health: 'green', index: '.kibana_-962704462_v992471_1' }, + { health: 'green', index: resultIndex }, + ], + requesting: false, + }, + ad: { + detectors: {}, + }, + alerting: { + monitors: {}, + }, + }; + + renderWithRouter(detectorId, initialState); + const element = screen.queryByTestId('missingResultIndexCallOut'); + + // Assert that the element is not in the document + expect(element).toBeNull(); + }); +}); diff --git a/public/redux/reducers/__tests__/utils.ts b/public/redux/reducers/__tests__/utils.ts index a85f04d2..797d5036 100644 --- a/public/redux/reducers/__tests__/utils.ts +++ b/public/redux/reducers/__tests__/utils.ts @@ -10,7 +10,7 @@ */ import chance from 'chance'; -import { snakeCase } from 'lodash'; +import { isEmpty, snakeCase } from 'lodash'; import { Detector, FeatureAttributes, @@ -82,7 +82,10 @@ const getUIMetadata = (features: FeatureAttributes[]) => { } as UiMetaData; }; -export const getRandomDetector = (isCreate: boolean = true): Detector => { +export const getRandomDetector = ( + isCreate: boolean = true, + customResultIndex: string = '' +): Detector => { const features = new Array(detectorFaker.natural({ min: 1, max: 5 })) .fill(null) .map(() => getRandomFeature(isCreate ? false : true)); @@ -116,6 +119,7 @@ export const getRandomDetector = (isCreate: boolean = true): Detector => { curState: DETECTOR_STATE.INIT, stateError: '', shingleSize: DEFAULT_SHINGLE_SIZE, + resultIndex: isEmpty(customResultIndex) ? undefined : customResultIndex }; }; diff --git a/test/jest.config.js b/test/jest.config.js index 3771a71e..b12d552f 100644 --- a/test/jest.config.js +++ b/test/jest.config.js @@ -54,4 +54,7 @@ module.exports = { '^.+\\.svg$': '/test/mocks/transformMock.ts', '^.+\\.html$': '/test/mocks/transformMock.ts', }, + setupFiles: [ + "jest-canvas-mock" + ] }; \ No newline at end of file diff --git a/test/setupTests.ts b/test/setupTests.ts index bb070e68..bd2f1764 100644 --- a/test/setupTests.ts +++ b/test/setupTests.ts @@ -10,3 +10,4 @@ */ require('babel-polyfill'); +import 'jest-canvas-mock'; \ No newline at end of file