Skip to content

Commit

Permalink
[eem] merging nulls fix (elastic#204183)
Browse files Browse the repository at this point in the history
Closes elastic#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.
  • Loading branch information
klacabane authored Dec 13, 2024
1 parent bd1c00f commit d89153d
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 | ' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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})`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 1 addition & 2 deletions x-pack/test/api_integration/apis/entity_manager/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down

0 comments on commit d89153d

Please sign in to comment.