From b89f3f811543bdc0a167ab9a5f25d73a3599a6a3 Mon Sep 17 00:00:00 2001 From: Kyle Brandt Date: Mon, 9 Sep 2024 09:56:43 -0400 Subject: [PATCH] Ad-Hoc Filters & Scopes: don't remap one-of to regex in frontend (#92995) * send "one-of" and "not-one-of" directly to datasource (instead of changing them to regex) * Added to Ad-hoc and and Scope Filters: The "values" prop ([]string) and the "one-of" and "not-one-"of" operators. "values" is used with one-of and not-one-of. * adds prometheus support for the above --------- Co-authored-by: Ashley Harrison Co-authored-by: Todd Treece --- packages/grafana-data/src/types/scopes.ts | 6 +++- .../grafana-prometheus/src/datasource.test.ts | 4 +++ packages/grafana-prometheus/src/datasource.ts | 29 +++++-------------- pkg/apis/scope/v0alpha1/types.go | 8 +++-- .../scope/v0alpha1/zz_generated.deepcopy.go | 9 +++++- .../scope/v0alpha1/zz_generated.openapi.go | 19 ++++++++++-- ...enerated.openapi_violation_exceptions.list | 1 + pkg/promlib/models/query.go | 8 +++-- pkg/promlib/models/query.panel.schema.json | 14 +++++++++ pkg/promlib/models/query.request.schema.json | 14 +++++++++ pkg/promlib/models/query.types.json | 16 +++++++++- pkg/promlib/models/scope.go | 11 +++++++ pkg/promlib/models/scope_test.go | 13 +++++++-- 13 files changed, 119 insertions(+), 33 deletions(-) diff --git a/packages/grafana-data/src/types/scopes.ts b/packages/grafana-data/src/types/scopes.ts index aac608e3e1d..90579b72017 100644 --- a/packages/grafana-data/src/types/scopes.ts +++ b/packages/grafana-data/src/types/scopes.ts @@ -17,18 +17,22 @@ export interface ScopeDashboardBinding { status: ScopeDashboardBindingStatus; } -export type ScopeFilterOperator = 'equals' | 'not-equals' | 'regex-match' | 'regex-not-match'; +export type ScopeFilterOperator = 'equals' | 'not-equals' | 'regex-match' | 'regex-not-match' | 'one-of' | 'not-one-of'; export const scopeFilterOperatorMap: Record = { '=': 'equals', '!=': 'not-equals', '=~': 'regex-match', '!~': 'regex-not-match', + '=|': 'one-of', + '!=|': 'not-one-of', }; export interface ScopeSpecFilter { key: string; value: string; + // values is used for operators that support multiple values (e.g. one-of, not-one-of) + values?: string[]; operator: ScopeFilterOperator; } diff --git a/packages/grafana-prometheus/src/datasource.test.ts b/packages/grafana-prometheus/src/datasource.test.ts index 39bf02561ed..664206c9f1f 100644 --- a/packages/grafana-prometheus/src/datasource.test.ts +++ b/packages/grafana-prometheus/src/datasource.test.ts @@ -1275,6 +1275,8 @@ describe('modifyQuery', () => { }, { key: 'reg', value: 'regv', operator: '=~' }, { key: 'nreg', value: 'nregv', operator: '!~' }, + { key: 'foo', value: 'bar', operator: '=|' }, + { key: 'bar', value: 'baz', operator: '!=|' }, ]; const expectedScopeFilter: ScopeSpecFilter[] = [ { key: 'eq', value: 'eqv', operator: 'equals' }, @@ -1285,6 +1287,8 @@ describe('modifyQuery', () => { }, { key: 'reg', value: 'regv', operator: 'regex-match' }, { key: 'nreg', value: 'nregv', operator: 'regex-not-match' }, + { key: 'foo', value: 'bar', operator: 'one-of' }, + { key: 'bar', value: 'baz', operator: 'not-one-of' }, ]; const result = ds.generateScopeFilters(adhocFilter); result.forEach((r, i) => { diff --git a/packages/grafana-prometheus/src/datasource.ts b/packages/grafana-prometheus/src/datasource.ts index 637e701507d..f75aa985b1a 100644 --- a/packages/grafana-prometheus/src/datasource.ts +++ b/packages/grafana-prometheus/src/datasource.ts @@ -603,7 +603,7 @@ export class PrometheusDatasource return this.languageProvider.getLabelKeys().map((k) => ({ value: k, text: k })); } - const labelFilters: QueryBuilderLabelFilter[] = options.filters.map(remapOneOf).map((f) => ({ + const labelFilters: QueryBuilderLabelFilter[] = options.filters.map((f) => ({ label: f.key, value: f.value, op: f.operator, @@ -620,7 +620,7 @@ export class PrometheusDatasource // By implementing getTagKeys and getTagValues we add ad-hoc filters functionality async getTagValues(options: DataSourceGetTagValuesOptions) { - const labelFilters: QueryBuilderLabelFilter[] = options.filters.map(remapOneOf).map((f) => ({ + const labelFilters: QueryBuilderLabelFilter[] = options.filters.map((f) => ({ label: f.key, value: f.value, op: f.operator, @@ -822,10 +822,11 @@ export class PrometheusDatasource return []; } - return filters.map(remapOneOf).map((f) => ({ - ...f, - value: this.templateSrv.replace(f.value, {}, this.interpolateQueryExpr), + return filters.map((f) => ({ + key: f.key, operator: scopeFilterOperatorMap[f.operator], + value: this.templateSrv.replace(f.value, {}, this.interpolateQueryExpr), + values: f.values?.map((v) => this.templateSrv.replace(v, {}, this.interpolateQueryExpr)), })); } @@ -834,7 +835,7 @@ export class PrometheusDatasource return expr; } - const finalQuery = filters.map(remapOneOf).reduce((acc, filter) => { + const finalQuery = filters.reduce((acc, filter) => { const { key, operator } = filter; let { value } = filter; if (operator === '=~' || operator === '!~') { @@ -1001,19 +1002,3 @@ export function prometheusRegularEscape(value: T) { export function prometheusSpecialRegexEscape(value: T) { return typeof value === 'string' ? value.replace(/\\/g, '\\\\\\\\').replace(/[$^*{}\[\]\'+?.()|]/g, '\\\\$&') : value; } - -export function remapOneOf(filter: AdHocVariableFilter) { - let { operator, value, values } = filter; - if (operator === '=|') { - operator = '=~'; - value = values?.map(prometheusRegularEscape).join('|') ?? ''; - } else if (operator === '!=|') { - operator = '!~'; - value = values?.map(prometheusRegularEscape).join('|') ?? ''; - } - return { - ...filter, - operator, - value, - }; -} diff --git a/pkg/apis/scope/v0alpha1/types.go b/pkg/apis/scope/v0alpha1/types.go index 3ba545134ba..c7ed0657ab5 100644 --- a/pkg/apis/scope/v0alpha1/types.go +++ b/pkg/apis/scope/v0alpha1/types.go @@ -26,8 +26,10 @@ type ScopeSpec struct { } type ScopeFilter struct { - Key string `json:"key"` - Value string `json:"value"` + Key string `json:"key"` + Value string `json:"value"` + // Values is used for operators that require multiple values (e.g. one-of and not-one-of). + Values []string `json:"values,omitempty"` Operator FilterOperator `json:"operator"` } @@ -41,6 +43,8 @@ const ( FilterOperatorNotEquals FilterOperator = "not-equals" FilterOperatorRegexMatch FilterOperator = "regex-match" FilterOperatorRegexNotMatch FilterOperator = "regex-not-match" + FilterOperatorOneOf FilterOperator = "one-of" + FilterOperatorNotOneOf FilterOperator = "not-one-of" ) // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/scope/v0alpha1/zz_generated.deepcopy.go b/pkg/apis/scope/v0alpha1/zz_generated.deepcopy.go index 197e04ca196..73a9723640e 100644 --- a/pkg/apis/scope/v0alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/scope/v0alpha1/zz_generated.deepcopy.go @@ -219,6 +219,11 @@ func (in *ScopeDashboardBindingStatus) DeepCopy() *ScopeDashboardBindingStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ScopeFilter) DeepCopyInto(out *ScopeFilter) { *out = *in + if in.Values != nil { + in, out := &in.Values, &out.Values + *out = make([]string, len(*in)) + copy(*out, *in) + } return } @@ -347,7 +352,9 @@ func (in *ScopeSpec) DeepCopyInto(out *ScopeSpec) { if in.Filters != nil { in, out := &in.Filters, &out.Filters *out = make([]ScopeFilter, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } diff --git a/pkg/apis/scope/v0alpha1/zz_generated.openapi.go b/pkg/apis/scope/v0alpha1/zz_generated.openapi.go index b5c97dba863..4018d2cf024 100644 --- a/pkg/apis/scope/v0alpha1/zz_generated.openapi.go +++ b/pkg/apis/scope/v0alpha1/zz_generated.openapi.go @@ -387,13 +387,28 @@ func schema_pkg_apis_scope_v0alpha1_ScopeFilter(ref common.ReferenceCallback) co Format: "", }, }, + "values": { + SchemaProps: spec.SchemaProps{ + Description: "Values is used for operators that require multiple values (e.g. one-of and not-one-of).", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, "operator": { SchemaProps: spec.SchemaProps{ - Description: "Possible enum values:\n - `\"equals\"`\n - `\"not-equals\"`\n - `\"regex-match\"`\n - `\"regex-not-match\"`", + Description: "Possible enum values:\n - `\"equals\"`\n - `\"not-equals\"`\n - `\"not-one-of\"`\n - `\"one-of\"`\n - `\"regex-match\"`\n - `\"regex-not-match\"`", Default: "", Type: []string{"string"}, Format: "", - Enum: []interface{}{"equals", "not-equals", "regex-match", "regex-not-match"}, + Enum: []interface{}{"equals", "not-equals", "not-one-of", "one-of", "regex-match", "regex-not-match"}, }, }, }, diff --git a/pkg/apis/scope/v0alpha1/zz_generated.openapi_violation_exceptions.list b/pkg/apis/scope/v0alpha1/zz_generated.openapi_violation_exceptions.list index c2dc43389ae..b473e1d66f0 100644 --- a/pkg/apis/scope/v0alpha1/zz_generated.openapi_violation_exceptions.list +++ b/pkg/apis/scope/v0alpha1/zz_generated.openapi_violation_exceptions.list @@ -1,3 +1,4 @@ API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/scope/v0alpha1,FindScopeDashboardBindingsResults,Items API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/scope/v0alpha1,ScopeDashboardBindingStatus,Groups +API rule violation: list_type_missing,github.com/grafana/grafana/pkg/apis/scope/v0alpha1,ScopeFilter,Values API rule violation: names_match,github.com/grafana/grafana/pkg/apis/scope/v0alpha1,ScopeNodeSpec,LinkID diff --git a/pkg/promlib/models/query.go b/pkg/promlib/models/query.go index 8acd496a264..002102db219 100644 --- a/pkg/promlib/models/query.go +++ b/pkg/promlib/models/query.go @@ -89,8 +89,10 @@ type ScopeSpec struct { // ScopeFilter is a hand copy of the ScopeFilter struct from pkg/apis/scope/v0alpha1/types.go // to avoid import (temp fix) type ScopeFilter struct { - Key string `json:"key"` - Value string `json:"value"` + Key string `json:"key"` + Value string `json:"value"` + // Values is used for operators that require multiple values (e.g. one-of and not-one-of). + Values []string `json:"values,omitempty"` Operator FilterOperator `json:"operator"` } @@ -103,6 +105,8 @@ const ( FilterOperatorNotEquals FilterOperator = "not-equals" FilterOperatorRegexMatch FilterOperator = "regex-match" FilterOperatorRegexNotMatch FilterOperator = "regex-not-match" + FilterOperatorOneOf FilterOperator = "one-of" + FilterOperatorNotOneOf FilterOperator = "not-one-of" ) // Internal interval and range variables diff --git a/pkg/promlib/models/query.panel.schema.json b/pkg/promlib/models/query.panel.schema.json index 0d3ca2c5d2e..778b233805d 100644 --- a/pkg/promlib/models/query.panel.schema.json +++ b/pkg/promlib/models/query.panel.schema.json @@ -34,6 +34,13 @@ }, "value": { "type": "string" + }, + "values": { + "description": "Values is used for operators that require multiple values (e.g. one-of and not-one-of).", + "type": "array", + "items": { + "type": "string" + } } }, "additionalProperties": false @@ -213,6 +220,13 @@ }, "value": { "type": "string" + }, + "values": { + "description": "Values is used for operators that require multiple values (e.g. one-of and not-one-of).", + "type": "array", + "items": { + "type": "string" + } } }, "additionalProperties": false diff --git a/pkg/promlib/models/query.request.schema.json b/pkg/promlib/models/query.request.schema.json index 27bbd8310d0..36ff78a2508 100644 --- a/pkg/promlib/models/query.request.schema.json +++ b/pkg/promlib/models/query.request.schema.json @@ -44,6 +44,13 @@ }, "value": { "type": "string" + }, + "values": { + "description": "Values is used for operators that require multiple values (e.g. one-of and not-one-of).", + "type": "array", + "items": { + "type": "string" + } } }, "additionalProperties": false @@ -223,6 +230,13 @@ }, "value": { "type": "string" + }, + "values": { + "description": "Values is used for operators that require multiple values (e.g. one-of and not-one-of).", + "type": "array", + "items": { + "type": "string" + } } }, "additionalProperties": false diff --git a/pkg/promlib/models/query.types.json b/pkg/promlib/models/query.types.json index 1d6a5b71fbf..052d0998eac 100644 --- a/pkg/promlib/models/query.types.json +++ b/pkg/promlib/models/query.types.json @@ -8,7 +8,7 @@ { "metadata": { "name": "default", - "resourceVersion": "1715871691891", + "resourceVersion": "1725885733879", "creationTimestamp": "2024-03-25T13:19:04Z" }, "spec": { @@ -31,6 +31,13 @@ }, "value": { "type": "string" + }, + "values": { + "description": "Values is used for operators that require multiple values (e.g. one-of and not-one-of).", + "items": { + "type": "string" + }, + "type": "array" } }, "required": [ @@ -117,6 +124,13 @@ }, "value": { "type": "string" + }, + "values": { + "description": "Values is used for operators that require multiple values (e.g. one-of and not-one-of).", + "items": { + "type": "string" + }, + "type": "array" } }, "required": [ diff --git a/pkg/promlib/models/scope.go b/pkg/promlib/models/scope.go index b2e3d0aa387..ec6c8f95b28 100644 --- a/pkg/promlib/models/scope.go +++ b/pkg/promlib/models/scope.go @@ -2,11 +2,13 @@ package models import ( "fmt" + "strings" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/promql/parser" ) +// ApplyFiltersAndGroupBy takes a raw promQL expression, converts the filters into PromQL matchers, and applies these matchers to the parsed expression. It also applies the group by clause to any aggregate expressions in the parsed expression. func ApplyFiltersAndGroupBy(rawExpr string, scopeFilters, adHocFilters []ScopeFilter, groupBy []string) (string, error) { expr, err := parser.ParseExpr(rawExpr) if err != nil { @@ -98,8 +100,17 @@ func filterToMatcher(f ScopeFilter) (*labels.Matcher, error) { mt = labels.MatchRegexp case FilterOperatorRegexNotMatch: mt = labels.MatchNotRegexp + case FilterOperatorOneOf: + mt = labels.MatchRegexp + case FilterOperatorNotOneOf: + mt = labels.MatchNotRegexp default: return nil, fmt.Errorf("unknown operator %q", f.Operator) } + if f.Operator == FilterOperatorOneOf || f.Operator == FilterOperatorNotOneOf { + if len(f.Values) > 0 { + return labels.NewMatcher(mt, f.Key, strings.Join(f.Values, "|")) + } + } return labels.NewMatcher(mt, f.Key, f.Value) } diff --git a/pkg/promlib/models/scope_test.go b/pkg/promlib/models/scope_test.go index b2c48870405..33ef2653416 100644 --- a/pkg/promlib/models/scope_test.go +++ b/pkg/promlib/models/scope_test.go @@ -68,7 +68,7 @@ func TestApplyQueryFiltersAndGroupBy_Filters(t *testing.T) { expectErr: false, }, { - name: "Adhoc and Scope filter conflict - adhoc wins", + name: "Adhoc and Scope filter conflict - adhoc wins (if not oneOf or notOneOf)", query: `http_requests_total{job="prometheus"}`, scopeFilters: []ScopeFilter{ {Key: "status", Value: "404", Operator: FilterOperatorEquals}, @@ -88,6 +88,15 @@ func TestApplyQueryFiltersAndGroupBy_Filters(t *testing.T) { expected: `capacity_bytes{job="alloy"} + available_bytes{job="alloy"} / 1024`, expectErr: false, }, + { + name: "OneOf Operator is combined into a single regex filter", + query: `http_requests_total{job="prometheus"}`, + scopeFilters: []ScopeFilter{ + {Key: "status", Values: []string{"404", "400"}, Operator: FilterOperatorOneOf}, + }, + expected: `http_requests_total{job="prometheus",status=~"404|400"}`, + expectErr: false, + }, } for _, tt := range tests { @@ -98,7 +107,7 @@ func TestApplyQueryFiltersAndGroupBy_Filters(t *testing.T) { require.Error(t, err) } else { require.NoError(t, err) - require.Equal(t, expr, tt.expected) + require.Equal(t, tt.expected, expr) } }) }