From d89153dc55bdeab635a69a7eab48ddae208d79d4 Mon Sep 17 00:00:00 2001 From: Kevin Lacabane Date: Fri, 13 Dec 2024 14:39:04 +0100 Subject: [PATCH] [eem] merging nulls fix (#204183) Closes https://github.com/elastic/kibana/issues/203982 Ignore nulls when merging metadata fields. The flakiness comes from the random arrival time of the queries. since we execute two queries in parallel, if the first one has the null value for a field the merging logic will ignore it and test succeeds, if the null value arrives second it fails. --- .../lib/v2/queries/entity_instances.test.ts | 2 +- .../server/lib/v2/queries/entity_instances.ts | 2 +- .../server/lib/v2/queries/utils.test.ts | 43 +++++++++++++++++++ .../server/lib/v2/queries/utils.ts | 4 +- .../apis/entity_manager/search.ts | 3 +- 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/entity_instances.test.ts b/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/entity_instances.test.ts index 8836b7635ff36..e7fa8882bfcb9 100644 --- a/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/entity_instances.test.ts +++ b/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/entity_instances.test.ts @@ -28,7 +28,7 @@ describe('getEntityInstancesQuery', () => { expect(query).toEqual( 'FROM logs-*, metrics-* | ' + - 'STATS host.name = VALUES(host.name::keyword), entity.last_seen_timestamp = MAX(custom_timestamp_field), service.id = MAX(service.id::keyword) BY service.name::keyword | ' + + 'STATS host.name = TOP(host.name::keyword, 10, "ASC"), entity.last_seen_timestamp = MAX(custom_timestamp_field), service.id = MAX(service.id::keyword) BY service.name::keyword | ' + 'RENAME `service.name::keyword` AS service.name | ' + 'EVAL entity.type = "service", entity.id = service.name, entity.display_name = COALESCE(service.id, entity.id) | ' + 'SORT entity.id DESC | ' + diff --git a/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/entity_instances.ts b/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/entity_instances.ts index c9a5948b55dc1..dc79d815abd37 100644 --- a/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/entity_instances.ts +++ b/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/entity_instances.ts @@ -46,7 +46,7 @@ const dslFilter = ({ const statsCommand = ({ source }: { source: EntitySourceDefinition }) => { const aggs = source.metadata_fields .filter((field) => !source.identity_fields.some((idField) => idField === field)) - .map((field) => `${field} = VALUES(${asKeyword(field)})`); + .map((field) => `${field} = TOP(${asKeyword(field)}, 10, "ASC")`); if (source.timestamp_field) { aggs.push(`entity.last_seen_timestamp = MAX(${source.timestamp_field})`); diff --git a/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/utils.test.ts b/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/utils.test.ts index 2aee58828defa..d588811f2aa19 100644 --- a/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/utils.test.ts +++ b/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/utils.test.ts @@ -277,5 +277,48 @@ describe('mergeEntitiesList', () => { service_name: 'foo', }); }); + + it('ignores null values when merging', () => { + const entities = [ + { + 'entity.id': 'foo', + 'entity.type': 'service', + 'entity.display_name': 'foo', + 'service.name': 'foo', + 'host.name': null, + }, + { + 'entity.id': 'foo', + 'entity.type': 'service', + 'entity.display_name': 'foo', + 'service.name': 'foo', + 'host.name': 'host-2', + }, + { + 'entity.id': 'foo', + 'entity.type': 'service', + 'entity.display_name': 'foo', + 'service.name': 'foo', + 'host.name': null, + }, + ]; + + const mergedEntities = mergeEntitiesList({ + entities, + metadataFields: ['host.name'], + sources: [ + { identity_fields: ['service.name'], metadata_fields: [] as string[] }, + { identity_fields: ['service.name'], metadata_fields: [] as string[] }, + ] as EntitySourceDefinition[], + }); + expect(mergedEntities.length).toEqual(1); + expect(mergedEntities[0]).toEqual({ + 'entity.id': 'foo', + 'entity.type': 'service', + 'entity.display_name': 'foo', + 'service.name': 'foo', + 'host.name': 'host-2', + }); + }); }); }); diff --git a/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/utils.ts b/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/utils.ts index 71bf85632a447..969128fcf4088 100644 --- a/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/utils.ts +++ b/x-pack/platform/plugins/shared/entity_manager/server/lib/v2/queries/utils.ts @@ -33,8 +33,8 @@ function mergeEntities( merged['entity.last_seen_timestamp'] = latestTimestamp; } - for (const [key, value] of Object.entries(entity2).filter(([_key]) => - mergeableFields.includes(_key) + for (const [key, value] of Object.entries(entity2).filter( + ([_key]) => mergeableFields.includes(_key) && entity2[_key] )) { if (merged[key]) { // we want to keep identity fields as single-value properties. diff --git a/x-pack/test/api_integration/apis/entity_manager/search.ts b/x-pack/test/api_integration/apis/entity_manager/search.ts index f164a309f0313..a20364c7256e4 100644 --- a/x-pack/test/api_integration/apis/entity_manager/search.ts +++ b/x-pack/test/api_integration/apis/entity_manager/search.ts @@ -20,8 +20,7 @@ export default function ({ getService }: FtrProviderContext) { const esClient = getService('es'); const supertest = getService('supertest'); - // Failing: See https://github.com/elastic/kibana/issues/203982 - describe.skip('_search API', () => { + describe('_search API', () => { let cleanup: Function[] = []; before(() => clearEntityDefinitions(esClient));