From f8d67301340becf728c0e8f8e43138b577c4c47f Mon Sep 17 00:00:00 2001 From: gtk-grafana <109082771+gtk-grafana@users.noreply.github.com> Date: Wed, 3 Aug 2022 09:57:29 -0500 Subject: [PATCH] Metrics: Refactor use of lezer nodes to reference ID instead of name (parsing.ts) (#53177) * Update public/app/plugins/datasource/prometheus/querybuilder/shared/parsingUtils.ts Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> * Since loki is still passing in node type strings, update the getAllByType shared function to allow string or number types * add todo to remove string type when last datasource has migrated Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> --- .../prometheus/querybuilder/parsing.ts | 95 ++++++++++++------- .../querybuilder/shared/parsingUtils.ts | 7 +- 2 files changed, 66 insertions(+), 36 deletions(-) diff --git a/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts b/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts index edf343cd234..10140f57077 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts +++ b/public/app/plugins/datasource/prometheus/querybuilder/parsing.ts @@ -1,5 +1,30 @@ import { SyntaxNode } from '@lezer/common'; -import { parser } from 'lezer-promql'; +import { + AggregateExpr, + AggregateModifier, + AggregateOp, + BinaryExpr, + BinModifiers, + Expr, + FunctionCall, + FunctionCallArgs, + FunctionCallBody, + FunctionIdentifier, + GroupingLabel, + GroupingLabelList, + GroupingLabels, + LabelMatcher, + LabelName, + MatchOp, + MetricIdentifier, + NumberLiteral, + On, + OnOrIgnoring, + ParenExpr, + parser, + StringLiteral, + Without, +} from 'lezer-promql'; import { binaryScalarOperatorToOperatorName } from './binaryScalarOperations'; import { @@ -69,6 +94,9 @@ interface Context { errors: ParsingError[]; } +// Although 0 isn't explicitly provided in the lezer-promql library as the error node ID, it does appear to be the ID of error nodes within lezer. +const ErrorId = 0; + /** * Handler for default state. It will traverse the tree and call the appropriate handler for each node. The node * handled here does not necessarily need to be of type == Expr. @@ -78,14 +106,15 @@ interface Context { */ export function handleExpression(expr: string, node: SyntaxNode, context: Context) { const visQuery = context.query; - switch (node.name) { - case 'MetricIdentifier': { + + switch (node.type.id) { + case MetricIdentifier: { // Expectation is that there is only one of those per query. visQuery.metric = getString(expr, node); break; } - case 'LabelMatcher': { + case LabelMatcher: { // Same as MetricIdentifier should be just one per query. visQuery.labels.push(getLabel(expr, node)); const err = node.getChild(ErrorName); @@ -95,22 +124,22 @@ export function handleExpression(expr: string, node: SyntaxNode, context: Contex break; } - case 'FunctionCall': { + case FunctionCall: { handleFunction(expr, node, context); break; } - case 'AggregateExpr': { + case AggregateExpr: { handleAggregation(expr, node, context); break; } - case 'BinaryExpr': { + case BinaryExpr: { handleBinary(expr, node, context); break; } - case ErrorName: { + case ErrorId: { if (isIntervalVariableError(node)) { break; } @@ -119,7 +148,7 @@ export function handleExpression(expr: string, node: SyntaxNode, context: Contex } default: { - if (node.name === 'ParenExpr') { + if (node.type.id === ParenExpr) { // We don't support parenthesis in the query to group expressions. We just report error but go on with the // parsing. context.errors.push(makeError(expr, node)); @@ -142,9 +171,9 @@ function isIntervalVariableError(node: SyntaxNode) { } function getLabel(expr: string, node: SyntaxNode): QueryBuilderLabelFilter { - const label = getString(expr, node.getChild('LabelName')); - const op = getString(expr, node.getChild('MatchOp')); - const value = getString(expr, node.getChild('StringLiteral')).replace(/"/g, ''); + const label = getString(expr, node.getChild(LabelName)); + const op = getString(expr, node.getChild(MatchOp)); + const value = getString(expr, node.getChild(StringLiteral)).replace(/"/g, ''); return { label, op, @@ -161,11 +190,11 @@ const rangeFunctions = ['changes', 'rate', 'irate', 'increase', 'delta']; */ function handleFunction(expr: string, node: SyntaxNode, context: Context) { const visQuery = context.query; - const nameNode = node.getChild('FunctionIdentifier'); + const nameNode = node.getChild(FunctionIdentifier); const funcName = getString(expr, nameNode); - const body = node.getChild('FunctionCallBody'); - const callArgs = body!.getChild('FunctionCallArgs'); + const body = node.getChild(FunctionCallBody); + const callArgs = body!.getChild(FunctionCallArgs); const params = []; let interval = ''; @@ -203,10 +232,10 @@ function handleFunction(expr: string, node: SyntaxNode, context: Context) { */ function handleAggregation(expr: string, node: SyntaxNode, context: Context) { const visQuery = context.query; - const nameNode = node.getChild('AggregateOp'); + const nameNode = node.getChild(AggregateOp); let funcName = getString(expr, nameNode); - const modifier = node.getChild('AggregateModifier'); + const modifier = node.getChild(AggregateModifier); const labels = []; if (modifier) { @@ -215,16 +244,16 @@ function handleAggregation(expr: string, node: SyntaxNode, context: Context) { funcName = `__${funcName}_by`; } - const withoutModifier = modifier.getChild(`Without`); + const withoutModifier = modifier.getChild(Without); if (withoutModifier) { funcName = `__${funcName}_without`; } - labels.push(...getAllByType(expr, modifier, 'GroupingLabel')); + labels.push(...getAllByType(expr, modifier, GroupingLabel)); } - const body = node.getChild('FunctionCallBody'); - const callArgs = body!.getChild('FunctionCallArgs'); + const body = node.getChild(FunctionCallBody); + const callArgs = body!.getChild(FunctionCallArgs); const op: QueryBuilderOperation = { id: funcName, params: [] }; visQuery.operations.unshift(op); @@ -249,11 +278,11 @@ function updateFunctionArgs(expr: string, node: SyntaxNode | null, context: Cont if (!node) { return; } - switch (node.name) { + switch (node.type.id) { // In case we have an expression we don't know what kind so we have to look at the child as it can be anything. - case 'Expr': + case Expr: // FunctionCallArgs are nested bit weirdly as mentioned so we have to go one deeper in this case. - case 'FunctionCallArgs': { + case FunctionCallArgs: { let child = node.firstChild; while (child) { updateFunctionArgs(expr, child, context, op); @@ -262,12 +291,12 @@ function updateFunctionArgs(expr: string, node: SyntaxNode | null, context: Cont break; } - case 'NumberLiteral': { + case NumberLiteral: { op.params.push(parseFloat(getString(expr, node))); break; } - case 'StringLiteral': { + case StringLiteral: { op.params.push(getString(expr, node).replace(/"/g, '')); break; } @@ -291,16 +320,16 @@ function handleBinary(expr: string, node: SyntaxNode, context: Context) { const visQuery = context.query; const left = node.firstChild!; const op = getString(expr, left.nextSibling); - const binModifier = getBinaryModifier(expr, node.getChild('BinModifiers')); + const binModifier = getBinaryModifier(expr, node.getChild(BinModifiers)); const right = node.lastChild!; const opDef = binaryScalarOperatorToOperatorName[op]; - const leftNumber = left.getChild('NumberLiteral'); - const rightNumber = right.getChild('NumberLiteral'); + const leftNumber = left.getChild(NumberLiteral); + const rightNumber = right.getChild(NumberLiteral); - const rightBinary = right.getChild('BinaryExpr'); + const rightBinary = right.getChild(BinaryExpr); if (leftNumber) { // TODO: this should be already handled in case parent is binary expression as it has to be added to parent @@ -359,17 +388,17 @@ function getBinaryModifier( if (node.getChild('Bool')) { return { isBool: true, isMatcher: false }; } else { - const matcher = node.getChild('OnOrIgnoring'); + const matcher = node.getChild(OnOrIgnoring); if (!matcher) { // Not sure what this could be, maybe should be an error. return undefined; } - const labels = getString(expr, matcher.getChild('GroupingLabels')?.getChild('GroupingLabelList')); + const labels = getString(expr, matcher.getChild(GroupingLabels)?.getChild(GroupingLabelList)); return { isMatcher: true, isBool: false, matches: labels, - matchType: matcher.getChild('On') ? 'on' : 'ignoring', + matchType: matcher.getChild(On) ? 'on' : 'ignoring', }; } } diff --git a/public/app/plugins/datasource/prometheus/querybuilder/shared/parsingUtils.ts b/public/app/plugins/datasource/prometheus/querybuilder/shared/parsingUtils.ts index 3e6f2a7c6f1..aac56d54a28 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/shared/parsingUtils.ts +++ b/public/app/plugins/datasource/prometheus/querybuilder/shared/parsingUtils.ts @@ -113,10 +113,11 @@ export function makeBinOp( * not be safe is it would also find arguments of nested functions. * @param expr * @param cur - * @param type + * @param type - can be string or number, some data-sources (loki) haven't migrated over to using numeric constants defined in the lezer parsing library (e.g. lezer-promql). + * @todo Remove string type definition when all data-sources have migrated to numeric constants */ -export function getAllByType(expr: string, cur: SyntaxNode, type: string): string[] { - if (cur.name === type) { +export function getAllByType(expr: string, cur: SyntaxNode, type: number | string): string[] { + if (cur.type.id === type || cur.name === type) { return [getString(expr, cur)]; } const values: string[] = [];