From aa22cf9e1f4df4ab83ba9f54824b1d7392812d61 Mon Sep 17 00:00:00 2001 From: ismail simsek Date: Thu, 3 Jul 2025 16:16:23 +0200 Subject: [PATCH] Prometheus: Don't use empty matcher if there is no match parameter (#107569) * don't use empty matcher * nit updates --- packages/grafana-prometheus/src/constants.ts | 5 +++-- .../grafana-prometheus/src/datasource.test.ts | 6 +++--- packages/grafana-prometheus/src/datasource.ts | 12 +++++++++-- .../src/language_provider.test.ts | 12 +++++------ .../src/language_provider.ts | 10 +++++----- .../src/resource_clients.ts | 20 +++++++++---------- 6 files changed, 37 insertions(+), 28 deletions(-) diff --git a/packages/grafana-prometheus/src/constants.ts b/packages/grafana-prometheus/src/constants.ts index f54ed124c20..2c8dffa179d 100644 --- a/packages/grafana-prometheus/src/constants.ts +++ b/packages/grafana-prometheus/src/constants.ts @@ -19,8 +19,9 @@ export const EMPTY_SELECTOR = '{}'; export const DEFAULT_SERIES_LIMIT = 40000; -export const MATCH_ALL_LABELS_STR = '__name__!=""'; - +/** + * Only for /series endpoint. Don't use this anywhere else as it cause an expensive query + */ export const MATCH_ALL_LABELS = '{__name__!=""}'; export const METRIC_LABEL = '__name__'; diff --git a/packages/grafana-prometheus/src/datasource.test.ts b/packages/grafana-prometheus/src/datasource.test.ts index 5b18f5e2e97..c5fc15d6a8f 100644 --- a/packages/grafana-prometheus/src/datasource.test.ts +++ b/packages/grafana-prometheus/src/datasource.test.ts @@ -1016,7 +1016,7 @@ describe('PrometheusDatasource', () => { ]; const result = extractResourceMatcher(queries, filters); - expect(result).toBe('{__name__!="",instance="localhost"}'); + expect(result).toBe('{instance="localhost"}'); }); it('should extract matcher from given filters only', () => { @@ -1035,7 +1035,7 @@ describe('PrometheusDatasource', () => { ]; const result = extractResourceMatcher(queries, filters); - expect(result).toBe('{__name__!="",instance="localhost",job!="testjob"}'); + expect(result).toBe('{instance="localhost",job!="testjob"}'); }); it('should extract matcher as match-all from no query and filter', () => { @@ -1043,7 +1043,7 @@ describe('PrometheusDatasource', () => { const filters: AdHocVariableFilter[] = []; const result = extractResourceMatcher(queries, filters); - expect(result).toBe('{__name__!=""}'); + expect(result).toBeUndefined(); }); it('should extract the correct matcher for queries with `... or vector(0)`', () => { diff --git a/packages/grafana-prometheus/src/datasource.ts b/packages/grafana-prometheus/src/datasource.ts index 204b452efd6..1cc48500a80 100644 --- a/packages/grafana-prometheus/src/datasource.ts +++ b/packages/grafana-prometheus/src/datasource.ts @@ -913,7 +913,10 @@ export function extractRuleMappingFromGroups(groups: RawRecordingRules[]): RuleQ * adhocFilters={key:"instance", operator:"=", value:"localhost"} * returns {__name__=~"metricName", instance="localhost"} */ -export const extractResourceMatcher = (queries: PromQuery[], adhocFilters: AdHocVariableFilter[]): string => { +export const extractResourceMatcher = ( + queries: PromQuery[], + adhocFilters: AdHocVariableFilter[] +): string | undefined => { // Extract metric names from queries we have already const metricMatch = populateMatchParamsFromQueries(queries); const labelFilters: QueryBuilderLabelFilter[] = adhocFilters.map((f) => ({ @@ -923,6 +926,11 @@ export const extractResourceMatcher = (queries: PromQuery[], adhocFilters: AdHoc })); // Extract label filters from the filters we have already const labelsMatch = renderLabelsWithoutBrackets(labelFilters); + + if (metricMatch.length === 0 && labelsMatch.length === 0) { + return undefined; + } + // Create a matcher using metric names and label filters - return `{${[metricMatch, ...labelsMatch].join(',')}}`; + return `{${[...metricMatch, ...labelsMatch].join(',')}}`; }; diff --git a/packages/grafana-prometheus/src/language_provider.test.ts b/packages/grafana-prometheus/src/language_provider.test.ts index 5d7396cda95..874c07bc151 100644 --- a/packages/grafana-prometheus/src/language_provider.test.ts +++ b/packages/grafana-prometheus/src/language_provider.test.ts @@ -1042,18 +1042,18 @@ describe('PrometheusLanguageProvider with feature toggle', () => { { expr: 'metric2', refId: '2' }, ]; const result = populateMatchParamsFromQueries(queries); - expect(result).toBe(`__name__=~"metric1|metric2"`); + expect(result).toEqual([`__name__=~"metric1|metric2"`]); }); it('should handle binary queries', () => { const queries: PromQuery[] = [{ expr: 'binary{label="val"} + second{}', refId: '1' }]; const result = populateMatchParamsFromQueries(queries); - expect(result).toBe(`__name__=~"binary|second"`); + expect(result).toEqual([`__name__=~"binary|second"`]); }); it('should handle undefined queries', () => { const result = populateMatchParamsFromQueries(undefined); - expect(result).toBe('__name__!=""'); + expect(result).toEqual([]); }); it('should handle UTF8 metrics', () => { @@ -1071,13 +1071,13 @@ describe('PrometheusLanguageProvider with feature toggle', () => { it('should return match-all matcher if there is no expr in queries', () => { const queries: PromQuery[] = [{ expr: '', refId: '1' }]; const result = populateMatchParamsFromQueries(queries); - expect(result).toBe('__name__!=""'); + expect(result).toEqual([]); }); it('should return match-all matcher if there is no query', () => { const queries: PromQuery[] = []; const result = populateMatchParamsFromQueries(queries); - expect(result).toBe('__name__!=""'); + expect(result).toEqual([]); }); it('should extract the correct matcher for queries with `... or vector(0)`', () => { @@ -1088,7 +1088,7 @@ describe('PrometheusLanguageProvider with feature toggle', () => { }, ]; const result = populateMatchParamsFromQueries(queries); - expect(result).toBe('__name__=~"go_cpu_classes_idle_cpu_seconds_total"'); + expect(result).toEqual(['__name__=~"go_cpu_classes_idle_cpu_seconds_total"']); }); }); }); diff --git a/packages/grafana-prometheus/src/language_provider.ts b/packages/grafana-prometheus/src/language_provider.ts index a61be690cb8..0b1495aa288 100644 --- a/packages/grafana-prometheus/src/language_provider.ts +++ b/packages/grafana-prometheus/src/language_provider.ts @@ -18,7 +18,7 @@ import { BackendSrvRequest } from '@grafana/runtime'; import { buildCacheHeaders, getDaysToCacheMetadata, getDefaultCacheHeaders } from './caching'; import { Label } from './components/monaco-query-field/monaco-completion-provider/situation'; -import { DEFAULT_SERIES_LIMIT, MATCH_ALL_LABELS_STR, EMPTY_SELECTOR, REMOVE_SERIES_LIMIT } from './constants'; +import { DEFAULT_SERIES_LIMIT, EMPTY_SELECTOR, REMOVE_SERIES_LIMIT } from './constants'; import { PrometheusDatasource } from './datasource'; import { extractLabelMatchers, @@ -796,11 +796,11 @@ function getNameLabelValue(promQuery: string, tokens: Array { +export const populateMatchParamsFromQueries = (queries?: PromQuery[]): string[] => { if (!queries) { - return MATCH_ALL_LABELS_STR; + return []; } const metrics = (queries ?? []).reduce((params, query) => { @@ -818,5 +818,5 @@ export const populateMatchParamsFromQueries = (queries?: PromQuery[]): string => return params; }, []); - return metrics.length === 0 ? MATCH_ALL_LABELS_STR : `__name__=~"${metrics.join('|')}"`; + return metrics.length === 0 ? [] : [`__name__=~"${metrics.join('|')}"`]; }; diff --git a/packages/grafana-prometheus/src/resource_clients.ts b/packages/grafana-prometheus/src/resource_clients.ts index b5f01e93aa4..04ab0559274 100644 --- a/packages/grafana-prometheus/src/resource_clients.ts +++ b/packages/grafana-prometheus/src/resource_clients.ts @@ -73,7 +73,7 @@ export abstract class BaseResourceClient { * @param {string} match - Label matcher to filter time series * @param {string} limit - Maximum number of series to return */ - public querySeries = async (timeRange: TimeRange, match: string, limit: number) => { + public querySeries = async (timeRange: TimeRange, match: string | undefined, limit: number) => { const effectiveMatch = !match || match === EMPTY_SELECTOR ? MATCH_ALL_LABELS : match; const timeParams = this.datasource.getTimeRangeParams(timeRange); const searchParams = { ...timeParams, 'match[]': effectiveMatch, limit }; @@ -97,7 +97,7 @@ export class LabelsApiClient extends BaseResourceClient implements ResourceApiCl public queryMetrics = async (timeRange: TimeRange): Promise<{ metrics: string[]; histogramMetrics: string[] }> => { this.metrics = await this.queryLabelValues(timeRange, METRIC_LABEL); this.histogramMetrics = processHistogramMetrics(this.metrics); - this._cache.setLabelValues(timeRange, MATCH_ALL_LABELS, DEFAULT_SERIES_LIMIT, this.metrics); + this._cache.setLabelValues(timeRange, undefined, DEFAULT_SERIES_LIMIT, this.metrics); return { metrics: this.metrics, histogramMetrics: this.histogramMetrics }; }; @@ -177,19 +177,19 @@ export class SeriesApiClient extends BaseResourceClient implements ResourceApiCl }; public queryMetrics = async (timeRange: TimeRange): Promise<{ metrics: string[]; histogramMetrics: string[] }> => { - const series = await this.querySeries(timeRange, MATCH_ALL_LABELS, DEFAULT_SERIES_LIMIT); + const series = await this.querySeries(timeRange, undefined, DEFAULT_SERIES_LIMIT); const { metrics, labelKeys } = processSeries(series, METRIC_LABEL); this.metrics = metrics; this.histogramMetrics = processHistogramMetrics(this.metrics); this.labelKeys = labelKeys; - this._cache.setLabelValues(timeRange, MATCH_ALL_LABELS, DEFAULT_SERIES_LIMIT, metrics); - this._cache.setLabelKeys(timeRange, MATCH_ALL_LABELS, DEFAULT_SERIES_LIMIT, labelKeys); + this._cache.setLabelValues(timeRange, undefined, DEFAULT_SERIES_LIMIT, metrics); + this._cache.setLabelKeys(timeRange, undefined, DEFAULT_SERIES_LIMIT, labelKeys); return { metrics: this.metrics, histogramMetrics: this.histogramMetrics }; }; public queryLabelKeys = async (timeRange: TimeRange, match?: string, limit?: number): Promise => { const effectiveLimit = this.getEffectiveLimit(limit); - const effectiveMatch = !match || match === EMPTY_SELECTOR ? MATCH_ALL_LABELS : match; + const effectiveMatch = !match || match === EMPTY_SELECTOR ? undefined : match; const maybeCachedKeys = this._cache.getLabelKeys(timeRange, effectiveMatch, effectiveLimit); if (maybeCachedKeys) { return maybeCachedKeys; @@ -247,7 +247,7 @@ class ResourceClientsCache { constructor(private cacheLevel: PrometheusCacheLevel = PrometheusCacheLevel.High) {} - public setLabelKeys(timeRange: TimeRange, match: string, limit: number, keys: string[]) { + public setLabelKeys(timeRange: TimeRange, match: string | undefined, limit: number, keys: string[]) { if (keys.length === 0) { return; } @@ -258,7 +258,7 @@ class ResourceClientsCache { this._accessTimestamps[cacheKey] = Date.now(); } - public getLabelKeys(timeRange: TimeRange, match: string, limit: number): string[] | undefined { + public getLabelKeys(timeRange: TimeRange, match: string | undefined, limit: number): string[] | undefined { const cacheKey = this.getCacheKey(timeRange, match, limit, 'key'); const result = this._cache[cacheKey]; if (result) { @@ -268,7 +268,7 @@ class ResourceClientsCache { return result; } - public setLabelValues(timeRange: TimeRange, match: string, limit: number, values: string[]) { + public setLabelValues(timeRange: TimeRange, match: string | undefined, limit: number, values: string[]) { if (values.length === 0) { return; } @@ -289,7 +289,7 @@ class ResourceClientsCache { return result; } - private getCacheKey(timeRange: TimeRange, match: string, limit: number, type: 'key' | 'value') { + private getCacheKey(timeRange: TimeRange, match: string | undefined, limit: number, type: 'key' | 'value') { const snappedTimeRange = getRangeSnapInterval(this.cacheLevel, timeRange); return [snappedTimeRange.start, snappedTimeRange.end, limit, match, type].join('|'); }