Chore: Better builtin variable check during parsing the code (#103952)

* replace and return the builtin variables

* don't parse the expression twice

* improve the replacement logic

* better code with more tests

* lint

* betterer

* rename the test suite
This commit is contained in:
ismail simsek
2025-04-22 19:57:01 +02:00
committed by GitHub
parent 4bf32f3651
commit 4b1fa81394
3 changed files with 135 additions and 44 deletions
@@ -34,7 +34,9 @@ import {
getString,
makeBinOp,
makeError,
replaceBuiltInVariable,
replaceVariables,
returnBuiltInVariable,
} from './parsingUtils';
import { QueryBuilderLabelFilter, QueryBuilderOperation } from './shared/types';
import { PromVisualQuery, PromVisualQueryBinary } from './types';
@@ -42,12 +44,11 @@ import { PromVisualQuery, PromVisualQueryBinary } from './types';
/**
* Parses a PromQL query into a visual query model.
*
* It traverses the tree and uses sort of state machine to update the query model. The query model is modified
* during the traversal and sent to each handler as context.
*
* @param expr
* It traverses the tree and uses sort of state machine to update the query model.
* The query model is modified during the traversal and sent to each handler as context.
*/
export function buildVisualQueryFromString(expr: string): Context {
expr = replaceBuiltInVariable(expr);
const replacedExpr = replaceVariables(expr);
const tree = parser.parse(replacedExpr);
const node = tree.topNode;
@@ -80,11 +81,6 @@ export function buildVisualQueryFromString(expr: string): Context {
context.errors = [];
}
// We don't want parsing errors related to Grafana global variables
if (isValidPromQLMinusGrafanaGlobalVariables(expr)) {
context.errors = [];
}
return context;
}
@@ -100,36 +96,6 @@ interface Context {
errors: ParsingError[];
}
// TODO find a better approach for grafana global variables
function isValidPromQLMinusGrafanaGlobalVariables(expr: string) {
const context: Context = {
query: {
metric: '',
labels: [],
operations: [],
},
errors: [],
};
expr = expr.replace(/\$__interval/g, '1s');
expr = expr.replace(/\$__interval_ms/g, '1000');
expr = expr.replace(/\$__rate_interval/g, '1s');
expr = expr.replace(/\$__range_ms/g, '1000');
expr = expr.replace(/\$__range_s/g, '1');
expr = expr.replace(/\$__range/g, '1s');
const tree = parser.parse(expr);
const node = tree.topNode;
try {
handleExpression(expr, node, context);
} catch (err) {
return false;
}
return context.errors.length === 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.
@@ -277,7 +243,9 @@ function handleFunction(expr: string, node: SyntaxNode, context: Context) {
let match = getString(expr, node).match(/\[(.+)\]/);
if (match?.[1]) {
interval = match[1];
params.push(match[1]);
// We were replaced the builtin variables to prevent errors
// Here we return those back
params.push(returnBuiltInVariable(match[1]));
}
}
@@ -1,7 +1,13 @@
// Core Grafana history https://github.com/grafana/grafana/blob/v11.0.0-preview/public/app/plugins/datasource/prometheus/querybuilder/parsingUtils.test.ts
import { parser } from '@prometheus-io/lezer-promql';
import { getLeftMostChild, getString, replaceVariables } from './parsingUtils';
import {
getLeftMostChild,
getString,
replaceBuiltInVariable,
replaceVariables,
returnBuiltInVariable,
} from './parsingUtils';
describe('getLeftMostChild', () => {
it('return left most child', () => {
@@ -41,3 +47,69 @@ describe('getString', () => {
expect(getString(replaced, tree.topNode)).toBe(expr);
});
});
describe('builtInTimeVariables', () => {
const testCases = [
{
expr: 'sum_over_time([[metric_var]]{bar="${app}"}[$__interval])',
expected: 'sum_over_time([[metric_var]]{bar="${app}"}[711_999_999])',
},
{
expr: 'sum_over_time([[metric_var]]{bar="${app}"}[$__rate_interval])',
expected: 'sum_over_time([[metric_var]]{bar="${app}"}[7999799979997999])',
},
{
expr: 'sum_over_time([[metric_var]]{bar="${app}"}[$__range_ms])',
expected: 'sum_over_time([[metric_var]]{bar="${app}"}[722_999_999])',
},
{
expr: 'histogram_quantile(0.95, sum(rate(process_max_fds[$__rate_interval])) by (le)) + rate(process_max_fds[$__interval])',
expected:
'histogram_quantile(0.95, sum(rate(process_max_fds[7999799979997999])) by (le)) + rate(process_max_fds[711_999_999])',
},
{
expr: 'rate(http_requests_total{job="api-server"}[$__interval_ms] offset $__interval_ms)',
expected: 'rate(http_requests_total{job="api-server"}[79_999_999_999] offset 79_999_999_999)',
},
{
expr: 'max_over_time(node_memory_usage[$__range_s])',
expected: 'max_over_time(node_memory_usage[79_299_999])',
},
{
expr: 'avg_over_time(cpu_usage{env="prod"}[$__range])',
expected: 'avg_over_time(cpu_usage{env="prod"}[799_999])',
},
{
expr: 'rate(requests[$__interval]) / rate(requests[$__interval] offset $__interval)',
expected: 'rate(requests[711_999_999]) / rate(requests[711_999_999] offset 711_999_999)',
},
{
expr: 'sum(rate(http_requests_total{status=~"5.."}[$__rate_interval])) / sum(rate(http_requests_total[$__rate_interval])) or vector($__range_ms / $__interval_ms)',
expected:
'sum(rate(http_requests_total{status=~"5.."}[7999799979997999])) / sum(rate(http_requests_total[7999799979997999])) or vector(722_999_999 / 79_999_999_999)',
},
{
expr: 'sum(rate(http_requests_total{job="api"}[5m]))',
expected: 'sum(rate(http_requests_total{job="api"}[5m]))',
},
{
expr: 'max_over_time(rate(cpu{instance="server-01"}[$__interval])[$__range_s:$__interval])',
expected: 'max_over_time(rate(cpu{instance="server-01"}[711_999_999])[79_299_999:711_999_999])',
},
{
expr: 'rate(cpu[$__interval]) + rate(memory[$__interval_ms]) + rate(disk[$__rate_interval]) + rate(network[$__range]) + rate(io[$__range_s]) + rate(gpu[$__range_ms])',
expected:
'rate(cpu[711_999_999]) + rate(memory[79_999_999_999]) + rate(disk[7999799979997999]) + rate(network[799_999]) + rate(io[79_299_999]) + rate(gpu[722_999_999])',
},
];
testCases.forEach((testCase) => {
it(testCase.expr, () => {
const actual1 = replaceBuiltInVariable(testCase.expr);
expect(actual1).toBe(testCase.expected);
const actual2 = returnBuiltInVariable(actual1);
expect(actual2).toBe(testCase.expr);
});
});
});
@@ -32,9 +32,8 @@ export function makeError(expr: string, node: SyntaxNode) {
const variableRegex = /\$(\w+)|\[\[([\s\S]+?)(?::(\w+))?\]\]|\${(\w+)(?:\.([^:^\}]+))?(?::([^\}]+))?}/g;
/**
* As variables with $ are creating parsing errors, we first replace them with magic string that is parsable and at
* the same time we can get the variable and its format back from it.
* @param expr
* As variables with $ are creating parsing errors, we first replace them with magic string that is
* parsable and at the same time we can get the variable and its format back from it.
*/
export function replaceVariables(expr: string) {
return expr.replace(variableRegex, (match, var1, var2, fmt2, var3, fieldPath, fmt3) => {
@@ -138,3 +137,55 @@ export const regexifyLabelValuesQueryString = (query: string) => {
const queryArray = query.split(' ');
return queryArray.map((query) => `${query}.*`).join('');
};
/**
* Built-in Grafana variables used for time ranges and intervals in Prometheus queries
* Each variable has a carefully crafted numeric replacement that:
* 1. Has exactly the same string length as the original variable
* 2. Is valid in Prometheus syntax to avoid parsing errors
* 3. Preserves error position information for accurate error reporting
* 4. Uses readable number formatting with digit grouping via underscores
* https://prometheus.io/docs/prometheus/latest/querying/basics/#float-literals-and-time-durations
*/
const BUILT_IN_VARIABLES = [
{ variable: '$__interval_ms', replacement: '79_999_999_999' },
{ variable: '$__interval', replacement: '711_999_999' },
{ variable: '$__rate_interval', replacement: '7999799979997999' },
{ variable: '$__range_ms', replacement: '722_999_999' },
{ variable: '$__range_s', replacement: '79_299_999' },
{ variable: '$__range', replacement: '799_999' },
];
// Derived maps for efficient lookups
const variableToReplacement = BUILT_IN_VARIABLES.reduce<Record<string, string>>((map, { variable, replacement }) => {
map[variable] = replacement;
return map;
}, {});
const replacementToVariable = BUILT_IN_VARIABLES.reduce<Record<string, string>>((map, { variable, replacement }) => {
map[replacement] = variable;
return map;
}, {});
// Pre-compiled regular expressions for efficient search/replace
const builtInVariablePattern = BUILT_IN_VARIABLES.map(({ variable }) => variable.replace(/\$/g, '\\$')).join('|');
const builtInVariableRegex = new RegExp(builtInVariablePattern, 'g');
const builtInReplacementPattern = BUILT_IN_VARIABLES.map(({ replacement }) => replacement).join('|');
const builtInReplacementRegex = new RegExp(builtInReplacementPattern, 'g');
/**
* Replaces Grafana built-in variables with numeric replacements
* This helps prevent these variables from causing parsing errors
*/
export function replaceBuiltInVariable(expr: string): string {
return expr.replace(builtInVariableRegex, (match) => variableToReplacement[match]);
}
/**
* Restores the original built-in variables from their replacement format
* Reverses the transformation done by replaceBuiltInVariable
*/
export function returnBuiltInVariable(expr: string): string {
return expr.replace(builtInReplacementRegex, (match) => replacementToVariable[match]);
}