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.

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

Strip field from events for event search #149113

Merged
merged 11 commits into from
Jan 27, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,61 @@ describe('create_signals', () => {
});
});

test('it respects overriderBody params', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test('it respects overriderBody params', () => {
test('it respects overrideBody params', () => {

small typo

const query = buildEventsSearchQuery({
index: ['auditbeat-*'],
from: 'now-5m',
to: 'today',
filter: {},
size: 100,
searchAfterSortIds: undefined,
primaryTimestamp: '@timestamp',
secondaryTimestamp: undefined,
runtimeMappings: undefined,
overrideBody: {
_source: false,
fields: ['@timestamp'],
},
});
expect(query).toEqual({
allow_no_indices: true,
index: ['auditbeat-*'],
size: 100,
ignore_unavailable: true,
body: {
query: {
bool: {
filter: [
{},
{
range: {
'@timestamp': {
gte: 'now-5m',
lte: 'today',
format: 'strict_date_optional_time',
},
},
},
{
match_all: {},
},
],
},
},
_source: false,
fields: ['@timestamp'],
sort: [
{
'@timestamp': {
order: 'asc',
unmapped_type: 'date',
},
},
],
},
});
});

describe('buildEqlSearchRequest', () => {
test('should build a basic request with time range', () => {
const request = buildEqlSearchRequest({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { isEmpty } from 'lodash';
import type { Filter } from '@kbn/es-query';
import type { OverrideBodyQuery } from './types';
import type {
RuleFilterArray,
TimestampOverride,
Expand All @@ -27,6 +28,7 @@ interface BuildEventsSearchQuery {
secondaryTimestamp: TimestampOverride | undefined;
trackTotalHits?: boolean;
additionalFilters?: estypes.QueryDslQueryContainer[];
overrideBody?: OverrideBodyQuery;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nkhristinin could we add some js docs comments explaining what this property is used for? Something like this suggestion?

Suggested change
overrideBody?: OverrideBodyQuery;
/**
* overrideBody allows the search after to ignore the _source property of the result, thus reducing the size of the response and increasing the performance of the query.
*/
overrideBody?: OverrideBodyQuery;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks! Added comment to a single search function.

}

interface BuildEqlSearchRequestParams {
Expand Down Expand Up @@ -132,6 +134,7 @@ export const buildEventsSearchQuery = ({
secondaryTimestamp,
trackTotalHits,
additionalFilters,
overrideBody,
}: BuildEventsSearchQuery) => {
const timestamps = secondaryTimestamp
? [primaryTimestamp, secondaryTimestamp]
Expand Down Expand Up @@ -193,6 +196,7 @@ export const buildEventsSearchQuery = ({
...(aggregations ? { aggregations } : {}),
runtime_mappings: runtimeMappings,
sort,
...overrideBody,
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import type { RuleExecutorServicesMock } from '@kbn/alerting-plugin/server/mocks
import { alertsMock } from '@kbn/alerting-plugin/server/mocks';
import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks';
import { ruleExecutionLogMock } from '../rule_monitoring/mocks';
import { buildEventsSearchQuery } from './build_events_query';

jest.mock('./build_events_query');
const mockBuildEventsSearchQuery = buildEventsSearchQuery as jest.Mock;

describe('singleSearchAfter', () => {
const mockService: RuleExecutorServicesMock = alertsMock.createRuleExecutorServices();
Expand Down Expand Up @@ -157,4 +161,47 @@ describe('singleSearchAfter', () => {
})
).rejects.toThrow('Fake Error');
});

test('singleSearchAfter passes overrideBody to buildEventsSearchQuery', async () => {
mockService.scopedClusterClient.asCurrentUser.search.mockResolvedValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise(sampleDocSearchResultsNoSortId())
);
await singleSearchAfter({
searchAfterSortIds: undefined,
index: [],
from: 'now-360s',
to: 'now',
services: mockService,
ruleExecutionLogger,
pageSize: 1,
filter: {},
primaryTimestamp: '@timestamp',
secondaryTimestamp: undefined,
runtimeMappings: undefined,
overrideBody: {
_source: false,
fields: ['@timestamp'],
},
});

expect(mockBuildEventsSearchQuery).toHaveBeenCalledWith({
additionalFilters: undefined,
aggregations: undefined,
filter: {},
from: 'now-360s',
index: [],
primaryTimestamp: '@timestamp',
runtimeMappings: undefined,
searchAfterSortIds: undefined,
secondaryTimestamp: undefined,
size: 1,
sortOrder: undefined,
to: 'now',
trackTotalHits: undefined,
overrideBody: {
_source: false,
fields: ['@timestamp'],
},
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import type {
AlertInstanceState,
RuleExecutorServices,
} from '@kbn/alerting-plugin/server';
import type { SignalSearchResponse, SignalSource } from './types';
import type { SignalSearchResponse, SignalSource, OverrideBodyQuery } from './types';
import { buildEventsSearchQuery } from './build_events_query';
import { createErrorsFromShard, makeFloatString } from './utils';
import type { TimestampOverride } from '../../../../common/detection_engine/rule_schema';
Expand All @@ -34,6 +34,7 @@ interface SingleSearchAfterParams {
trackTotalHits?: boolean;
runtimeMappings: estypes.MappingRuntimeFields | undefined;
additionalFilters?: estypes.QueryDslQueryContainer[];
overrideBody?: OverrideBodyQuery;
}

// utilize search_after for paging results into bulk.
Expand All @@ -55,6 +56,7 @@ export const singleSearchAfter = async <
secondaryTimestamp,
trackTotalHits,
additionalFilters,
overrideBody,
}: SingleSearchAfterParams): Promise<{
searchResult: SignalSearchResponse<TAggregations>;
searchDuration: string;
Expand All @@ -76,6 +78,7 @@ export const singleSearchAfter = async <
secondaryTimestamp,
trackTotalHits,
additionalFilters,
overrideBody,
});

const start = performance.now();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ export const createThreatSignals = async ({
_source: false,
};

const eventListConfig = {
fields: threatMapping.map((mapping) => mapping.entries.map((item) => item.field)).flat(),
_source: false,
};

const threatEnrichment = buildThreatEnrichment({
ruleExecutionLogger,
services,
Expand Down Expand Up @@ -197,6 +202,7 @@ export const createThreatSignals = async ({
primaryTimestamp,
secondaryTimestamp,
exceptionFilter,
eventListConfig,
}),

createSignal: (slicedChunk) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const getEventList = async ({
secondaryTimestamp,
runtimeMappings,
exceptionFilter,
eventListConfig,
}: EventsOptions): Promise<estypes.SearchResponse<EventDoc>> => {
const calculatedPerPage = perPage ?? MAX_PER_PAGE;
if (calculatedPerPage > 10000) {
Expand Down Expand Up @@ -59,6 +60,7 @@ export const getEventList = async ({
sortOrder: 'desc',
trackTotalHits: false,
runtimeMappings,
overrideBody: eventListConfig,
});

ruleExecutionLogger.debug(`Retrieved events items of size: ${searchResult.hits.hits.length}`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import type {
SearchAfterAndBulkCreateReturnType,
SignalsEnrichment,
WrapHits,
OverrideBodyQuery,
} from '../types';
import type { CompleteRule, ThreatRuleParams } from '../../rule_schema';
import type { IRuleExecutionLogForExecutors } from '../../rule_monitoring';
Expand Down Expand Up @@ -265,6 +266,7 @@ export interface EventsOptions {
tuple: RuleRangeTuple;
runtimeMappings: estypes.MappingRuntimeFields | undefined;
exceptionFilter: Filter | undefined;
eventListConfig?: OverrideBodyQuery;
}

export interface EventDoc {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,3 +342,8 @@ export type EventGroupingMultiBucketAggregationResult = ESSearchResponse<
};
}
>;

export interface OverrideBodyQuery {
_source: estypes.SearchSourceConfig;
fields: estypes.Fields;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

search query body has some other fields as well, as: sort, query etc.
Defining only 2 of these might be unnecessary restrictive, given its name suggests the whole body override.

On second note - would it make sense to define them as non required fields in this interface? It would make this parameter more flexible, if you would want to override _source field only, for example

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion!

I changed those fields to optional.

I was trying to change OverrideBodyQuery to estypes.SearchRequest['body'], but it wasn't working, because looks like
buildEventsSearchQuery return compatible but not the exact type.

Later in a single search after we cast this query to estypes.SearchRequest

So if somebody need to add new fields, it shouldn't be a problem to add it into OverrideBodyQuery

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for looking into it.
I think it's good to go.
With added comment it should not be an issue to add new fields if needed

}