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 45 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
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', 'metrics', 'row', 'dissect', 'where', 'enrich'],
vadimkibana marked this conversation as resolved.
Show resolved Hide resolved
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