[8.12] [Discover][Alerts] Fix Discover results when alert excludes matches from previous runs (#176690) (#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 (#176690)](https://github.com/elastic/kibana/pull/176690)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Julia Rechkunova <julia.rechkunova@elastic.co>
2025-04-24 01:38:56 -04:00 · 2024-02-14 13:07:49 -05:00 · 2024-02-14 13:07:49 -05:00 · e446672754
commit e446672754
parent 4884e48e60
2 changed files with 166 additions and 30 deletions
--- 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', () => {
+  const dataViewMock = createDataView();
+
+  afterAll(() => {
+    jest.resetAllMocks();
+  });
+
+  const fakeNow = new Date('2020-02-09T23:15:41.941Z');
+
+  beforeAll(() => {
+    jest.resetAllMocks();
+    global.Date.now = jest.fn(() => fakeNow.getTime());
+  });
+
  describe('updateSearchSource', () => {
-    const dataViewMock = createDataView();
-    afterAll(() => {
-      jest.resetAllMocks();
-    });
-
-    const fakeNow = new Date('2020-02-09T23:15:41.941Z');
-
-    beforeAll(() => {
-      jest.resetAllMocks();
-      global.Date.now = jest.fn(() => fakeNow.getTime());
-    });
-
    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 = {
--- 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 {