From 8ef6773ac89f267f970b9447f5254a499febc64c Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Mon, 19 Dec 2022 18:35:31 +0100 Subject: [PATCH] [ML] Explain Log Rate Spikes: Fix field candidate selection. (#147614) The field candidate selection for Explain Log Rate Spikes was missing a check if the supported field type is also aggregatable. For example, a `keyword` type field could still be non-aggregatable if it was both not indexed and `doc_values` set to `false`. Additionally, if no groups were detected, we showed a "Try to continue analysis" button in the UI even if the analysis was able to finish. In this PR the artificial logs dataset for functional tests was extended to include a field like that. (cherry picked from commit aecad27159764d8ea2d0aeddc94fd03954d480e5) --- .../explain_log_rate_spikes_analysis.tsx | 5 +++-- .../server/routes/queries/fetch_index_info.test.ts | 13 ++++++++++--- .../aiops/server/routes/queries/fetch_index_info.ts | 3 ++- .../aiops/explain_log_rate_spikes_data_generator.ts | 5 +++++ 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/aiops/public/components/explain_log_rate_spikes/explain_log_rate_spikes_analysis.tsx b/x-pack/plugins/aiops/public/components/explain_log_rate_spikes/explain_log_rate_spikes_analysis.tsx index dd67151d1e2c3c..4b8ad8a8919617 100644 --- a/x-pack/plugins/aiops/public/components/explain_log_rate_spikes/explain_log_rate_spikes_analysis.tsx +++ b/x-pack/plugins/aiops/public/components/explain_log_rate_spikes/explain_log_rate_spikes_analysis.tsx @@ -112,8 +112,9 @@ export const ExplainLogRateSpikesAnalysis: FC const { loaded, remainingFieldCandidates, groupsMissing } = data; if ( - (Array.isArray(remainingFieldCandidates) && remainingFieldCandidates.length > 0) || - groupsMissing + loaded < 1 && + ((Array.isArray(remainingFieldCandidates) && remainingFieldCandidates.length > 0) || + groupsMissing) ) { setOverrides({ loaded, remainingFieldCandidates, changePoints: data.changePoints }); } else { diff --git a/x-pack/plugins/aiops/server/routes/queries/fetch_index_info.test.ts b/x-pack/plugins/aiops/server/routes/queries/fetch_index_info.test.ts index 084c415a652cd0..f38feaee8a58b2 100644 --- a/x-pack/plugins/aiops/server/routes/queries/fetch_index_info.test.ts +++ b/x-pack/plugins/aiops/server/routes/queries/fetch_index_info.test.ts @@ -70,9 +70,16 @@ describe('fetch_index_info', () => { it('returns field candidates and total hits', async () => { const esClientFieldCapsMock = jest.fn(() => ({ fields: { - myIpFieldName: { ip: {} }, - myKeywordFieldName: { keyword: {} }, - myUnpopulatedKeywordFieldName: { keyword: {} }, + // Should end up as a field candidate + myIpFieldName: { ip: { aggregatable: true } }, + // Should end up as a field candidate + myKeywordFieldName: { keyword: { aggregatable: true } }, + // Should not end up as a field candidate, it's a keyword but non-aggregatable + myKeywordFieldNameToBeIgnored: { keyword: { aggregatable: false } }, + // Should not end up as a field candidate, based on this field caps result it would be + // but it will not be part of the mocked search result so will count as unpopulated. + myUnpopulatedKeywordFieldName: { keyword: { aggregatable: true } }, + // Should not end up as a field candidate since fields of type number will not be considered myNumericFieldName: { number: {} }, }, })); diff --git a/x-pack/plugins/aiops/server/routes/queries/fetch_index_info.ts b/x-pack/plugins/aiops/server/routes/queries/fetch_index_info.ts index 93378911c7201d..33a298af8e98f9 100644 --- a/x-pack/plugins/aiops/server/routes/queries/fetch_index_info.ts +++ b/x-pack/plugins/aiops/server/routes/queries/fetch_index_info.ts @@ -68,9 +68,10 @@ export const fetchIndexInfo = async ( Object.entries(respMapping.fields).forEach(([key, value]) => { const fieldTypes = Object.keys(value) as ES_FIELD_TYPES[]; const isSupportedType = fieldTypes.some((type) => SUPPORTED_ES_FIELD_TYPES.includes(type)); + const isAggregatable = fieldTypes.some((type) => value[type].aggregatable); // Check if fieldName is something we can aggregate on - if (isSupportedType) { + if (isSupportedType && isAggregatable) { acceptableFields.add(key); } }); diff --git a/x-pack/test/functional/services/aiops/explain_log_rate_spikes_data_generator.ts b/x-pack/test/functional/services/aiops/explain_log_rate_spikes_data_generator.ts index 2039ab7b7ae3c1..6e5f7044efbc38 100644 --- a/x-pack/test/functional/services/aiops/explain_log_rate_spikes_data_generator.ts +++ b/x-pack/test/functional/services/aiops/explain_log_rate_spikes_data_generator.ts @@ -15,6 +15,7 @@ export interface GeneratedDoc { url: string; version: string; '@timestamp': number; + should_ignore_this_field: string; } const REFERENCE_TS = 1669018354793; @@ -50,6 +51,7 @@ function getArtificialLogsWithSpike(index: string) { url, version: 'v1.0.0', '@timestamp': ts + tsOffset, + should_ignore_this_field: 'should_ignore_this_field', }; bulkBody.push(action); @@ -74,6 +76,7 @@ function getArtificialLogsWithSpike(index: string) { url, version: 'v1.0.0', '@timestamp': DEVIATION_TS + tsOffset, + should_ignore_this_field: 'should_ignore_this_field', }); }); }); @@ -91,6 +94,7 @@ function getArtificialLogsWithSpike(index: string) { url, version: 'v1.0.0', '@timestamp': DEVIATION_TS + tsOffset, + should_ignore_this_field: 'should_ignore_this_field', }); }); }); @@ -158,6 +162,7 @@ export function ExplainLogRateSpikesDataGeneratorProvider({ getService }: FtrPro url: { type: 'keyword' }, version: { type: 'keyword' }, '@timestamp': { type: 'date' }, + should_ignore_this_field: { type: 'keyword', doc_values: false, index: false }, }, }, });