From 2fe4ecde19e4e3548f10f256eb520d7b9f446852 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 28 Sep 2023 16:28:58 +0200 Subject: [PATCH] DataSourceAPI: Add adhoc filters to DataQueryRequest and make it not depend on global templateSrv (#75552) * DataSourceAPI: Add adhoc filters to DataQueryRequest and some methods to make it not depend on global templateSrv * Minor tweaks/fixes * Renamed to filters * Fix test * Log deprecation warning * I give up --- packages/grafana-data/src/types/datasource.ts | 5 ++- .../src/utils/DataSourceWithBackend.ts | 9 +++-- .../expressions/ExpressionDatasource.ts | 2 +- .../query/state/PanelQueryRunner.test.ts | 5 +++ .../features/query/state/PanelQueryRunner.ts | 10 +++-- .../features/templating/template_srv.mock.ts | 4 ++ .../app/features/templating/template_srv.ts | 17 ++++++++ .../datasource/prometheus/datasource.test.ts | 39 ++++++++----------- .../datasource/prometheus/datasource.tsx | 32 ++++++++++----- 9 files changed, 82 insertions(+), 41 deletions(-) diff --git a/packages/grafana-data/src/types/datasource.ts b/packages/grafana-data/src/types/datasource.ts index 06180bae65e..d6d565cbc0e 100644 --- a/packages/grafana-data/src/types/datasource.ts +++ b/packages/grafana-data/src/types/datasource.ts @@ -333,7 +333,7 @@ abstract class DataSourceApi< getVersion?(optionalOptions?: any): Promise; - interpolateVariablesInQueries?(queries: TQuery[], scopedVars: ScopedVars): TQuery[]; + interpolateVariablesInQueries?(queries: TQuery[], scopedVars: ScopedVars, filters?: AdHocVariableFilter[]): TQuery[]; /** * An annotation processor allows explicit control for how annotations are managed. @@ -552,6 +552,9 @@ export interface DataQueryRequest { panelId?: number; dashboardUID?: string; + /** Filters to dynamically apply to all queries */ + filters?: AdHocVariableFilter[]; + // Request Timing startTime: number; endTime?: number; diff --git a/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts b/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts index 3a3fa775306..a863a051d2e 100644 --- a/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts +++ b/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts @@ -16,6 +16,7 @@ import { makeClassES5Compatible, parseLiveChannelAddress, ScopedVars, + AdHocVariableFilter, } from '@grafana/data'; import { config } from '../config'; @@ -263,8 +264,8 @@ class DataSourceWithBackend< /** * Apply template variables for explore */ - interpolateVariablesInQueries(queries: TQuery[], scopedVars: ScopedVars | {}): TQuery[] { - return queries.map((q) => this.applyTemplateVariables(q, scopedVars) as TQuery); + interpolateVariablesInQueries(queries: TQuery[], scopedVars: ScopedVars, filters?: AdHocVariableFilter[]): TQuery[] { + return queries.map((q) => this.applyTemplateVariables(q, scopedVars, filters) as TQuery); } /** @@ -278,7 +279,7 @@ class DataSourceWithBackend< filterQuery?(query: TQuery): boolean; /** - * Override to apply template variables. The result is usually also `TQuery`, but sometimes this can + * Override to apply template variables and adhoc filters. The result is usually also `TQuery`, but sometimes this can * be used to modify the query structure before sending to the backend. * * NOTE: if you do modify the structure or use template variables, alerting queries may not work @@ -286,7 +287,7 @@ class DataSourceWithBackend< * * @virtual */ - applyTemplateVariables(query: TQuery, scopedVars: ScopedVars): Record { + applyTemplateVariables(query: TQuery, scopedVars: ScopedVars, filters?: AdHocVariableFilter[]): Record { return query; } diff --git a/public/app/features/expressions/ExpressionDatasource.ts b/public/app/features/expressions/ExpressionDatasource.ts index 5ad53694354..795407c0d4a 100644 --- a/public/app/features/expressions/ExpressionDatasource.ts +++ b/public/app/features/expressions/ExpressionDatasource.ts @@ -43,7 +43,7 @@ export class ExpressionDatasourceApi extends DataSourceWithBackend ({ }, })); +jest.mock('app/features/templating/template_srv', () => ({ + getTemplateSrv: () => new TemplateSrvMock({}), +})); + interface ScenarioContext { setup: (fn: () => void) => void; diff --git a/public/app/features/query/state/PanelQueryRunner.ts b/public/app/features/query/state/PanelQueryRunner.ts index 79f9a50d00a..7ca0c30791b 100644 --- a/public/app/features/query/state/PanelQueryRunner.ts +++ b/public/app/features/query/state/PanelQueryRunner.ts @@ -29,10 +29,11 @@ import { ApplyFieldOverrideOptions, StreamingDataFrame, } from '@grafana/data'; -import { getTemplateSrv, toDataQueryError } from '@grafana/runtime'; +import { toDataQueryError } from '@grafana/runtime'; import { ExpressionDatasourceRef } from '@grafana/runtime/src/utils/DataSourceWithBackend'; import { isStreamingDataFrame } from 'app/features/live/data/utils'; import { getDatasourceSrv } from 'app/features/plugins/datasource_srv'; +import { getTemplateSrv } from 'app/features/templating/template_srv'; import { isSharedDashboardQuery, runSharedRequest } from '../../../plugins/datasource/dashboard'; import { PanelModel } from '../../dashboard/state'; @@ -78,6 +79,7 @@ export class PanelQueryRunner { private lastResult?: PanelData; private dataConfigSource: DataConfigSource; private lastRequest?: DataQueryRequest; + private templateSrv = getTemplateSrv(); constructor(dataConfigSource: DataConfigSource) { this.subject = new ReplaySubject(1); @@ -215,7 +217,7 @@ export class PanelQueryRunner { } const ctx: DataTransformContext = { - interpolate: (v: string) => getTemplateSrv().replace(v, data?.request?.scopedVars), + interpolate: (v: string) => this.templateSrv.replace(v, data?.request?.scopedVars), }; return transformDataFrame(transformations, data.series, ctx).pipe( @@ -274,6 +276,7 @@ export class PanelQueryRunner { try { const ds = await getDataSource(datasource, request.scopedVars); + const isMixedDS = ds.meta?.mixed; // Attach the data source to each query @@ -287,7 +290,7 @@ export class PanelQueryRunner { return query; }); - const lowerIntervalLimit = minInterval ? getTemplateSrv().replace(minInterval, request.scopedVars) : ds.interval; + const lowerIntervalLimit = minInterval ? this.templateSrv.replace(minInterval, request.scopedVars) : ds.interval; const norm = rangeUtil.calculateInterval(timeRange, maxDataPoints, lowerIntervalLimit); // make shallow copy of scoped vars, @@ -299,6 +302,7 @@ export class PanelQueryRunner { request.interval = norm.interval; request.intervalMs = norm.intervalMs; + request.filters = this.templateSrv.getAdhocFilters(ds.name); this.lastRequest = request; diff --git a/public/app/features/templating/template_srv.mock.ts b/public/app/features/templating/template_srv.mock.ts index 70ab70fb988..53dad5ead93 100644 --- a/public/app/features/templating/template_srv.mock.ts +++ b/public/app/features/templating/template_srv.mock.ts @@ -60,4 +60,8 @@ export class TemplateSrvMock implements TemplateSrv { } updateTimeRange(timeRange: TimeRange) {} + + getAdhocFilters(dsName: string) { + return [{ key: 'key', operator: '=', value: 'a' }]; + } } diff --git a/public/app/features/templating/template_srv.ts b/public/app/features/templating/template_srv.ts index 359a7c5af43..c4ea8ba5a6a 100644 --- a/public/app/features/templating/template_srv.ts +++ b/public/app/features/templating/template_srv.ts @@ -51,6 +51,7 @@ export class TemplateSrv implements BaseTemplateSrv { private index: any = {}; private grafanaVariables = new Map(); private timeRange?: TimeRange | null = null; + private _adhocFiltersDeprecationWarningLogged = new Map(); constructor(private dependencies: TemplateSrvDependencies = runtimeDependencies) { this._variables = []; @@ -111,6 +112,11 @@ export class TemplateSrv implements BaseTemplateSrv { this.index[variable.name] = variable; } + /** + * @deprecated + * Use filters property on the request (DataQueryRequest) or if this is called from + * interpolateVariablesInQueries or applyTemplateVariables it is passed as a new argument + **/ getAdhocFilters(datasourceName: string): AdHocVariableFilter[] { let filters: any = []; let ds = getDataSourceSrv().getInstanceSettings(datasourceName); @@ -119,6 +125,17 @@ export class TemplateSrv implements BaseTemplateSrv { return []; } + if (!this._adhocFiltersDeprecationWarningLogged.get(ds.type)) { + if (process.env.NODE_ENV !== 'test') { + deprecationWarning( + `DataSource ${ds.type}`, + 'templateSrv.getAdhocFilters', + 'filters property on the request (DataQueryRequest). Or if this is called from interpolateVariablesInQueries or applyTemplateVariables it is passed as a new argument' + ); + } + this._adhocFiltersDeprecationWarningLogged.set(ds.type, true); + } + for (const variable of this.getAdHocVariables()) { const variableUid = variable.datasource?.uid; diff --git a/public/app/plugins/datasource/prometheus/datasource.test.ts b/public/app/plugins/datasource/prometheus/datasource.test.ts index b66c0e212c4..e717b5089be 100644 --- a/public/app/plugins/datasource/prometheus/datasource.test.ts +++ b/public/app/plugins/datasource/prometheus/datasource.test.ts @@ -42,11 +42,9 @@ jest.mock('@grafana/runtime', () => ({ }), })); -const getAdhocFiltersMock = jest.fn().mockImplementation(() => []); const replaceMock = jest.fn().mockImplementation((a: string, ...rest: unknown[]) => a); const templateSrvStub = { - getAdhocFilters: getAdhocFiltersMock, replace: replaceMock, } as unknown as TemplateSrv; @@ -310,17 +308,13 @@ describe('PrometheusDatasource', () => { const DEFAULT_QUERY_EXPRESSION = 'metric{job="foo"} - metric'; const target: PromQuery = { expr: DEFAULT_QUERY_EXPRESSION, refId: 'A' }; - afterAll(() => { - getAdhocFiltersMock.mockImplementation(() => []); - }); - it('should not modify expression with no filters', () => { const result = ds.createQuery(target, { interval: '15s' } as DataQueryRequest, 0, 0); expect(result).toMatchObject({ expr: DEFAULT_QUERY_EXPRESSION }); }); it('should add filters to expression', () => { - getAdhocFiltersMock.mockReturnValue([ + const filters = [ { key: 'k1', operator: '=', @@ -331,13 +325,13 @@ describe('PrometheusDatasource', () => { operator: '!=', value: 'v2', }, - ]); - const result = ds.createQuery(target, { interval: '15s' } as DataQueryRequest, 0, 0); + ]; + const result = ds.createQuery(target, { interval: '15s', filters } as DataQueryRequest, 0, 0); expect(result).toMatchObject({ expr: 'metric{job="foo", k1="v1", k2!="v2"} - metric{k1="v1", k2!="v2"}' }); }); it('should add escaping if needed to regex filter expressions', () => { - getAdhocFiltersMock.mockReturnValue([ + const filters = [ { key: 'k1', operator: '=~', @@ -348,8 +342,9 @@ describe('PrometheusDatasource', () => { operator: '=~', value: `v'.*`, }, - ]); - const result = ds.createQuery(target, { interval: '15s' } as DataQueryRequest, 0, 0); + ]; + + const result = ds.createQuery(target, { interval: '15s', filters } as DataQueryRequest, 0, 0); expect(result).toMatchObject({ expr: `metric{job="foo", k1=~"v.*", k2=~"v\\\\'.*"} - metric{k1=~"v.*", k2=~"v\\\\'.*"}`, }); @@ -358,6 +353,7 @@ describe('PrometheusDatasource', () => { describe('When converting prometheus histogram to heatmap format', () => { let query: DataQueryRequest; + beforeEach(() => { query = { range: { from: dateTime(1443454528000), to: dateTime(1443454528000) }, @@ -770,7 +766,6 @@ describe('PrometheusDatasource', () => { describe('applyTemplateVariables', () => { afterAll(() => { - getAdhocFiltersMock.mockImplementation(() => []); replaceMock.mockImplementation((a: string, ...rest: unknown[]) => a); }); @@ -814,7 +809,7 @@ describe('PrometheusDatasource', () => { it('should add ad-hoc filters to expr', () => { replaceMock.mockImplementation((a: string) => a); - getAdhocFiltersMock.mockReturnValue([ + const filters = [ { key: 'k1', operator: '=', @@ -825,20 +820,20 @@ describe('PrometheusDatasource', () => { operator: '!=', value: 'v2', }, - ]); + ]; const query = { expr: 'test{job="bar"}', refId: 'A', }; - const result = ds.applyTemplateVariables(query, {}); + const result = ds.applyTemplateVariables(query, {}, filters); expect(result).toMatchObject({ expr: 'test{job="bar", k1="v1", k2!="v2"}' }); }); it('should add ad-hoc filters only to expr', () => { replaceMock.mockImplementation((a: string) => a?.replace('$A', '99') ?? a); - getAdhocFiltersMock.mockReturnValue([ + const filters = [ { key: 'k1', operator: '=', @@ -849,21 +844,21 @@ describe('PrometheusDatasource', () => { operator: '!=', value: 'v2', }, - ]); + ]; const query = { expr: 'test{job="bar"} > $A', refId: 'A', }; - const result = ds.applyTemplateVariables(query, {}); + const result = ds.applyTemplateVariables(query, {}, filters); expect(result).toMatchObject({ expr: 'test{job="bar", k1="v1", k2!="v2"} > 99' }); }); it('should add ad-hoc filters only to expr and expression has template variable as label value??', () => { const searchPattern = /\$A/g; replaceMock.mockImplementation((a: string) => a?.replace(searchPattern, '99') ?? a); - getAdhocFiltersMock.mockReturnValue([ + const filters = [ { key: 'k1', operator: '=', @@ -874,14 +869,14 @@ describe('PrometheusDatasource', () => { operator: '!=', value: 'v2', }, - ]); + ]; const query = { expr: 'test{job="$A"} > $A', refId: 'A', }; - const result = ds.applyTemplateVariables(query, {}); + const result = ds.applyTemplateVariables(query, {}, filters); expect(result).toMatchObject({ expr: 'test{job="99", k1="v1", k2!="v2"} > 99' }); }); }); diff --git a/public/app/plugins/datasource/prometheus/datasource.tsx b/public/app/plugins/datasource/prometheus/datasource.tsx index a75bb9a7abb..bb0e2c01894 100644 --- a/public/app/plugins/datasource/prometheus/datasource.tsx +++ b/public/app/plugins/datasource/prometheus/datasource.tsx @@ -27,6 +27,7 @@ import { DataSourceGetTagKeysOptions, DataSourceGetTagValuesOptions, MetricFindValue, + AdHocVariableFilter, } from '@grafana/data'; import { BackendDataSourceResponse, @@ -680,7 +681,7 @@ export class PrometheusDatasource let expr = target.expr; // Apply adhoc filters - expr = this.enhanceExprWithAdHocFilters(expr); + expr = this.enhanceExprWithAdHocFilters(options.filters, expr); // Only replace vars in expression after having (possibly) updated interval vars query.expr = this.templateSrv.replace(expr, scopedVars, this.interpolateQueryExpr); @@ -1112,16 +1113,21 @@ export class PrometheusDatasource ); } - interpolateVariablesInQueries(queries: PromQuery[], scopedVars: ScopedVars): PromQuery[] { + interpolateVariablesInQueries( + queries: PromQuery[], + scopedVars: ScopedVars, + filters?: AdHocVariableFilter[] + ): PromQuery[] { let expandedQueries = queries; 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 expandedQuery = { ...query, datasource: this.getRef(), - expr: this.enhanceExprWithAdHocFilters( - this.templateSrv.replace(query.expr, scopedVars, this.interpolateQueryExpr) - ), + expr: withAdhocFilters, interval: this.templateSrv.replace(query.interval, scopedVars), }; return expandedQuery; @@ -1250,10 +1256,12 @@ export class PrometheusDatasource return getOriginalMetricName(labelData); } - enhanceExprWithAdHocFilters(expr: string) { - const adhocFilters = this.templateSrv.getAdhocFilters(this.name); + enhanceExprWithAdHocFilters(filters: AdHocVariableFilter[] | undefined, expr: string) { + if (!filters || filters.length === 0) { + return expr; + } - const finalQuery = adhocFilters.reduce((acc: string, filter: { key?: any; operator?: any; value?: any }) => { + const finalQuery = filters.reduce((acc: string, filter: { key?: any; operator?: any; value?: any }) => { const { key, operator } = filter; let { value } = filter; if (operator === '=~' || operator === '!~') { @@ -1273,7 +1281,11 @@ export class PrometheusDatasource } // Used when running queries through backend - applyTemplateVariables(target: PromQuery, scopedVars: ScopedVars): Record { + applyTemplateVariables( + target: PromQuery, + scopedVars: ScopedVars, + filters?: AdHocVariableFilter[] + ): Record { const variables = cloneDeep(scopedVars); // We want to interpolate these variables on backend @@ -1284,7 +1296,7 @@ export class PrometheusDatasource const expr = this.templateSrv.replace(target.expr, variables, this.interpolateQueryExpr); // Add ad hoc filters - const exprWithAdHocFilters = this.enhanceExprWithAdHocFilters(expr); + const exprWithAdHocFilters = this.enhanceExprWithAdHocFilters(filters, expr); return { ...target,