From 2cc2eb15d45c7b7574adc9e9c3bf478575611a63 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 25 Oct 2024 09:49:34 +0000 Subject: [PATCH] refactor: Update alerting DSL verify mechanism (#359) (cherry picked from commit 0f4e48a0d25aa68fb78d4de012ce135ec6578a00) Signed-off-by: github-actions[bot] # Conflicts: # CHANGELOG.md --- .../generate_popover_body.test.tsx | 23 +++++-- .../generate_popover_body.tsx | 50 ++++++++++---- public/types.ts | 12 ++++ public/utils/alerting.ts | 54 ++++++++++++--- public/utils/tests/alerting.test.ts | 66 +++++++++++++++++++ 5 files changed, 176 insertions(+), 29 deletions(-) create mode 100644 public/utils/tests/alerting.test.ts diff --git a/public/components/incontext_insight/generate_popover_body.test.tsx b/public/components/incontext_insight/generate_popover_body.test.tsx index 4b3c1302..7c61d7f6 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 @@ -370,9 +381,7 @@ describe('GeneratePopoverBody', () => { const button = getByText('Discover details'); expect(button).toBeInTheDocument(); fireEvent.click(button); - expect(coreStart.application.navigateToUrl).toHaveBeenCalledWith( - 'data-explorer/discover#?query' - ); }); + expect(window.open).toHaveBeenCalledWith('formattedUrl', '_blank'); }); }); diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index a24b0120..6afdba14 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,25 @@ 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) { + 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; + hasTimeRangeFilter = !!extractTimeRangeDSL(filters).timeRangeDSL; + } + setDisplayDiscoverButton(isSupportedMonitorType && hasTimeRangeFilter); }; getMonitorType(); }, [incontextInsight, setDisplayDiscoverButton]); @@ -83,7 +99,7 @@ export const GeneratePopoverBody: React.FC<{ defaultMessage: 'Generate summary error', }) ); - closePopover(); + // closePopover(); return; } const contextContent = contextObj?.context || ''; @@ -142,7 +158,7 @@ export const GeneratePopoverBody: React.FC<{ defaultMessage: 'Generate summary error', }) ); - closePopover(); + // closePopover(); }); }; @@ -195,12 +211,11 @@ 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, timeFieldName } = extractTimeRangeDSL(filters); + // Filter out time range DSL and use this result to build filter query. + if (!timeFieldName || !timeRangeDSL) return; + dslObject.query.bool.filter = newFilters; if (getStartServices) { const [coreStart, startDeps] = await getStartServices(); @@ -222,11 +237,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..dd80f9ea 100644 --- a/public/types.ts +++ b/public/types.ts @@ -139,3 +139,15 @@ export type IncontextInsightType = | 'error'; 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 d5c02cba..b7d91489 100644 --- a/public/utils/alerting.ts +++ b/public/utils/alerting.ts @@ -5,16 +5,19 @@ 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, DSL } from '../types'; -export const buildFilter = (indexPatternId: string, dsl: Record) => { +export const buildFilter = (indexPatternId: string, dsl: DSL) => { const filterAlias = 'Alerting-filters'; return buildCustomFilter( indexPatternId, @@ -69,16 +72,19 @@ export const buildUrlQuery = async ( dataStart: DataPublicPluginStart, savedObjects: CoreStart['savedObjects'], indexPattern: IndexPattern, - dsl: Record, + dsl: DSL, 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) > 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,35 @@ export const buildUrlQuery = async ( ); return hash; }; + +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; + let timeFieldName; + 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]; + if (typeof rangeValue === 'object' && 'to' in rangeValue) { + const toValue = rangeValue.to; + if (typeof toValue === 'string' && validateToTimeRange(toValue)) { + timeRangeDSL = filter.range[key]; + timeFieldName = key; + return false; + } + } + } + } + return true; + }); + return { + newFilters, + timeRangeDSL, + timeFieldName, + }; +}; 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, + }); + }); +});