Skip to content

Commit

Permalink
cmd/bosun: Fix merge to error on duplicate groups
Browse files Browse the repository at this point in the history
also, no stack trace on errors.
  • Loading branch information
kylebrandt committed Mar 16, 2016
1 parent 400ca82 commit 22c91a9
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 5 deletions.
5 changes: 3 additions & 2 deletions cmd/bosun/expr/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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))
Expand Down
6 changes: 3 additions & 3 deletions cmd/bosun/expr/funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
54 changes: 54 additions & 0 deletions cmd/bosun/expr/funcs_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package expr

import (
"fmt"
"testing"
"time"

"bosun.org/opentsdb"

"github.com/influxdata/influxdb/client"
)

Expand Down Expand Up @@ -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")
}
}

0 comments on commit 22c91a9

Please sign in to comment.