[8.12] [Discover][Alerts] Fix Discover results when alert excludes ma…

…tches from previous runs (elastic#176690) (elastic#176931) # Backport This will backport the following commits from `main` to `8.12`: - [[Discover][Alerts] Fix Discover results when alert excludes matches from previous runs (elastic#176690)](elastic#176690)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Julia Rechkunova <[email protected]>
criamico · Feb 14, 2024 · e446672 · e446672
1 parent 4884e48
commit e446672
Show file tree

Hide file tree

Showing 2 changed files with 164 additions and 28 deletions.
diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.test.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.test.ts
@@ -7,14 +7,22 @@
 
 import { OnlySearchSourceRuleParams } from '../types';
 import { createSearchSourceMock } from '@kbn/data-plugin/common/search/search_source/mocks';
-import { updateSearchSource, getSmallerDataViewSpec } from './fetch_search_source_query';
+import {
+  updateSearchSource,
+  generateLink,
+  updateFilterReferences,
+  getSmallerDataViewSpec,
+} from './fetch_search_source_query';
 import {
   createStubDataView,
   stubbedSavedObjectIndexPattern,
 } from '@kbn/data-views-plugin/common/data_view.stub';
-import { DataView } from '@kbn/data-views-plugin/common';
+import { DataView, DataViewSpec } from '@kbn/data-views-plugin/common';
 import { fieldFormatsMock } from '@kbn/field-formats-plugin/common/mocks';
 import { Comparator } from '../../../../common/comparator_types';
+import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks';
+import { DiscoverAppLocatorParams } from '@kbn/discover-plugin/common';
+import { LocatorPublic } from '@kbn/share-plugin/common';
 
 const createDataView = () => {
   const id = 'test-id';
@@ -55,26 +63,27 @@ const defaultParams: OnlySearchSourceRuleParams = {
 };
 
 describe('fetchSearchSourceQuery', () => {
-  describe('updateSearchSource', () => {
-    const dataViewMock = createDataView();
-    afterAll(() => {
-      jest.resetAllMocks();
-    });
+  const dataViewMock = createDataView();
 
-    const fakeNow = new Date('2020-02-09T23:15:41.941Z');
+  afterAll(() => {
+    jest.resetAllMocks();
+  });
 
-    beforeAll(() => {
-      jest.resetAllMocks();
-      global.Date.now = jest.fn(() => fakeNow.getTime());
-    });
+  const fakeNow = new Date('2020-02-09T23:15:41.941Z');
+
+  beforeAll(() => {
+    jest.resetAllMocks();
+    global.Date.now = jest.fn(() => fakeNow.getTime());
+  });
 
+  describe('updateSearchSource', () => {
     it('without latest timestamp', async () => {
       const params = { ...defaultParams, thresholdComparator: Comparator.GT_OR_EQ, threshold: [3] };
 
       const searchSourceInstance = createSearchSourceMock({ index: dataViewMock });
 
       const { dateStart, dateEnd } = getTimeRange();
-      const searchSource = updateSearchSource(
+      const { searchSource, filterToExcludeHitsFromPreviousRun } = updateSearchSource(
         searchSourceInstance,
         dataViewMock,
         params,
@@ -83,6 +92,7 @@ describe('fetchSearchSourceQuery', () => {
         dateEnd
       );
       const searchRequest = searchSource.getSearchRequestBody();
+      expect(filterToExcludeHitsFromPreviousRun).toBe(null);
       expect(searchRequest.size).toMatchInlineSnapshot(`100`);
       expect(searchRequest.query).toMatchInlineSnapshot(`
         Object {
@@ -113,7 +123,7 @@ describe('fetchSearchSourceQuery', () => {
       const searchSourceInstance = createSearchSourceMock({ index: dataViewMock });
 
       const { dateStart, dateEnd } = getTimeRange();
-      const searchSource = updateSearchSource(
+      const { searchSource, filterToExcludeHitsFromPreviousRun } = updateSearchSource(
         searchSourceInstance,
         dataViewMock,
         params,
@@ -122,6 +132,23 @@ describe('fetchSearchSourceQuery', () => {
         dateEnd
       );
       const searchRequest = searchSource.getSearchRequestBody();
+      expect(filterToExcludeHitsFromPreviousRun).toMatchInlineSnapshot(`
+        Object {
+          "meta": Object {
+            "field": "time",
+            "index": "test-id",
+            "params": Object {},
+          },
+          "query": Object {
+            "range": Object {
+              "time": Object {
+                "format": "strict_date_optional_time",
+                "gt": "2020-02-09T23:12:41.941Z",
+              },
+            },
+          },
+        }
+      `);
       expect(searchRequest.size).toMatchInlineSnapshot(`100`);
       expect(searchRequest.query).toMatchInlineSnapshot(`
         Object {
@@ -160,7 +187,7 @@ describe('fetchSearchSourceQuery', () => {
       const searchSourceInstance = createSearchSourceMock({ index: dataViewMock });
 
       const { dateStart, dateEnd } = getTimeRange();
-      const searchSource = updateSearchSource(
+      const { searchSource, filterToExcludeHitsFromPreviousRun } = updateSearchSource(
         searchSourceInstance,
         dataViewMock,
         params,
@@ -169,6 +196,7 @@ describe('fetchSearchSourceQuery', () => {
         dateEnd
       );
       const searchRequest = searchSource.getSearchRequestBody();
+      expect(filterToExcludeHitsFromPreviousRun).toBe(null);
       expect(searchRequest.size).toMatchInlineSnapshot(`100`);
       expect(searchRequest.query).toMatchInlineSnapshot(`
         Object {
@@ -199,7 +227,7 @@ describe('fetchSearchSourceQuery', () => {
       const searchSourceInstance = createSearchSourceMock({ index: dataViewMock });
 
       const { dateStart, dateEnd } = getTimeRange();
-      const searchSource = updateSearchSource(
+      const { searchSource, filterToExcludeHitsFromPreviousRun } = updateSearchSource(
         searchSourceInstance,
         dataViewMock,
         params,
@@ -208,6 +236,7 @@ describe('fetchSearchSourceQuery', () => {
         dateEnd
       );
       const searchRequest = searchSource.getSearchRequestBody();
+      expect(filterToExcludeHitsFromPreviousRun).toBe(null);
       expect(searchRequest.size).toMatchInlineSnapshot(`100`);
       expect(searchRequest.query).toMatchInlineSnapshot(`
         Object {
@@ -244,7 +273,7 @@ describe('fetchSearchSourceQuery', () => {
       const searchSourceInstance = createSearchSourceMock({ index: dataViewMock });
 
       const { dateStart, dateEnd } = getTimeRange();
-      const searchSource = updateSearchSource(
+      const { searchSource } = updateSearchSource(
         searchSourceInstance,
         dataViewMock,
         params,
@@ -307,6 +336,95 @@ describe('fetchSearchSourceQuery', () => {
     });
   });
 
+  describe('generateLink', () => {
+    it('should include additional time filter', async () => {
+      const params = { ...defaultParams, thresholdComparator: Comparator.GT_OR_EQ, threshold: [3] };
+
+      const searchSourceInstance = createSearchSourceMock({ index: dataViewMock });
+
+      const { dateStart, dateEnd } = getTimeRange();
+      const { filterToExcludeHitsFromPreviousRun } = updateSearchSource(
+        searchSourceInstance,
+        dataViewMock,
+        params,
+        '2020-02-09T23:12:41.941Z',
+        dateStart,
+        dateEnd
+      );
+
+      expect(filterToExcludeHitsFromPreviousRun).toMatchInlineSnapshot(`
+        Object {
+          "meta": Object {
+            "field": "time",
+            "index": "test-id",
+            "params": Object {},
+          },
+          "query": Object {
+            "range": Object {
+              "time": Object {
+                "format": "strict_date_optional_time",
+                "gt": "2020-02-09T23:12:41.941Z",
+              },
+            },
+          },
+        }
+      `);
+
+      const locatorMock = {
+        getRedirectUrl: jest.fn(() => '/app/r?l=DISCOVER_APP_LOCATOR'),
+      } as unknown as LocatorPublic<DiscoverAppLocatorParams>;
+
+      const dataViews = {
+        ...dataViewPluginMocks.createStartContract(),
+        create: async (spec: DataViewSpec) =>
+          new DataView({ spec, fieldFormats: fieldFormatsMock }),
+      };
+
+      const linkWithoutExcludedRuns = await generateLink(
+        searchSourceInstance,
+        locatorMock,
+        dataViews,
+        dataViewMock,
+        dateStart,
+        dateEnd,
+        'test1',
+        null
+      );
+
+      expect(linkWithoutExcludedRuns).toBe('test1/app/r?l=DISCOVER_APP_LOCATOR');
+      expect(locatorMock.getRedirectUrl).toHaveBeenCalledWith(
+        expect.objectContaining({
+          filters: [],
+        })
+      );
+
+      const linkWithExcludedRuns = await generateLink(
+        searchSourceInstance,
+        locatorMock,
+        dataViews,
+        dataViewMock,
+        dateStart,
+        dateEnd,
+        'test2',
+        filterToExcludeHitsFromPreviousRun
+      );
+
+      expect(linkWithExcludedRuns).toBe('test2/app/r?l=DISCOVER_APP_LOCATOR');
+      expect(locatorMock.getRedirectUrl).toHaveBeenNthCalledWith(
+        2,
+        expect.objectContaining({
+          filters: expect.arrayContaining(
+            updateFilterReferences(
+              [filterToExcludeHitsFromPreviousRun!],
+              dataViewMock.id!,
+              undefined
+            )
+          ),
+        })
+      );
+    });
+  });
+
   describe('getSmallerDataViewSpec', () => {
     it('should remove "count"s but keep other props like "customLabel"', async () => {
       const fieldsMap = {

diff --git a/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.ts b/x-pack/plugins/stack_alerts/server/rule_types/es_query/lib/fetch_search_source_query.ts
@@ -60,7 +60,7 @@ export async function fetchSearchSourceQuery({
   const initialSearchSource = await searchSourceClient.create(params.searchConfiguration);
 
   const index = initialSearchSource.getField('index') as DataView;
-  const searchSource = updateSearchSource(
+  const { searchSource, filterToExcludeHitsFromPreviousRun } = updateSearchSource(
     initialSearchSource,
     index,
     params,
@@ -85,7 +85,8 @@ export async function fetchSearchSourceQuery({
     index,
     dateStart,
     dateEnd,
-    spacePrefix
+    spacePrefix,
+    filterToExcludeHitsFromPreviousRun
   );
   return {
     link,
@@ -104,7 +105,7 @@ export function updateSearchSource(
   dateStart: string,
   dateEnd: string,
   alertLimit?: number
-) {
+): { searchSource: ISearchSource; filterToExcludeHitsFromPreviousRun: Filter | null } {
   const isGroupAgg = isGroupAggregation(params.termField);
   const timeFieldName = params.timeField || index.timeFieldName;
 
@@ -123,16 +124,17 @@ export function updateSearchSource(
     ),
   ];
 
+  let filterToExcludeHitsFromPreviousRun = null;
   if (params.excludeHitsFromPreviousRun) {
     if (latestTimestamp && latestTimestamp > dateStart) {
-      // add additional filter for documents with a timestamp greater then
+      // add additional filter for documents with a timestamp greater than
       // the timestamp of the previous run, so that those documents are not counted twice
-      const addTimeRangeField = buildRangeFilter(
+      filterToExcludeHitsFromPreviousRun = buildRangeFilter(
         field!,
         { gt: latestTimestamp, format: 'strict_date_optional_time' },
         index
       );
-      filters.push(addTimeRangeField);
+      filters.push(filterToExcludeHitsFromPreviousRun);
     }
   }
 
@@ -164,19 +166,31 @@ export function updateSearchSource(
       ...(isGroupAgg ? { topHitsSize: params.size } : {}),
     })
   );
-  return searchSourceChild;
+  return {
+    searchSource: searchSourceChild,
+    filterToExcludeHitsFromPreviousRun,
+  };
 }
 
-async function generateLink(
+export async function generateLink(
   searchSource: ISearchSource,
   discoverLocator: LocatorPublic<DiscoverAppLocatorParams>,
   dataViews: DataViewsContract,
   dataViewToUpdate: DataView,
   dateStart: string,
   dateEnd: string,
-  spacePrefix: string
+  spacePrefix: string,
+  filterToExcludeHitsFromPreviousRun: Filter | null
 ) {
-  const prevFilters = searchSource.getField('filter') as Filter[];
+  const prevFilters = [...((searchSource.getField('filter') as Filter[]) || [])];
+
+  if (filterToExcludeHitsFromPreviousRun) {
+    // Using the same additional filter as in the alert check above.
+    // We cannot simply pass `latestTimestamp` to `timeRange.from` Discover locator params
+    // as that would include `latestTimestamp` itself in the Discover results which would be wrong.
+    // Results should be after `latestTimestamp` and within `dateStart` and `dateEnd`.
+    prevFilters.push(filterToExcludeHitsFromPreviousRun);
+  }
 
   // make new adhoc data view
   const newDataView = await dataViews.create({
@@ -202,7 +216,11 @@ async function generateLink(
   return start + spacePrefix + '/app' + end;
 }
 
-function updateFilterReferences(filters: Filter[], fromDataView: string, toDataView: string) {
+export function updateFilterReferences(
+  filters: Filter[],
+  fromDataView: string,
+  toDataView: string | undefined
+) {
   return (filters || []).map((filter) => {
     if (filter.meta.index === fromDataView) {
       return {