Skip to content

Commit

Permalink
[SIEM][Detection Engine] Silence 409 errors on signal creation (#53859)…
Browse files Browse the repository at this point in the history
… (#53903)

* 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 <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
rylnd and elasticmachine authored Jan 3, 2020
1 parent 44afac2 commit 126da30
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 31 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand All @@ -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);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
};

0 comments on commit 126da30

Please sign in to comment.