From 8ff152f98f0092f2a5c55da7d0b2be40dccdce75 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Mon, 18 Jul 2022 14:13:34 +0200 Subject: [PATCH] ModifyQuery: Improve typing for modifyQuery and query hints (#52326) * ModifyQuery: Improve typing * Elasticsearch: Added `modifyQuery` method to add filters in Explore (#52313) * fixed elasticsearch `QueryFixAction` type Co-authored-by: Sven Grossmann --- .betterer.results | 6 ++---- packages/grafana-data/src/types/datasource.ts | 1 + public/app/features/explore/Explore.tsx | 4 ++-- .../elasticsearch/datasource.test.ts | 16 +++++++++------ .../datasource/elasticsearch/datasource.ts | 11 +++++++--- .../datasource/loki/datasource.test.ts | 20 +++++++++---------- .../app/plugins/datasource/loki/datasource.ts | 11 +++++++--- .../prometheus/components/PromQueryField.tsx | 5 +++-- .../datasource/prometheus/datasource.test.ts | 8 ++++---- .../datasource/prometheus/datasource.tsx | 18 ++++++++++++----- .../datasource/prometheus/query_hints.ts | 2 +- .../querybuilder/shared/QueryBuilderHints.tsx | 10 ++++++---- 12 files changed, 68 insertions(+), 44 deletions(-) diff --git a/.betterer.results b/.betterer.results index 0ae53be6813..ceabba5c265 100644 --- a/.betterer.results +++ b/.betterer.results @@ -7685,8 +7685,7 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "3"], [0, 0, 0, "Unexpected any. Specify a different type.", "4"], [0, 0, 0, "Unexpected any. Specify a different type.", "5"], - [0, 0, 0, "Unexpected any. Specify a different type.", "6"], - [0, 0, 0, "Unexpected any. Specify a different type.", "7"] + [0, 0, 0, "Unexpected any. Specify a different type.", "6"] ], "public/app/plugins/datasource/loki/getDerivedFields.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], @@ -8246,8 +8245,7 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "32"], [0, 0, 0, "Unexpected any. Specify a different type.", "33"], [0, 0, 0, "Unexpected any. Specify a different type.", "34"], - [0, 0, 0, "Unexpected any. Specify a different type.", "35"], - [0, 0, 0, "Unexpected any. Specify a different type.", "36"] + [0, 0, 0, "Unexpected any. Specify a different type.", "35"] ], "public/app/plugins/datasource/prometheus/language_provider.test.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/packages/grafana-data/src/types/datasource.ts b/packages/grafana-data/src/types/datasource.ts index 1be984a4dbd..83bbe98502e 100644 --- a/packages/grafana-data/src/types/datasource.ts +++ b/packages/grafana-data/src/types/datasource.ts @@ -506,6 +506,7 @@ export interface QueryFixAction { type: string; query?: string; preventSubmit?: boolean; + options?: KeyValue; } export interface QueryHint { diff --git a/public/app/features/explore/Explore.tsx b/public/app/features/explore/Explore.tsx index 4d0a513972b..55233403242 100644 --- a/public/app/features/explore/Explore.tsx +++ b/public/app/features/explore/Explore.tsx @@ -150,11 +150,11 @@ export class Explore extends React.PureComponent { }; onClickFilterLabel = (key: string, value: string) => { - this.onModifyQueries({ type: 'ADD_FILTER', key, value }); + this.onModifyQueries({ type: 'ADD_FILTER', options: { key, value } }); }; onClickFilterOutLabel = (key: string, value: string) => { - this.onModifyQueries({ type: 'ADD_FILTER_OUT', key, value }); + this.onModifyQueries({ type: 'ADD_FILTER_OUT', options: { key, value } }); }; onClickAddQueryRowButton = () => { diff --git a/public/app/plugins/datasource/elasticsearch/datasource.test.ts b/public/app/plugins/datasource/elasticsearch/datasource.test.ts index 78f622211cc..bdb6dd52cff 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.test.ts +++ b/public/app/plugins/datasource/elasticsearch/datasource.test.ts @@ -1033,15 +1033,19 @@ describe('modifyQuery', () => { }); it('should add the filter', () => { - expect(ds.modifyQuery(query, { type: 'ADD_FILTER', key: 'foo', value: 'bar' }).query).toBe('foo:"bar"'); + expect(ds.modifyQuery(query, { type: 'ADD_FILTER', options: { key: 'foo', value: 'bar' } }).query).toBe( + 'foo:"bar"' + ); }); it('should add the negative filter', () => { - expect(ds.modifyQuery(query, { type: 'ADD_FILTER_OUT', key: 'foo', value: 'bar' }).query).toBe('-foo:"bar"'); + expect(ds.modifyQuery(query, { type: 'ADD_FILTER_OUT', options: { key: 'foo', value: 'bar' } }).query).toBe( + '-foo:"bar"' + ); }); it('should do nothing on unknown type', () => { - expect(ds.modifyQuery(query, { type: 'unknown', key: 'foo', value: 'bar' }).query).toBe(query.query); + expect(ds.modifyQuery(query, { type: 'unknown', options: { key: 'foo', value: 'bar' } }).query).toBe(query.query); }); }); @@ -1052,19 +1056,19 @@ describe('modifyQuery', () => { }); it('should add the filter', () => { - expect(ds.modifyQuery(query, { type: 'ADD_FILTER', key: 'foo', value: 'bar' }).query).toBe( + expect(ds.modifyQuery(query, { type: 'ADD_FILTER', options: { key: 'foo', value: 'bar' } }).query).toBe( 'test:"value" AND foo:"bar"' ); }); it('should add the negative filter', () => { - expect(ds.modifyQuery(query, { type: 'ADD_FILTER_OUT', key: 'foo', value: 'bar' }).query).toBe( + expect(ds.modifyQuery(query, { type: 'ADD_FILTER_OUT', options: { key: 'foo', value: 'bar' } }).query).toBe( 'test:"value" AND -foo:"bar"' ); }); it('should do nothing on unknown type', () => { - expect(ds.modifyQuery(query, { type: 'unknown', key: 'foo', value: 'bar' }).query).toBe(query.query); + expect(ds.modifyQuery(query, { type: 'unknown', options: { key: 'foo', value: 'bar' } }).query).toBe(query.query); }); }); }); diff --git a/public/app/plugins/datasource/elasticsearch/datasource.ts b/public/app/plugins/datasource/elasticsearch/datasource.ts index 4aa1df7f555..84a55da6fc8 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.ts +++ b/public/app/plugins/datasource/elasticsearch/datasource.ts @@ -25,6 +25,7 @@ import { ScopedVars, TimeRange, toUtc, + QueryFixAction, } from '@grafana/data'; import { BackendSrvRequest, getBackendSrv, getDataSourceSrv } from '@grafana/runtime'; import { RowContextOptions } from '@grafana/ui/src/components/Logs/LogRowContextProvider'; @@ -953,21 +954,25 @@ export class ElasticDatasource return false; } - modifyQuery(query: ElasticsearchQuery, action: { type: string; key: string; value: string }): ElasticsearchQuery { + modifyQuery(query: ElasticsearchQuery, action: QueryFixAction): ElasticsearchQuery { + if (!action.options) { + return query; + } + let expression = query.query ?? ''; switch (action.type) { case 'ADD_FILTER': { if (expression.length > 0) { expression += ' AND '; } - expression += `${action.key}:"${action.value}"`; + expression += `${action.options.key}:"${action.options.value}"`; break; } case 'ADD_FILTER_OUT': { if (expression.length > 0) { expression += ' AND '; } - expression += `-${action.key}:"${action.value}"`; + expression += `-${action.options.key}:"${action.options.value}"`; break; } } diff --git a/public/app/plugins/datasource/loki/datasource.test.ts b/public/app/plugins/datasource/loki/datasource.test.ts index bf39da555b0..e34a051a02c 100644 --- a/public/app/plugins/datasource/loki/datasource.test.ts +++ b/public/app/plugins/datasource/loki/datasource.test.ts @@ -522,7 +522,7 @@ describe('LokiDatasource', () => { describe('and query has no parser', () => { it('then the correct label should be added for logs query', () => { const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; - const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER' }; + const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' }; const ds = createLokiDSForTests(); const result = ds.modifyQuery(query, action); @@ -532,7 +532,7 @@ describe('LokiDatasource', () => { it('then the correctly escaped label should be added for logs query', () => { const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; - const action = { key: 'job', value: '\\test', type: 'ADD_FILTER' }; + const action = { options: { key: 'job', value: '\\test' }, type: 'ADD_FILTER' }; const ds = createLokiDSForTests(); const result = ds.modifyQuery(query, action); @@ -542,7 +542,7 @@ describe('LokiDatasource', () => { it('then the correct label should be added for metrics query', () => { const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"}[5m])' }; - const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER' }; + const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' }; const ds = createLokiDSForTests(); const result = ds.modifyQuery(query, action); @@ -552,7 +552,7 @@ describe('LokiDatasource', () => { describe('and query has parser', () => { it('then the correct label should be added for logs query', () => { const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' }; - const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER' }; + const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' }; const ds = createLokiDSForTests(); const result = ds.modifyQuery(query, action); @@ -561,7 +561,7 @@ describe('LokiDatasource', () => { }); it('then the correct label should be added for metrics query', () => { const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' }; - const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER' }; + const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' }; const ds = createLokiDSForTests(); const result = ds.modifyQuery(query, action); @@ -576,7 +576,7 @@ describe('LokiDatasource', () => { describe('and query has no parser', () => { it('then the correct label should be added for logs query', () => { const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; - const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER_OUT' }; + const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' }; const ds = createLokiDSForTests(); const result = ds.modifyQuery(query, action); @@ -586,7 +586,7 @@ describe('LokiDatasource', () => { it('then the correctly escaped label should be added for logs query', () => { const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; - const action = { key: 'job', value: '"test', type: 'ADD_FILTER_OUT' }; + const action = { options: { key: 'job', value: '"test' }, type: 'ADD_FILTER_OUT' }; const ds = createLokiDSForTests(); const result = ds.modifyQuery(query, action); @@ -596,7 +596,7 @@ describe('LokiDatasource', () => { it('then the correct label should be added for metrics query', () => { const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"}[5m])' }; - const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER_OUT' }; + const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' }; const ds = createLokiDSForTests(); const result = ds.modifyQuery(query, action); @@ -606,7 +606,7 @@ describe('LokiDatasource', () => { describe('and query has parser', () => { it('then the correct label should be added for logs query', () => { const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' }; - const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER_OUT' }; + const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' }; const ds = createLokiDSForTests(); const result = ds.modifyQuery(query, action); @@ -615,7 +615,7 @@ describe('LokiDatasource', () => { }); it('then the correct label should be added for metrics query', () => { const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' }; - const action = { key: 'job', value: 'grafana', type: 'ADD_FILTER_OUT' }; + const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' }; const ds = createLokiDSForTests(); const result = ds.modifyQuery(query, action); diff --git a/public/app/plugins/datasource/loki/datasource.ts b/public/app/plugins/datasource/loki/datasource.ts index d1b18db48d1..e91e0500297 100644 --- a/public/app/plugins/datasource/loki/datasource.ts +++ b/public/app/plugins/datasource/loki/datasource.ts @@ -32,6 +32,7 @@ import { toUtc, QueryHint, getDefaultTimeRange, + QueryFixAction, } from '@grafana/data'; import { FetchError, config, DataSourceWithBackend } from '@grafana/runtime'; import { RowContextOptions } from '@grafana/ui/src/components/Logs/LogRowContextProvider'; @@ -389,15 +390,19 @@ export class LokiDatasource return escapedValues.join('|'); } - modifyQuery(query: LokiQuery, action: any): LokiQuery { + modifyQuery(query: LokiQuery, action: QueryFixAction): LokiQuery { let expression = query.expr ?? ''; switch (action.type) { case 'ADD_FILTER': { - expression = this.addLabelToQuery(expression, action.key, '=', action.value); + if (action.options?.key && action.options?.value) { + expression = this.addLabelToQuery(expression, action.options.key, '=', action.options.value); + } break; } case 'ADD_FILTER_OUT': { - expression = this.addLabelToQuery(expression, action.key, '!=', action.value); + if (action.options?.key && action.options?.value) { + expression = this.addLabelToQuery(expression, action.options.key, '!=', action.options.value); + } break; } case 'ADD_LOGFMT_PARSER': { diff --git a/public/app/plugins/datasource/prometheus/components/PromQueryField.tsx b/public/app/plugins/datasource/prometheus/components/PromQueryField.tsx index 07e44b67135..7139ac2c7be 100644 --- a/public/app/plugins/datasource/prometheus/components/PromQueryField.tsx +++ b/public/app/plugins/datasource/prometheus/components/PromQueryField.tsx @@ -224,8 +224,9 @@ class PromQueryField extends React.PureComponent { const { datasource, query, onChange, onRunQuery } = this.props; const { hint } = this.state; - - onChange(datasource.modifyQuery(query, hint!.fix!.action)); + if (hint?.fix?.action) { + onChange(datasource.modifyQuery(query, hint.fix.action)); + } onRunQuery(); }; diff --git a/public/app/plugins/datasource/prometheus/datasource.test.ts b/public/app/plugins/datasource/prometheus/datasource.test.ts index a80f1f4279d..de61312c84b 100644 --- a/public/app/plugins/datasource/prometheus/datasource.test.ts +++ b/public/app/plugins/datasource/prometheus/datasource.test.ts @@ -2154,7 +2154,7 @@ describe('modifyQuery', () => { describe('and query has no labels', () => { it('then the correct label should be added', () => { const query: PromQuery = { refId: 'A', expr: 'go_goroutines' }; - const action = { key: 'cluster', value: 'us-cluster', type: 'ADD_FILTER' }; + const action = { options: { key: 'cluster', value: 'us-cluster' }, type: 'ADD_FILTER' }; const instanceSettings = { jsonData: {} } as unknown as DataSourceInstanceSettings; const ds = new PrometheusDatasource(instanceSettings, templateSrvStub as any, timeSrvStub as any); @@ -2168,7 +2168,7 @@ describe('modifyQuery', () => { describe('and query has labels', () => { it('then the correct label should be added', () => { const query: PromQuery = { refId: 'A', expr: 'go_goroutines{cluster="us-cluster"}' }; - const action = { key: 'pod', value: 'pod-123', type: 'ADD_FILTER' }; + const action = { options: { key: 'pod', value: 'pod-123' }, type: 'ADD_FILTER' }; const instanceSettings = { jsonData: {} } as unknown as DataSourceInstanceSettings; const ds = new PrometheusDatasource(instanceSettings, templateSrvStub as any, timeSrvStub as any); @@ -2184,7 +2184,7 @@ describe('modifyQuery', () => { describe('and query has no labels', () => { it('then the correct label should be added', () => { const query: PromQuery = { refId: 'A', expr: 'go_goroutines' }; - const action = { key: 'cluster', value: 'us-cluster', type: 'ADD_FILTER_OUT' }; + const action = { options: { key: 'cluster', value: 'us-cluster' }, type: 'ADD_FILTER_OUT' }; const instanceSettings = { jsonData: {} } as unknown as DataSourceInstanceSettings; const ds = new PrometheusDatasource(instanceSettings, templateSrvStub as any, timeSrvStub as any); @@ -2198,7 +2198,7 @@ describe('modifyQuery', () => { describe('and query has labels', () => { it('then the correct label should be added', () => { const query: PromQuery = { refId: 'A', expr: 'go_goroutines{cluster="us-cluster"}' }; - const action = { key: 'pod', value: 'pod-123', type: 'ADD_FILTER_OUT' }; + const action = { options: { key: 'pod', value: 'pod-123' }, type: 'ADD_FILTER_OUT' }; const instanceSettings = { jsonData: {} } as unknown as DataSourceInstanceSettings; const ds = new PrometheusDatasource(instanceSettings, templateSrvStub as any, timeSrvStub as any); diff --git a/public/app/plugins/datasource/prometheus/datasource.tsx b/public/app/plugins/datasource/prometheus/datasource.tsx index 5c834905386..c6a3f089e90 100644 --- a/public/app/plugins/datasource/prometheus/datasource.tsx +++ b/public/app/plugins/datasource/prometheus/datasource.tsx @@ -22,6 +22,7 @@ import { TimeRange, DataFrame, dateTime, + QueryFixAction, } from '@grafana/data'; import { BackendSrvRequest, @@ -1030,15 +1031,22 @@ export class PrometheusDatasource } } - modifyQuery(query: PromQuery, action: any): PromQuery { + modifyQuery(query: PromQuery, action: QueryFixAction): PromQuery { let expression = query.expr ?? ''; switch (action.type) { case 'ADD_FILTER': { - expression = addLabelToQuery(expression, action.key, action.value); + const { key, value } = action.options ?? {}; + if (key && value) { + expression = addLabelToQuery(expression, key, value); + } + break; } case 'ADD_FILTER_OUT': { - expression = addLabelToQuery(expression, action.key, action.value, '!='); + const { key, value } = action.options ?? {}; + if (key && value) { + expression = addLabelToQuery(expression, key, value, '!='); + } break; } case 'ADD_HISTOGRAM_QUANTILE': { @@ -1054,8 +1062,8 @@ export class PrometheusDatasource break; } case 'EXPAND_RULES': { - if (action.mapping) { - expression = expandRecordingRules(expression, action.mapping); + if (action.options) { + expression = expandRecordingRules(expression, action.options); } break; } diff --git a/public/app/plugins/datasource/prometheus/query_hints.ts b/public/app/plugins/datasource/prometheus/query_hints.ts index 0b6be60ed99..08dd3495099 100644 --- a/public/app/plugins/datasource/prometheus/query_hints.ts +++ b/public/app/plugins/datasource/prometheus/query_hints.ts @@ -103,7 +103,7 @@ export function getQueryHints(query: string, series?: any[], datasource?: Promet action: { type: 'EXPAND_RULES', query, - mapping: mappingForQuery, + options: mappingForQuery, }, } as any as QueryFix, }); diff --git a/public/app/plugins/datasource/prometheus/querybuilder/shared/QueryBuilderHints.tsx b/public/app/plugins/datasource/prometheus/querybuilder/shared/QueryBuilderHints.tsx index 93b0d9ba721..3deb9c3e13a 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/shared/QueryBuilderHints.tsx +++ b/public/app/plugins/datasource/prometheus/querybuilder/shared/QueryBuilderHints.tsx @@ -45,10 +45,12 @@ export const QueryBuilderHints = ({