Skip to content

Commit

Permalink
Fix preview API not considering rules and imputation options (opensea…
Browse files Browse the repository at this point in the history
…rch-project#898)

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 <kaituo@amazon.com>
kaituo committed Oct 17, 2024
1 parent 4f22813 commit 22acbf2
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
@@ -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()
2 changes: 2 additions & 0 deletions public/pages/ConfigureModel/containers/ConfigureModel.tsx
Original file line number Diff line number Diff line change
@@ -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>
8 changes: 6 additions & 2 deletions public/pages/ConfigureModel/containers/SampleAnomalies.tsx
Original file line number Diff line number Diff line change
@@ -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';
@@ -59,6 +59,8 @@ interface SampleAnomaliesProps {
categoryFields: string[];
errors: any;
setFieldTouched: any;
imputationOption?: ImputationFormikValues;
suppressionRules?: RuleFormikValues[];
}

export function SampleAnomalies(props: SampleAnomaliesProps) {
@@ -183,7 +185,9 @@ export function SampleAnomalies(props: SampleAnomaliesProps) {
props.shingleSize,
props.categoryFields,
newDetector,
true
true,
props.imputationOption,
props.suppressionRules,
);
setPreviewDone(false);
setZoomRange({ ...dateRange });
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
@@ -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,
6 changes: 5 additions & 1 deletion public/pages/ConfigureModel/utils/helpers.ts
Original file line number Diff line number Diff line change
@@ -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);
@@ -354,6 +356,8 @@ export function prepareDetector(
...detector.uiMetadata,
features: { ...featuresToUIMetadata(featureValues) },
},
imputationOption: formikToImputationOption(imputationOption),
rules: formikToRules(suppressionRules),
};
}

0 comments on commit 22acbf2

Please sign in to comment.