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 <[email protected]>
opensearch-project · May 17, 2024 · fc28db3 · fc28db3
1 parent 9cb2b00
commit fc28db3
Show file tree

Hide file tree

Showing 6 changed files with 313 additions and 4 deletions.
diff --git 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
@@ -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');
@@ -179,10 +201,12 @@ export const DetectorDetail = (props: DetectorDetailProps) => {
   // detector starts, result index recreated or user switches tabs to re-fetch detector)
   useEffect(() => {
     const getInitialIndices = async () => {
-      await dispatch(getIndices('', dataSourceId)).catch((error: any) => {
+      try {
+        await dispatch(getIndices('', dataSourceId));
+      } catch (error) {
         console.error(error);
         core.notifications.toasts.addDanger('Error getting all indices');
-      });
+      }
     };
     // only need to check if indices exist after detector finishes loading
     if (!isLoadingDetector) {
@@ -464,6 +488,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"
             ></EuiCallOut>
           ) : null}
 

diff --git a/public/pages/DetectorDetail/containers/__tests__/CustomIndexErrorMsg.test.tsx 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(
+    <Provider store={mockStore(initialState)}>
+      <Router>
+        <Switch>
+          <Route
+            path={`/detectors/${detectorId}/results`}
+            render={(props: RouteComponentProps<DetectorRouterProps>) => {
+              const testProps = {
+                ...props,
+                match: {
+                  params: { detectorId: detectorId },
+                  isExact: false,
+                  path: '',
+                  url: '',
+                },
+              };
+              return (
+                <CoreServicesContext.Provider value={coreServicesMock}>
+                  <DetectorDetail {...testProps} />
+                </CoreServicesContext.Provider>
+              );
+            }}
+          />
+          <Redirect from="/" to={`/detectors/${detectorId}/results`} />
+        </Switch>
+      </Router>
+    </Provider>
+  ),
+});
+
+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
@@ -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
@@ -54,4 +54,7 @@ module.exports = {
     '^.+\\.svg$': '<rootDir>/test/mocks/transformMock.ts',
     '^.+\\.html$': '<rootDir>/test/mocks/transformMock.ts',
   },
+  setupFiles: [
+    "jest-canvas-mock"
+  ]
 };
diff --git a/test/setupTests.ts b/test/setupTests.ts
@@ -10,3 +10,4 @@
  */
 
 require('babel-polyfill');
+import 'jest-canvas-mock';
Original file line number	Diff line number	Diff line change
Expand Up		@@ -10,3 +10,4 @@
		*/

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