Skip to content

Commit

Permalink
[Fields Metadata] Improve integration fields resolution and caching (e…
Browse files Browse the repository at this point in the history
…lastic#195405)

## 📓 Summary

Browsing fields from the Discover sidebar, I noticed integration fields
never show a related description even if they exist. The same is
happening in the fields table for the document detail flyout.

This happens due to `integration` and `dataset` parameters not being
passed to the service.


https://github.com/user-attachments/assets/0946cc71-44fb-4fc7-8e9d-b146bdd811f2

These changes improve the resolution of the integration field metadata:

- The `integration` and `dataset` params are no longer required to
attempt resolving and integration field metadata.
They are still accepted as an explicit hint in case we cannot infer
correctly some integration packages from the field name.
- The above change enables querying fields from different integrations
and datasets at once, enabling metadata retrieval for mixed data
sources.
- The integration retrieved from the EPR is now cached with its relevant
version, solving a potential corner case as explained
[here](elastic#183806 (review)).


https://github.com/user-attachments/assets/ae9cafd8-2581-4ce0-9242-cbb4e37c7702

---------

Co-authored-by: Marco Antonio Ghiani <[email protected]>
  • Loading branch information
tonyghiani and Marco Antonio Ghiani authored Oct 14, 2024
1 parent 87c91f4 commit 2b7c72c
Show file tree
Hide file tree
Showing 13 changed files with 162 additions and 18 deletions.
2 changes: 2 additions & 0 deletions x-pack/plugins/fields_metadata/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ const fields = await client.find({
*/
```

> The service will try to extract the integration and dataset name as they are conventionally named in their static definition, providing a much simpler usage of this API for integration fields.
> N.B. Passing the `dataset` name parameter to `.find` helps narrowing the scope of the integration assets that need to be fetched, increasing the performance of the request.
In case the exact dataset for a field is unknown, is it still possible to pass a `*` value as `dataset` parameter to access all the integration datasets' fields.
Still, is recommended always passing the `dataset` as well if known or unless the required fields come from different datasets of the same integration.
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/fields_metadata/server/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { FieldsMetadataServerSetup, FieldsMetadataServerStart } from './types';
const createFieldsMetadataServerSetupMock = (): jest.Mocked<FieldsMetadataServerSetup> => ({
registerIntegrationFieldsExtractor:
createFieldsMetadataServiceSetupMock().registerIntegrationFieldsExtractor,
registerIntegrationListExtractor:
createFieldsMetadataServiceSetupMock().registerIntegrationListExtractor,
});

const createFieldsMetadataServerStartMock = (): jest.Mocked<FieldsMetadataServerStart> => ({
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/fields_metadata/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export class FieldsMetadataPlugin

return {
registerIntegrationFieldsExtractor: fieldsMetadata.registerIntegrationFieldsExtractor,
registerIntegrationListExtractor: fieldsMetadata.registerIntegrationListExtractor,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,41 @@ const integrationFields = {
'The version of the browser or computer where the 1Password app is installed, or the CPU of the machine where the 1Password command-line tool is installed',
},
},
'mysql.slowlog': {
'mysql.slowlog.filesort': {
name: 'filesort',
type: 'boolean',
description: 'Whether filesort optimization was used.',
flat_name: 'mysql.slowlog.filesort',
source: 'integration',
dashed_name: 'mysql-slowlog-filesort',
normalize: [],
short: 'Whether filesort optimization was used.',
},
},
};

describe('FieldsMetadataClient class', () => {
const logger = loggerMock.create();
const ecsFieldsRepository = EcsFieldsRepository.create({ ecsFields });
const metadataFieldsRepository = MetadataFieldsRepository.create({ metadataFields });
const integrationFieldsExtractor = jest.fn();
const integrationListExtractor = jest.fn();
integrationFieldsExtractor.mockImplementation(() => Promise.resolve(integrationFields));
integrationListExtractor.mockImplementation(() =>
Promise.resolve([
{
id: '1password',
name: '1password',
version: '1.0.0',
},
{
id: 'mysql',
name: 'mysql',
version: '1.0.0',
},
])
);

let integrationFieldsRepository: IntegrationFieldsRepository;
let fieldsMetadataClient: FieldsMetadataClient;
Expand All @@ -73,6 +100,7 @@ describe('FieldsMetadataClient class', () => {
integrationFieldsExtractor.mockClear();
integrationFieldsRepository = IntegrationFieldsRepository.create({
integrationFieldsExtractor,
integrationListExtractor,
});
fieldsMetadataClient = FieldsMetadataClient.create({
ecsFieldsRepository,
Expand Down Expand Up @@ -105,6 +133,26 @@ describe('FieldsMetadataClient class', () => {
expect(Object.hasOwn(timestampField, 'type')).toBeTruthy();
});

it('should attempt resolving the field from an integration if it does not exist in ECS/Metadata by inferring the integration from the field name', async () => {
const mysqlFieldInstance = await fieldsMetadataClient.getByName('mysql.slowlog.filesort');

expect(integrationFieldsExtractor).toHaveBeenCalled();

expectToBeDefined(mysqlFieldInstance);
expect(mysqlFieldInstance).toBeInstanceOf(FieldMetadata);

const mysqlField = mysqlFieldInstance.toPlain();

expect(Object.hasOwn(mysqlField, 'name')).toBeTruthy();
expect(Object.hasOwn(mysqlField, 'type')).toBeTruthy();
expect(Object.hasOwn(mysqlField, 'description')).toBeTruthy();
expect(Object.hasOwn(mysqlField, 'flat_name')).toBeTruthy();
expect(Object.hasOwn(mysqlField, 'source')).toBeTruthy();
expect(Object.hasOwn(mysqlField, 'dashed_name')).toBeTruthy();
expect(Object.hasOwn(mysqlField, 'normalize')).toBeTruthy();
expect(Object.hasOwn(mysqlField, 'short')).toBeTruthy();
});

it('should attempt resolving the field from an integration if it does not exist in ECS/Metadata and the integration and dataset params are provided', async () => {
const onePasswordFieldInstance = await fieldsMetadataClient.getByName(
'onepassword.client.platform_version',
Expand All @@ -128,13 +176,13 @@ describe('FieldsMetadataClient class', () => {
expect(Object.hasOwn(onePasswordField, 'short')).toBeTruthy();
});

it('should not resolve the field from an integration if the integration and dataset params are not provided', async () => {
const onePasswordFieldInstance = await fieldsMetadataClient.getByName(
'onepassword.client.platform_version'
it('should not resolve the field from an integration if the integration name cannot be inferred from the field name and integration and dataset params are not provided', async () => {
const unknownFieldInstance = await fieldsMetadataClient.getByName(
'customField.duration.milliseconds'
);

expect(integrationFieldsExtractor).not.toHaveBeenCalled();
expect(onePasswordFieldInstance).toBeUndefined();
expect(unknownFieldInstance).toBeUndefined();
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class FieldsMetadataClient implements IFieldsMetadataClient {
}

// 2. Try searching for the fiels in the Elastic Package Registry
if (!field && integration) {
if (!field) {
field = await this.integrationFieldsRepository.getByName(fieldName, { integration, dataset });
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { FieldsMetadataServiceSetup, FieldsMetadataServiceStart } from './types'
export const createFieldsMetadataServiceSetupMock =
(): jest.Mocked<FieldsMetadataServiceSetup> => ({
registerIntegrationFieldsExtractor: jest.fn(),
registerIntegrationListExtractor: jest.fn(),
});

export const createFieldsMetadataServiceStartMock =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ import { FieldsMetadataClient } from './fields_metadata_client';
import { EcsFieldsRepository } from './repositories/ecs_fields_repository';
import { IntegrationFieldsRepository } from './repositories/integration_fields_repository';
import { MetadataFieldsRepository } from './repositories/metadata_fields_repository';
import { IntegrationFieldsExtractor } from './repositories/types';
import { IntegrationFieldsExtractor, IntegrationListExtractor } from './repositories/types';
import { FieldsMetadataServiceSetup, FieldsMetadataServiceStart } from './types';
import { MetadataFields as metadataFields } from '../../../common/metadata_fields';

export class FieldsMetadataService {
private integrationFieldsExtractor: IntegrationFieldsExtractor = () => Promise.resolve({});
private integrationListExtractor: IntegrationListExtractor = () => Promise.resolve([]);

constructor(private readonly logger: Logger) {}

Expand All @@ -25,16 +26,20 @@ export class FieldsMetadataService {
registerIntegrationFieldsExtractor: (extractor: IntegrationFieldsExtractor) => {
this.integrationFieldsExtractor = extractor;
},
registerIntegrationListExtractor: (extractor: IntegrationListExtractor) => {
this.integrationListExtractor = extractor;
},
};
}

public start(): FieldsMetadataServiceStart {
const { logger, integrationFieldsExtractor } = this;
const { logger, integrationFieldsExtractor, integrationListExtractor } = this;

const ecsFieldsRepository = EcsFieldsRepository.create({ ecsFields });
const metadataFieldsRepository = MetadataFieldsRepository.create({ metadataFields });
const integrationFieldsRepository = IntegrationFieldsRepository.create({
integrationFieldsExtractor,
integrationListExtractor,
});

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,46 @@ import { ANY_DATASET } from '../../../../common/fields_metadata';
import { HashedCache } from '../../../../common/hashed_cache';
import { FieldMetadata, IntegrationFieldName } from '../../../../common';
import {
ExtractedIntegration,
ExtractedIntegrationFields,
IntegrationFieldsExtractor,
IntegrationFieldsSearchParams,
IntegrationListExtractor,
IntegrationName,
} from './types';
import { PackageNotFoundError } from '../errors';
interface IntegrationFieldsRepositoryDeps {
integrationFieldsExtractor: IntegrationFieldsExtractor;
integrationListExtractor: IntegrationListExtractor;
}

type DatasetFieldsMetadata = Record<string, FieldMetadata>;
type IntegrationFieldsMetadataTree = Record<IntegrationName, DatasetFieldsMetadata>;

export class IntegrationFieldsRepository {
private cache: HashedCache<IntegrationFieldsSearchParams, IntegrationFieldsMetadataTree>;
private integrationsMap: Map<string, ExtractedIntegration>;

private constructor(private readonly fieldsExtractor: IntegrationFieldsExtractor) {
private constructor(
private readonly integrationFieldsExtractor: IntegrationFieldsExtractor,
private readonly integrationListExtractor: IntegrationListExtractor
) {
this.cache = new HashedCache();
this.integrationsMap = new Map();

this.extractIntegrationList();
}

async getByName(
fieldName: IntegrationFieldName,
{ integration, dataset }: IntegrationFieldsSearchParams
params: Partial<IntegrationFieldsSearchParams>
): Promise<FieldMetadata | undefined> {
const { integration, dataset } = this.extractIntegrationFieldsSearchParams(fieldName, params);

if (!integration || !this.integrationsMap.has(integration)) {
return undefined;
}

let field = this.getCachedField(fieldName, { integration, dataset });

if (!field) {
Expand All @@ -48,8 +64,29 @@ export class IntegrationFieldsRepository {
return field;
}

public static create({ integrationFieldsExtractor }: IntegrationFieldsRepositoryDeps) {
return new IntegrationFieldsRepository(integrationFieldsExtractor);
public static create({
integrationFieldsExtractor,
integrationListExtractor,
}: IntegrationFieldsRepositoryDeps) {
return new IntegrationFieldsRepository(integrationFieldsExtractor, integrationListExtractor);
}

private extractIntegrationFieldsSearchParams(
fieldName: IntegrationFieldName,
params: Partial<IntegrationFieldsSearchParams>
) {
const parts = fieldName.split('.');

if (parts.length < 3) {
return params;
}

const [extractedIntegration, extractedDataset] = parts;

return {
integration: params.integration ?? extractedIntegration,
dataset: params.dataset ?? [extractedIntegration, extractedDataset].join('.'),
};
}

private async extractFields({
Expand All @@ -63,11 +100,17 @@ export class IntegrationFieldsRepository {
return undefined;
}

return this.fieldsExtractor({ integration, dataset })
return this.integrationFieldsExtractor({ integration, dataset })
.then(this.mapExtractedFieldsToFieldMetadataTree)
.then((fieldMetadataTree) => this.storeFieldsInCache(cacheKey, fieldMetadataTree));
}

private extractIntegrationList(): void {
void this.integrationListExtractor()
.then(this.mapExtractedIntegrationListToMap)
.then((integrationsMap) => (this.integrationsMap = integrationsMap));
}

private getCachedField(
fieldName: IntegrationFieldName,
{ integration, dataset }: IntegrationFieldsSearchParams
Expand Down Expand Up @@ -113,7 +156,19 @@ export class IntegrationFieldsRepository {
}
};

private getCacheKey = (params: IntegrationFieldsSearchParams) => params;
private getCacheKey = ({ integration, dataset }: IntegrationFieldsSearchParams) => {
const integrationDetails = this.integrationsMap.get(integration);

if (integrationDetails) {
return {
dataset,
integration,
version: integrationDetails.version,
};
}

return { integration, dataset };
};

private mapExtractedFieldsToFieldMetadataTree = (extractedFields: ExtractedIntegrationFields) => {
const datasetGroups = Object.entries(extractedFields);
Expand All @@ -132,4 +187,8 @@ export class IntegrationFieldsRepository {
return integrationGroup;
}, {} as IntegrationFieldsMetadataTree);
};

private mapExtractedIntegrationListToMap = (extractedIntegrations: ExtractedIntegration[]) => {
return new Map(extractedIntegrations.map((integration) => [integration.name, integration]));
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,11 @@ export type ExtractedDatasetFields = Record<DatasetName, FieldMetadataPlain>;
export type IntegrationFieldsExtractor = (
params: IntegrationFieldsSearchParams
) => Promise<ExtractedIntegrationFields>;

export interface ExtractedIntegration {
id: string;
name: string;
version: string;
}

export type IntegrationListExtractor = () => Promise<ExtractedIntegration[]>;
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
*/

import { FieldName, FieldMetadata, FieldsMetadataDictionary } from '../../../common';
import { IntegrationFieldsExtractor, IntegrationFieldsSearchParams } from './repositories/types';
import {
IntegrationFieldsExtractor,
IntegrationFieldsSearchParams,
IntegrationListExtractor,
} from './repositories/types';

export * from './repositories/types';

Expand All @@ -15,6 +19,7 @@ export interface FieldsMetadataServiceStartDeps {}

export interface FieldsMetadataServiceSetup {
registerIntegrationFieldsExtractor: (extractor: IntegrationFieldsExtractor) => void;
registerIntegrationListExtractor: (extractor: IntegrationListExtractor) => void;
}

export interface FieldsMetadataServiceStart {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/fields_metadata/server/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type FieldsMetadataPluginStartServicesAccessor =

export interface FieldsMetadataServerSetup {
registerIntegrationFieldsExtractor: FieldsMetadataServiceSetup['registerIntegrationFieldsExtractor'];
registerIntegrationListExtractor: FieldsMetadataServiceSetup['registerIntegrationListExtractor'];
}

export interface FieldsMetadataServerStart {
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/fleet/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ import { PolicyWatcher } from './services/agent_policy_watch';
import { getPackageSpecTagId } from './services/epm/kibana/assets/tag_assets';
import { FleetMetricsTask } from './services/metrics/fleet_metrics_task';
import { fetchAgentMetrics } from './services/metrics/fetch_agent_metrics';
import { registerIntegrationFieldsExtractor } from './services/register_integration_fields_extractor';
import { registerFieldsMetadataExtractors } from './services/register_fields_metadata_extractors';
import { registerUpgradeManagedPackagePoliciesTask } from './services/setup/managed_package_policies';
import { registerDeployAgentPoliciesTask } from './services/agent_policies/deploy_agent_policies_task';
import { DeleteUnenrolledAgentsTask } from './tasks/delete_unenrolled_agents_task';
Expand Down Expand Up @@ -637,8 +637,8 @@ export class FleetPlugin
logFactory: this.initializerContext.logger,
});

// Register fields metadata extractor
registerIntegrationFieldsExtractor({ core, fieldsMetadata: deps.fieldsMetadata });
// Register fields metadata extractors
registerFieldsMetadataExtractors({ core, fieldsMetadata: deps.fieldsMetadata });
}

public start(core: CoreStart, plugins: FleetStartDeps): FleetStartContract {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ interface RegistrationDeps {
fieldsMetadata: FieldsMetadataServerSetup;
}

export const registerIntegrationFieldsExtractor = ({ core, fieldsMetadata }: RegistrationDeps) => {
export const registerFieldsMetadataExtractors = ({ core, fieldsMetadata }: RegistrationDeps) => {
fieldsMetadata.registerIntegrationFieldsExtractor(async ({ integration, dataset }) => {
const [_core, _startDeps, { packageService }] = await core.getStartServices();

Expand All @@ -24,4 +24,16 @@ export const registerIntegrationFieldsExtractor = ({ core, fieldsMetadata }: Reg
datasetName: dataset,
});
});

fieldsMetadata.registerIntegrationListExtractor(async () => {
const [_core, _startDeps, { packageService }] = await core.getStartServices();

try {
const packages = await packageService.asInternalUser.getPackages();

return packages.map(({ id, name, version }) => ({ id, name, version }));
} catch (error) {
return [];
}
});
};

0 comments on commit 2b7c72c

Please sign in to comment.