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

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;

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,

packages/grafana-prometheus/src/components/VariableQueryEditor.tsx

@@ -499,10 +499,10 @@ export const PromVariableQueryEditor = ({ onChange, query, datasource, range }:
                 <Trans
                   i18nKey="grafana-prometheus.components.prom-variable-query-editor.tooltip-classic-query"
                   values={{
-                    exampleQuery: 'label_values(label, metric)',
+                    exampleQuery: 'label_values(metric, label)',
                   }}
                 >
-                  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}}'}.
                 </Trans>
               </div>

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') {

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: '',

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;
 

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}}.",

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 () => {

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 }));
   }
 

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();
   });
 });

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', () => {

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<string[]> => {
-    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;
   };