From ac2f39e987c8865e5faff2115880fa95622939af Mon Sep 17 00:00:00 2001 From: Kyle Brandt Date: Wed, 16 Mar 2016 12:28:56 -0400 Subject: [PATCH] cmd/bosun: Fix merge to error on duplicate groups also, no stack trace on errors. --- cmd/bosun/expr/expr.go | 5 ++-- cmd/bosun/expr/funcs.go | 6 ++-- cmd/bosun/expr/funcs_test.go | 54 ++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/cmd/bosun/expr/expr.go b/cmd/bosun/expr/expr.go index b645a16b7a..f67b6f7f37 100644 --- a/cmd/bosun/expr/expr.go +++ b/cmd/bosun/expr/expr.go @@ -128,13 +128,14 @@ func (e *Expr) ExecuteState(s *State, T miniprofiler.Timer) (r *Results, queries func errRecover(errp *error) { e := recover() if e != nil { - slog.Infof("%s: %s", e, debug.Stack()) switch err := e.(type) { case runtime.Error: + slog.Infof("%s: %s", e, debug.Stack()) panic(e) case error: *errp = err default: + slog.Infof("%s: %s", e, debug.Stack()) panic(e) } } @@ -293,7 +294,7 @@ func (a *Results) Equal(b *Results) (bool, error) { } case Series: if !t.Equal(sortedB[i].Value.(Series)) { - return false, fmt.Errorf("mismatched series in result a: %v, b: %v", t, sortedB[i].Value.(Series)) + return false, fmt.Errorf("mismatched series in result (Group: %s) a: %v, b: %v", result.Group, t, sortedB[i].Value.(Series)) } default: panic(fmt.Sprintf("can't compare results with type %T", t)) diff --git a/cmd/bosun/expr/funcs.go b/cmd/bosun/expr/funcs.go index 49253212fd..e10a31aa93 100644 --- a/cmd/bosun/expr/funcs.go +++ b/cmd/bosun/expr/funcs.go @@ -471,15 +471,15 @@ func Filter(e *State, T miniprofiler.Timer, series *Results, number *Results) (* } func Merge(e *State, T miniprofiler.Timer, series ...*Results) (*Results, error) { + res := &Results{} if len(series) == 0 { - return &Results{}, fmt.Errorf("merge requires at least one result") + return res, fmt.Errorf("merge requires at least one result") } if len(series) == 1 { return series[0], nil } - res := series[0] seen := make(map[string]bool) - for _, r := range series[1:] { + for _, r := range series { for _, entry := range r.Results { if _, ok := seen[entry.Group.String()]; ok { return res, fmt.Errorf("duplicate group in merge: %s", entry.Group.String()) diff --git a/cmd/bosun/expr/funcs_test.go b/cmd/bosun/expr/funcs_test.go index 91cb26b440..49cfe24e82 100644 --- a/cmd/bosun/expr/funcs_test.go +++ b/cmd/bosun/expr/funcs_test.go @@ -1,9 +1,12 @@ package expr import ( + "fmt" "testing" "time" + "bosun.org/opentsdb" + "github.com/influxdata/influxdb/client" ) @@ -60,3 +63,54 @@ func TestToDuration(t *testing.T) { t.Error(err) } } + +func TestMerge(t *testing.T) { + seriesA := `series("foo=bar", 0, 1)` + seriesB := `series("foo=baz", 0, 1)` + err := testExpression(exprInOut{ + fmt.Sprintf("merge(%v, %v)", seriesA, seriesB), + Results{ + Results: ResultSlice{ + &Result{ + Value: Series{ + time.Unix(0, 0): 1, + }, + Group: opentsdb.TagSet{"foo": "bar"}, + }, + &Result{ + Value: Series{ + time.Unix(0, 0): 1, + }, + Group: opentsdb.TagSet{"foo": "baz"}, + }, + }, + }, + }) + if err != nil { + t.Error(err) + } + + //Should Error due to identical groups in merge + err = testExpression(exprInOut{ + fmt.Sprintf("merge(%v, %v)", seriesA, seriesA), + Results{ + Results: ResultSlice{ + &Result{ + Value: Series{ + time.Unix(0, 0): 1, + }, + Group: opentsdb.TagSet{"foo": "bar"}, + }, + &Result{ + Value: Series{ + time.Unix(0, 0): 1, + }, + Group: opentsdb.TagSet{"foo": "bar"}, + }, + }, + }, + }) + if err == nil { + t.Errorf("error expected due to identical groups in merge but did not get one") + } +}