Prometheus: Don't use empty matcher if there is no match parameter (#107569)

* don't use empty matcher

* nit updates
iortega/new-ds-model-backup^2
ismail simsek 2 weeks ago committed by GitHub
parent e76f470b44
commit aa22cf9e1f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 5
      packages/grafana-prometheus/src/constants.ts
  2. 6
      packages/grafana-prometheus/src/datasource.test.ts
  3. 12
      packages/grafana-prometheus/src/datasource.ts
  4. 12
      packages/grafana-prometheus/src/language_provider.test.ts
  5. 10
      packages/grafana-prometheus/src/language_provider.ts
  6. 20
      packages/grafana-prometheus/src/resource_clients.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__';

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

@ -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(',')}}`;
};

@ -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"']);
});
});
});

@ -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<string | Prism.Token
* Handles UTF8 metrics by properly escaping them.
*
* @param {PromQuery[]} queries - Array of Prometheus queries
* @returns {string} Metric names as a regex matcher
* @returns {string[]} Metric names as a regex matcher inside the array for easy handling
*/
export const populateMatchParamsFromQueries = (queries?: PromQuery[]): string => {
export const populateMatchParamsFromQueries = (queries?: PromQuery[]): string[] => {
if (!queries) {
return MATCH_ALL_LABELS_STR;
return [];
}
const metrics = (queries ?? []).reduce<string[]>((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('|')}"`];
};

@ -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<string[]> => {
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('|');
}

Loading…
Cancel
Save