Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] [ES|QL] Improve performance for ECS schema information in editor (#192646) #193199

Merged
merged 1 commit into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,6 @@ export const buildFieldsDefinitionsWithMetadata = (
options?: { advanceCursor?: boolean; openSuggestions?: boolean; addComma?: boolean }
): SuggestionRawDefinition[] => {
return fields.map((field) => {
const description = field.metadata?.description;

const titleCaseType = field.type.charAt(0).toUpperCase() + field.type.slice(1);
return {
label: field.name,
Expand All @@ -151,16 +149,8 @@ export const buildFieldsDefinitionsWithMetadata = (
(options?.advanceCursor ? ' ' : ''),
kind: 'Variable',
detail: titleCaseType,
documentation: description
? {
value: `
---

${description}`,
}
: undefined,
// If there is a description, it is a field from ECS, so it should be sorted to the top
sortText: description ? '1D' : 'D',
// If detected to be an ECS field, push it up to the top of the list
sortText: field.isEcs ? '1D' : 'D',
command: options?.openSuggestions ? TRIGGER_SUGGESTION_COMMAND : undefined,
};
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,18 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { getColumnsWithMetadata } from './ecs_metadata_helper';
import type { FieldsMetadataPublicStart } from '@kbn/fields-metadata-plugin/public';
import { ESQLRealField } from '@kbn/esql-validation-autocomplete';
import type { ESQLRealField } from '../../validation/types';
import { type ECSMetadata, enrichFieldsWithECSInfo } from './ecs_metadata_helper';

describe('getColumnsWithMetadata', () => {
describe('enrichFieldsWithECSInfo', () => {
it('should return original columns if fieldsMetadata is not provided', async () => {
const columns: ESQLRealField[] = [
{ name: 'ecs.version', type: 'keyword' },
{ name: 'field1', type: 'text' },
{ name: 'field2', type: 'double' },
];

const result = await getColumnsWithMetadata(columns);
const result = await enrichFieldsWithECSInfo(columns);
expect(result).toEqual(columns);
});

Expand All @@ -42,15 +41,23 @@ describe('getColumnsWithMetadata', () => {
},
}),
}),
} as unknown as FieldsMetadataPublicStart;
};
const fieldsMetadataCache = await (
await fieldsMetadata.getClient()
).find({
attributes: ['type'],
});

const result = await getColumnsWithMetadata(columns, fieldsMetadata);
const result = await enrichFieldsWithECSInfo(
columns,
fieldsMetadataCache.fields as ECSMetadata
);

expect(result).toEqual([
{
name: 'ecs.field',
type: 'text',
metadata: { description: 'ECS field description' },
isEcs: true,
},
{ name: 'ecs.fakeBooleanField', type: 'boolean' },
{ name: 'field2', type: 'double' },
Expand All @@ -68,19 +75,27 @@ describe('getColumnsWithMetadata', () => {
find: jest.fn().mockResolvedValue({
fields: {
'ecs.version': { description: 'ECS version field', type: 'keyword' },
},
} as unknown as ECSMetadata,
}),
}),
} as unknown as FieldsMetadataPublicStart;
};

const result = await getColumnsWithMetadata(columns, fieldsMetadata);
const fieldsMetadataCache = await (
await fieldsMetadata.getClient()
).find({
attributes: ['type'],
});
const result = await enrichFieldsWithECSInfo(
columns,
fieldsMetadataCache.fields as ECSMetadata
);

expect(result).toEqual([
{ name: 'ecs.version', type: 'keyword', metadata: { description: 'ECS version field' } },
{ name: 'ecs.version', type: 'keyword', isEcs: true },
{
name: 'ecs.version.keyword',
type: 'keyword',
metadata: { description: 'ECS version field' },
isEcs: true,
},
{ name: 'field2', type: 'double' },
]);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import type { ESQLRealField } from '../../validation/types';

const removeKeywordSuffix = (name: string) => {
return name.endsWith('.keyword') ? name.slice(0, -8) : name;
};

export interface ECSMetadata {
[key: string]: {
type?: string;
source?: string;
description?: string;
};
}
/**
* Returns columns with the metadata/description (e.g ECS info)
* if available
*
* @param columns
* @param fieldsMetadata
* @returns
*/
export function enrichFieldsWithECSInfo(
columns: Array<Omit<ESQLRealField, 'metadata'>>,
ecsMetadataCache?: ECSMetadata
): ESQLRealField[] {
if (!ecsMetadataCache) return columns;

try {
if (ecsMetadataCache) {
return columns.map((c) => {
// Metadata services gives description for
// 'ecs.version' but not 'ecs.version.keyword'
// but both should show description if available
const metadata = ecsMetadataCache[removeKeywordSuffix(c.name)];

// Need to convert metadata's type (e.g. keyword) to ES|QL type (e.g. string) to check if they are the same
if (!metadata || (metadata?.type && metadata.type !== c.type)) return c;
return {
...c,
isEcs: true,
};
});
}

return columns;
} catch (error) {
// eslint-disable-next-line no-console
console.error('Unable to fetch field metadata', error);
}
return columns;
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import { getActions } from './actions';
import { validateQuery } from '../validation/validation';
import { getAllFunctions } from '../shared/helpers';
import { getAstAndSyntaxErrors } from '@kbn/esql-ast';
import { CodeActionOptions } from './types';
import { ESQLRealField } from '../validation/types';
import { FieldType } from '../definitions/types';
import type { CodeActionOptions } from './types';
import type { ESQLRealField } from '../validation/types';
import type { FieldType } from '../definitions/types';
import type { ESQLCallbacks, PartialFieldsMetadataClient } from '../shared/types';

function getCallbackMocks() {
function getCallbackMocks(): jest.Mocked<ESQLCallbacks> {
return {
getFieldsFor: jest.fn<Promise<ESQLRealField[]>, any>(async ({ query }) => {
if (/enrich/.test(query)) {
Expand Down Expand Up @@ -65,6 +66,11 @@ function getCallbackMocks() {
enrichFields: ['other-field', 'yetAnotherField'],
},
]),
getFieldsMetadata: jest.fn(async () => ({
find: jest.fn(async () => ({
fields: {},
})),
})) as unknown as Promise<PartialFieldsMetadataClient>,
};
}

Expand Down Expand Up @@ -395,6 +401,7 @@ describe('quick fixes logic', () => {
const { errors } = await validateQuery(statement, getAstAndSyntaxErrors, undefined, {
...callbackMocks,
getFieldsFor: undefined,
getFieldsMetadata: undefined,
});
const actions = await getActions(
statement,
Expand All @@ -403,7 +410,11 @@ describe('quick fixes logic', () => {
{
relaxOnMissingCallbacks: true,
},
{ ...callbackMocks, getFieldsFor: undefined }
{
...callbackMocks,
getFieldsFor: undefined,
getFieldsMetadata: undefined,
}
);
const edits = actions.map(({ edits: actionEdits }) => actionEdits[0].text);
expect(edits).toEqual(['`any#Char$Field`']);
Expand Down Expand Up @@ -452,6 +463,7 @@ describe('quick fixes logic', () => {
getFieldsFor: undefined,
getSources: undefined,
getPolicies: undefined,
getFieldsMetadata: undefined,
}
);
} catch {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import type { ESQLAst } from '@kbn/esql-ast';
import type { ESQLCallbacks } from './types';
import type { ESQLRealField } from '../validation/types';
import { enrichFieldsWithECSInfo } from '../autocomplete/utils/ecs_metadata_helper';

export function buildQueryUntilPreviousCommand(ast: ESQLAst, queryString: string) {
const prevCommand = ast[Math.max(ast.length - 2, 0)];
Expand All @@ -18,14 +19,31 @@ export function buildQueryUntilPreviousCommand(ast: ESQLAst, queryString: string

export function getFieldsByTypeHelper(queryText: string, resourceRetriever?: ESQLCallbacks) {
const cacheFields = new Map<string, ESQLRealField>();

const getEcsMetadata = async () => {
if (!resourceRetriever?.getFieldsMetadata) {
return undefined;
}
const client = await resourceRetriever?.getFieldsMetadata;
if (client.find) {
// Fetch full list of ECS field
// This list should be cached already by fieldsMetadataClient
const results = await client.find({ attributes: ['type'] });
return results?.fields;
}
};

const getFields = async () => {
const metadata = await getEcsMetadata();
if (!cacheFields.size && queryText) {
const fieldsOfType = await resourceRetriever?.getFieldsFor?.({ query: queryText });
for (const field of fieldsOfType || []) {
const fieldsWithMetadata = enrichFieldsWithECSInfo(fieldsOfType || [], metadata);
for (const field of fieldsWithMetadata || []) {
cacheFields.set(field.name, field);
}
}
};

return {
getFieldsByType: async (
expectedType: string | string[] = 'any',
Expand Down
17 changes: 17 additions & 0 deletions packages/kbn-esql-validation-autocomplete/src/shared/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,22 @@ import type { ESQLRealField } from '../validation/types';
/** @internal **/
type CallbackFn<Options = {}, Result = string> = (ctx?: Options) => Result[] | Promise<Result[]>;

/**
* Partial fields metadata client, used to avoid circular dependency with @kbn/monaco
/** @internal **/
export interface PartialFieldsMetadataClient {
find: ({ fieldNames, attributes }: { fieldNames?: string[]; attributes: string[] }) => Promise<{
fields: Record<
string,
{
type: string;
source: string;
description?: string;
}
>;
}>;
}

/** @public **/
export interface ESQLSourceResult {
name: string;
Expand All @@ -29,6 +45,7 @@ export interface ESQLCallbacks {
{ name: string; sourceIndices: string[]; matchField: string; enrichFields: string[] }
>;
getPreferences?: () => Promise<{ histogramBarTarget: number }>;
getFieldsMetadata?: Promise<PartialFieldsMetadataClient>;
}

export type ReasonTypes = 'missingCommand' | 'unsupportedFunction' | 'unknownFunction';
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface ESQLVariable {
export interface ESQLRealField {
name: string;
type: FieldType;
isEcs?: boolean;
metadata?: {
description?: string;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1709,6 +1709,7 @@ describe('validation logic', () => {
getPolicies: /Unknown policy/,
getFieldsFor: /Unknown column|Argument of|it is unsupported or not indexed/,
getPreferences: /Unknown/,
getFieldsMetadata: /Unknown/,
};
return excludedCallback.map((callback) => contentByCallback[callback]) || [];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,7 @@ export const ignoreErrorsMap: Record<keyof ESQLCallbacks, ErrorTypes[]> = {
getSources: ['unknownIndex'],
getPolicies: ['unknownPolicy'],
getPreferences: [],
getFieldsMetadata: [],
};

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-esql-validation-autocomplete/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"kbn_references": [
"@kbn/i18n",
"@kbn/esql-ast",
"@kbn/utility-types",
"@kbn/utility-types"
],
"exclude": [
"target/**/*",
Expand Down
Loading