Merge pull request influxdata#2448 from influxdata/fix/topic-handler

fix: topic-handler duration() -> alertDuration()

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Bugfixes
+- [#2448](https://github.com/influxdata/kapacitor/pull/2448): Changes the alert-handler match function duration() to be alertDuration() to avoid name collision with the type conversion function of the same name.
+
+
 ### Features
 - [#1839](https://github.com/influxdata/kapacitor/pull/1839): Add Subscription path configuration option to allow Kapacitor to run behind a reverse proxy, thanks @aspring
 - [#2055](https://github.com/influxdata/kapacitor/pull/2055): Add support for correlate in the Alerta AlertNode, thanks @nermolaev!

services/alert/dao.go

@@ -168,7 +168,7 @@ func (kv *handlerSpecKV) getHelper(o storage.BinaryObject, err error) (HandlerSp
 }
 
 func (kv *handlerSpecKV) Create(h HandlerSpec) error {
-	return kv.store.Create(&h)
+	return kv.store.Put(&h)
 }
 func (kv *handlerSpecKV) CreateTx(tx storage.Tx, h HandlerSpec) error {
 	return kv.store.CreateTx(tx, &h)

services/alert/handlers.go

@@ -373,7 +373,7 @@ const (
 	levelFunc    = "level"
 	nameFunc     = "name"
 	taskNameFunc = "taskName"
-	durationFunc = "duration"
+	durationFunc = "alertDuration"
 )
 
 var matchIdentifiers = map[string]interface{}{
@@ -408,7 +408,6 @@ func newMatchHandler(match string, h alert.Handler, d HandlerDiagnostic) (*match
 	if err != nil {
 		return nil, errors.Wrap(err, "invalid match expression")
 	}
-
 	mh := &matchHandler{
 		h:     h,
 		expr:  expr,
@@ -440,6 +439,17 @@ func newMatchHandler(match string, h alert.Handler, d HandlerDiagnostic) (*match
 }
 
 func (h *matchHandler) Handle(event alert.Event) {
+	defer func() {
+		if r := recover(); r != nil {
+			switch r := r.(type) {
+			case string:
+				h.diag.Error("recovered from panic", errors.New(r))
+			case error:
+				h.diag.Error("recovered from panic", r)
+			}
+		}
+	}()
+
 	if ok, err := h.match(event); err != nil {
 		h.diag.Error("failed to evaluate match expression", err)
 	} else if ok {
@@ -534,7 +544,6 @@ func (h *matchHandler) match(event alert.Event) (bool, error) {
 			return false, fmt.Errorf("no tag exists for %s", v)
 		}
 	}
-
 	// Evaluate expression with scope
 	return h.expr.EvalBool(h.scope)
 }

services/alert/handlers_test.go

@@ -0,0 +1,69 @@
+package alert
+
+import (
+	"testing"
+	"time"
+
+	"github.com/influxdata/kapacitor/alert"
+)
+
+func TestMatchHandlerAlertDuration(t *testing.T) {
+	cases := []struct {
+		name        string
+		event       alert.Event
+		shouldMatch bool
+		match       string
+	}{{
+		event: alert.Event{
+			Topic: "topi",
+			State: alert.EventState{
+				ID:       "woot",
+				Message:  "message",
+				Details:  "details",
+				Time:     time.Now(),
+				Duration: time.Second * 30,
+				Level:    alert.Critical,
+			},
+			Data: alert.EventData{},
+		},
+		shouldMatch: true,
+		match:       "alertDuration() < 1m",
+	}, {
+		event: alert.Event{
+			Topic: "topi",
+			State: alert.EventState{
+				ID:       "woot",
+				Message:  "message",
+				Details:  "details",
+				Time:     time.Now(),
+				Duration: time.Second * 130,
+				Level:    alert.Critical,
+			},
+			Data: alert.EventData{},
+		},
+		shouldMatch: false,
+		match:       "alertDuration() < 1m",
+	}}
+
+	for _, testCase := range cases {
+		t.Run(testCase.name, func(t *testing.T) {
+			var h alert.Handler
+			mh, err := newMatchHandler(testCase.match, h, nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			matched, err := mh.match(testCase.event)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if matched != testCase.shouldMatch {
+				if matched {
+					t.Errorf("expected event to match %s but it didn't", testCase.match)
+				} else {
+					t.Errorf("expected event to not match %s but it did", testCase.match)
+				}
+			}
+		})
+	}
+}

services/alert/service.go

@@ -1017,10 +1017,20 @@ func (s *Service) createHandlerFromSpec(spec HandlerSpec) (handler, error) {
 	default:
 		err = fmt.Errorf("unsupported action kind %q", spec.Kind)
 	}
+	if h == nil && err != nil {
+		return handler{}, err
+	}
 	if spec.Match != "" {
 		// Wrap handler in match handler
 		handlerDiag := s.diag.WithHandlerContext(ctx...)
-		h, err = newMatchHandler(spec.Match, h, handlerDiag)
+		if h == nil {
+			panic("handler is nil, this should not happen")
+		}
+		var err2 error
+		h, err2 = newMatchHandler(spec.Match, h, handlerDiag)
+		if err2 != nil {
+			return handler{Spec: spec, Handler: h}, err2
+		}
 	}
 	return handler{Spec: spec, Handler: h}, err
 }