Skip to content

Commit

Permalink
Report metrics for suggest anomaly detector (#876)
Browse files Browse the repository at this point in the history 
* Add feedback button to the flyout of suggest anomaly detector

Signed-off-by: gaobinlong <gbinlong@amazon.com>

* Check feature flag before registering action to Discover page

Signed-off-by: gaobinlong <gbinlong@amazon.com>

* Fix url bug

Signed-off-by: gaobinlong <gbinlong@amazon.com>

* Remove unused dependency

Signed-off-by: gaobinlong <gbinlong@amazon.com>

* Fix e2e test failure

Signed-off-by: gaobinlong <gbinlong@amazon.com>

---------

Signed-off-by: gaobinlong <gbinlong@amazon.com>
  • Loading branch information
@gaobinlong
gaobinlong authored Sep 19, 2024
1 parent ec02b63 commit 26b12d1
Show file tree
Hide file tree
Showing 7 changed files with 251 additions and 26 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/remote-integ-tests-workflow.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,14 +155,14 @@ jobs:
working-directory: opensearch-dashboards-functional-test

- name: Capture failure screenshots
uses: actions/upload-artifact@v1
uses: actions/upload-artifact@v4
if: failure()
with:
name: cypress-screenshots-${{ matrix.os }}
path: opensearch-dashboards-functional-test/cypress/screenshots

- name: Capture failure test video
uses: actions/upload-artifact@v1
uses: actions/upload-artifact@v4
if: failure()
with:
name: cypress-videos-${{ matrix.os }}
Expand Down
3 changes: 2 additions & 1 deletion opensearch_dashboards.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"optionalPlugins": [
"dataSource",
"dataSourceManagement",
"assistantDashboards"
"assistantDashboards",
"usageCollection"
],
"requiredPlugins": [
"opensearchDashboardsUtils",
Expand Down
167 changes: 154 additions & 13 deletions public/components/DiscoverAction/SuggestAnomalyDetector.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import configureStore from '../../redux/configureStore';
import SuggestAnomalyDetector from './SuggestAnomalyDetector';
import userEvent from '@testing-library/user-event';
import { HttpFetchOptionsWithPath } from '../../../../../src/core/public';
import { getAssistantClient, getQueryService } from '../../services';
import { getAssistantClient, getQueryService, getUsageCollection } from '../../services';

const notifications = {
toasts: {
Expand All @@ -41,8 +41,10 @@ jest.mock('../../services', () => ({
},
}),
getAssistantClient: jest.fn().mockReturnValue({
executeAgentByName: jest.fn(),
})
agentConfigExists: jest.fn(),
executeAgentByConfigName: jest.fn(),
}),
getUsageCollection: jest.fn(),
}));

const renderWithRouter = () => ({
Expand Down Expand Up @@ -126,11 +128,13 @@ describe('GenerateAnomalyDetector spec', () => {
timeFieldName: '@timestamp',
},
});

(getAssistantClient().agentConfigExists as jest.Mock).mockResolvedValueOnce({
exists: true
});
});

it('renders with empty generated parameters', async () => {
(getAssistantClient().executeAgentByName as jest.Mock).mockResolvedValueOnce({
(getAssistantClient().executeAgentByConfigName as jest.Mock).mockResolvedValueOnce({
body: {
inference_results: [
{
Expand All @@ -154,7 +158,7 @@ describe('GenerateAnomalyDetector spec', () => {
});

it('renders with empty parameter', async () => {
(getAssistantClient().executeAgentByName as jest.Mock).mockResolvedValueOnce({
(getAssistantClient().executeAgentByConfigName as jest.Mock).mockResolvedValueOnce({
body: {
inference_results: [
{
Expand All @@ -178,7 +182,7 @@ describe('GenerateAnomalyDetector spec', () => {
});

it('renders with empty aggregation field or empty aggregation method', async () => {
(getAssistantClient().executeAgentByName as jest.Mock).mockResolvedValueOnce({
(getAssistantClient().executeAgentByConfigName as jest.Mock).mockResolvedValueOnce({
body: {
inference_results: [
{
Expand All @@ -202,7 +206,7 @@ describe('GenerateAnomalyDetector spec', () => {
});

it('renders with different number of aggregation methods and fields', async () => {
(getAssistantClient().executeAgentByName as jest.Mock).mockResolvedValueOnce({
(getAssistantClient().executeAgentByConfigName as jest.Mock).mockResolvedValueOnce({
body: {
inference_results: [
{
Expand All @@ -226,7 +230,7 @@ describe('GenerateAnomalyDetector spec', () => {
});

it('renders component completely', async () => {
(getAssistantClient().executeAgentByName as jest.Mock).mockResolvedValueOnce({
(getAssistantClient().executeAgentByConfigName as jest.Mock).mockResolvedValueOnce({
body: {
inference_results: [
{
Expand All @@ -246,9 +250,143 @@ describe('GenerateAnomalyDetector spec', () => {
expect(queryByText('Detector details')).not.toBeNull();
expect(queryByText('Advanced configuration')).not.toBeNull();
expect(queryByText('Model Features')).not.toBeNull();
expect(queryByText('Was this helpful?')).not.toBeNull();
});
});
});

describe('Test agent not configured', () => {
beforeEach(() => {
jest.clearAllMocks();
const queryService = getQueryService();
queryService.queryString.getQuery.mockReturnValue({
dataset: {
id: 'test-pattern',
title: 'test-pattern',
type: 'INDEX_PATTERN',
timeFieldName: '@timestamp',
},
});
});

it('renders with empty generated parameters', async () => {
(getAssistantClient().agentConfigExists as jest.Mock).mockResolvedValueOnce({
exists: false
});

const { queryByText } = renderWithRouter();
expect(queryByText('Suggested anomaly detector')).not.toBeNull();

await waitFor(() => {
expect(getNotifications().toasts.addDanger).toHaveBeenCalledTimes(1);
expect(getNotifications().toasts.addDanger).toHaveBeenCalledWith(
'Generate parameters for creating anomaly detector failed, reason: Error: Agent for suggest anomaly detector not found, please configure an agent firstly!'
);
});
});
});

describe('Test feedback', () => {
let reportUiStatsMock: any;

beforeEach(() => {
const queryService = getQueryService();
queryService.queryString.getQuery.mockReturnValue({
dataset: {
id: 'test-pattern',
title: 'test-pattern',
type: 'INDEX_PATTERN',
timeFieldName: '@timestamp',
},
});

reportUiStatsMock = jest.fn();
(getUsageCollection as jest.Mock).mockReturnValue({
reportUiStats: reportUiStatsMock,
METRIC_TYPE: {
CLICK: 'click',
},
});

(getAssistantClient().agentConfigExists as jest.Mock).mockResolvedValueOnce({
exists: true
});
});

afterEach(() => {
jest.clearAllMocks();
});

it('should call reportMetric with thumbup when thumbs up is clicked', async () => {
(getAssistantClient().executeAgentByConfigName as jest.Mock).mockResolvedValueOnce({
body: {
inference_results: [
{
output: [
{ result: "{\"index\":\"opensearch_dashboards_sample_data_logs\",\"categoryField\":\"ip\",\"aggregationField\":\"responseLatency,response\",\"aggregationMethod\":\"avg,sum\",\"dateFields\":\"utc_time,timestamp\"}" }
]
}
]
}
});

const { queryByText, getByLabelText } = renderWithRouter();
expect(queryByText('Suggested anomaly detector')).not.toBeNull();

await waitFor(() => {
expect(queryByText('Create detector')).not.toBeNull();
expect(queryByText('Was this helpful?')).not.toBeNull();
});

userEvent.click(getByLabelText('feedback thumbs up'));
expect(reportUiStatsMock).toHaveBeenCalled();
expect(reportUiStatsMock).toHaveBeenCalledWith(
'suggestAD',
'click',
expect.stringContaining('generated-')
);
expect(reportUiStatsMock).toHaveBeenCalledWith(
'suggestAD',
'click',
expect.stringContaining('thumbup-')
);
});


it('should call reportMetric with thumbdown when thumbs down is clicked', async () => {
(getAssistantClient().executeAgentByConfigName as jest.Mock).mockResolvedValueOnce({
body: {
inference_results: [
{
output: [
{ result: "{\"index\":\"opensearch_dashboards_sample_data_logs\",\"categoryField\":\"ip\",\"aggregationField\":\"responseLatency,response\",\"aggregationMethod\":\"avg,sum\",\"dateFields\":\"utc_time,timestamp\"}" }
]
}
]
}
});

const { queryByText, getByLabelText } = renderWithRouter();
expect(queryByText('Suggested anomaly detector')).not.toBeNull();

await waitFor(() => {
expect(queryByText('Create detector')).not.toBeNull();
expect(queryByText('Was this helpful?')).not.toBeNull();
});

userEvent.click(getByLabelText('feedback thumbs down'));
expect(reportUiStatsMock).toHaveBeenCalled();
expect(reportUiStatsMock).toHaveBeenCalledWith(
'suggestAD',
'click',
expect.stringContaining('generated-')
);
expect(reportUiStatsMock).toHaveBeenCalledWith(
'suggestAD',
'click',
expect.stringContaining('thumbdown-')
);
});
});

describe('Test API calls', () => {
Expand All @@ -263,6 +401,9 @@ describe('GenerateAnomalyDetector spec', () => {
timeFieldName: '@timestamp',
},
});
(getAssistantClient().agentConfigExists as jest.Mock).mockResolvedValueOnce({
exists: true
});
});

it('All API calls execute successfully', async () => {
Expand All @@ -282,7 +423,7 @@ describe('GenerateAnomalyDetector spec', () => {
});
}
});
(getAssistantClient().executeAgentByName as jest.Mock).mockResolvedValueOnce({
(getAssistantClient().executeAgentByConfigName as jest.Mock).mockResolvedValueOnce({
body: {
inference_results: [
{
Expand Down Expand Up @@ -314,7 +455,7 @@ describe('GenerateAnomalyDetector spec', () => {
});

it('Generate parameters failed', async () => {
(getAssistantClient().executeAgentByName as jest.Mock).mockRejectedValueOnce('Generate parameters failed');
(getAssistantClient().executeAgentByConfigName as jest.Mock).mockRejectedValueOnce('Generate parameters failed');

const { queryByText } = renderWithRouter();
expect(queryByText('Suggested anomaly detector')).not.toBeNull();
Expand All @@ -341,7 +482,7 @@ describe('GenerateAnomalyDetector spec', () => {
});
}
});
(getAssistantClient().executeAgentByName as jest.Mock).mockResolvedValueOnce({
(getAssistantClient().executeAgentByConfigName as jest.Mock).mockResolvedValueOnce({
body: {
inference_results: [
{
Expand Down Expand Up @@ -412,7 +553,7 @@ describe('GenerateAnomalyDetector spec', () => {
},
});

(getAssistantClient().executeAgentByName as jest.Mock).mockResolvedValueOnce({
(getAssistantClient().executeAgentByConfigName as jest.Mock).mockResolvedValueOnce({
body: {
inference_results: [
{
Expand Down
Loading

0 comments on commit 26b12d1

Please sign in to comment.