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

[ES|QL] METRICS command definition and validation #184905

Merged
merged 92 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from 80 commits
Commits
Show all changes
92 commits
Select commit Hold shift + click to select a range
3a9a5de
add basic validation test cases
vadimkibana Jun 3, 2024
bbc0540
add initial command definition
vadimkibana Jun 3, 2024
ced48fc
add more basic tests
vadimkibana Jun 3, 2024
965ea88
group metrics and from tests
vadimkibana Jun 3, 2024
8dbfcca
support aggregation functions in metrics command
vadimkibana Jun 3, 2024
7bf1203
add standalone metrics tests
vadimkibana Jun 3, 2024
9bb3dcc
move from command tests into subfolder
vadimkibana Jun 3, 2024
8317155
remove metrics test from the main validation file
vadimkibana Jun 3, 2024
e837471
add metrics to builtins definition
vadimkibana Jun 4, 2024
5f8d096
setup grouping and aggregates tests
vadimkibana Jun 4, 2024
57a9415
setup metrics aggregates and grouping validation
vadimkibana Jun 4, 2024
8582d3e
rename metrics command indices to sources
vadimkibana Jun 4, 2024
ce1ba83
move from command tests to its own file
vadimkibana Jun 4, 2024
0ec92c3
adjust test titles
vadimkibana Jun 4, 2024
f4cca4f
move stats tests into a separate file
vadimkibana Jun 4, 2024
2601b9e
improve test assertion
vadimkibana Jun 4, 2024
13c66a9
cleanup metrics tests
vadimkibana Jun 5, 2024
a68a5f7
implement walker
vadimkibana Jun 5, 2024
8ed1724
expose walker
vadimkibana Jun 5, 2024
cdc4b97
continue on metrics validation
vadimkibana Jun 5, 2024
7ee91b6
improve walker, add smoke test
vadimkibana Jun 5, 2024
b4391c0
harder types for field collection
vadimkibana Jun 5, 2024
2f2acc6
error on missing aggregation function
vadimkibana Jun 5, 2024
81e8d69
cleanup tests and error message
vadimkibana Jun 5, 2024
0ce1161
validate that aggregate expressions are aggregation-closed
vadimkibana Jun 5, 2024
bf75ded
emit error when aggregate expression is a column
vadimkibana Jun 6, 2024
2131b02
cleanup unknown agg function factory
vadimkibana Jun 6, 2024
76cfe0e
improve function definition
vadimkibana Jun 6, 2024
ae956d4
populate variables with aggregated fields
vadimkibana Jun 6, 2024
8e420de
automated test snapshot update
vadimkibana Jun 6, 2024
98df644
cleanup unknown column errors
vadimkibana Jun 6, 2024
ab97b79
create shared variable collection function
vadimkibana Jun 6, 2024
725d9ea
enable all metrics tests
vadimkibana Jun 6, 2024
f515e6c
add metrics function validation support
vadimkibana Jun 6, 2024
50cb9ec
fix double unknown function error
vadimkibana Jun 6, 2024
5048cf1
add more metrics tests
vadimkibana Jun 6, 2024
df65c06
improve command definition
vadimkibana Jun 6, 2024
9dbaec5
Merge remote-tracking branch 'upstream/main' into esql-metrics-defini…
vadimkibana Jun 6, 2024
ca9cb97
Merge branch 'main' into esql-metrics-definitions
vadimkibana Jun 6, 2024
e42ed51
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Jun 6, 2024
afd8160
format code as per linter
vadimkibana Jun 7, 2024
8efa81b
fix linter warnings
vadimkibana Jun 7, 2024
db534b1
Merge remote-tracking branch 'origin/esql-metrics-definitions' into e…
vadimkibana Jun 7, 2024
1bc4754
Merge branch 'main' into esql-metrics-definitions
kibanamachine Jun 7, 2024
760abc5
move walker to the autocomplete and validation package
vadimkibana Jun 7, 2024
d6cbb1b
typos
vadimkibana Jun 7, 2024
b23f34e
create higher-order function for collection errors
vadimkibana Jun 7, 2024
d48c106
add ability to collect test cases to a fixture file
vadimkibana Jun 7, 2024
7194a37
collect test fixtures from executed tests
vadimkibana Jun 7, 2024
cc1fc91
add jsdocs to test helpers
vadimkibana Jun 7, 2024
3f8c617
separate from command test suite into own file
vadimkibana Jun 7, 2024
02240b3
new naming for autogenerated files
vadimkibana Jun 7, 2024
d28b023
collect from command tests into main fixture file
vadimkibana Jun 7, 2024
6fbfefa
remove metrics dupes
vadimkibana Jun 10, 2024
a239636
add metrics supported command in generator file
vadimkibana Jun 10, 2024
31505ec
larger "do not edit" warning
vadimkibana Jun 10, 2024
2fe90f6
js ts
vadimkibana Jun 10, 2024
f3d7d92
Merge branch 'main' into esql-metrics-definitions
kibanamachine Jun 10, 2024
705f907
make agg close check function more readable
vadimkibana Jun 10, 2024
180aa25
use snake case file names
vadimkibana Jun 10, 2024
b3b3cee
setup jest integration testing for validation and autocomplete
vadimkibana Jun 10, 2024
ebec8d9
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Jun 10, 2024
084b923
improve integration test suite
vadimkibana Jun 10, 2024
eed37fd
separate FROM callback tests into its own file
vadimkibana Jun 10, 2024
83b6dac
augment fields as in the legacy test file
vadimkibana Jun 10, 2024
3e7986c
cleanup integration test implementation
vadimkibana Jun 10, 2024
508086b
test all number and string type combination
vadimkibana Jun 10, 2024
e716a1c
consolidate esql test env setup
vadimkibana Jun 10, 2024
af8248a
add comments
vadimkibana Jun 10, 2024
d004d83
move helpers into a separate file
vadimkibana Jun 10, 2024
959b665
cleanup integration tests and rename from test suite
vadimkibana Jun 10, 2024
e47b8d6
create test suites for stats and metrics commands
vadimkibana Jun 10, 2024
90d170a
remove test case collection to .json
vadimkibana Jun 11, 2024
a509c16
remove autogenerated .json files
vadimkibana Jun 11, 2024
c28ab5b
error on nested aggregation functions
vadimkibana Jun 11, 2024
9717ac4
exit early on first found agg-in-agg error
vadimkibana Jun 11, 2024
1b05483
add extra success metrics test
vadimkibana Jun 11, 2024
bc03874
use FROM command to use METRICS fields
vadimkibana Jun 11, 2024
04fbf71
Merge remote-tracking branch 'upstream/main' into esql-metrics-defini…
vadimkibana Jun 11, 2024
bbc273a
rerun validation tests
vadimkibana Jun 11, 2024
b1eb61b
remove defensive coding check
vadimkibana Jun 17, 2024
cad62b1
ignore open handles as "core.setup()" does not close something
vadimkibana Jun 17, 2024
dc653d9
Merge remote-tracking branch 'upstream/main' into esql-metrics-defini…
vadimkibana Jun 17, 2024
02d525f
rename function
vadimkibana Jun 17, 2024
9131be3
Merge remote-tracking branch 'upstream/main' into esql-metrics-defini…
vadimkibana Jun 17, 2024
fa198f3
update tests
vadimkibana Jun 17, 2024
587d9ea
Merge remote-tracking branch 'upstream/main' into esql-metrics-defini…
vadimkibana Jun 19, 2024
341713e
rerun unit tests
vadimkibana Jun 19, 2024
7e98030
use ast walker implementation
vadimkibana Jun 19, 2024
f065d9f
skip integration tests
vadimkibana Jun 19, 2024
fce4bc0
remove circular dependency
vadimkibana Jun 19, 2024
7715403
fix type check error
vadimkibana Jun 19, 2024
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
1 change: 1 addition & 0 deletions packages/kbn-esql-ast/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export type {
ESQLLiteral,
AstProviderFn,
EditorError,
ESQLAstNode,
} from './src/types';

// Low level functions to parse grammar
Expand Down
10 changes: 5 additions & 5 deletions packages/kbn-esql-ast/src/__tests__/ast_parser.metrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('METRICS', () => {
{
type: 'command',
name: 'metrics',
indices: [
sources: [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought "sources" is more accurate name for this field.

{
type: 'source',
name: 'foo',
Expand All @@ -30,7 +30,7 @@ describe('METRICS', () => {
]);
});

it('can parse multiple "indices"', () => {
it('can parse multiple "sources"', () => {
const text = 'METRICS foo ,\nbar\t,\t\nbaz \n';
const { ast, errors } = parse(text);

Expand All @@ -39,7 +39,7 @@ describe('METRICS', () => {
{
type: 'command',
name: 'metrics',
indices: [
sources: [
{
type: 'source',
name: 'foo',
Expand Down Expand Up @@ -69,7 +69,7 @@ describe('METRICS', () => {
{
type: 'command',
name: 'metrics',
indices: [
sources: [
{
type: 'source',
name: 'foo',
Expand Down Expand Up @@ -99,7 +99,7 @@ describe('METRICS', () => {
{
type: 'command',
name: 'metrics',
indices: [
sources: [
{
type: 'source',
name: 'foo',
Expand Down
16 changes: 8 additions & 8 deletions packages/kbn-esql-ast/src/ast_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {
import { getPosition } from './ast_position_utils';
import {
collectAllSourceIdentifiers,
collectAllFieldsStatements,
collectAllFields,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed "Statements" suffix, as it fields are more like "expressions".

visitByOption,
collectAllColumnIdentifiers,
visitRenameClauses,
Expand Down Expand Up @@ -120,7 +120,7 @@ export class AstListener implements ESQLParserListener {
exitRowCommand(ctx: RowCommandContext) {
const command = createCommand('row', ctx);
this.ast.push(command);
command.args.push(...collectAllFieldsStatements(ctx.fields()));
command.args.push(...collectAllFields(ctx.fields()));
}

/**
Expand Down Expand Up @@ -153,20 +153,20 @@ export class AstListener implements ESQLParserListener {
...createAstBaseItem('metrics', ctx),
type: 'command',
args: [],
indices: ctx
sources: ctx
.getTypedRuleContexts(IndexIdentifierContext)
.map((sourceCtx) => createSource(sourceCtx)),
};
this.ast.push(node);
const aggregates = collectAllFieldsStatements(ctx.fields(0));
const grouping = collectAllFieldsStatements(ctx.fields(1));
const aggregates = collectAllFields(ctx.fields(0));
const grouping = collectAllFields(ctx.fields(1));
if (aggregates && aggregates.length) {
node.aggregates = aggregates;
}
if (grouping && grouping.length) {
node.grouping = grouping;
}
node.args.push(...node.indices, ...aggregates, ...grouping);
node.args.push(...node.sources, ...aggregates, ...grouping);
}

/**
Expand All @@ -176,7 +176,7 @@ export class AstListener implements ESQLParserListener {
exitEvalCommand(ctx: EvalCommandContext) {
const commandAst = createCommand('eval', ctx);
this.ast.push(commandAst);
commandAst.args.push(...collectAllFieldsStatements(ctx.fields()));
commandAst.args.push(...collectAllFields(ctx.fields()));
}

/**
Expand All @@ -189,7 +189,7 @@ export class AstListener implements ESQLParserListener {

// STATS expression is optional
if (ctx._stats) {
command.args.push(...collectAllFieldsStatements(ctx.fields(0)));
command.args.push(...collectAllFields(ctx.fields(0)));
}
if (ctx._grouping) {
command.args.push(...visitByOption(ctx, ctx._stats ? ctx.fields(1) : ctx.fields(0)));
Expand Down
9 changes: 5 additions & 4 deletions packages/kbn-esql-ast/src/ast_walker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ import type {
ESQLFunction,
ESQLCommandOption,
ESQLAstItem,
ESQLAstField,
} from './types';

export function collectAllSourceIdentifiers(ctx: FromCommandContext): ESQLAstItem[] {
Expand Down Expand Up @@ -474,14 +475,14 @@ export function visitField(ctx: FieldContext) {
return collectBooleanExpression(ctx.booleanExpression());
}

export function collectAllFieldsStatements(ctx: FieldsContext | undefined): ESQLAstItem[] {
const ast: ESQLAstItem[] = [];
export function collectAllFields(ctx: FieldsContext | undefined): ESQLAstField[] {
const ast: ESQLAstField[] = [];
if (!ctx) {
return ast;
}
try {
for (const field of ctx.field_list()) {
ast.push(...visitField(field));
ast.push(...(visitField(field) as ESQLAstField[]));
}
} catch (e) {
// do nothing
Expand All @@ -494,7 +495,7 @@ export function visitByOption(ctx: StatsCommandContext, expr: FieldsContext | un
return [];
}
const option = createOption(ctx.BY()!.getText().toLowerCase(), ctx);
option.args.push(...collectAllFieldsStatements(expr));
option.args.push(...collectAllFields(expr));
return [option];
}

Expand Down
10 changes: 7 additions & 3 deletions packages/kbn-esql-ast/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ export type ESQLAst = ESQLAstCommand[];

export type ESQLAstCommand = ESQLCommand | ESQLAstMetricsCommand;

export type ESQLAstNode = ESQLAstCommand | ESQLAstItem;

export type ESQLSingleAstItem =
| ESQLFunction
| ESQLCommandOption
Expand All @@ -20,6 +22,8 @@ export type ESQLSingleAstItem =
| ESQLLiteral
| ESQLCommandMode;

export type ESQLAstField = ESQLFunction | ESQLColumn;

export type ESQLAstItem = ESQLSingleAstItem | ESQLAstItem[];

export interface ESQLLocation {
Expand All @@ -40,9 +44,9 @@ export interface ESQLCommand<Name = string> extends ESQLAstBaseItem<Name> {
}

export interface ESQLAstMetricsCommand extends ESQLCommand<'metrics'> {
indices: ESQLSource[];
aggregates?: ESQLAstItem[];
grouping?: ESQLAstItem[];
sources: ESQLSource[];
aggregates?: ESQLAstField[];
grouping?: ESQLAstField[];
}

export interface ESQLCommandOption extends ESQLAstBaseItem {
Expand Down
10 changes: 5 additions & 5 deletions packages/kbn-esql-validation-autocomplete/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const myCallbacks = {
const { errors, warnings } = await validateQuery("from index | stats 1 + avg(myColumn)", getAstAndSyntaxErrors, undefined, myCallbacks);
```

If not all callbacks are available it is possible to gracefully degradate the validation experience with the `ignoreOnMissingCallbacks` option:
If not all callbacks are available it is possible to gracefully degrade the validation experience with the `ignoreOnMissingCallbacks` option:

```js
import { getAstAndSyntaxErrors } from '@kbn/esql-ast';
Expand All @@ -61,7 +61,7 @@ const { errors, warnings } = await validateQuery(

#### Autocomplete

This is the complete logic for the ES|QL autocomplete language, it is completely indepedent from the actual editor (i.e. Monaco) and the suggestions reported need to be wrapped against the specific editor shape.
This is the complete logic for the ES|QL autocomplete language, it is completely independent from the actual editor (i.e. Monaco) and the suggestions reported need to be wrapped against the specific editor shape.

```js
import { getAstAndSyntaxErrors } from '@kbn/esql-ast';
Expand Down Expand Up @@ -207,13 +207,13 @@ The autocomplete/suggest task takes a query as input together with the current c
Note that autocomplete works most of the time with incomplete/invalid queries, so some logic to manipulate the query into something valid (see the `EDITOR_MARKER` or the `countBracketsUnclosed` functions for more).

Once the AST is produced there's a `getAstContext` function that finds the cursor position node (and its parent command), together with some hint like the type of current context: `expression`, `function`, `newCommand`, `option`.
The most complex case is the `expression` as it can cover a moltitude of cases. The function is highly commented in order to identify the specific cases, but there's probably some obscure area still to comment/clarify.
The most complex case is the `expression` as it can cover a multitude of cases. The function is highly commented in order to identify the specific cases, but there's probably some obscure area still to comment/clarify.

### Adding new commands/options/functions/erc...
### Adding new commands/options/functions/etc...

To update the definitions:

1. open either approriate definition file within the `definitions` folder and add a new entry to the relative array
1. open either appropriate definition file within the `definitions` folder and add a new entry to the relative array
2. if you are adding a function, run `yarn maketests` to add a set of fundamental validation tests for the new definition. If any of the suggested tests are wrong, feel free to correct them by hand. If it seems like a general problem, open an issue with the details so that we can update the generator code.
3. write new tests for validation and autocomplete

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* 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 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 or the Server
* Side Public License, v 1.
*/

module.exports = {
preset: '@kbn/test/jest_integration_node',
rootDir: '../..',
roots: ['<rootDir>/packages/kbn-esql-validation-autocomplete'],
};
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const aliasTable: Record<string, string[]> = {
const aliases = new Set(Object.values(aliasTable).flat());

const evalSupportedCommandsAndOptions = {
supportedCommands: ['stats', 'eval', 'where', 'row', 'sort'],
supportedCommands: ['stats', 'metrics', 'eval', 'where', 'row', 'sort'],
supportedOptions: ['by'],
};

Expand Down Expand Up @@ -317,8 +317,24 @@ function printGeneratedFunctionsFile(functionDefinitions: FunctionDefinition[])
}`;
};

const fileHeader = `// NOTE: This file is generated by the generate_function_definitions.ts script
// Do not edit it manually
const fileHeader = `/**
* __AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.__
*
* @note This file is generated by the \`generate_function_definitions.ts\`
* script. Do not edit it manually.
*
*
*
*
*
*
*
*
*
*
*
*
*/

import type { ESQLFunction } from '@kbn/esql-ast';
import { i18n } from '@kbn/i18n';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function createNumericAggDefinition({
name,
type: 'agg',
description,
supportedCommands: ['stats'],
supportedCommands: ['stats', 'metrics'],
signatures: [
{
params: [
Expand Down Expand Up @@ -98,7 +98,7 @@ export const statsAggregationFunctionDefinitions: FunctionDefinition[] = [
defaultMessage: 'Returns the maximum value in a field.',
}),
type: 'agg',
supportedCommands: ['stats'],
supportedCommands: ['stats', 'metrics'],
signatures: [
{
params: [{ name: 'column', type: 'number', noNestingFunctions: true }],
Expand All @@ -117,7 +117,7 @@ export const statsAggregationFunctionDefinitions: FunctionDefinition[] = [
defaultMessage: 'Returns the minimum value in a field.',
}),
type: 'agg',
supportedCommands: ['stats'],
supportedCommands: ['stats', 'metrics'],
signatures: [
{
params: [{ name: 'column', type: 'number', noNestingFunctions: true }],
Expand All @@ -138,7 +138,7 @@ export const statsAggregationFunctionDefinitions: FunctionDefinition[] = [
description: i18n.translate('kbn-esql-validation-autocomplete.esql.definitions.countDoc', {
defaultMessage: 'Returns the count of the values in a field.',
}),
supportedCommands: ['stats'],
supportedCommands: ['stats', 'metrics'],
signatures: [
{
params: [
Expand All @@ -164,7 +164,7 @@ export const statsAggregationFunctionDefinitions: FunctionDefinition[] = [
defaultMessage: 'Returns the count of distinct values in a field.',
}
),
supportedCommands: ['stats'],
supportedCommands: ['stats', 'metrics'],
signatures: [
{
params: [
Expand All @@ -188,7 +188,7 @@ export const statsAggregationFunctionDefinitions: FunctionDefinition[] = [
defaultMessage: 'Returns the count of distinct values in a field.',
}
),
supportedCommands: ['stats'],
supportedCommands: ['stats', 'metrics'],
signatures: [
{
params: [{ name: 'column', type: 'cartesian_point', noNestingFunctions: true }],
Expand All @@ -212,7 +212,7 @@ export const statsAggregationFunctionDefinitions: FunctionDefinition[] = [
description: i18n.translate('kbn-esql-validation-autocomplete.esql.definitions.values', {
defaultMessage: 'Returns all values in a group as an array.',
}),
supportedCommands: ['stats'],
supportedCommands: ['stats', 'metrics'],
signatures: [
{
params: [{ name: 'expression', type: 'any', noNestingFunctions: true }],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ function createMathDefinition(
type: 'builtin',
name,
description,
supportedCommands: ['eval', 'where', 'row', 'stats', 'sort'],
supportedCommands: ['eval', 'where', 'row', 'stats', 'metrics', 'sort'],
supportedOptions: ['by'],
signatures: types.map((type) => {
if (Array.isArray(type)) {
Expand Down Expand Up @@ -507,7 +507,7 @@ const otherDefinitions: FunctionDefinition[] = [
description: i18n.translate('kbn-esql-validation-autocomplete.esql.definition.assignDoc', {
defaultMessage: 'Assign (=)',
}),
supportedCommands: ['eval', 'stats', 'row', 'dissect', 'where', 'enrich'],
supportedCommands: ['eval', 'stats', 'metrics', 'row', 'dissect', 'where', 'enrich'],
supportedOptions: ['by', 'with'],
signatures: [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,36 @@ export const commandDefinitions: CommandDefinition[] = [
params: [{ name: 'functions', type: 'function' }],
},
},
{
name: 'metrics',
description: i18n.translate('kbn-esql-validation-autocomplete.esql.definitions.metricsDoc', {
defaultMessage:
'A metrics-specific source command, use this command to load data from TSDB indices. ' +
'Similar to STATS command on can calculate aggregate statistics, such as average, count, and sum, over the incoming search results set. ' +
'When used without a BY clause, only one row is returned, which is the aggregation over the entire incoming search results set. ' +
'When you use a BY clause, one row is returned for each distinct value in the field specified in the BY clause. ' +
'The command returns only the fields in the aggregation, and you can use a wide range of statistical functions with the stats command. ' +
'When you perform more than one aggregation, separate each aggregation with a comma.',
}),
examples: [
'metrics index',
'metrics index, index2',
'metrics index avg = avg(a)',
'metrics index sum(b) by b',
'metrics index, index2 sum(b) by b % 2',
'metrics <sources> [ <aggregates> [ by <grouping> ]]',
'metrics src1, src2 agg1, agg2 by field1, field2',
],
options: [],
modes: [],
signature: {
multipleParams: true,
params: [
{ name: 'index', type: 'source', wildcards: true },
{ name: 'expression', type: 'function', optional: true },
],
},
},
{
name: 'stats',
description: i18n.translate('kbn-esql-validation-autocomplete.esql.definitions.statsDoc', {
Expand Down
Loading