From 6aa5b45d62fea8c902fd958c1fb2cb6aaf9f0095 Mon Sep 17 00:00:00 2001 From: Pavel Zavora Date: Tue, 24 Nov 2020 05:38:05 +0100 Subject: [PATCH] fix(kapacitor): AND not equal conditions in where filter --- kapacitor/ast.go | 2 +- kapacitor/ast_test.go | 119 +++++++++++++++++++++++++++++++++++++++++ kapacitor/vars.go | 4 +- kapacitor/vars_test.go | 96 +++++++++++++++++++++++++++++++++ 4 files changed, 219 insertions(+), 2 deletions(-) diff --git a/kapacitor/ast.go b/kapacitor/ast.go index 0c1358a65..04f8d376c 100644 --- a/kapacitor/ast.go +++ b/kapacitor/ast.go @@ -106,7 +106,7 @@ func varWhereFilter(vars map[string]tick.Var) (WhereFilter, bool) { } lambda := value.ExpressionString() - // Chronograf TICKScripts use lambda: TRUE as a pass-throug where clause + // Chronograf TICKScripts use lambda: TRUE as a pass-through where clause // if the script does not have a where clause set. // isPresent(fieldName) is also used since chronograf 1.8.7 in place of TRUE // https://github.com/influxdata/chronograf/issues/5566 diff --git a/kapacitor/ast_test.go b/kapacitor/ast_test.go index d1e88d553..2c80e6e49 100644 --- a/kapacitor/ast_test.go +++ b/kapacitor/ast_test.go @@ -134,6 +134,125 @@ func TestReverse(t *testing.T) { }, }, }, + { + name: "simple stream tickscript - negative tags", + script: chronograf.TICKScript(` + var name = 'name' + var triggerType = 'threshold' + var every = 30s + var period = 10m + var groupBy = ['host', 'cluster_id'] + var db = 'telegraf' + var rp = 'autogen' + var measurement = 'cpu' + var message = 'message' + var details = 'details' + var crit = 90 + var idVar = name + ':{{.Group}}' + var idTag = 'alertID' + var levelTag = 'level' + var messageField = 'message' + var durationField = 'duration' + var whereFilter = lambda: ("cpu" != 'cpu_total') AND ("host" != 'acc-0eabc309-eu-west-1-data-3' AND "host" != 'prod') + + var data = stream + |from() + .database(db) + .retentionPolicy(rp) + .measurement(measurement) + |window() + .period(period) + .every(every) + .align() + |mean('usage_user') + .as('value') + var trigger = data + |alert() + .crit(lambda: "value" > crit) + .stateChangesOnly() + .message(message) + .id(idVar) + .idTag(idTag) + .levelTag(levelTag) + .messageField(messageField) + .durationField(durationField) + .slack() + .victorOps() + .email('howdy@howdy.com', 'doody@doody.com') + .log('/tmp/alerts.log') + .post('http://backin.tm') + .endpoint('myendpoint') + .header('key', 'value') + `), + + want: chronograf.AlertRule{ + Name: "name", + Trigger: "threshold", + AlertNodes: chronograf.AlertNodes{ + IsStateChangesOnly: true, + Slack: []*chronograf.Slack{ + {}, + }, + VictorOps: []*chronograf.VictorOps{ + {}, + }, + Email: []*chronograf.Email{ + { + To: []string{"howdy@howdy.com", "doody@doody.com"}, + }, + }, + Log: []*chronograf.Log{ + { + FilePath: "/tmp/alerts.log", + }, + }, + Posts: []*chronograf.Post{ + { + URL: "http://backin.tm", + Headers: map[string]string{"key": "value"}, + }, + }, + }, + TriggerValues: chronograf.TriggerValues{ + Operator: "greater than", + Value: "90", + }, + Every: "30s", + Message: "message", + Details: "details", + Query: &chronograf.QueryConfig{ + Database: "telegraf", + RetentionPolicy: "autogen", + Measurement: "cpu", + Fields: []chronograf.Field{ + { + Value: "mean", + Args: []chronograf.Field{ + { + Value: "usage_user", + Type: "field", + }, + }, + Type: "func", + }, + }, + GroupBy: chronograf.GroupBy{ + Time: "10m0s", + Tags: []string{"host", "cluster_id"}, + }, + Tags: map[string][]string{ + "cpu": { + "cpu_total", + }, + "host": { + "acc-0eabc309-eu-west-1-data-3", + "prod", + }, + }, + AreTagsAccepted: false, + }, + }, + }, { name: "Test Threshold", script: `var db = 'telegraf' diff --git a/kapacitor/vars.go b/kapacitor/vars.go index 774c2572a..37fbd2a49 100644 --- a/kapacitor/vars.go +++ b/kapacitor/vars.go @@ -235,8 +235,10 @@ func field(q *chronograf.QueryConfig) (string, error) { func whereFilter(q *chronograf.QueryConfig) string { if q != nil { operator := "==" + combineOperators := " OR " if !q.AreTagsAccepted { operator = "!=" + combineOperators = " AND " // negative comparisons are ANDed } outer := []string{} @@ -245,7 +247,7 @@ func whereFilter(q *chronograf.QueryConfig) string { for _, value := range values { inner = append(inner, fmt.Sprintf(`"%s" %s '%s'`, tag, operator, value)) } - outer = append(outer, "("+strings.Join(inner, " OR ")+")") + outer = append(outer, "("+strings.Join(inner, combineOperators)+")") } // add isPresent filters, see https://github.com/influxdata/chronograf/issues/5566 diff --git a/kapacitor/vars_test.go b/kapacitor/vars_test.go index 305685ad7..91e11e275 100644 --- a/kapacitor/vars_test.go +++ b/kapacitor/vars_test.go @@ -4,6 +4,7 @@ import ( "fmt" "testing" + "github.com/google/go-cmp/cmp" "github.com/influxdata/chronograf" ) @@ -85,3 +86,98 @@ func Test_formatValue(t *testing.T) { }) } } + +func TestVarsWhereFilter(t *testing.T) { + tests := []struct { + name string + queryConfig chronograf.QueryConfig + whereValue string + }{ + { + name: "simple", + queryConfig: chronograf.QueryConfig{ + Database: "telegraf", + Measurement: "haproxy", + RetentionPolicy: "autogen", + Fields: []chronograf.Field{ + { + Value: "status", + Type: "field", + }, + }, + GroupBy: chronograf.GroupBy{ + Time: "10m", + Tags: []string{"pxname"}, + }, + AreTagsAccepted: true, + }, + whereValue: `lambda: isPresent("status")`, + }, + { + name: "accepted tags", + queryConfig: chronograf.QueryConfig{ + Database: "telegraf", + Measurement: "haproxy", + RetentionPolicy: "autogen", + Fields: []chronograf.Field{ + { + Value: "status", + Type: "field", + }, + }, + GroupBy: chronograf.GroupBy{ + Time: "10m", + Tags: []string{"pxname"}, + }, + Tags: map[string][]string{ + "cpu": { + "cpu_total", + }, + "host": { + "acc-0eabc309-eu-west-1-data-3", + "prod", + }, + }, + AreTagsAccepted: true, + }, + whereValue: `lambda: ("cpu" == 'cpu_total') AND ("host" == 'acc-0eabc309-eu-west-1-data-3' OR "host" == 'prod') AND isPresent("status")`, + }, + { + name: "rejected tags", + queryConfig: chronograf.QueryConfig{ + Database: "telegraf", + Measurement: "haproxy", + RetentionPolicy: "autogen", + Fields: []chronograf.Field{ + { + Value: "status", + Type: "field", + }, + }, + GroupBy: chronograf.GroupBy{ + Time: "10m", + Tags: []string{"pxname"}, + }, + Tags: map[string][]string{ + "cpu": { + "cpu_total", + }, + "host": { + "acc-0eabc309-eu-west-1-data-3", + "prod", + }, + }, + AreTagsAccepted: false, + }, + whereValue: `lambda: ("cpu" != 'cpu_total') AND ("host" != 'acc-0eabc309-eu-west-1-data-3' AND "host" != 'prod') AND isPresent("status")`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := whereFilter(&tt.queryConfig) + if !cmp.Equal(result, tt.whereValue) { + t.Errorf("Reverse() = QueryConfig not equal %s", cmp.Diff(result, tt.whereValue)) + } + }) + } +}