From 0eb7960d0e48574a838f46860089260c2cd434a8 Mon Sep 17 00:00:00 2001 From: tygao Date: Thu, 24 Oct 2024 17:01:59 +0800 Subject: [PATCH 1/5] update verify method for time range DSL Signed-off-by: tygao --- .../generate_popover_body.test.tsx | 26 +++++++--- .../generate_popover_body.tsx | 41 +++++++++++----- public/types.ts | 4 ++ public/utils/alerting.ts | 49 ++++++++++++++++--- 4 files changed, 94 insertions(+), 26 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.test.tsx b/public/components/incontext_insight/generate_popover_body.test.tsx index 4b3c1302..442030cc 100644 --- a/public/components/incontext_insight/generate_popover_body.test.tsx +++ b/public/components/incontext_insight/generate_popover_body.test.tsx @@ -15,9 +15,20 @@ import { dataPluginMock } from '../../../../../src/plugins/data/public/mocks'; jest.mock('../../services'); -jest.mock('../../utils', () => ({ - createIndexPatterns: jest.fn().mockResolvedValue('index pattern'), - buildUrlQuery: jest.fn().mockResolvedValue('query'), +jest.mock('../../utils', () => { + const originUtils = jest.requireActual('../../utils'); + return { + ...originUtils, + createIndexPatterns: jest.fn().mockResolvedValue('index pattern'), + buildUrlQuery: jest.fn().mockResolvedValue('query'), + }; +}); + +jest.spyOn(window, 'open').mockImplementation(() => null); + +jest.mock('../../../../../src/core/public/utils', () => ({ + ...jest.requireActual('../../../../../src/core/public/utils'), + formatUrlWithWorkspaceId: jest.fn().mockReturnValue('formattedUrl'), })); const mockToasts = { @@ -48,7 +59,7 @@ const mockDSL = `{ "range": { "timestamp": { "from": "2024-09-06T04:02:52||-1h", - "to": "2024-09-06T04:02:52", + "to": "2024-10-09T17:40:47+00:00", "include_lower": true, "include_upper": true, "boost": 1 @@ -70,6 +81,9 @@ const mockDSL = `{ } }`; +const spyWindowOpen = jest.spyOn(window, 'open'); +spyWindowOpen.mockImplementation(jest.fn()); + describe('GeneratePopoverBody', () => { const incontextInsightMock = { contextProvider: jest.fn(), @@ -370,9 +384,7 @@ describe('GeneratePopoverBody', () => { const button = getByText('Discover details'); expect(button).toBeInTheDocument(); fireEvent.click(button); - expect(coreStart.application.navigateToUrl).toHaveBeenCalledWith( - 'data-explorer/discover#?query' - ); + expect(spyWindowOpen).toHaveBeenCalledWith('data-explorer/discover#?query'); }); }); }); diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index a24b0120..0a783bea 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -29,9 +29,10 @@ import { SUMMARY_ASSISTANT_API } from '../../../common/constants/llm'; import shiny_sparkle from '../../assets/shiny_sparkle.svg'; import { UsageCollectionSetup } from '../../../../../src/plugins/usage_collection/public'; import { reportMetric } from '../../utils/report_metric'; -import { buildUrlQuery, createIndexPatterns } from '../../utils'; +import { buildUrlQuery, createIndexPatterns, extractTimeRangeDSL } from '../../utils'; import { AssistantPluginStartDependencies } from '../../types'; import { UI_SETTINGS } from '../../../../../src/plugins/data/public'; +import { formatUrlWithWorkspaceId } from '../../../../../src/core/public/utils'; export const GeneratePopoverBody: React.FC<{ incontextInsight: IncontextInsightInput; @@ -55,10 +56,19 @@ export const GeneratePopoverBody: React.FC<{ const getMonitorType = async () => { const context = await incontextInsight.contextProvider?.(); const monitorType = context?.additionalInfo?.monitorType; + const dsl = context?.additionalInfo?.dsl; // Only this two types from alerting contain DSL and index. - const shoudDisplayDiscoverButton = + const isSupportedMonitorType = monitorType === 'query_level_monitor' || monitorType === 'bucket_level_monitor'; - setDisplayDiscoverButton(shoudDisplayDiscoverButton); + let hasTimeRangeFilter = false; + if (dsl) { + const dslObject = JSON.parse(dsl); + const filters = dslObject?.query?.bool?.filter; + // Filters contains time range filter,if no filters, return. + if (!filters?.length) return; + hasTimeRangeFilter = !!extractTimeRangeDSL(filters).timeRangeDSL; + } + setDisplayDiscoverButton(isSupportedMonitorType && hasTimeRangeFilter); }; getMonitorType(); }, [incontextInsight, setDisplayDiscoverButton]); @@ -195,12 +205,12 @@ export const GeneratePopoverBody: React.FC<{ if (!dsl || !indexName) return; const dslObject = JSON.parse(dsl); const filters = dslObject?.query?.bool?.filter; - if (!filters) return; - const timeDslIndex = filters?.findIndex((filter: Record) => filter?.range); - const timeDsl = filters[timeDslIndex]?.range; - const timeFieldName = Object.keys(timeDsl)[0]; - if (!timeFieldName) return; - filters?.splice(timeDslIndex, 1); + if (!filters?.length) return; + const { timeRangeDSL, newFilters } = extractTimeRangeDSL(filters); + // Filter out time range DSL and use this result to build filter query. + if (!timeRangeDSL) return; + dslObject.query.bool.filter = newFilters; + const timeFieldName = Object.keys(timeRangeDSL)[0]; if (getStartServices) { const [coreStart, startDeps] = await getStartServices(); @@ -222,11 +232,18 @@ export const GeneratePopoverBody: React.FC<{ coreStart.savedObjects, indexPattern, dslObject, - timeDsl[timeFieldName], + timeRangeDSL, context?.dataSourceId ); - // Navigate to new discover with query built to populate - coreStart.application.navigateToUrl(`data-explorer/discover#?${query}`); + // Navigate to new discover with query built to populate, use new window to avoid discover search failed. + const discoverUrl = `data-explorer/discover#?${query}`; + const currentWorkspace = coreStart.workspaces.currentWorkspace$.getValue(); + const url = formatUrlWithWorkspaceId( + discoverUrl, + currentWorkspace?.id ?? '', + coreStart.http.basePath + ); + window.open(url, '_blank'); } } finally { setDiscoverLoading(false); diff --git a/public/types.ts b/public/types.ts index 6a95238f..334f8e55 100644 --- a/public/types.ts +++ b/public/types.ts @@ -139,3 +139,7 @@ export type IncontextInsightType = | 'error'; export type TabId = 'chat' | 'compose' | 'insights' | 'history' | 'trace'; + +export interface NestedRecord { + [key: string]: T | NestedRecord; +} diff --git a/public/utils/alerting.ts b/public/utils/alerting.ts index d5c02cba..fd8fcab2 100644 --- a/public/utils/alerting.ts +++ b/public/utils/alerting.ts @@ -5,14 +5,17 @@ import rison from 'rison-node'; import { stringify } from 'query-string'; +import moment from 'moment'; import { buildCustomFilter } from '../../../../src/plugins/data/common'; import { url } from '../../../../src/plugins/opensearch_dashboards_utils/public'; import { DataPublicPluginStart, opensearchFilters, IndexPattern, + Filter, } from '../../../../src/plugins/data/public'; import { CoreStart } from '../../../../src/core/public'; +import { NestedRecord } from '../types'; export const buildFilter = (indexPatternId: string, dsl: Record) => { const filterAlias = 'Alerting-filters'; @@ -69,16 +72,19 @@ export const buildUrlQuery = async ( dataStart: DataPublicPluginStart, savedObjects: CoreStart['savedObjects'], indexPattern: IndexPattern, - dsl: Record, + dsl: NestedRecord, timeDsl: Record<'from' | 'to', string>, dataSourceId?: string ) => { - const filter = buildFilter(indexPattern.id!, dsl); - - const filterManager = dataStart.query.filterManager; - // There are some map and flatten operations to filters in filterManager, use this to keep aligned with discover. - filterManager.setAppFilters([filter]); - const filters = filterManager.getAppFilters(); + let filters: Filter[] = []; + // If there is none filter after filtering timeRange filter, skip to build filter query. + if (dsl?.query?.bool?.filter?.length > 0) { + const filter = buildFilter(indexPattern.id!, dsl); + const filterManager = dataStart.query.filterManager; + // There are some map and flatten operations to filters in filterManager, use this to keep aligned with discover. + filterManager.setAppFilters([filter]); + filters = filterManager.getAppFilters(); + } const refreshInterval = { pause: true, @@ -142,3 +148,32 @@ export const buildUrlQuery = async ( ); return hash; }; + +export const validateToTimeRange = (time: string) => { + // Alerting uses this format in to field of time range filter. + const TO_TIME_FORMAT = 'YYYY-MM-DDTHH:mm:ssZ'; + return moment.utc(time, TO_TIME_FORMAT, true).isValid(); +}; + +export const extractTimeRangeDSL = (filters: NestedRecord[]) => { + let timeRangeDSL; + const newFilters = filters.filter((filter: NestedRecord) => { + if (filter?.range && typeof filter.range === 'object') { + for (const key of Object.keys(filter.range)) { + const rangeValue = filter.range[key]; + if (typeof rangeValue === 'object' && 'to' in rangeValue) { + const toValue = rangeValue.to; + if (typeof toValue === 'string' && validateToTimeRange(toValue)) { + timeRangeDSL = filter.range[key]; + return false; + } + } + } + } + return true; + }); + return { + newFilters, + timeRangeDSL, + }; +}; From 9f9eca3e6624446240386ab528efcb76ce691dd4 Mon Sep 17 00:00:00 2001 From: tygao Date: Thu, 24 Oct 2024 17:26:05 +0800 Subject: [PATCH 2/5] add time field key Signed-off-by: tygao --- .../incontext_insight/generate_popover_body.tsx | 9 ++++----- public/utils/alerting.ts | 3 +++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 0a783bea..2f16af4c 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -93,7 +93,7 @@ export const GeneratePopoverBody: React.FC<{ defaultMessage: 'Generate summary error', }) ); - closePopover(); + // closePopover(); return; } const contextContent = contextObj?.context || ''; @@ -152,7 +152,7 @@ export const GeneratePopoverBody: React.FC<{ defaultMessage: 'Generate summary error', }) ); - closePopover(); + // closePopover(); }); }; @@ -206,11 +206,10 @@ export const GeneratePopoverBody: React.FC<{ const dslObject = JSON.parse(dsl); const filters = dslObject?.query?.bool?.filter; if (!filters?.length) return; - const { timeRangeDSL, newFilters } = extractTimeRangeDSL(filters); + const { timeRangeDSL, newFilters, timeFieldName } = extractTimeRangeDSL(filters); // Filter out time range DSL and use this result to build filter query. - if (!timeRangeDSL) return; + if (!timeFieldName || !timeRangeDSL) return; dslObject.query.bool.filter = newFilters; - const timeFieldName = Object.keys(timeRangeDSL)[0]; if (getStartServices) { const [coreStart, startDeps] = await getStartServices(); diff --git a/public/utils/alerting.ts b/public/utils/alerting.ts index fd8fcab2..6f640a85 100644 --- a/public/utils/alerting.ts +++ b/public/utils/alerting.ts @@ -157,6 +157,7 @@ export const validateToTimeRange = (time: string) => { export const extractTimeRangeDSL = (filters: NestedRecord[]) => { let timeRangeDSL; + let timeFieldName; const newFilters = filters.filter((filter: NestedRecord) => { if (filter?.range && typeof filter.range === 'object') { for (const key of Object.keys(filter.range)) { @@ -165,6 +166,7 @@ export const extractTimeRangeDSL = (filters: NestedRecord[]) => { const toValue = rangeValue.to; if (typeof toValue === 'string' && validateToTimeRange(toValue)) { timeRangeDSL = filter.range[key]; + timeFieldName = key; return false; } } @@ -175,5 +177,6 @@ export const extractTimeRangeDSL = (filters: NestedRecord[]) => { return { newFilters, timeRangeDSL, + timeFieldName, }; }; From 2244cfe2093d124bcce02de21c418a93eb9900b6 Mon Sep 17 00:00:00 2001 From: tygao Date: Thu, 24 Oct 2024 17:44:21 +0800 Subject: [PATCH 3/5] update type and test Signed-off-by: tygao --- CHANGELOG.md | 1 + .../incontext_insight/generate_popover_body.test.tsx | 5 +---- public/types.ts | 8 ++++++++ public/utils/alerting.ts | 10 +++++----- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f12273e1..de5fbe62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - feat: take index pattern and query assistant input to text2viz app([#349](https://github.com/opensearch-project/dashboards-assistant/pull/349)) - feat: Hide incompatible index patterns ([#354] (https://github.com/opensearch-project/dashboards-assistant/pull/354)) - feat: edit visualization with natural language in a dialog([#351](https://github.com/opensearch-project/dashboards-assistant/pull/351)) +- fix: Update alerting DSL verify mechanism([#359](https://github.com/opensearch-project/dashboards-assistant/pull/359)) ### 📈 Features/Enhancements diff --git a/public/components/incontext_insight/generate_popover_body.test.tsx b/public/components/incontext_insight/generate_popover_body.test.tsx index 442030cc..7c61d7f6 100644 --- a/public/components/incontext_insight/generate_popover_body.test.tsx +++ b/public/components/incontext_insight/generate_popover_body.test.tsx @@ -81,9 +81,6 @@ const mockDSL = `{ } }`; -const spyWindowOpen = jest.spyOn(window, 'open'); -spyWindowOpen.mockImplementation(jest.fn()); - describe('GeneratePopoverBody', () => { const incontextInsightMock = { contextProvider: jest.fn(), @@ -384,7 +381,7 @@ describe('GeneratePopoverBody', () => { const button = getByText('Discover details'); expect(button).toBeInTheDocument(); fireEvent.click(button); - expect(spyWindowOpen).toHaveBeenCalledWith('data-explorer/discover#?query'); }); + expect(window.open).toHaveBeenCalledWith('formattedUrl', '_blank'); }); }); diff --git a/public/types.ts b/public/types.ts index 334f8e55..dd80f9ea 100644 --- a/public/types.ts +++ b/public/types.ts @@ -143,3 +143,11 @@ export type TabId = 'chat' | 'compose' | 'insights' | 'history' | 'trace'; export interface NestedRecord { [key: string]: T | NestedRecord; } + +export interface DSL { + query?: { + bool?: { + filter?: unknown[]; + }; + }; +} diff --git a/public/utils/alerting.ts b/public/utils/alerting.ts index 6f640a85..9ec848a4 100644 --- a/public/utils/alerting.ts +++ b/public/utils/alerting.ts @@ -15,9 +15,9 @@ import { Filter, } from '../../../../src/plugins/data/public'; import { CoreStart } from '../../../../src/core/public'; -import { NestedRecord } from '../types'; +import { NestedRecord, DSL } from '../types'; -export const buildFilter = (indexPatternId: string, dsl: Record) => { +export const buildFilter = (indexPatternId: string, dsl: DSL) => { const filterAlias = 'Alerting-filters'; return buildCustomFilter( indexPatternId, @@ -72,13 +72,13 @@ export const buildUrlQuery = async ( dataStart: DataPublicPluginStart, savedObjects: CoreStart['savedObjects'], indexPattern: IndexPattern, - dsl: NestedRecord, + dsl: DSL, timeDsl: Record<'from' | 'to', string>, dataSourceId?: string ) => { let filters: Filter[] = []; // If there is none filter after filtering timeRange filter, skip to build filter query. - if (dsl?.query?.bool?.filter?.length > 0) { + if ((dsl?.query?.bool?.filter?.length ?? 0) > 0) { const filter = buildFilter(indexPattern.id!, dsl); const filterManager = dataStart.query.filterManager; // There are some map and flatten operations to filters in filterManager, use this to keep aligned with discover. @@ -158,7 +158,7 @@ export const validateToTimeRange = (time: string) => { export const extractTimeRangeDSL = (filters: NestedRecord[]) => { let timeRangeDSL; let timeFieldName; - const newFilters = filters.filter((filter: NestedRecord) => { + const newFilters = filters.filter((filter) => { if (filter?.range && typeof filter.range === 'object') { for (const key of Object.keys(filter.range)) { const rangeValue = filter.range[key]; From f2114ff162cded7a0e6c5603bf228d7d37d6f3f9 Mon Sep 17 00:00:00 2001 From: tygao Date: Thu, 24 Oct 2024 17:47:34 +0800 Subject: [PATCH 4/5] remove extra comment Signed-off-by: tygao --- public/components/incontext_insight/generate_popover_body.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 2f16af4c..747e31bc 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -93,7 +93,7 @@ export const GeneratePopoverBody: React.FC<{ defaultMessage: 'Generate summary error', }) ); - // closePopover(); + closePopover(); return; } const contextContent = contextObj?.context || ''; @@ -152,7 +152,7 @@ export const GeneratePopoverBody: React.FC<{ defaultMessage: 'Generate summary error', }) ); - // closePopover(); + closePopover(); }); }; From 26d1cf54c1143642de98b2c16d0f528fefd145ee Mon Sep 17 00:00:00 2001 From: tygao Date: Fri, 25 Oct 2024 14:31:45 +0800 Subject: [PATCH 5/5] test: add tests Signed-off-by: tygao --- .../generate_popover_body.tsx | 12 +++- public/utils/alerting.ts | 2 +- public/utils/tests/alerting.test.ts | 66 +++++++++++++++++++ 3 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 public/utils/tests/alerting.test.ts diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 747e31bc..6afdba14 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -62,7 +62,13 @@ export const GeneratePopoverBody: React.FC<{ monitorType === 'query_level_monitor' || monitorType === 'bucket_level_monitor'; let hasTimeRangeFilter = false; if (dsl) { - const dslObject = JSON.parse(dsl); + let dslObject; + try { + dslObject = JSON.parse(dsl); + } catch (e) { + console.error('Invalid DSL', e); + return; + } const filters = dslObject?.query?.bool?.filter; // Filters contains time range filter,if no filters, return. if (!filters?.length) return; @@ -93,7 +99,7 @@ export const GeneratePopoverBody: React.FC<{ defaultMessage: 'Generate summary error', }) ); - closePopover(); + // closePopover(); return; } const contextContent = contextObj?.context || ''; @@ -152,7 +158,7 @@ export const GeneratePopoverBody: React.FC<{ defaultMessage: 'Generate summary error', }) ); - closePopover(); + // closePopover(); }); }; diff --git a/public/utils/alerting.ts b/public/utils/alerting.ts index 9ec848a4..b7d91489 100644 --- a/public/utils/alerting.ts +++ b/public/utils/alerting.ts @@ -149,7 +149,7 @@ export const buildUrlQuery = async ( return hash; }; -export const validateToTimeRange = (time: string) => { +const validateToTimeRange = (time: string) => { // Alerting uses this format in to field of time range filter. const TO_TIME_FORMAT = 'YYYY-MM-DDTHH:mm:ssZ'; return moment.utc(time, TO_TIME_FORMAT, true).isValid(); diff --git a/public/utils/tests/alerting.test.ts b/public/utils/tests/alerting.test.ts new file mode 100644 index 00000000..11164171 --- /dev/null +++ b/public/utils/tests/alerting.test.ts @@ -0,0 +1,66 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { extractTimeRangeDSL } from '../alerting'; + +describe('extractTimeRangeDSL', () => { + it('should only extract utc time range filter', () => { + expect(extractTimeRangeDSL([{ range: { timestamp: { to: 'now' } } }]).timeRangeDSL).toEqual( + undefined + ); + }); + + it('should return undefined timeFiledName if no time range filter', () => { + expect( + extractTimeRangeDSL([ + { + bool: {}, + }, + ]).timeRangeDSL + ).toBe(undefined); + }); + + it('should extract timeFiledName normally', () => { + expect( + extractTimeRangeDSL([ + { + range: { + timestamp: { + from: '2024-10-09T17:40:47+00:00||-1h', + to: '2024-10-09T17:40:47+00:00', + include_lower: true, + include_upper: true, + boost: 1, + }, + }, + }, + { + bool: { + must_not: [ + { + match_phrase: { + response: { + query: '200', + slop: 0, + zero_terms_query: 'NONE', + boost: 1, + }, + }, + }, + ], + adjust_pure_negative: true, + boost: 1, + }, + }, + ]).timeRangeDSL + ).toStrictEqual({ + from: '2024-10-09T17:40:47+00:00||-1h', + to: '2024-10-09T17:40:47+00:00', + include_lower: true, + include_upper: true, + boost: 1, + }); + }); +});