diff --git a/.github/workflows/remote-integ-tests-workflow.yml b/.github/workflows/remote-integ-tests-workflow.yml index 31f56a6c..4bf7ffe7 100644 --- a/.github/workflows/remote-integ-tests-workflow.yml +++ b/.github/workflows/remote-integ-tests-workflow.yml @@ -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 }} diff --git a/opensearch_dashboards.json b/opensearch_dashboards.json index 5aa5d906..a4375851 100644 --- a/opensearch_dashboards.json +++ b/opensearch_dashboards.json @@ -8,7 +8,8 @@ "optionalPlugins": [ "dataSource", "dataSourceManagement", - "assistantDashboards" + "assistantDashboards", + "usageCollection" ], "requiredPlugins": [ "opensearchDashboardsUtils", diff --git a/public/components/DiscoverAction/SuggestAnomalyDetector.test.tsx b/public/components/DiscoverAction/SuggestAnomalyDetector.test.tsx index 4e0de621..ac4479a5 100644 --- a/public/components/DiscoverAction/SuggestAnomalyDetector.test.tsx +++ b/public/components/DiscoverAction/SuggestAnomalyDetector.test.tsx @@ -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: { @@ -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 = () => ({ @@ -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: [ { @@ -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: [ { @@ -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: [ { @@ -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: [ { @@ -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: [ { @@ -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', () => { @@ -263,6 +401,9 @@ describe('GenerateAnomalyDetector spec', () => { timeFieldName: '@timestamp', }, }); + (getAssistantClient().agentConfigExists as jest.Mock).mockResolvedValueOnce({ + exists: true + }); }); it('All API calls execute successfully', async () => { @@ -282,7 +423,7 @@ describe('GenerateAnomalyDetector spec', () => { }); } }); - (getAssistantClient().executeAgentByName as jest.Mock).mockResolvedValueOnce({ + (getAssistantClient().executeAgentByConfigName as jest.Mock).mockResolvedValueOnce({ body: { inference_results: [ { @@ -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(); @@ -341,7 +482,7 @@ describe('GenerateAnomalyDetector spec', () => { }); } }); - (getAssistantClient().executeAgentByName as jest.Mock).mockResolvedValueOnce({ + (getAssistantClient().executeAgentByConfigName as jest.Mock).mockResolvedValueOnce({ body: { inference_results: [ { @@ -412,7 +553,7 @@ describe('GenerateAnomalyDetector spec', () => { }, }); - (getAssistantClient().executeAgentByName as jest.Mock).mockResolvedValueOnce({ + (getAssistantClient().executeAgentByConfigName as jest.Mock).mockResolvedValueOnce({ body: { inference_results: [ { diff --git a/public/components/DiscoverAction/SuggestAnomalyDetector.tsx b/public/components/DiscoverAction/SuggestAnomalyDetector.tsx index 96e60234..1a3e202e 100644 --- a/public/components/DiscoverAction/SuggestAnomalyDetector.tsx +++ b/public/components/DiscoverAction/SuggestAnomalyDetector.tsx @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useState, useEffect, Fragment } from 'react'; +import React, { useState, useEffect, Fragment, useCallback } from 'react'; import { EuiFlyoutHeader, EuiFlyoutBody, @@ -22,6 +22,7 @@ import { EuiButtonEmpty, EuiPanel, EuiComboBox, + EuiSmallButtonIcon, } from '@elastic/eui'; import '../FeatureAnywhereContextMenu/CreateAnomalyDetector/styles.scss'; import { useDispatch, useSelector } from 'react-redux'; @@ -62,8 +63,8 @@ import { import { formikToDetector } from '../../pages/ReviewAndCreate/utils/helpers'; import { FormattedFormRow } from '../FormattedFormRow/FormattedFormRow'; import { FeatureAccordion } from '../../pages/ConfigureModel/components/FeatureAccordion'; -import { AD_DOCS_LINK, DEFAULT_SHINGLE_SIZE, MAX_FEATURE_NUM, PLUGIN_NAME } from '../../utils/constants'; -import { getAssistantClient, getNotifications, getQueryService } from '../../services'; +import { AD_DOCS_LINK, DEFAULT_SHINGLE_SIZE, MAX_FEATURE_NUM, PLUGIN_NAME, SUGGEST_ANOMALY_DETECTOR_METRIC_TYPE } from '../../utils/constants'; +import { getAssistantClient, getNotifications, getQueryService, getUsageCollection } from '../../services'; import { prettifyErrorMessage } from '../../../server/utils/helpers'; import EnhancedAccordion from '../FeatureAnywhereContextMenu/EnhancedAccordion'; import MinimalAccordion from '../FeatureAnywhereContextMenu/MinimalAccordion'; @@ -75,6 +76,7 @@ import { mountReactNode } from '../../../../../src/core/public/utils'; import { formikToDetectorName } from '../FeatureAnywhereContextMenu/CreateAnomalyDetector/helpers'; import { DEFAULT_DATA } from '../../../../../src/plugins/data/common'; import { AppState } from '../../redux/reducers'; +import { v4 as uuidv4 } from 'uuid'; export interface GeneratedParameters { categoryField: string; @@ -89,6 +91,7 @@ function SuggestAnomalyDetector({ }) { const dispatch = useDispatch(); const notifications = getNotifications(); + const usageCollection = getUsageCollection(); const assistantClient = getAssistantClient(); const queryString = getQueryService().queryString; @@ -136,12 +139,19 @@ function SuggestAnomalyDetector({ const dateNanoFields = get(indexDataTypes, 'date_nanos', []) as string[]; const allDateFields = dateFields.concat(dateNanoFields); + const [feedbackResult, setFeedbackResult] = useState(undefined); + // let LLM to generate parameters for creating anomaly detector async function getParameters() { try { + const checkAgentExistsResponse = await assistantClient.agentConfigExists(SUGGEST_ANOMALY_DETECTOR_CONFIG_ID, { dataSourceId }); + if (!checkAgentExistsResponse?.exists) { + throw new Error('Agent for suggest anomaly detector not found, please configure an agent firstly!'); + } const executeAgentResponse = await - assistantClient.executeAgentByName(SUGGEST_ANOMALY_DETECTOR_CONFIG_ID, { index: indexName }, { dataSourceId } + assistantClient.executeAgentByConfigName(SUGGEST_ANOMALY_DETECTOR_CONFIG_ID, { index: indexName }, { dataSourceId } ); + reportMetric(SUGGEST_ANOMALY_DETECTOR_METRIC_TYPE.GENERATED); const rawGeneratedParameters = executeAgentResponse?.body?.inference_results?.[0]?.output?.[0]?.result; if (!rawGeneratedParameters) { throw new Error('Cannot get generated parameters!'); @@ -240,6 +250,28 @@ function SuggestAnomalyDetector({ setDelayValue(e.target.value); }; + const reportMetric = usageCollection + ? (metric: string) => { + usageCollection.reportUiStats( + `suggestAD`, + usageCollection.METRIC_TYPE.CLICK, + metric + '-' + uuidv4() + ); + } + : () => { }; + + const feedbackOutput = useCallback( + (correct: boolean, result: boolean | undefined) => { + // No repeated feedback. + if (result !== undefined) { + return; + } + reportMetric(correct ? SUGGEST_ANOMALY_DETECTOR_METRIC_TYPE.THUMBUP : SUGGEST_ANOMALY_DETECTOR_METRIC_TYPE.THUMBDOWN); + setFeedbackResult(correct); + }, + [] + ); + const handleValidationAndSubmit = (formikProps: any) => { if (formikProps.values.featureList.length !== 0) { formikProps.setFieldTouched('featureList', true); @@ -264,6 +296,7 @@ function SuggestAnomalyDetector({ const detectorToCreate = formikToDetector(formikProps.values); await dispatch(createDetector(detectorToCreate, dataSourceId)) .then(async (response: any) => { + reportMetric(SUGGEST_ANOMALY_DETECTOR_METRIC_TYPE.CREATED); const detectorId = response.response.id; dispatch(startDetector(detectorId, dataSourceId)) .then(() => { }) @@ -288,7 +321,7 @@ function SuggestAnomalyDetector({ Detector created: { e.preventDefault(); - const url = `../${PLUGIN_NAME}#/detectors/${detectorId}`; + const url = `../${PLUGIN_NAME}#/detectors/${detectorId}/results?dataSourceId=${dataSourceId || ''}`; window.open(url, '_blank'); }} style={{ textDecoration: 'underline' }}>{formikProps.values.name} @@ -839,6 +872,34 @@ function SuggestAnomalyDetector({ Cancel + + + Was this helpful? + + {feedbackResult !== false ? ( + + feedbackOutput(true, feedbackResult)} + /> + + ) : null} + {feedbackResult !== true ? ( + + feedbackOutput(false, feedbackResult)} + /> + + ) : null} + + { + const [coreStart] = await core.getStartServices(); + const assistantEnabled = coreStart.application.capabilities?.assistant?.enabled === true; + if (assistantEnabled) { + // Add suggest anomaly detector action to the uiActions in Discover + const suggestAnomalyDetectorAction = getSuggestAnomalyDetectorAction(); + plugins.uiActions.addTriggerAction(plugins.assistantDashboards.assistantTriggers.AI_ASSISTANT_QUERY_EDITOR_TRIGGER, suggestAnomalyDetectorAction); + // set usageCollection for metric report + setUsageCollection(plugins.usageCollection); + } + } + checkAndRegisterAction(); } // registers the expression function used to render anomalies on an Augmented Visualization plugins.expressions.registerFunction(overlayAnomaliesFunction); diff --git a/public/services.ts b/public/services.ts index d582ac59..df21fcb5 100644 --- a/public/services.ts +++ b/public/services.ts @@ -16,6 +16,7 @@ import { createGetterSetter } from '../../../src/plugins/opensearch_dashboards_u import { UiActionsStart } from '../../../src/plugins/ui_actions/public'; import { SavedAugmentVisLoader } from '../../../src/plugins/vis_augmenter/public'; import { NavigationPublicPluginStart } from '../../../src/plugins/navigation/public'; +import { UsageCollectionSetup } from '../../../src/plugins/usage_collection/public/plugin'; import { AssistantPublicPluginStart } from '../../../plugins/dashboards-assistant/public/'; export interface DataSourceEnabled { @@ -46,6 +47,9 @@ export const [getUISettings, setUISettings] = export const [getQueryService, setQueryService] = createGetterSetter('Query'); +export const [getUsageCollection, setUsageCollection] = + createGetterSetter('UsageCollection'); + export const [getAssistantEnabled, setAssistantEnabled] = createGetterSetter('AssistantClient'); diff --git a/public/utils/constants.ts b/public/utils/constants.ts index 6b7f60b4..5b92e355 100644 --- a/public/utils/constants.ts +++ b/public/utils/constants.ts @@ -117,4 +117,11 @@ export const DASHBOARD_PAGE_NAV_ID = `anomaly_detection_dashboard-dashboard`; export const DETECTORS_PAGE_NAV_ID = `anomaly_detection_dashboard-detectors`; -export const USE_NEW_HOME_PAGE = 'home:useNewHomePage'; \ No newline at end of file +export const USE_NEW_HOME_PAGE = 'home:useNewHomePage'; + +export enum SUGGEST_ANOMALY_DETECTOR_METRIC_TYPE { + THUMBUP = 'thumbup', + THUMBDOWN = 'thumbdown', + GENERATED = 'generated', + CREATED = 'created', +} \ No newline at end of file