From 6a3b84ebd280988c59ebcf2c2f9f5549f679bf9c Mon Sep 17 00:00:00 2001 From: ismail simsek Date: Thu, 7 Nov 2024 08:39:29 +0100 Subject: [PATCH] [v11.0.x] Prometheus: Fix interpolating adhoc filters with template variables (#95986) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Prometheus: Fix interpolating adhoc filters with template variables (#88626) * Prometheus: replace variables on adhoc filters Fixes #87979 Signed-off-by: Stéphane Cazeaux * Prometheus: replace variable filters on adhoc variables also when promQLScope=true Signed-off-by: Stéphane Cazeaux --------- Signed-off-by: Stéphane Cazeaux (cherry picked from commit 6b876f1e3845e0b5ec89dbf451e81d8fdec461bb) * fix unit test * revert * revert some unnecessary changes * remove unnecessary promqlscope ft checks * linting --------- Co-authored-by: Stéphane Cazeaux --- .../grafana-prometheus/src/datasource.test.ts | 24 +++++++++++++++-- packages/grafana-prometheus/src/datasource.ts | 18 ++++++++++--- .../datasource/prometheus/datasource.test.ts | 27 ++++++++++++++++--- .../datasource/prometheus/datasource.ts | 18 ++++++++++--- 4 files changed, 74 insertions(+), 13 deletions(-) diff --git a/packages/grafana-prometheus/src/datasource.test.ts b/packages/grafana-prometheus/src/datasource.test.ts index 72f9c4572d8..b57d973bf21 100644 --- a/packages/grafana-prometheus/src/datasource.test.ts +++ b/packages/grafana-prometheus/src/datasource.test.ts @@ -539,7 +539,7 @@ describe('PrometheusDatasource', () => { }); describe('interpolateVariablesInQueries', () => { - it('should call replace function 2 times', () => { + it('should call replace function 3 times', () => { const query: PromQuery = { expr: 'test{job="testjob"}', format: 'time_series', @@ -550,7 +550,7 @@ describe('PrometheusDatasource', () => { replaceMock.mockReturnValue(interval); const queries = ds.interpolateVariablesInQueries([query], { Interval: { text: interval, value: interval } }); - expect(templateSrvStub.replace).toBeCalledTimes(2); + expect(templateSrvStub.replace).toBeCalledTimes(3); expect(queries[0].interval).toBe(interval); }); @@ -682,6 +682,26 @@ describe('PrometheusDatasource', () => { const result = ds.applyTemplateVariables(query, {}, filters); expect(result).toMatchObject({ expr: 'test{job="99", k1="v1", k2!="v2"} > 99' }); }); + + it('should replace variables in ad-hoc filters', () => { + const searchPattern = /\$A/g; + replaceMock.mockImplementation((a: string) => a?.replace(searchPattern, '99') ?? a); + + const query = { + expr: 'test', + refId: 'A', + }; + const filters = [ + { + key: 'job', + operator: '=~', + value: '$A', + }, + ]; + + const result = ds.applyTemplateVariables(query, {}, filters); + expect(result).toMatchObject({ expr: 'test{job=~"99"}' }); + }); }); describe('metricFindQuery', () => { diff --git a/packages/grafana-prometheus/src/datasource.ts b/packages/grafana-prometheus/src/datasource.ts index 84a728676a8..120703f1d1a 100644 --- a/packages/grafana-prometheus/src/datasource.ts +++ b/packages/grafana-prometheus/src/datasource.ts @@ -726,7 +726,11 @@ export class PrometheusDatasource if (queries && queries.length) { expandedQueries = queries.map((query) => { const interpolatedQuery = this.templateSrv.replace(query.expr, scopedVars, this.interpolateQueryExpr); - const withAdhocFilters = this.enhanceExprWithAdHocFilters(filters, interpolatedQuery); + const withAdhocFilters = this.templateSrv.replace( + this.enhanceExprWithAdHocFilters(filters, interpolatedQuery), + scopedVars, + this.interpolateQueryExpr + ); const expandedQuery = { ...query, @@ -893,10 +897,18 @@ export class PrometheusDatasource }; // interpolate expression + + // We need a first replace to evaluate variables before applying adhoc filters + // This is required for an expression like `metric > $VAR` where $VAR is a float to which we must not add adhoc filters const expr = this.templateSrv.replace(target.expr, variables, this.interpolateQueryExpr); - // Add ad hoc filters - const exprWithAdHocFilters = this.enhanceExprWithAdHocFilters(filters, expr); + // Apply ad-hoc filters + // When ad-hoc filters are applied, we replace again the variables in case the ad-hoc filters also reference a variable + const exprWithAdHocFilters = this.templateSrv.replace( + this.enhanceExprWithAdHocFilters(filters, expr), + variables, + this.interpolateQueryExpr + ); return { ...target, diff --git a/public/app/plugins/datasource/prometheus/datasource.test.ts b/public/app/plugins/datasource/prometheus/datasource.test.ts index c740af7f85d..dd3e896d997 100644 --- a/public/app/plugins/datasource/prometheus/datasource.test.ts +++ b/public/app/plugins/datasource/prometheus/datasource.test.ts @@ -29,7 +29,6 @@ import { PromApplication, PrometheusCacheLevel, PromOptions, PromQuery, PromQuer const fetchMock = jest.fn().mockReturnValue(of(createDefaultPromResponse())); jest.mock('./metric_find_query'); - jest.mock('@grafana/runtime', () => ({ ...jest.requireActual('@grafana/runtime'), getBackendSrv: () => ({ @@ -75,9 +74,9 @@ describe('PrometheusDatasource', () => { url: 'proxied', id: 1, uid: 'ABCDEF', + access: 'proxy', user: 'test', password: 'mupp', - access: 'proxy', jsonData: { customQueryParameters: '', cacheLevel: PrometheusCacheLevel.Low, @@ -548,7 +547,7 @@ describe('PrometheusDatasource', () => { }); describe('interpolateVariablesInQueries', () => { - it('should call replace function 2 times', () => { + it('should call replace function 3 times', () => { const query: PromQuery = { expr: 'test{job="testjob"}', format: 'time_series', @@ -559,7 +558,7 @@ describe('PrometheusDatasource', () => { replaceMock.mockReturnValue(interval); const queries = ds.interpolateVariablesInQueries([query], { Interval: { text: interval, value: interval } }); - expect(templateSrvStub.replace).toBeCalledTimes(2); + expect(templateSrvStub.replace).toBeCalledTimes(3); expect(queries[0].interval).toBe(interval); }); @@ -691,6 +690,26 @@ describe('PrometheusDatasource', () => { const result = ds.applyTemplateVariables(query, {}, filters); expect(result).toMatchObject({ expr: 'test{job="99", k1="v1", k2!="v2"} > 99' }); }); + + it('should replace variables in ad-hoc filters', () => { + const searchPattern = /\$A/g; + replaceMock.mockImplementation((a: string) => a?.replace(searchPattern, '99') ?? a); + + const query = { + expr: 'test', + refId: 'A', + }; + const filters = [ + { + key: 'job', + operator: '=~', + value: '$A', + }, + ]; + + const result = ds.applyTemplateVariables(query, {}, filters); + expect(result).toMatchObject({ expr: 'test{job=~"99"}' }); + }); }); describe('metricFindQuery', () => { diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index 70a40c154a1..11582ff289f 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -728,7 +728,11 @@ export class PrometheusDatasource if (queries && queries.length) { expandedQueries = queries.map((query) => { const interpolatedQuery = this.templateSrv.replace(query.expr, scopedVars, this.interpolateQueryExpr); - const withAdhocFilters = this.enhanceExprWithAdHocFilters(filters, interpolatedQuery); + const withAdhocFilters = this.templateSrv.replace( + this.enhanceExprWithAdHocFilters(filters, interpolatedQuery), + scopedVars, + this.interpolateQueryExpr + ); const expandedQuery = { ...query, @@ -894,11 +898,17 @@ export class PrometheusDatasource value: '$__interval_ms', }; - // interpolate expression + // We need a first replace to evaluate variables before applying adhoc filters + // This is required for an expression like `metric > $VAR` where $VAR is a float to which we must not add adhoc filters const expr = this.templateSrv.replace(target.expr, variables, this.interpolateQueryExpr); - // Add ad hoc filters - const exprWithAdHocFilters = this.enhanceExprWithAdHocFilters(filters, expr); + // Apply ad-hoc filters + // When ad-hoc filters are applied, we replace again the variables in case the ad-hoc filters also reference a variable + const exprWithAdHocFilters = this.templateSrv.replace( + this.enhanceExprWithAdHocFilters(filters, expr), + variables, + this.interpolateQueryExpr + ); return { ...target,