Skip to content

Commit

Permalink
Fix preview API not considering rules and imputation options (#898)
Browse files Browse the repository at this point in the history
This commit resolves a bug where the preview API did not account for suppression rules and imputation options. The fix involves passing additional parameters to handle these configurations correctly.

Testing:
* e2e testing completed.
* UT added to cover the new logic.

Signed-off-by: Kaituo Li <[email protected]>
  • Loading branch information
kaituo committed Oct 17, 2024
1 parent 4f22813 commit 22acbf2
Show file tree
Hide file tree
Showing 6 changed files with 174 additions and 24 deletions.
42 changes: 27 additions & 15 deletions .github/workflows/remote-integ-tests-workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,36 +71,48 @@ jobs:
node-version: ${{ steps.tool-versions.outputs.node_version }}
registry-url: 'https://registry.npmjs.org'

- name: Setup Opensearch Dashboards
- name: Install correct yarn version for OpenSearch Dashboards
id: setup-yarn
run: |
npm uninstall -g yarn
echo "Installing yarn ${{ steps.tool-versions.outputs.yarn_version }}"
npm i -g yarn@${{ steps.tool-versions.outputs.yarn_version }}
yarn cache clean
yarn add sha.js
working-directory: OpenSearch-Dashboards
shell: bash
echo "Installing yarn ${{ steps.versions_step.outputs.yarn_version }}"
npm i -g yarn@${{ steps.versions_step.outputs.yarn_version }}
- name: Boodstrap Opensearch Dashboards
- name: Yarn Cache
uses: actions/cache@v4
with:
path: |
OpenSearch-Dashboards/**/target
OpenSearch-Dashboards/**/node_modules
key: ${{ runner.OS }}-build-${{ hashFiles('OpenSearch-Dashboards/**/yarn.lock') }}
restore-keys: |
${{ runner.OS }}-build-
- name: Bootstrap OpenSearch Dashboards
run: |
cd OpenSearch-Dashboards
yarn osd bootstrap --single-version=loose
working-directory: OpenSearch-Dashboards/plugins/anomaly-detection-dashboards-plugin
- name: Compile OpenSearch Dashboards
run: |
cd OpenSearch-Dashboards
node scripts/build_opensearch_dashboards_platform_plugins --no-examples --workers=10 --verbose
- name: Run Opensearch Dashboards with AD Installed
run: |
nohup yarn start --no-base-path --no-watch --server.host="0.0.0.0" | tee dashboard.log &
working-directory: OpenSearch-Dashboards

- name : Check If OpenSearch Dashboards Is Ready
- name: Check If OpenSearch Dashboards Is Ready
if: ${{ runner.os == 'Linux' }}
run: |
if timeout 600 grep -q "bundles compiled successfully after" <(tail -n0 -f dashboard.log); then
echo "OpenSearch Dashboards compiled successfully."
cd ./OpenSearch-Dashboards
if timeout 60 grep -q "http server running" <(tail -n +1 -f dashboard.log); then
echo "OpenSearch Dashboards started successfully."
else
echo "Timeout for 600 seconds reached. OpenSearch Dashboards did not finish compiling."
echo "Timeout of 60 seconds reached. OpenSearch Dashboards did not start successfully."
exit 1
fi
working-directory: OpenSearch-Dashboards
fi&
- name: Show OpenSearch Dashboards Logs
if: always()
Expand Down
2 changes: 2 additions & 0 deletions public/pages/ConfigureModel/containers/ConfigureModel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,8 @@ export function ConfigureModel(props: ConfigureModelProps) {
categoryFields={formikProps.values.categoryField}
errors={formikProps.errors}
setFieldTouched={formikProps.setFieldTouched}
imputationOption={formikProps.values.imputationOption}
suppressionRules={formikProps.values.suppressionRules}
/>
) : null}
</EuiPageBody>
Expand Down
8 changes: 6 additions & 2 deletions public/pages/ConfigureModel/containers/SampleAnomalies.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {
} from '../../utils/anomalyResultUtils';
import { focusOnFirstWrongFeature } from '../utils/helpers';
import { prepareDetector } from '../utils/helpers';
import { FeaturesFormikValues } from '../models/interfaces';
import { FeaturesFormikValues, ImputationFormikValues, RuleFormikValues} from '../models/interfaces';
import { BASE_DOCS_LINK } from '../../../utils/constants';
import { prettifyErrorMessage } from '../../../../server/utils/helpers';
import { CoreStart } from '../../../../../../src/core/public';
Expand All @@ -59,6 +59,8 @@ interface SampleAnomaliesProps {
categoryFields: string[];
errors: any;
setFieldTouched: any;
imputationOption?: ImputationFormikValues;
suppressionRules?: RuleFormikValues[];
}

export function SampleAnomalies(props: SampleAnomaliesProps) {
Expand Down Expand Up @@ -183,7 +185,9 @@ export function SampleAnomalies(props: SampleAnomaliesProps) {
props.shingleSize,
props.categoryFields,
newDetector,
true
true,
props.imputationOption,
props.suppressionRules,
);
setPreviewDone(false);
setZoomRange({ ...dateRange });
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
// SPDX-License-Identifier: Apache-2.0

import React from 'react';
import { Provider } from 'react-redux';
import {
HashRouter as Router,
RouteComponentProps,
Route,
Switch,
} from 'react-router-dom';
import { render, fireEvent, waitFor } from '@testing-library/react';
import { SampleAnomalies } from '../SampleAnomalies';
import configureStore from '../../../../redux/configureStore';
import { httpClientMock, coreServicesMock } from '../../../../../test/mocks';
import { mockedStore } from '../../../../redux/utils/testUtils';
import { CoreServicesContext } from '../../../../components/CoreServices/CoreServices';
import { prepareDetector, focusOnFirstWrongFeature } from '../../utils/helpers';
import { createMemoryHistory } from 'history';
import {
FeaturesFormikValues,
ImputationFormikValues,
RuleFormikValues,
} from '../../../../pages/ConfigureModel/models/interfaces';
import { getRandomDetector } from '../../../../redux/reducers/__tests__/utils';
import { FEATURE_TYPE } from '../../../../models/interfaces';

// Mock the helper functions
jest.mock('../../utils/helpers', () => ({
...jest.requireActual('../../utils/helpers'),
prepareDetector: jest.fn(() => ({
// mock a detector object with at least an id, preventing the undefined error when detector.id is accessed in getSampleAdResult.
id: 'test-detector-id',
name: 'test-detector',
description: 'test-detector-description',
})),
focusOnFirstWrongFeature: jest.fn(() => false),
}));

// Mock the Redux actions if necessary
jest.mock('../../../../redux/reducers/previewAnomalies', () => ({
previewDetector: jest.fn(() => async () => Promise.resolve()),
}));

describe('<SampleAnomalies /> spec', () => {
beforeEach(() => {
jest.clearAllMocks();
console.error = jest.fn();
console.warn = jest.fn();
});

test('calls prepareDetector with imputationOption and suppressionRules when previewDetector button is clicked', async () => {
// Set up the mock props
const mockDetector = getRandomDetector();

const mockFeatureList: FeaturesFormikValues[] = [
{
featureId: 'feature1',
featureName: 'Feature 1',
featureEnabled: true,
aggregationBy: 'sum',
aggregationOf: [{ label: 'bytes' }],
featureType: FEATURE_TYPE.SIMPLE,
aggregationQuery: '',
},
];

const mockImputationOption: ImputationFormikValues = {
imputationMethod: 'zero',
};

const mockSuppressionRules: RuleFormikValues[] = [
{
featureName: '',
absoluteThreshold: undefined,
relativeThreshold: undefined,
aboveBelow: 'above',
},
];

const props = {
detector: mockDetector,
featureList: mockFeatureList,
shingleSize: 8,
categoryFields: ['category1'],
errors: {},
setFieldTouched: jest.fn(),
imputationOption: mockImputationOption,
suppressionRules: mockSuppressionRules,
};

// Create a mock history and store
const history = createMemoryHistory();
const initialState = {
anomalies: {
anomaliesResult: {
anomalies: [],
},
},
};
const store = mockedStore();

const { getByText } = render(
<Provider store={store}>
<Router history={history}>
<CoreServicesContext.Provider value={coreServicesMock}>
<SampleAnomalies {...props} />
</CoreServicesContext.Provider>
</Router>
</Provider>
);

// Simulate clicking the previewDetector button
fireEvent.click(getByText('Preview anomalies'));

// Wait for async actions to complete
await waitFor(() => {
expect(prepareDetector).toHaveBeenCalledWith(
props.featureList,
props.shingleSize,
props.categoryFields,
expect.anything(), // newDetector (could be the same as props.detector or modified)
true,
props.imputationOption,
props.suppressionRules
);
});
});
});
12 changes: 6 additions & 6 deletions public/pages/ConfigureModel/utils/__tests__/helpers.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ describe('featuresToFormik', () => {
aggregationQuery: '',
};
const randomPositiveInt = Math.ceil(Math.random() * 100);
const apiRequest = prepareDetector(
[newFeature],
randomPositiveInt,
randomDetector,
false
);
// const apiRequest = prepareDetector(
// [newFeature],
// randomPositiveInt,
// randomDetector,
// false
// );
// expect(apiRequest.featureAttributes).toEqual([
// {
// featureId: newFeature.featureId,
Expand Down
6 changes: 5 additions & 1 deletion public/pages/ConfigureModel/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,9 @@ export function prepareDetector(
shingleSizeValue: number,
categoryFields: string[],
ad: Detector,
forPreview: boolean = false
forPreview: boolean = false,
imputationOption?: ImputationFormikValues,
suppressionRules?: RuleFormikValues[]
): Detector {
const detector = cloneDeep(ad);
const featureAttributes = formikToFeatures(featureValues, forPreview);
Expand All @@ -354,6 +356,8 @@ export function prepareDetector(
...detector.uiMetadata,
features: { ...featuresToUIMetadata(featureValues) },
},
imputationOption: formikToImputationOption(imputationOption),
rules: formikToRules(suppressionRules),
};
}

Expand Down

0 comments on commit 22acbf2

Please sign in to comment.