From 65f4dd301b67492c86810014fd999e0a684becc6 Mon Sep 17 00:00:00 2001 From: Marco Antonio Ghiani Date: Mon, 14 Oct 2024 12:15:36 +0200 Subject: [PATCH] [Fields Metadata] Improve integration fields resolution and caching (#195405) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 📓 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](https://github.com/elastic/kibana/pull/183806#pullrequestreview-2088102130). https://github.com/user-attachments/assets/ae9cafd8-2581-4ce0-9242-cbb4e37c7702 --------- Co-authored-by: Marco Antonio Ghiani (cherry picked from commit 2b7c72c6193cf46c5cf883dafb8521f4a6805cd4) --- x-pack/plugins/fields_metadata/README.md | 2 + .../plugins/fields_metadata/server/mocks.ts | 2 + .../plugins/fields_metadata/server/plugin.ts | 1 + .../fields_metadata_client.test.ts | 56 +++++++++++++-- .../fields_metadata/fields_metadata_client.ts | 2 +- .../fields_metadata_service.mock.ts | 1 + .../fields_metadata_service.ts | 9 ++- .../integration_fields_repository.ts | 71 +++++++++++++++++-- .../fields_metadata/repositories/types.ts | 8 +++ .../server/services/fields_metadata/types.ts | 7 +- .../plugins/fields_metadata/server/types.ts | 1 + x-pack/plugins/fleet/server/plugin.ts | 6 +- ...=> register_fields_metadata_extractors.ts} | 14 +++- 13 files changed, 162 insertions(+), 18 deletions(-) rename x-pack/plugins/fleet/server/services/{register_integration_fields_extractor.ts => register_fields_metadata_extractors.ts} (66%) diff --git a/x-pack/plugins/fields_metadata/README.md b/x-pack/plugins/fields_metadata/README.md index ea561c52febce..dacd6d8acabd3 100755 --- a/x-pack/plugins/fields_metadata/README.md +++ b/x-pack/plugins/fields_metadata/README.md @@ -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. diff --git a/x-pack/plugins/fields_metadata/server/mocks.ts b/x-pack/plugins/fields_metadata/server/mocks.ts index b46ca661b6210..03415fcc6ddda 100644 --- a/x-pack/plugins/fields_metadata/server/mocks.ts +++ b/x-pack/plugins/fields_metadata/server/mocks.ts @@ -14,6 +14,8 @@ import { FieldsMetadataServerSetup, FieldsMetadataServerStart } from './types'; const createFieldsMetadataServerSetupMock = (): jest.Mocked => ({ registerIntegrationFieldsExtractor: createFieldsMetadataServiceSetupMock().registerIntegrationFieldsExtractor, + registerIntegrationListExtractor: + createFieldsMetadataServiceSetupMock().registerIntegrationListExtractor, }); const createFieldsMetadataServerStartMock = (): jest.Mocked => ({ diff --git a/x-pack/plugins/fields_metadata/server/plugin.ts b/x-pack/plugins/fields_metadata/server/plugin.ts index da7ede5efba3f..70ccb7b5d30a3 100644 --- a/x-pack/plugins/fields_metadata/server/plugin.ts +++ b/x-pack/plugins/fields_metadata/server/plugin.ts @@ -52,6 +52,7 @@ export class FieldsMetadataPlugin return { registerIntegrationFieldsExtractor: fieldsMetadata.registerIntegrationFieldsExtractor, + registerIntegrationListExtractor: fieldsMetadata.registerIntegrationListExtractor, }; } diff --git a/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_client.test.ts b/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_client.test.ts index 93c43fa69e5c8..4ef2e3c693fb5 100644 --- a/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_client.test.ts +++ b/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_client.test.ts @@ -57,6 +57,18 @@ 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', () => { @@ -64,7 +76,22 @@ describe('FieldsMetadataClient class', () => { 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; @@ -73,6 +100,7 @@ describe('FieldsMetadataClient class', () => { integrationFieldsExtractor.mockClear(); integrationFieldsRepository = IntegrationFieldsRepository.create({ integrationFieldsExtractor, + integrationListExtractor, }); fieldsMetadataClient = FieldsMetadataClient.create({ ecsFieldsRepository, @@ -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', @@ -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(); }); }); diff --git a/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_client.ts b/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_client.ts index 87c9b6547f4f5..baaac903a7b3a 100644 --- a/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_client.ts +++ b/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_client.ts @@ -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 }); } diff --git a/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_service.mock.ts b/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_service.mock.ts index 6fab587c9ca7a..b6395d4c96f6b 100644 --- a/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_service.mock.ts +++ b/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_service.mock.ts @@ -11,6 +11,7 @@ import { FieldsMetadataServiceSetup, FieldsMetadataServiceStart } from './types' export const createFieldsMetadataServiceSetupMock = (): jest.Mocked => ({ registerIntegrationFieldsExtractor: jest.fn(), + registerIntegrationListExtractor: jest.fn(), }); export const createFieldsMetadataServiceStartMock = diff --git a/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_service.ts b/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_service.ts index 8313f0337d769..dc8aa976e34be 100644 --- a/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_service.ts +++ b/x-pack/plugins/fields_metadata/server/services/fields_metadata/fields_metadata_service.ts @@ -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) {} @@ -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 { diff --git a/x-pack/plugins/fields_metadata/server/services/fields_metadata/repositories/integration_fields_repository.ts b/x-pack/plugins/fields_metadata/server/services/fields_metadata/repositories/integration_fields_repository.ts index cf3c2b0454c7d..7bf3ed871d1c5 100644 --- a/x-pack/plugins/fields_metadata/server/services/fields_metadata/repositories/integration_fields_repository.ts +++ b/x-pack/plugins/fields_metadata/server/services/fields_metadata/repositories/integration_fields_repository.ts @@ -9,14 +9,17 @@ 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; @@ -24,15 +27,28 @@ type IntegrationFieldsMetadataTree = Record; + private integrationsMap: Map; - 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 ): Promise { + const { integration, dataset } = this.extractIntegrationFieldsSearchParams(fieldName, params); + + if (!integration || !this.integrationsMap.has(integration)) { + return undefined; + } + let field = this.getCachedField(fieldName, { integration, dataset }); if (!field) { @@ -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 + ) { + 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({ @@ -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 @@ -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); @@ -132,4 +187,8 @@ export class IntegrationFieldsRepository { return integrationGroup; }, {} as IntegrationFieldsMetadataTree); }; + + private mapExtractedIntegrationListToMap = (extractedIntegrations: ExtractedIntegration[]) => { + return new Map(extractedIntegrations.map((integration) => [integration.name, integration])); + }; } diff --git a/x-pack/plugins/fields_metadata/server/services/fields_metadata/repositories/types.ts b/x-pack/plugins/fields_metadata/server/services/fields_metadata/repositories/types.ts index e258c46b569b4..a9fff964f2b19 100644 --- a/x-pack/plugins/fields_metadata/server/services/fields_metadata/repositories/types.ts +++ b/x-pack/plugins/fields_metadata/server/services/fields_metadata/repositories/types.ts @@ -20,3 +20,11 @@ export type ExtractedDatasetFields = Record; export type IntegrationFieldsExtractor = ( params: IntegrationFieldsSearchParams ) => Promise; + +export interface ExtractedIntegration { + id: string; + name: string; + version: string; +} + +export type IntegrationListExtractor = () => Promise; diff --git a/x-pack/plugins/fields_metadata/server/services/fields_metadata/types.ts b/x-pack/plugins/fields_metadata/server/services/fields_metadata/types.ts index 5b87f3299d61b..533b4fd0bb2c2 100644 --- a/x-pack/plugins/fields_metadata/server/services/fields_metadata/types.ts +++ b/x-pack/plugins/fields_metadata/server/services/fields_metadata/types.ts @@ -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'; @@ -15,6 +19,7 @@ export interface FieldsMetadataServiceStartDeps {} export interface FieldsMetadataServiceSetup { registerIntegrationFieldsExtractor: (extractor: IntegrationFieldsExtractor) => void; + registerIntegrationListExtractor: (extractor: IntegrationListExtractor) => void; } export interface FieldsMetadataServiceStart { diff --git a/x-pack/plugins/fields_metadata/server/types.ts b/x-pack/plugins/fields_metadata/server/types.ts index 4e2bf7ce2c0b3..dd84bc2c24559 100644 --- a/x-pack/plugins/fields_metadata/server/types.ts +++ b/x-pack/plugins/fields_metadata/server/types.ts @@ -21,6 +21,7 @@ export type FieldsMetadataPluginStartServicesAccessor = export interface FieldsMetadataServerSetup { registerIntegrationFieldsExtractor: FieldsMetadataServiceSetup['registerIntegrationFieldsExtractor']; + registerIntegrationListExtractor: FieldsMetadataServiceSetup['registerIntegrationListExtractor']; } export interface FieldsMetadataServerStart { diff --git a/x-pack/plugins/fleet/server/plugin.ts b/x-pack/plugins/fleet/server/plugin.ts index c767424cef36a..526d8a1a016ab 100644 --- a/x-pack/plugins/fleet/server/plugin.ts +++ b/x-pack/plugins/fleet/server/plugin.ts @@ -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'; @@ -629,8 +629,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 { diff --git a/x-pack/plugins/fleet/server/services/register_integration_fields_extractor.ts b/x-pack/plugins/fleet/server/services/register_fields_metadata_extractors.ts similarity index 66% rename from x-pack/plugins/fleet/server/services/register_integration_fields_extractor.ts rename to x-pack/plugins/fleet/server/services/register_fields_metadata_extractors.ts index d06c1e528469d..9a53c235622bf 100644 --- a/x-pack/plugins/fleet/server/services/register_integration_fields_extractor.ts +++ b/x-pack/plugins/fleet/server/services/register_fields_metadata_extractors.ts @@ -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(); @@ -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 []; + } + }); };