Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] [Detections] Multiple timestamp fields #86368

Merged
merged 11 commits into from
Dec 24, 2020
+689 −476
Merged

[Security Solution] [Detections] Multiple timestamp fields #86368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d4ab7a7
query timestamp override and default @timestamp field, adds functiona…
dhurley14 Dec 17, 2020
7997e15
fix logic for when to filter out timestamp override documents
dhurley14 Dec 17, 2020
7f1d7a8
update the total hits field of the search result if we find hits with…
dhurley14 Dec 17, 2020
436fd9e
update integration test, updates logic for performing second search a…
dhurley14 Dec 18, 2020
418e550
cleanup comments, remove commented out console logs, fix logic to bre…
dhurley14 Dec 21, 2020
cfbf038
default param to 'succeeded'
dhurley14 Dec 21, 2020
0e8d154
remove commented out code
dhurley14 Dec 21, 2020
fe827de
always perform a secondary search when timestamp override field is pr…
dhurley14 Dec 21, 2020
9b21ace
perf improvement and fix bug where sortIds were being mixed between s…
dhurley14 Dec 22, 2020
78d7433
set sortIds to undefined when not present on search result
dhurley14 Dec 22, 2020
14f9c77
exit loop and prevent extraneous searches from occurring if we exhaus…
dhurley14 Dec 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .../plugins/security_solution/server/lib/detection_engine/signals/build_events_query.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ describe('create_signals', () => {
size: 100,
searchAfterSortId: undefined,
timestampOverride: undefined,
excludeDocsWithTimestampOverride: false,
});
expect(query).toEqual({
allowNoIndices: true,
Expand Down Expand Up @@ -95,6 +96,7 @@ describe('create_signals', () => {
size: 100,
searchAfterSortId: '',
timestampOverride: undefined,
excludeDocsWithTimestampOverride: false,
});
expect(query).toEqual({
allowNoIndices: true,
Expand Down Expand Up @@ -175,6 +177,7 @@ describe('create_signals', () => {
size: 100,
searchAfterSortId: fakeSortId,
timestampOverride: undefined,
excludeDocsWithTimestampOverride: false,
});
expect(query).toEqual({
allowNoIndices: true,
Expand Down Expand Up @@ -256,6 +259,7 @@ describe('create_signals', () => {
size: 100,
searchAfterSortId: fakeSortIdNumber,
timestampOverride: undefined,
excludeDocsWithTimestampOverride: false,
});
expect(query).toEqual({
allowNoIndices: true,
Expand Down Expand Up @@ -336,6 +340,7 @@ describe('create_signals', () => {
size: 100,
searchAfterSortId: undefined,
timestampOverride: undefined,
excludeDocsWithTimestampOverride: false,
});
expect(query).toEqual({
allowNoIndices: true,
Expand Down Expand Up @@ -423,6 +428,7 @@ describe('create_signals', () => {
size: 100,
searchAfterSortId: undefined,
timestampOverride: undefined,
excludeDocsWithTimestampOverride: false,
});
expect(query).toEqual({
allowNoIndices: true,
Expand Down
94 changes: 47 additions & 47 deletions x-pack/plugins/security_solution/server/lib/detection_engine/signals/build_events_query.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ interface BuildEventsSearchQuery {
sortOrder?: SortOrderOrUndefined;
searchAfterSortId: string | number | undefined;
timestampOverride: TimestampOverrideOrUndefined;
excludeDocsWithTimestampOverride: boolean;
}

export const buildEventsSearchQuery = ({
Expand All @@ -31,66 +32,65 @@ export const buildEventsSearchQuery = ({
searchAfterSortId,
sortOrder,
timestampOverride,
excludeDocsWithTimestampOverride,
}: BuildEventsSearchQuery) => {
const timestamp = timestampOverride ?? '@timestamp';
const docFields =
timestampOverride != null
? [
{
field: '@timestamp',
format: 'strict_date_optional_time',
},
{
field: timestampOverride,
format: 'strict_date_optional_time',
},
]
: [
{
field: '@timestamp',
format: 'strict_date_optional_time',
},
];
const defaultTimeFields = ['@timestamp'];
const timestamps =
timestampOverride != null ? [timestampOverride, ...defaultTimeFields] : defaultTimeFields;
const docFields = timestamps.map((tstamp) => ({
field: tstamp,
format: 'strict_date_optional_time',
}));

const sortField =
timestampOverride != null && !excludeDocsWithTimestampOverride
? timestampOverride
: '@timestamp';

const filterWithTime = [
filter,
const rangeFilter: unknown[] = [
{
bool: {
filter: [
should: [
{
bool: {
should: [
{
range: {
[timestamp]: {
gte: from,
format: 'strict_date_optional_time',
},
},
},
],
minimum_should_match: 1,
range: {
[sortField]: {
gte: from,
format: 'strict_date_optional_time',
},
},
},
],
minimum_should_match: 1,
},
},
{
bool: {
should: [
{
bool: {
should: [
{
range: {
[timestamp]: {
lte: to,
format: 'strict_date_optional_time',
},
},
},
],
minimum_should_match: 1,
range: {
[sortField]: {
lte: to,
format: 'strict_date_optional_time',
},
},
},
],
minimum_should_match: 1,
},
},
];
if (excludeDocsWithTimestampOverride) {
rangeFilter.push({
bool: {
must_not: {
exists: {
field: timestampOverride,
},
},
},
});
}
const filterWithTime = [filter, { bool: { filter: rangeFilter } }];

const searchQuery = {
allowNoIndices: true,
Expand All @@ -112,7 +112,7 @@ export const buildEventsSearchQuery = ({
...(aggregations ? { aggregations } : {}),
sort: [
{
[timestamp]: {
[sortField]: {
order: sortOrder ?? 'asc',
},
},
Expand Down
1 change: 1 addition & 0 deletions ...k/plugins/security_solution/server/lib/detection_engine/signals/find_threshold_signals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,6 @@ export const findThresholdSignals = async ({
pageSize: 1,
sortOrder: 'desc',
buildRuleMessage,
excludeDocsWithTimestampOverride: false,
});
};
33 changes: 4 additions & 29 deletions ...ns/security_solution/server/lib/detection_engine/signals/search_after_bulk_create.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,31 +183,6 @@ describe('searchAfterAndBulkCreate', () => {
},
],
})
.mockResolvedValueOnce(sampleDocSearchResultsNoSortIdNoHits())
.mockResolvedValueOnce(repeatedSearchResultsWithSortId(4, 1, someGuids.slice(9, 12)))
.mockResolvedValueOnce({
took: 100,
errors: false,
items: [
{
create: {
status: 201,
},
},
],
})
.mockResolvedValueOnce(repeatedSearchResultsWithSortId(4, 1, someGuids.slice(0, 3)))
.mockResolvedValueOnce({
took: 100,
errors: false,
items: [
{
create: {
status: 201,
},
},
],
})
.mockResolvedValueOnce(sampleDocSearchResultsNoSortIdNoHits());

const exceptionItem = getExceptionListItemSchemaMock();
Expand Down Expand Up @@ -250,8 +225,8 @@ describe('searchAfterAndBulkCreate', () => {
buildRuleMessage,
});
expect(success).toEqual(true);
expect(mockService.callCluster).toHaveBeenCalledTimes(12);
expect(createdSignalsCount).toEqual(5);
expect(mockService.callCluster).toHaveBeenCalledTimes(8);
expect(createdSignalsCount).toEqual(3);
expect(lastLookBackDate).toEqual(new Date('2020-04-20T21:27:45+0000'));
});

Expand Down Expand Up @@ -461,7 +436,7 @@ describe('searchAfterAndBulkCreate', () => {
// I don't like testing log statements since logs change but this is the best
// way I can think of to ensure this section is getting hit with this test case.
expect(((mockLogger.debug as unknown) as jest.Mock).mock.calls[8][0]).toContain(
'sortIds was empty on searchResult'
'ran out of sort ids to sort on name: "fake name" id: "fake id" rule id: "fake rule id" signals index: "fakeindex"'
);
});

Expand Down Expand Up @@ -542,7 +517,7 @@ describe('searchAfterAndBulkCreate', () => {
// I don't like testing log statements since logs change but this is the best
// way I can think of to ensure this section is getting hit with this test case.
expect(((mockLogger.debug as unknown) as jest.Mock).mock.calls[15][0]).toContain(
'sortIds was empty on searchResult name: "fake name" id: "fake id" rule id: "fake rule id" signals index: "fakeindex"'
'ran out of sort ids to sort on name: "fake name" id: "fake id" rule id: "fake rule id" signals index: "fakeindex"'
);
});

Expand Down
Loading