Skip to content

Commit

Permalink
[8.x] [Fields Metadata] Improve integration fields resolution and cac…
Browse files Browse the repository at this point in the history
…hing (#195405) (#196086)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Fields Metadata] Improve integration fields resolution and caching
(#195405)](#195405)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Marco Antonio
Ghiani","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-14T10:15:36Z","message":"[Fields
Metadata] Improve integration fields resolution and caching
(#195405)\n\n## 📓 Summary\r\n\r\nBrowsing fields from the Discover
sidebar, I noticed integration fields\r\nnever show a related
description even if they exist. The same is\r\nhappening in the fields
table for the document detail flyout.\r\n\r\nThis happens due to
`integration` and `dataset` parameters not being\r\npassed to the
service.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/0946cc71-44fb-4fc7-8e9d-b146bdd811f2\r\n\r\nThese
changes improve the resolution of the integration field
metadata:\r\n\r\n- The `integration` and `dataset` params are no longer
required to\r\nattempt resolving and integration field metadata.\r\nThey
are still accepted as an explicit hint in case we cannot
infer\r\ncorrectly some integration packages from the field name.\r\n-
The above change enables querying fields from different
integrations\r\nand datasets at once, enabling metadata retrieval for
mixed data\r\nsources.\r\n- The integration retrieved from the EPR is
now cached with its relevant\r\nversion, solving a potential corner case
as
explained\r\n[here](https://github.com/elastic/kibana/pull/183806#pullrequestreview-2088102130).\r\n\r\n\r\nhttps://github.com/user-attachments/assets/ae9cafd8-2581-4ce0-9242-cbb4e37c7702\r\n\r\n---------\r\n\r\nCo-authored-by:
Marco Antonio Ghiani
<[email protected]>","sha":"2b7c72c6193cf46c5cf883dafb8521f4a6805cd4","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","v9.0.0","backport:prev-minor","Team:obs-ux-logs"],"title":"[Fields
Metadata] Improve integration fields resolution and
caching","number":195405,"url":"https://github.com/elastic/kibana/pull/195405","mergeCommit":{"message":"[Fields
Metadata] Improve integration fields resolution and caching
(#195405)\n\n## 📓 Summary\r\n\r\nBrowsing fields from the Discover
sidebar, I noticed integration fields\r\nnever show a related
description even if they exist. The same is\r\nhappening in the fields
table for the document detail flyout.\r\n\r\nThis happens due to
`integration` and `dataset` parameters not being\r\npassed to the
service.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/0946cc71-44fb-4fc7-8e9d-b146bdd811f2\r\n\r\nThese
changes improve the resolution of the integration field
metadata:\r\n\r\n- The `integration` and `dataset` params are no longer
required to\r\nattempt resolving and integration field metadata.\r\nThey
are still accepted as an explicit hint in case we cannot
infer\r\ncorrectly some integration packages from the field name.\r\n-
The above change enables querying fields from different
integrations\r\nand datasets at once, enabling metadata retrieval for
mixed data\r\nsources.\r\n- The integration retrieved from the EPR is
now cached with its relevant\r\nversion, solving a potential corner case
as
explained\r\n[here](https://github.com/elastic/kibana/pull/183806#pullrequestreview-2088102130).\r\n\r\n\r\nhttps://github.com/user-attachments/assets/ae9cafd8-2581-4ce0-9242-cbb4e37c7702\r\n\r\n---------\r\n\r\nCo-authored-by:
Marco Antonio Ghiani
<[email protected]>","sha":"2b7c72c6193cf46c5cf883dafb8521f4a6805cd4"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195405","number":195405,"mergeCommit":{"message":"[Fields
Metadata] Improve integration fields resolution and caching
(#195405)\n\n## 📓 Summary\r\n\r\nBrowsing fields from the Discover
sidebar, I noticed integration fields\r\nnever show a related
description even if they exist. The same is\r\nhappening in the fields
table for the document detail flyout.\r\n\r\nThis happens due to
`integration` and `dataset` parameters not being\r\npassed to the
service.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/0946cc71-44fb-4fc7-8e9d-b146bdd811f2\r\n\r\nThese
changes improve the resolution of the integration field
metadata:\r\n\r\n- The `integration` and `dataset` params are no longer
required to\r\nattempt resolving and integration field metadata.\r\nThey
are still accepted as an explicit hint in case we cannot
infer\r\ncorrectly some integration packages from the field name.\r\n-
The above change enables querying fields from different
integrations\r\nand datasets at once, enabling metadata retrieval for
mixed data\r\nsources.\r\n- The integration retrieved from the EPR is
now cached with its relevant\r\nversion, solving a potential corner case
as
explained\r\n[here](https://github.com/elastic/kibana/pull/183806#pullrequestreview-2088102130).\r\n\r\n\r\nhttps://github.com/user-attachments/assets/ae9cafd8-2581-4ce0-9242-cbb4e37c7702\r\n\r\n---------\r\n\r\nCo-authored-by:
Marco Antonio Ghiani
<[email protected]>","sha":"2b7c72c6193cf46c5cf883dafb8521f4a6805cd4"}}]}]
BACKPORT-->

Co-authored-by: Marco Antonio Ghiani <[email protected]>
  • Loading branch information
kibanamachine and tonyghiani authored Oct 14, 2024
1 parent b7c0e07 commit 89623aa
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';

Expand Down Expand Up @@ -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 {
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 89623aa

Please sign in to comment.