From b29276dc4a33b390f7e85e3ad341368afecd2639 Mon Sep 17 00:00:00 2001 From: ismail simsek Date: Thu, 7 Nov 2024 15:34:13 +0100 Subject: [PATCH] [v10.4.x] Prometheus: Fix interpolating adhoc filters with template variables (#95989) 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) * apply same changes to core prometheus * 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 | 24 +++++++++++++++++-- .../datasource/prometheus/datasource.ts | 18 ++++++++++---- 4 files changed, 72 insertions(+), 12 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 956b4a268be..919319b73f3 100644 --- a/packages/grafana-prometheus/src/datasource.ts +++ b/packages/grafana-prometheus/src/datasource.ts @@ -722,7 +722,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, @@ -888,11 +892,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, diff --git a/public/app/plugins/datasource/prometheus/datasource.test.ts b/public/app/plugins/datasource/prometheus/datasource.test.ts index 7d7d1f613e7..cad3a39d18c 100644 --- a/public/app/plugins/datasource/prometheus/datasource.test.ts +++ b/public/app/plugins/datasource/prometheus/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/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index 5a0264a4571..afd0f242838 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -722,7 +722,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, @@ -888,11 +892,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,