Prometheus: Improve handling of special chars in label values (#96067)

* fix: handling of special chars

* docs: add clarity

* fix: escaping

* refactor: put changes behind new feature toggle

* docs: use consistent comment style

* refactor: rename feature toggle for brevity

* use single quotes

* fix unit tests

* remove redundant json entry

* fix: keep all changes behind feature toggle

* fix: support builder mode

* fix: don't escape when using regex operators

* fix: code mode label values completions with special chars

* refactor: remove unneeded changes

* move feature toggle up so new changes from main won't conflict with ours

* fix: escape label values in metric select scene

* refactor: ensure changes are behind feature toggle

---------

Co-authored-by: ismail simsek <ismailsimsek09@gmail.com>
This commit is contained in:
Nick Richmond
2024-12-18 16:31:08 -05:00
committed by GitHub
parent c9c77a6a6c
commit 721c50a304
14 changed files with 513 additions and 109 deletions

View File

@@ -232,6 +232,7 @@ export interface FeatureToggles {
playlistsWatcher?: boolean;
passwordlessMagicLinkAuthentication?: boolean;
exploreMetricsRelatedLogs?: boolean;
prometheusSpecialCharsInLabelValues?: boolean;
enableExtensionsAdminPage?: boolean;
zipkinBackendMigration?: boolean;
enableSCIM?: boolean;

View File

@@ -274,3 +274,167 @@ describe.each(metricNameCompletionSituations)('metric name completions in situat
expect(completions.length).toBeLessThanOrEqual(expectedCompletionsCount);
});
});
describe('Label value completions', () => {
let dataProvider: DataProvider;
beforeEach(() => {
dataProvider = {
getAllMetricNames: jest.fn(),
metricNamesToMetrics: jest.fn(),
getHistory: jest.fn(),
getLabelNames: jest.fn(),
getLabelValues: jest.fn().mockResolvedValue(['value1', 'value"2', 'value\\3', "value'4"]),
getSeriesLabels: jest.fn(),
getSeriesValues: jest.fn(),
monacoSettings: {
setInputInRange: jest.fn(),
inputInRange: '',
suggestionsIncomplete: false,
enableAutocompleteSuggestionsUpdate: jest.fn(),
},
metricNamesSuggestionLimit: 100,
} as unknown as DataProvider;
});
afterEach(() => {
jest.restoreAllMocks();
});
describe('with prometheusSpecialCharsInLabelValues disabled', () => {
beforeEach(() => {
jest.replaceProperty(config, 'featureToggles', {
prometheusSpecialCharsInLabelValues: false,
});
});
it('should not escape special characters when between quotes', async () => {
const situation: Situation = {
type: 'IN_LABEL_SELECTOR_WITH_LABEL_NAME',
labelName: 'testLabel',
betweenQuotes: true,
otherLabels: [],
};
const completions = await getCompletions(situation, dataProvider);
expect(completions).toHaveLength(4);
expect(completions[0].insertText).toBe('value1');
expect(completions[1].insertText).toBe('value"2');
expect(completions[2].insertText).toBe('value\\3');
expect(completions[3].insertText).toBe("value'4");
});
it('should wrap in quotes but not escape special characters when not between quotes', async () => {
const situation: Situation = {
type: 'IN_LABEL_SELECTOR_WITH_LABEL_NAME',
labelName: 'testLabel',
betweenQuotes: false,
otherLabels: [],
};
const completions = await getCompletions(situation, dataProvider);
expect(completions).toHaveLength(4);
expect(completions[0].insertText).toBe('"value1"');
expect(completions[1].insertText).toBe('"value"2"');
expect(completions[2].insertText).toBe('"value\\3"');
expect(completions[3].insertText).toBe('"value\'4"');
});
});
describe('with prometheusSpecialCharsInLabelValues enabled', () => {
beforeEach(() => {
jest.replaceProperty(config, 'featureToggles', {
prometheusSpecialCharsInLabelValues: true,
});
});
it('should escape special characters when between quotes', async () => {
const situation: Situation = {
type: 'IN_LABEL_SELECTOR_WITH_LABEL_NAME',
labelName: 'testLabel',
betweenQuotes: true,
otherLabels: [],
};
const completions = await getCompletions(situation, dataProvider);
expect(completions).toHaveLength(4);
expect(completions[0].insertText).toBe('value1');
expect(completions[1].insertText).toBe('value\\"2');
expect(completions[2].insertText).toBe('value\\\\3');
expect(completions[3].insertText).toBe("value'4");
});
it('should wrap in quotes and escape special characters when not between quotes', async () => {
const situation: Situation = {
type: 'IN_LABEL_SELECTOR_WITH_LABEL_NAME',
labelName: 'testLabel',
betweenQuotes: false,
otherLabels: [],
};
const completions = await getCompletions(situation, dataProvider);
expect(completions).toHaveLength(4);
expect(completions[0].insertText).toBe('"value1"');
expect(completions[1].insertText).toBe('"value\\"2"');
expect(completions[2].insertText).toBe('"value\\\\3"');
expect(completions[3].insertText).toBe('"value\'4"');
});
});
describe('label value escaping edge cases', () => {
beforeEach(() => {
jest.replaceProperty(config, 'featureToggles', {
prometheusSpecialCharsInLabelValues: true,
});
});
it('should handle empty values', async () => {
jest.spyOn(dataProvider, 'getLabelValues').mockResolvedValue(['']);
const situation: Situation = {
type: 'IN_LABEL_SELECTOR_WITH_LABEL_NAME',
labelName: 'testLabel',
betweenQuotes: false,
otherLabels: [],
};
const completions = await getCompletions(situation, dataProvider);
expect(completions).toHaveLength(1);
expect(completions[0].insertText).toBe('""');
});
it('should handle values with multiple special characters', async () => {
jest.spyOn(dataProvider, 'getLabelValues').mockResolvedValue(['test"\\value']);
const situation: Situation = {
type: 'IN_LABEL_SELECTOR_WITH_LABEL_NAME',
labelName: 'testLabel',
betweenQuotes: true,
otherLabels: [],
};
const completions = await getCompletions(situation, dataProvider);
expect(completions).toHaveLength(1);
expect(completions[0].insertText).toBe('test\\"\\\\value');
});
it('should handle non-string values', async () => {
jest.spyOn(dataProvider, 'getLabelValues').mockResolvedValue([123 as unknown as string]);
const situation: Situation = {
type: 'IN_LABEL_SELECTOR_WITH_LABEL_NAME',
labelName: 'testLabel',
betweenQuotes: false,
otherLabels: [],
};
const completions = await getCompletions(situation, dataProvider);
expect(completions).toHaveLength(1);
expect(completions[0].insertText).toBe('"123"');
});
});
});

View File

@@ -3,6 +3,7 @@ import UFuzzy from '@leeoniya/ufuzzy';
import { config } from '@grafana/runtime';
import { prometheusRegularEscape } from '../../../datasource';
import { escapeLabelValueInExactSelector } from '../../../language_utils';
import { FUNCTIONS } from '../../../promql';
@@ -208,10 +209,15 @@ async function getLabelValuesForMetricCompletions(
return values.map((text) => ({
type: 'LABEL_VALUE',
label: text,
insertText: betweenQuotes ? text : `"${text}"`, // FIXME: escaping strange characters?
insertText: formatLabelValueForCompletion(text, betweenQuotes),
}));
}
function formatLabelValueForCompletion(value: string, betweenQuotes: boolean): string {
const text = config.featureToggles.prometheusSpecialCharsInLabelValues ? prometheusRegularEscape(value) : value;
return betweenQuotes ? text : `"${text}"`;
}
export function getCompletions(situation: Situation, dataProvider: DataProvider): Promise<Completion[]> {
switch (situation.type) {
case 'IN_DURATION':

View File

@@ -245,60 +245,118 @@ describe('PrometheusDatasource', () => {
const DEFAULT_QUERY_EXPRESSION = 'metric{job="foo"} - metric';
const target: PromQuery = { expr: DEFAULT_QUERY_EXPRESSION, refId: 'A' };
it('should not modify expression with no filters', async () => {
ds.query({
interval: '15s',
range: getMockTimeRange(),
targets: [target],
} as DataQueryRequest<PromQuery>);
const [result] = fetchMockCalledWith(fetchMock);
expect(result).toMatchObject({ expr: DEFAULT_QUERY_EXPRESSION });
describe('with prometheusSpecialCharsInLabelValues disabled', () => {
beforeAll(() => {
config.featureToggles.prometheusSpecialCharsInLabelValues = false;
});
it('should not modify expression with no filters', async () => {
ds.query({
interval: '15s',
range: getMockTimeRange(),
targets: [target],
} as DataQueryRequest<PromQuery>);
const [result] = fetchMockCalledWith(fetchMock);
expect(result).toMatchObject({ expr: DEFAULT_QUERY_EXPRESSION });
});
it('should add filters to expression', () => {
const filters = [
{
key: 'k1',
operator: '=',
value: 'v1',
},
{
key: 'k2',
operator: '!=',
value: 'v2',
},
];
ds.query({
interval: '15s',
range: getMockTimeRange(),
filters,
targets: [target],
} as DataQueryRequest<PromQuery>);
const [result] = fetchMockCalledWith(fetchMock);
expect(result).toMatchObject({ expr: 'metric{job="foo", k1="v1", k2!="v2"} - metric{k1="v1", k2!="v2"}' });
});
it('should add escaping if needed to regex filter expressions', () => {
const filters = [
{
key: 'k1',
operator: '=~',
value: 'v.*',
},
{
key: 'k2',
operator: '=~',
value: `v'.*`,
},
];
ds.query({
interval: '15s',
range: getMockTimeRange(),
filters,
targets: [target],
} as DataQueryRequest<PromQuery>);
const [result] = fetchMockCalledWith(fetchMock);
expect(result).toMatchObject({
expr: `metric{job="foo", k1=~"v.*", k2=~"v\\\\'.*"} - metric{k1=~"v.*", k2=~"v\\\\'.*"}`,
});
});
});
it('should add filters to expression', () => {
const filters = [
{
key: 'k1',
operator: '=',
value: 'v1',
},
{
key: 'k2',
operator: '!=',
value: 'v2',
},
];
ds.query({
interval: '15s',
range: getMockTimeRange(),
filters,
targets: [target],
} as DataQueryRequest<PromQuery>);
const [result] = fetchMockCalledWith(fetchMock);
expect(result).toMatchObject({ expr: 'metric{job="foo", k1="v1", k2!="v2"} - metric{k1="v1", k2!="v2"}' });
});
it('should add escaping if needed to regex filter expressions', () => {
const filters = [
{
key: 'k1',
operator: '=~',
value: 'v.*',
},
{
key: 'k2',
operator: '=~',
value: `v'.*`,
},
];
ds.query({
interval: '15s',
range: getMockTimeRange(),
filters,
targets: [target],
} as DataQueryRequest<PromQuery>);
const [result] = fetchMockCalledWith(fetchMock);
expect(result).toMatchObject({
expr: `metric{job="foo", k1=~"v.*", k2=~"v\\\\'.*"} - metric{k1=~"v.*", k2=~"v\\\\'.*"}`,
describe('with prometheusSpecialCharsInLabelValues enabled', () => {
beforeAll(() => {
config.featureToggles.prometheusSpecialCharsInLabelValues = true;
});
it('should not modify expression with no filters', async () => {
ds.query({
interval: '15s',
range: getMockTimeRange(),
targets: [target],
} as DataQueryRequest<PromQuery>);
const [result] = fetchMockCalledWith(fetchMock);
expect(result).toMatchObject({ expr: DEFAULT_QUERY_EXPRESSION });
});
it('should add escaping if needed to regex filter expressions', () => {
const filters = [
{
key: 'k1',
operator: '=~',
value: 'v.*',
},
{
key: 'k2',
operator: '=~',
value: `v'.*`,
},
{
key: 'k3',
operator: '=~',
value: `v".*`,
},
{
key: 'k4',
operator: '=~',
value: `\\v.*`,
},
];
ds.query({
interval: '15s',
range: getMockTimeRange(),
filters,
targets: [target],
} as DataQueryRequest<PromQuery>);
const [result] = fetchMockCalledWith(fetchMock);
expect(result).toMatchObject({
expr: `metric{job="foo", k1=~"v.*", k2=~"v'.*", k3=~"v\\".*", k4=~"\\\\v.*"} - metric{k1=~"v.*", k2=~"v'.*", k3=~"v\\".*", k4=~"\\\\v.*"}`,
});
});
});
});
@@ -470,56 +528,126 @@ describe('PrometheusDatasource', () => {
});
describe('Prometheus regular escaping', () => {
it('should not escape non-string', () => {
expect(prometheusRegularEscape(12)).toEqual(12);
describe('with prometheusSpecialCharsInLabelValues disabled', () => {
beforeAll(() => {
config.featureToggles.prometheusSpecialCharsInLabelValues = false;
});
it('should not escape non-string', () => {
expect(prometheusRegularEscape(12)).toEqual(12);
});
it('should not escape strings without special characters', () => {
expect(prometheusRegularEscape('cryptodepression')).toEqual('cryptodepression');
});
it('should escape single quotes', () => {
expect(prometheusRegularEscape("looking'glass")).toEqual("looking\\\\'glass");
});
it('should escape backslashes', () => {
expect(prometheusRegularEscape('looking\\glass')).toEqual('looking\\\\glass');
});
});
it('should not escape simple string', () => {
expect(prometheusRegularEscape('cryptodepression')).toEqual('cryptodepression');
});
describe('with prometheusSpecialCharsInLabelValues enabled', () => {
beforeAll(() => {
config.featureToggles.prometheusSpecialCharsInLabelValues = true;
});
it("should escape '", () => {
expect(prometheusRegularEscape("looking'glass")).toEqual("looking\\\\'glass");
});
it('should not escape non-string', () => {
expect(prometheusRegularEscape(12)).toEqual(12);
});
it('should escape \\', () => {
expect(prometheusRegularEscape('looking\\glass')).toEqual('looking\\\\glass');
});
it('should not escape strings without special characters', () => {
expect(prometheusRegularEscape('cryptodepression')).toEqual('cryptodepression');
});
it('should escape multiple characters', () => {
expect(prometheusRegularEscape("'looking'glass'")).toEqual("\\\\'looking\\\\'glass\\\\'");
});
it('should not escape complete label matcher', () => {
expect(prometheusRegularEscape('job="grafana"')).toEqual('job="grafana"');
expect(prometheusRegularEscape('job!="grafana"')).toEqual('job!="grafana"');
expect(prometheusRegularEscape('job=~"grafana"')).toEqual('job=~"grafana"');
expect(prometheusRegularEscape('job!~"grafana"')).toEqual('job!~"grafana"');
});
it('should escape multiple different characters', () => {
expect(prometheusRegularEscape("'loo\\king'glass'")).toEqual("\\\\'loo\\\\king\\\\'glass\\\\'");
it('should not escape single quotes', () => {
expect(prometheusRegularEscape("looking'glass")).toEqual("looking'glass");
});
it('should escape double quotes', () => {
expect(prometheusRegularEscape('looking"glass')).toEqual('looking\\"glass');
});
it('should escape backslashes', () => {
expect(prometheusRegularEscape('looking\\glass')).toEqual('looking\\\\glass');
});
it('should handle complete label matchers with escaped content', () => {
expect(prometheusRegularEscape('job="my\\"service"')).toEqual('job="my\\"service"');
expect(prometheusRegularEscape('job="\\\\server"')).toEqual('job="\\\\server"');
});
});
});
describe('Prometheus regexes escaping', () => {
it('should not escape simple string', () => {
expect(prometheusSpecialRegexEscape('cryptodepression')).toEqual('cryptodepression');
describe('with prometheusSpecialCharsInLabelValues disabled', () => {
beforeAll(() => {
config.featureToggles.prometheusSpecialCharsInLabelValues = false;
});
it('should not escape strings without special characters', () => {
expect(prometheusSpecialRegexEscape('cryptodepression')).toEqual('cryptodepression');
});
it('should escape special characters', () => {
expect(prometheusSpecialRegexEscape('looking{glass')).toEqual('looking\\\\{glass');
expect(prometheusSpecialRegexEscape('looking$glass')).toEqual('looking\\\\$glass');
expect(prometheusSpecialRegexEscape('looking\\glass')).toEqual('looking\\\\\\\\glass');
expect(prometheusSpecialRegexEscape('looking|glass')).toEqual('looking\\\\|glass');
});
it('should handle multiple special characters', () => {
expect(prometheusSpecialRegexEscape('+looking$glass?')).toEqual('\\\\+looking\\\\$glass\\\\?');
});
});
it('should escape $^*+?.()|\\', () => {
expect(prometheusSpecialRegexEscape("looking'glass")).toEqual("looking\\\\'glass");
expect(prometheusSpecialRegexEscape('looking{glass')).toEqual('looking\\\\{glass');
expect(prometheusSpecialRegexEscape('looking}glass')).toEqual('looking\\\\}glass');
expect(prometheusSpecialRegexEscape('looking[glass')).toEqual('looking\\\\[glass');
expect(prometheusSpecialRegexEscape('looking]glass')).toEqual('looking\\\\]glass');
expect(prometheusSpecialRegexEscape('looking$glass')).toEqual('looking\\\\$glass');
expect(prometheusSpecialRegexEscape('looking^glass')).toEqual('looking\\\\^glass');
expect(prometheusSpecialRegexEscape('looking*glass')).toEqual('looking\\\\*glass');
expect(prometheusSpecialRegexEscape('looking+glass')).toEqual('looking\\\\+glass');
expect(prometheusSpecialRegexEscape('looking?glass')).toEqual('looking\\\\?glass');
expect(prometheusSpecialRegexEscape('looking.glass')).toEqual('looking\\\\.glass');
expect(prometheusSpecialRegexEscape('looking(glass')).toEqual('looking\\\\(glass');
expect(prometheusSpecialRegexEscape('looking)glass')).toEqual('looking\\\\)glass');
expect(prometheusSpecialRegexEscape('looking\\glass')).toEqual('looking\\\\\\\\glass');
expect(prometheusSpecialRegexEscape('looking|glass')).toEqual('looking\\\\|glass');
});
describe('with prometheusSpecialCharsInLabelValues enabled', () => {
beforeAll(() => {
config.featureToggles.prometheusSpecialCharsInLabelValues = true;
});
it('should escape multiple special characters', () => {
expect(prometheusSpecialRegexEscape('+looking$glass?')).toEqual('\\\\+looking\\\\$glass\\\\?');
it('should not escape strings without special characters', () => {
expect(prometheusSpecialRegexEscape('cryptodepression')).toEqual('cryptodepression');
});
it('should escape special characters', () => {
expect(prometheusSpecialRegexEscape('looking{glass')).toEqual('looking\\\\{glass');
expect(prometheusSpecialRegexEscape('looking}glass')).toEqual('looking\\\\}glass');
expect(prometheusSpecialRegexEscape('looking[glass')).toEqual('looking\\\\[glass');
expect(prometheusSpecialRegexEscape('looking]glass')).toEqual('looking\\\\]glass');
expect(prometheusSpecialRegexEscape('looking$glass')).toEqual('looking\\\\$glass');
expect(prometheusSpecialRegexEscape('looking^glass')).toEqual('looking\\\\^glass');
expect(prometheusSpecialRegexEscape('looking*glass')).toEqual('looking\\\\*glass');
expect(prometheusSpecialRegexEscape('looking+glass')).toEqual('looking\\\\+glass');
expect(prometheusSpecialRegexEscape('looking?glass')).toEqual('looking\\\\?glass');
expect(prometheusSpecialRegexEscape('looking.glass')).toEqual('looking\\\\.glass');
expect(prometheusSpecialRegexEscape('looking(glass')).toEqual('looking\\\\(glass');
expect(prometheusSpecialRegexEscape('looking)glass')).toEqual('looking\\\\)glass');
expect(prometheusSpecialRegexEscape('looking\\glass')).toEqual('looking\\\\\\\\glass');
expect(prometheusSpecialRegexEscape('looking|glass')).toEqual('looking\\\\|glass');
});
it('should escape double quotes with special regex escaping', () => {
expect(prometheusSpecialRegexEscape('looking"glass')).toEqual('looking\\\\\\"glass');
});
it('should handle multiple special characters', () => {
expect(prometheusSpecialRegexEscape('+looking$glass?')).toEqual('\\\\+looking\\\\$glass\\\\?');
});
it('should handle mixed quotes and special characters', () => {
expect(prometheusSpecialRegexEscape('+looking"$glass?')).toEqual('\\\\+looking\\\\\\"\\\\$glass\\\\?');
});
});
});
@@ -548,9 +676,27 @@ describe('PrometheusDatasource', () => {
};
});
describe('and value is a string', () => {
it('should only escape single quotes', () => {
expect(ds.interpolateQueryExpr("abc'$^*{}[]+?.()|", customVariable)).toEqual("abc\\\\'$^*{}[]+?.()|");
describe('with prometheusSpecialCharsInLabelValues disabled', () => {
beforeAll(() => {
config.featureToggles.prometheusSpecialCharsInLabelValues = false;
});
describe('and value is a string', () => {
it('should escape single quotes', () => {
expect(ds.interpolateQueryExpr("abc'$^*{}[]+?.()|", customVariable)).toEqual("abc\\\\'$^*{}[]+?.()|");
});
});
});
describe('with prometheusSpecialCharsInLabelValues enabled', () => {
beforeAll(() => {
config.featureToggles.prometheusSpecialCharsInLabelValues = true;
});
describe('and value is a string', () => {
it('should only escape double quotes and backslashes', () => {
expect(ds.interpolateQueryExpr('abc\'"$^*{}[]+?.()|\\', customVariable)).toEqual('abc\'\\"$^*{}[]+?.()|\\\\');
});
});
});

View File

@@ -1046,13 +1046,46 @@ export function extractRuleMappingFromGroups(groups: RawRecordingRules[]): RuleQ
);
}
// NOTE: these two functions are very similar to the escapeLabelValueIn* functions
// NOTE: these two functions are similar to the escapeLabelValueIn* functions
// in language_utils.ts, but they are not exactly the same algorithm, and we found
// no way to reuse one in the another or vice versa.
export function prometheusRegularEscape<T>(value: T) {
return typeof value === 'string' ? value.replace(/\\/g, '\\\\').replace(/'/g, "\\\\'") : value;
if (typeof value !== 'string') {
return value;
}
if (config.featureToggles.prometheusSpecialCharsInLabelValues) {
// if the string looks like a complete label matcher (e.g. 'job="grafana"' or 'job=~"grafana"'),
// don't escape the encapsulating quotes
if (/^\w+(=|!=|=~|!~)".*"$/.test(value)) {
return value;
}
return value
.replace(/\\/g, '\\\\') // escape backslashes
.replace(/"/g, '\\"'); // escape double quotes
}
// classic behavior
return value
.replace(/\\/g, '\\\\') // escape backslashes
.replace(/'/g, "\\\\'"); // escape single quotes
}
export function prometheusSpecialRegexEscape<T>(value: T) {
return typeof value === 'string' ? value.replace(/\\/g, '\\\\\\\\').replace(/[$^*{}\[\]\'+?.()|]/g, '\\\\$&') : value;
if (typeof value !== 'string') {
return value;
}
if (config.featureToggles.prometheusSpecialCharsInLabelValues) {
return value
.replace(/\\/g, '\\\\\\\\') // escape backslashes
.replace(/"/g, '\\\\\\"') // escape double quotes
.replace(/[$^*{}\[\]\'+?.()|]/g, '\\\\$&'); // escape regex metacharacters
}
// classic behavior
return value
.replace(/\\/g, '\\\\\\\\') // escape backslashes
.replace(/[$^*{}\[\]+?.()|]/g, '\\\\$&'); // escape regex metacharacters
}

View File

@@ -1,6 +1,8 @@
// Core Grafana history https://github.com/grafana/grafana/blob/v11.0.0-preview/public/app/plugins/datasource/prometheus/querybuilder/shared/LokiAndPromQueryModellerBase.ts
import { Registry } from '@grafana/data';
import { config } from '@grafana/runtime';
import { prometheusRegularEscape } from '../../datasource';
import { PromVisualQueryOperationCategory } from '../types';
import { QueryBuilderLabelFilter, QueryBuilderOperation, QueryBuilderOperationDef, VisualQueryModeller } from './types';
@@ -89,7 +91,13 @@ export abstract class LokiAndPromQueryModellerBase implements VisualQueryModelle
expr += ', ';
}
expr += `${filter.label}${filter.op}"${filter.value}"`;
let labelValue = filter.value;
const usingRegexOperator = filter.op === '=~' || filter.op === '!~';
if (config.featureToggles.prometheusSpecialCharsInLabelValues && !usingRegexOperator) {
labelValue = prometheusRegularEscape(labelValue);
}
expr += `${filter.label}${filter.op}"${labelValue}"`;
}
return expr + `}`;