[SIEM][Detection Engine] Silence 409 errors on signal creation (#53859)

* Remove punctuation from translation We already had a colon on both uses of this key, resulting in '::' on the page. * Ignore 409 errors from our signal creation In my experience these are always due to a rule being run multiple times on the same document, generating a duplicate signal with a (correctly) duplicate id. Only if we encounter non-409 errors do we log a message to the user. * Hide 409 errors during signal creation These are expected and potentially confusing to the user. Instead, we only show unexpected errors. Co-authored-by: Elastic Machine <[email protected]>
elastic · Jan 2, 2020 · d9554ff · d9554ff
1 parent 47e5342
commit d9554ff
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 31 deletions.
diff --git a/x-pack/legacy/plugins/siem/public/pages/detection_engine/translations.ts b/x-pack/legacy/plugins/siem/public/pages/detection_engine/translations.ts
@@ -11,7 +11,7 @@ export const PAGE_TITLE = i18n.translate('xpack.siem.detectionEngine.pageTitle',
 });
 
 export const LAST_SIGNAL = i18n.translate('xpack.siem.detectionEngine.lastSignalTitle', {
-  defaultMessage: 'Last signal:',
+  defaultMessage: 'Last signal',
 });
 
 export const TOTAL_SIGNAL = i18n.translate('xpack.siem.detectionEngine.totalSignalTitle', {

diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/__mocks__/es_results.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/__mocks__/es_results.ts
@@ -144,6 +144,28 @@ export const sampleBulkCreateDuplicateResult = {
   ],
 };
 
+export const sampleBulkCreateErrorResult = {
+  ...sampleBulkCreateDuplicateResult,
+  items: [
+    ...sampleBulkCreateDuplicateResult.items,
+    {
+      create: {
+        _index: 'test',
+        _type: '_doc',
+        _id: '5',
+        status: 500,
+        error: {
+          type: 'internal_server_error',
+          reason: '[4]: internal server error',
+          index_uuid: 'cXmq4Rt3RGGswDTTwZFzvA',
+          shard: '0',
+          index: 'test',
+        },
+      },
+    },
+  ],
+};
+
 export const sampleDocSearchResultsNoSortId = (
   someUuid: string = sampleIdGuid
 ): SignalSearchResponse => ({

diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/single_bulk_create.test.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/single_bulk_create.test.ts
@@ -13,6 +13,7 @@ import {
   sampleDocSearchResultsNoSortIdNoVersion,
   sampleEmptyDocSearchResults,
   sampleBulkCreateDuplicateResult,
+  sampleBulkCreateErrorResult,
 } from './__mocks__/es_results';
 import { savedObjectsClientMock } from 'src/core/server/mocks';
 import { DEFAULT_SIGNALS_INDEX } from '../../../../common/constants';
@@ -206,7 +207,8 @@ describe('singleBulkCreate', () => {
     });
     expect(successfulsingleBulkCreate).toEqual(true);
   });
-  test('create successful bulk create when bulk create has errors', async () => {
+
+  test('create successful bulk create when bulk create has duplicate errors', async () => {
     const sampleParams = sampleRuleAlertParams();
     const sampleSearchResult = sampleDocSearchResultsNoSortId;
     mockService.callCluster.mockReturnValue(sampleBulkCreateDuplicateResult);
@@ -224,6 +226,30 @@ describe('singleBulkCreate', () => {
       enabled: true,
       tags: ['some fake tag 1', 'some fake tag 2'],
     });
+
+    expect(mockLogger.error).not.toHaveBeenCalled();
+    expect(successfulsingleBulkCreate).toEqual(true);
+  });
+
+  test('create successful bulk create when bulk create has multiple error statuses', async () => {
+    const sampleParams = sampleRuleAlertParams();
+    const sampleSearchResult = sampleDocSearchResultsNoSortId;
+    mockService.callCluster.mockReturnValue(sampleBulkCreateErrorResult);
+    const successfulsingleBulkCreate = await singleBulkCreate({
+      someResult: sampleSearchResult(),
+      ruleParams: sampleParams,
+      services: mockService,
+      logger: mockLogger,
+      id: sampleRuleGuid,
+      signalsIndex: DEFAULT_SIGNALS_INDEX,
+      name: 'rule-name',
+      createdBy: 'elastic',
+      updatedBy: 'elastic',
+      interval: '5m',
+      enabled: true,
+      tags: ['some fake tag 1', 'some fake tag 2'],
+    });
+
     expect(mockLogger.error).toHaveBeenCalled();
     expect(successfulsingleBulkCreate).toEqual(true);
   });

diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/single_bulk_create.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/single_bulk_create.ts
@@ -4,6 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
+import { countBy, isEmpty } from 'lodash';
 import { performance } from 'perf_hooks';
 import { AlertServices } from '../../../../../alerting/server/types';
 import { SignalSearchResponse, BulkResponse } from './types';
@@ -68,39 +69,30 @@ export const singleBulkCreate = async ({
     },
     buildBulkBody({ doc, ruleParams, id, name, createdBy, updatedBy, interval, enabled, tags }),
   ]);
-  const time1 = performance.now();
-  const firstResult: BulkResponse = await services.callCluster('bulk', {
+  const start = performance.now();
+  const response: BulkResponse = await services.callCluster('bulk', {
     index: signalsIndex,
     refresh: false,
     body: bulkBody,
   });
-  const time2 = performance.now();
-  logger.debug(
-    `individual bulk process time took: ${Number(time2 - time1).toFixed(2)} milliseconds`
-  );
-  logger.debug(`took property says bulk took: ${firstResult.took} milliseconds`);
-  if (firstResult.errors) {
-    // go through the response status errors and see what
-    // types of errors they are, count them up, and log them.
-    const errorCountMap = firstResult.items.reduce((acc: { [key: string]: number }, item) => {
-      if (item.create.error) {
-        const responseStatusKey = item.create.status.toString();
-        acc[responseStatusKey] = acc[responseStatusKey] ? acc[responseStatusKey] + 1 : 1;
-      }
-      return acc;
-    }, {});
-    /*
-     the logging output below should look like
-     {'409': 55}
-     which is read as "there were 55 counts of 409 errors returned from bulk create"
-    */
-    logger.error(
-      `[-] bulkResponse had errors with response statuses:counts of...\n${JSON.stringify(
-        errorCountMap,
-        null,
-        2
-      )}`
-    );
+  const end = performance.now();
+  logger.debug(`individual bulk process time took: ${Number(end - start).toFixed(2)} milliseconds`);
+  logger.debug(`took property says bulk took: ${response.took} milliseconds`);
+
+  if (response.errors) {
+    const itemsWithErrors = response.items.filter(item => item.create.error);
+    const errorCountsByStatus = countBy(itemsWithErrors, item => item.create.status);
+    delete errorCountsByStatus['409']; // Duplicate signals are expected
+
+    if (!isEmpty(errorCountsByStatus)) {
+      logger.error(
+        `[-] bulkResponse had errors with response statuses:counts of...\n${JSON.stringify(
+          errorCountsByStatus,
+          null,
+          2
+        )}`
+      );
+    }
   }
   return true;
 };