Prometheus: Don't show expand rules warning for unique rule (#99540)

* fix recording rule query hints

* don't show the hint twice in code editor

* provide testing rules
This commit is contained in:
ismail simsek 2025-01-31 11:46:38 +01:00 committed by GitHub
parent a2b1a85dc4
commit d062453c49
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 189 additions and 3 deletions

View File

@ -1,4 +1,18 @@
groups:
- name: TESTING_RULES
rules:
- record: test:rate5m
expr: rate(ismail_prometheus_tsdb_reloads_total{job="prometheus"}[5m])
labels:
identifier: special_idx
- record: agent:test:rate5m
expr: rate(prometheus_tsdb_reloads_total{job="prometheus"}[5m])
labels:
identifier: special_idx
- record: agent:test:rate5m_agg_
expr: rate(prometheus_tsdb_reloads_total_agg_{job="aggregated"}[5m])
labels:
identifier: special_idx
- name: RECORDING_RULES
rules:
- record: instance_path:requests:rate5m
@ -9,6 +23,14 @@ groups:
expr: rate(prometheus_tsdb_reloads_failures_total{job="prometheus"}[5m])
- record: instance_path:reloads:rate5m
expr: rate(prometheus_tsdb_reloads_total{job="prometheus"}[5m])
- record: test:rate5m
expr: rate(prometheus_tsdb_reloads_total{job="prometheus"}[5m])
- record: test:rate5m_agg_
expr: rate(prometheus_tsdb_reloads_total_agg_{job="aggregated"}[5m])
- record: agent:test:rate5m
expr: rate(prometheus_tsdb_reloads_total{job="prometheus"}[5m])
- record: agent:test:rate5m_agg_
expr: rate(prometheus_tsdb_reloads_total_agg_{job="aggregated"}[5m])
- record: instance_path:request_failures_per_requests:ratio_rate5m
expr: |2
instance_path:reloads_failures:rate5m{job="prometheus"}

View File

@ -8,6 +8,7 @@ import {
getQueryHints,
getQueryLabelsForRuleName,
getRecordingRuleIdentifierIdx,
isRuleInQuery,
SUM_HINT_THRESHOLD_COUNT,
} from './query_hints';
import { buildVisualQueryFromString } from './querybuilder/parsing';
@ -365,6 +366,92 @@ describe('getExpandRulesHints', () => {
},
]);
});
it('should return expand rule hint, when given query include a non-unique rule name', () => {
const extractedMapping: RuleQueryMapping = {
'duration:p95': [
{
query: 'expanded_duration_p95{}',
labels: {},
},
{
query: 'expanded_duration_p95_aggregated{}',
labels: {
span_name: '__aggregated__',
},
},
],
'duration:p95:upper_threshold': [
{
query: 'expanded_duration_p95_upper_threshold{}',
labels: {},
},
],
};
const query = 'sum(rate(duration:p95:upper_threshold{label="foo"}[5m])) by(bar)';
const hints = getExpandRulesHints(query, extractedMapping);
expect(hints).toEqual([
{
type: 'EXPAND_RULES',
label: 'Query contains recording rules.',
fix: {
label: 'Expand rules',
action: {
type: 'EXPAND_RULES',
query,
options: {
'duration:p95:upper_threshold': {
expandedQuery: 'expanded_duration_p95_upper_threshold{}',
},
},
},
},
},
]);
});
it('should return expand rule hint, when given query include a non-unique rule name - second case', () => {
const extractedMapping: RuleQueryMapping = {
'duration:p95': [
{
query: 'expanded_duration_p95{}',
labels: {},
},
{
query: 'expanded_duration_p95_aggregated{}',
labels: {
span_name: '__aggregated__',
},
},
],
'upper_threshold:duration:p95': [
{
query: 'expanded_duration_p95_upper_threshold{}',
labels: {},
},
],
};
const query = 'sum(rate(upper_threshold:duration:p95{label="foo"}[5m])) by(bar)';
const hints = getExpandRulesHints(query, extractedMapping);
expect(hints).toEqual([
{
type: 'EXPAND_RULES',
label: 'Query contains recording rules.',
fix: {
label: 'Expand rules',
action: {
type: 'EXPAND_RULES',
query,
options: {
'upper_threshold:duration:p95': {
expandedQuery: 'expanded_duration_p95_upper_threshold{}',
},
},
},
},
},
]);
});
});
describe('getRecordingRuleIdentifierIdx', () => {
@ -505,3 +592,73 @@ describe('getQueryLabelsForRuleName', () => {
expect(result).toEqual(expected);
});
});
describe('ruleInQuery', () => {
it('should return true when ruleName is present in the query', () => {
expect(isRuleInQuery('rate(http_requests_total{job="api"}[5m])', 'http_requests_total')).toBe(true);
});
it('should return false when ruleName is not present in the query', () => {
expect(isRuleInQuery('rate(cpu_usage{instance="localhost"}[5m])', 'http_requests_total')).toBe(false);
});
it('should return true for ruleName at the start of the query', () => {
expect(isRuleInQuery('http_requests_total{job="api"}', 'http_requests_total')).toBe(true);
});
it('should return true for ruleName at the end of the query', () => {
expect(isRuleInQuery('sum by (instance) (http_requests_total)', 'http_requests_total')).toBe(true);
expect(isRuleInQuery('sum(http_requests_total)', 'http_requests_total')).toBe(true);
expect(isRuleInQuery('http_requests_total', 'http_requests_total')).toBe(true);
});
it('should return true when ruleName is followed by spaces', () => {
expect(isRuleInQuery('http_requests_total { job="api" }', 'http_requests_total')).toBe(true);
});
it('should return false when ruleName is a substring of another metric', () => {
expect(isRuleInQuery('rate(http_requests_total_new{job="api"}[5m])', 'http_requests_total')).toBe(false);
});
it('should return false for escaped ruleName usage', () => {
expect(isRuleInQuery('rate(\"http_requests_total\"{job="api"}[5m])', 'http_requests_total')).toBe(false);
});
it('should return false when query is empty', () => {
expect(isRuleInQuery('', 'http_requests_total')).toBe(false);
});
it('should return false when ruleName is an empty string', () => {
expect(isRuleInQuery('rate(http_requests_total{job="api"}[5m])', '')).toBe(false);
});
it('should return false when both query and ruleName are empty', () => {
expect(isRuleInQuery('', '')).toBe(false);
});
it('should return true when used with binary operations', () => {
expect(isRuleInQuery('rate(http_requests_total{job="api"}[5m]) + my:rule', 'my:rule')).toBe(true);
expect(isRuleInQuery('rate(http_requests_total{job="api"}[5m])+my:rule', 'my:rule')).toBe(true);
});
it('should return true when ruleName is inside nested functions', () => {
expect(isRuleInQuery('sum(rate(http_requests_total[5m]))', 'http_requests_total')).toBe(true);
});
it('should return false when ruleName is part of a label value', () => {
expect(isRuleInQuery('http_requests_total_bytes{rule="http_requests_total"}', 'http_requests_total')).toBe(false);
});
it('should return true when ruleName contains special characters like colon', () => {
expect(isRuleInQuery('rate(my_namespace:http_requests_total[5m])', 'my_namespace:http_requests_total')).toBe(true);
});
it('should return false when ruleName appears within string literals', () => {
expect(
isRuleInQuery(
'label_replace(http_requests_total, "label", "value", "instance", "http_requests_total")',
'http_requests_total'
)
).toBe(false);
});
});

View File

@ -175,10 +175,19 @@ export function getInitHints(datasource: PrometheusDatasource): QueryHint[] {
return hints;
}
export function isRuleInQuery(query: string, ruleName: string) {
if (!query || !ruleName) {
return false;
}
const getRuleRegex = new RegExp(`(?<![\\w:])${ruleName}(?=[\\[{(\\s\\)]|$)`);
return getRuleRegex.test(query);
}
export function getExpandRulesHints(query: string, mapping: RuleQueryMapping): QueryHint[] {
const hints: QueryHint[] = [];
const mappingForQuery = Object.keys(mapping).reduce((acc, ruleName) => {
if (query.search(ruleName) === -1) {
if (!isRuleInQuery(query, ruleName)) {
return acc;
}

View File

@ -7,7 +7,6 @@ import { useStyles2 } from '@grafana/ui';
import { PromQueryField } from '../../components/PromQueryField';
import { PromQueryEditorProps } from '../../components/types';
import { QueryEditorHints } from '../shared/QueryEditorHints';
import { PromQueryBuilderExplained } from './PromQueryBuilderExplained';
@ -35,7 +34,6 @@ export function PromQueryCodeEditor(props: PromQueryCodeEditorProps) {
app={app}
/>
{showExplain && <PromQueryBuilderExplained query={query.expr} />}
<QueryEditorHints query={query} datasource={datasource} data={data} onChange={onChange} onRunQuery={onRunQuery} />
</div>
);
}