[v11.0.x] Prometheus: Fix interpolating adhoc filters with template variables (#95986)

* Prometheus: Fix interpolating adhoc filters with template variables (#88626)

* Prometheus: replace variables on adhoc filters

Fixes #87979

Signed-off-by: Stéphane Cazeaux <stephane.cazeaux@orange.com>

* Prometheus: replace variable filters on adhoc variables also when promQLScope=true

Signed-off-by: Stéphane Cazeaux <stephane.cazeaux@orange.com>

---------

Signed-off-by: Stéphane Cazeaux <stephane.cazeaux@orange.com>
(cherry picked from commit 6b876f1e38)

* fix unit test

* revert

* revert some unnecessary changes

* remove unnecessary promqlscope ft checks

* linting

---------

Co-authored-by: Stéphane Cazeaux <stephane.cazeaux@casper-online.net>

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

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,

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

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,