From 704d91f2be0295a29fc5953f14e767ebbc44c16e Mon Sep 17 00:00:00 2001 From: ismail simsek Date: Fri, 20 Jun 2025 12:12:42 +0200 Subject: [PATCH] Prometheus: Fix utf8 label value handling (#106963) * clean up * fix tooltip * adjust label values query for utf8 label names * fix series call with utf8 label * remove commented code * fix test --- .../components/PromQueryEditorByApp.test.tsx | 2 - .../src/components/PromQueryField.test.tsx | 2 +- .../src/components/VariableQueryEditor.tsx | 4 +- .../useMetricsLabelsValues.test.ts | 2 - .../completions.test.ts | 9 ---- .../usePromQueryFieldEffects.test.ts | 8 ---- .../src/locales/en-US/grafana-prometheus.json | 2 +- .../src/metric_find_query.test.ts | 6 +-- .../src/metric_find_query.ts | 4 +- .../components/MetricsLabelsSection.test.tsx | 15 +++---- .../src/resource_clients.test.ts | 44 +++++++++++++++++++ .../src/resource_clients.ts | 7 ++- 12 files changed, 60 insertions(+), 45 deletions(-) diff --git a/packages/grafana-prometheus/src/components/PromQueryEditorByApp.test.tsx b/packages/grafana-prometheus/src/components/PromQueryEditorByApp.test.tsx index 640fe7e64bb..31979cb5be0 100644 --- a/packages/grafana-prometheus/src/components/PromQueryEditorByApp.test.tsx +++ b/packages/grafana-prometheus/src/components/PromQueryEditorByApp.test.tsx @@ -28,8 +28,6 @@ function setup(app: CoreApp): { onRunQuery: jest.Mock } { getQueryHints: jest.fn(() => []), languageProvider: { start: () => Promise.resolve([]), - syntax: () => {}, - getLabelKeys: () => [], retrieveMetrics: () => [], }, } as unknown as PrometheusDatasource; diff --git a/packages/grafana-prometheus/src/components/PromQueryField.test.tsx b/packages/grafana-prometheus/src/components/PromQueryField.test.tsx index 9c1c27f03b7..d3fc7b80360 100644 --- a/packages/grafana-prometheus/src/components/PromQueryField.test.tsx +++ b/packages/grafana-prometheus/src/components/PromQueryField.test.tsx @@ -28,7 +28,7 @@ const defaultProps = { languageProvider: { start: () => Promise.resolve([]), syntax: () => {}, - getLabelKeys: () => [], + // getLabelKeys: () => [], retrieveMetrics: () => [], }, } as unknown as PrometheusDatasource, diff --git a/packages/grafana-prometheus/src/components/VariableQueryEditor.tsx b/packages/grafana-prometheus/src/components/VariableQueryEditor.tsx index f89baf6338f..c0e64fa2356 100644 --- a/packages/grafana-prometheus/src/components/VariableQueryEditor.tsx +++ b/packages/grafana-prometheus/src/components/VariableQueryEditor.tsx @@ -499,10 +499,10 @@ export const PromVariableQueryEditor = ({ onChange, query, datasource, range }: - The original implemetation of the Prometheus variable query editor. Enter a string with the correct + The original implementation of the Prometheus variable query editor. Enter a string with the correct query type and parameters as described in these docs. For example, {'{{exampleQuery}}'}. diff --git a/packages/grafana-prometheus/src/components/metrics-browser/useMetricsLabelsValues.test.ts b/packages/grafana-prometheus/src/components/metrics-browser/useMetricsLabelsValues.test.ts index c7ebd4418fb..4f4403fe26a 100644 --- a/packages/grafana-prometheus/src/components/metrics-browser/useMetricsLabelsValues.test.ts +++ b/packages/grafana-prometheus/src/components/metrics-browser/useMetricsLabelsValues.test.ts @@ -549,7 +549,6 @@ describe('useMetricsLabelsValues', () => { describe('testing with invalid values or special characters', () => { it('should handle metric names with special characters', async () => { - // Mock fetchSeriesValuesWithMatch to return metrics with special characters (mocks.mockLanguageProvider.queryLabelValues as jest.Mock).mockImplementation( (_timeRange: TimeRange, label: string) => { if (label === METRIC_LABEL) { @@ -576,7 +575,6 @@ describe('useMetricsLabelsValues', () => { }); it('should handle label values with special characters', async () => { - // Mock fetchSeriesValuesWithMatch to return label values with special characters (mocks.mockLanguageProvider.queryLabelValues as jest.Mock).mockImplementation( (_timeRange: TimeRange, label: string) => { if (label === 'job') { diff --git a/packages/grafana-prometheus/src/components/monaco-query-field/monaco-completion-provider/completions.test.ts b/packages/grafana-prometheus/src/components/monaco-query-field/monaco-completion-provider/completions.test.ts index 600777343ea..28b55e16f73 100644 --- a/packages/grafana-prometheus/src/components/monaco-query-field/monaco-completion-provider/completions.test.ts +++ b/packages/grafana-prometheus/src/components/monaco-query-field/monaco-completion-provider/completions.test.ts @@ -14,16 +14,10 @@ const dataProviderSettings = { datasource: { metricNamesAutocompleteSuggestionLimit: SUGGESTIONS_LIMIT, }, - getLabelKeys: jest.fn(), - getLabelValues: jest.fn(), queryLabelKeys: jest.fn(), queryLabelValues: jest.fn(), retrieveLabelKeys: jest.fn(), retrieveMetricsMetadata: jest.fn(), - getSeriesLabels: jest.fn(), - getSeriesValues: jest.fn(), - metrics: [], - metricsMetadata: {}, }, historyProvider: history.map((expr, idx) => ({ query: { expr, refId: 'some-ref' }, ts: idx })), } as unknown as DataProviderParams; @@ -290,10 +284,7 @@ describe('Label value completions', () => { 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: '', diff --git a/packages/grafana-prometheus/src/components/usePromQueryFieldEffects.test.ts b/packages/grafana-prometheus/src/components/usePromQueryFieldEffects.test.ts index 2141f4d51bc..d3e5017bba1 100644 --- a/packages/grafana-prometheus/src/components/usePromQueryFieldEffects.test.ts +++ b/packages/grafana-prometheus/src/components/usePromQueryFieldEffects.test.ts @@ -15,15 +15,12 @@ type TestProps = { describe('usePromQueryFieldEffects', () => { const mockLanguageProvider = { start: jest.fn().mockResolvedValue([]), - histogramMetrics: [], timeRange: {}, metrics: ['metric1'], startTask: Promise.resolve(), datasource: {}, lookupsDisabled: false, syntax: jest.fn(), - getLabelKeys: jest.fn(), - cleanText: jest.fn(), hasLookupsDisabled: jest.fn(), getBeginningCompletionItems: jest.fn(), getLabelCompletionItems: jest.fn(), @@ -31,13 +28,8 @@ describe('usePromQueryFieldEffects', () => { getTermCompletionItems: jest.fn(), request: jest.fn(), importQueries: jest.fn(), - labelKeys: [], labelFetchTs: 0, getDefaultCacheHeaders: jest.fn(), - loadMetricsMetadata: jest.fn(), - loadMetrics: jest.fn(), - loadLabelKeys: jest.fn(), - loadLabelValues: jest.fn(), modifyQuery: jest.fn(), } as unknown as PrometheusLanguageProviderInterface; diff --git a/packages/grafana-prometheus/src/locales/en-US/grafana-prometheus.json b/packages/grafana-prometheus/src/locales/en-US/grafana-prometheus.json index 37d6b802968..4eb44134b69 100644 --- a/packages/grafana-prometheus/src/locales/en-US/grafana-prometheus.json +++ b/packages/grafana-prometheus/src/locales/en-US/grafana-prometheus.json @@ -82,7 +82,7 @@ "placeholder-select-query-type": "Select query type", "placeholder-series-query": "Series Query", "returns-metrics-matching-specified-metric-regex": "Returns a list of metrics matching the specified metric regex.", - "tooltip-classic-query": "The original implemetation of the Prometheus variable query editor. Enter a string with the correct query type and parameters as described in these docs. For example, {{exampleQuery}}.", + "tooltip-classic-query": "The original implementation of the Prometheus variable query editor. Enter a string with the correct query type and parameters as described in these docs. For example, {{exampleQuery}}.", "tooltip-label": "Returns a list of label values for the label name in all metrics unless the metric is specified.", "tooltip-metric-regex": "Returns a list of label names, optionally filtering by specified metric regex.", "tooltip-query": "Returns a list of Prometheus query results for the query. This can include Prometheus functions, i.e.{{exampleQuery}}.", diff --git a/packages/grafana-prometheus/src/metric_find_query.test.ts b/packages/grafana-prometheus/src/metric_find_query.test.ts index 74adce9bfb1..af4b9ce84c5 100644 --- a/packages/grafana-prometheus/src/metric_find_query.test.ts +++ b/packages/grafana-prometheus/src/metric_find_query.test.ts @@ -214,11 +214,7 @@ describe('PrometheusMetricFindQuery', () => { const query = new PrometheusMetricFindQuery(datasource, 'label_values(metric,instance.test)'); await query.process(timeRange); - expect(datasource.languageProvider.queryLabelValues).toHaveBeenCalledWith( - timeRange, - 'U__instance_2e_test', - 'metric' - ); + expect(datasource.languageProvider.queryLabelValues).toHaveBeenCalledWith(timeRange, 'instance.test', 'metric'); }); it('should handle UTF-8 metric names in label_values query', async () => { diff --git a/packages/grafana-prometheus/src/metric_find_query.ts b/packages/grafana-prometheus/src/metric_find_query.ts index 6568548ea28..7643d56f676 100644 --- a/packages/grafana-prometheus/src/metric_find_query.ts +++ b/packages/grafana-prometheus/src/metric_find_query.ts @@ -14,7 +14,6 @@ import { } from './migrations/variableMigration'; import { getOriginalMetricName } from './result_transformer'; import { METRIC_LABEL } from './types'; -import { escapeForUtf8Support } from './utf8_support'; export class PrometheusMetricFindQuery { constructor( @@ -76,8 +75,7 @@ export class PrometheusMetricFindQuery { } async labelValuesQuery(label: string, range: TimeRange, metric?: string) { - const escapedLabel = escapeForUtf8Support(label); - const values = await this.datasource.languageProvider.queryLabelValues(range, escapedLabel, metric); + const values = await this.datasource.languageProvider.queryLabelValues(range, label, metric); return values.map((value) => ({ text: value })); } diff --git a/packages/grafana-prometheus/src/querybuilder/components/MetricsLabelsSection.test.tsx b/packages/grafana-prometheus/src/querybuilder/components/MetricsLabelsSection.test.tsx index b21dc473723..b351e4670b1 100644 --- a/packages/grafana-prometheus/src/querybuilder/components/MetricsLabelsSection.test.tsx +++ b/packages/grafana-prometheus/src/querybuilder/components/MetricsLabelsSection.test.tsx @@ -205,9 +205,9 @@ describe('MetricsLabelsSection', () => { // Call it await onGetLabelNamesCallback({}); - // Check that fetchLabels was called + // Check that queryLabelKeys was called expect(datasource.languageProvider.queryLabelKeys).toHaveBeenCalledWith(defaultTimeRange); - // Check that getLabelKeys was called + // Check that retrieveLabelKeys was called expect(datasource.languageProvider.retrieveLabelKeys).toHaveBeenCalled(); }); @@ -231,7 +231,7 @@ describe('MetricsLabelsSection', () => { // Call it await onGetLabelNamesCallback({}); - // Check that fetchLabelsWithMatch was called + // Check that queryLabelKeys was called expect(datasource.languageProvider.queryLabelKeys).toHaveBeenCalled(); }); @@ -255,7 +255,7 @@ describe('MetricsLabelsSection', () => { // Call it await getLabelValuesCallback('val', 'label1'); - // Check that fetchSeriesValuesWithMatch was called (since hasLabelsMatchAPISupport is true) + // Check that queryLabelValues was called (since hasLabelsMatchAPISupport is true) expect(datasource.languageProvider.queryLabelValues).toHaveBeenCalled(); }); @@ -280,7 +280,7 @@ describe('MetricsLabelsSection', () => { // Call it await onGetLabelValuesCallback({ label: 'label1' }); - // Check that getLabelValues was called + // Check that queryLabelValues was called expect(datasource.languageProvider.queryLabelValues).toHaveBeenCalledWith(defaultTimeRange, 'label1'); }); @@ -304,7 +304,7 @@ describe('MetricsLabelsSection', () => { // Call it await onGetLabelValuesCallback({ label: 'label1' }); - // Check that fetchSeriesValuesWithMatch was called (since hasLabelsMatchAPISupport is true) + // Check that queryLabelValues was called (since hasLabelsMatchAPISupport is true) expect(datasource.languageProvider.queryLabelValues).toHaveBeenCalled(); }); @@ -357,7 +357,6 @@ describe('MetricsLabelsSection', () => { // Check that we get back variables and metrics expect(result).toContainEqual(expect.objectContaining({ label: '$var1', value: '$var1' })); expect(result).toContainEqual(expect.objectContaining({ label: '$var2', value: '$var2' })); - // Metrics should be included too, but they come from the mocked getSeries or getLabelValues }); it('should load metrics metadata if not present', async () => { @@ -379,7 +378,7 @@ describe('MetricsLabelsSection', () => { // Call it await onGetMetricsCallback(); - // loadMetricsMetadata should be called + // queryMetricsMetadata should be called expect(datasource.languageProvider.queryMetricsMetadata).toHaveBeenCalled(); }); }); diff --git a/packages/grafana-prometheus/src/resource_clients.test.ts b/packages/grafana-prometheus/src/resource_clients.test.ts index ac1499af17f..98c4dfc4e21 100644 --- a/packages/grafana-prometheus/src/resource_clients.test.ts +++ b/packages/grafana-prometheus/src/resource_clients.test.ts @@ -491,6 +491,50 @@ describe('SeriesApiClient', () => { expect.any(Object) ); }); + + it('should be able make the right request with utf8 label keys and matchers', async () => { + mockRequest.mockResolvedValue([ + { __name__: 'metric1', job: 'grafana' }, + { __name__: 'metric2', job: 'prometheus' }, + ]); + + await client.queryLabelValues(mockTimeRange, '"label with space"', '{"label with space"="space"}'); + + expect(mockRequest).toHaveBeenCalledTimes(1); + expect(mockRequest).toHaveBeenCalledWith( + '/api/v1/series', + expect.objectContaining({ + 'match[]': '{"label with space"="space","label with space"!=""}', + }), + expect.any(Object) + ); + }); + + it('should be able to return the right utf8 label key value', async () => { + mockRequest.mockResolvedValue([ + { + __name__: 'a.utf8.metric 🤘', + a_legacy_label: 'legacy', + 'label with space': 'space', + }, + ]); + + const response = await client.queryLabelValues( + mockTimeRange, + '"label with space"', + '{a_legacy_label="legacy",__name__="a.utf8.metric 🤘"}' + ); + + expect(mockRequest).toHaveBeenCalledTimes(1); + expect(mockRequest).toHaveBeenCalledWith( + '/api/v1/series', + expect.objectContaining({ + 'match[]': '{a_legacy_label="legacy",__name__="a.utf8.metric 🤘","label with space"!=""}', + }), + expect.any(Object) + ); + expect(response).toEqual(['space']); + }); }); describe('SeriesCache', () => { diff --git a/packages/grafana-prometheus/src/resource_clients.ts b/packages/grafana-prometheus/src/resource_clients.ts index d15da2f60ab..e86edd3f6d1 100644 --- a/packages/grafana-prometheus/src/resource_clients.ts +++ b/packages/grafana-prometheus/src/resource_clients.ts @@ -204,17 +204,16 @@ export class SeriesApiClient extends BaseResourceClient implements ResourceApiCl match?: string, limit: string = DEFAULT_SERIES_LIMIT ): Promise => { - const utf8SafeLabelKey = utf8Support(labelKey); let effectiveMatch = ''; if (!match || match === EMPTY_MATCHER) { // Just and empty matcher {} or no matcher - effectiveMatch = `{${utf8SafeLabelKey}!=""}`; + effectiveMatch = `{${utf8Support(removeQuotesIfExist(labelKey))}!=""}`; } else { const { query: { metric, labels }, } = buildVisualQueryFromString(match); labels.push({ - label: utf8SafeLabelKey, + label: removeQuotesIfExist(labelKey), op: '!=', value: '', }); @@ -229,7 +228,7 @@ export class SeriesApiClient extends BaseResourceClient implements ResourceApiCl } const series = await this.querySeries(timeRange, effectiveMatch, limit); - const { labelValues } = processSeries(series, labelKey); + const { labelValues } = processSeries(series, removeQuotesIfExist(labelKey)); this._cache.setLabelValues(timeRange, effectiveMatch, limit, labelValues); return labelValues; };