From a29860d80e7851fe134e65114701b74a09307c9e Mon Sep 17 00:00:00 2001 From: Matt Bostock Date: Sat, 4 Jun 2016 09:05:54 +0100 Subject: [PATCH 1/7] Allow route receiver to be inherited This is useful when you're using a label to determine the receiver but want to override other options such as `group_by`. Currently you'd have to duplicate the matchers for the receivers to be able to do this. `checkReceiver()` now returns no error if a receiver is empty and we add a check to ensure that the root route has a receiver defined. I've added a test for this. This brings the `receiver` option into line with the other options (`group_by`, `group_wait`, etc) in the sense that routes can now inherit the receiver from the parent node. From https://prometheus.io/docs/alerting/configuration/: > A route block defines a node in a routing tree and its children. Its > optional configuration parameters are inherited from its parent node > if not set. --- config/config.go | 6 ++++++ config/config_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 config/config_test.go diff --git a/config/config.go b/config/config.go index 06d648d251..7d8e017bc1 100644 --- a/config/config.go +++ b/config/config.go @@ -225,6 +225,9 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { if c.Route == nil { return fmt.Errorf("No routes provided") } + if len(c.Route.Receiver) == 0 { + return fmt.Errorf("Root route must specify a default receiver") + } if len(c.Route.Match) > 0 || len(c.Route.MatchRE) > 0 { return fmt.Errorf("Root route must not have any matchers") } @@ -240,6 +243,9 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { // checkReceiver returns an error if a node in the routing tree // references a receiver not in the given map. func checkReceiver(r *Route, receivers map[string]struct{}) error { + if r.Receiver == "" { + return nil + } if _, ok := receivers[r.Receiver]; !ok { return fmt.Errorf("Undefined receiver %q used in route", r.Receiver) } diff --git a/config/config_test.go b/config/config_test.go new file mode 100644 index 0000000000..f48a3b6704 --- /dev/null +++ b/config/config_test.go @@ -0,0 +1,39 @@ +// Copyright 2016 Prometheus Team +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "testing" + + "gopkg.in/yaml.v2" +) + +func TestDefaultReceiverExists(t *testing.T) { + in := ` +route: + group_wait: 30s +` + + conf := &Config{} + err := yaml.Unmarshal([]byte(in), conf) + + expected := "Root route must specify a default receiver" + + if err == nil { + t.Fatalf("no error returned, expected:\n%v", expected) + } + if err.Error() != expected { + t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) + } +} From aec84c0288a836c471dc5922996faea9b0b23851 Mon Sep 17 00:00:00 2001 From: Matt Bostock Date: Sat, 4 Jun 2016 10:08:08 +0100 Subject: [PATCH 2/7] Add route tests for receiver inheritance Tests for: - inheriting a receiver from the default route - overriding the receiver when the parent route has not set one - inheriting a receiver from a parent route that's not the default Adds a route which matches on a `group_by` label which inherits the default receiver and also has a child route, matching on an `env` label, which overrides the receiver. To be sure of edge case behaviour, we test inheriting the receiver from a child route. Hopefully no one would actually implement this edge case in reality as it's overly complex but we test anyway to be sure that it doesn't trigger unexpected behaviour. --- route_test.go | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/route_test.go b/route_test.go index f405f09fc5..3578dfc062 100644 --- a/route_test.go +++ b/route_test.go @@ -65,6 +65,19 @@ routes: group_by: ['foo', 'bar'] group_wait: 2m receiver: 'notify-BC' + +- match: + group_by: 'role' + group_by: ['role'] + + routes: + - match: + env: 'testing' + receiver: 'notify-testing' + routes: + - match: + wait: 'long' + group_wait: 2m ` var ctree config.Route @@ -167,6 +180,51 @@ routes: }, }, }, + { + input: model.LabelSet{ + "group_by": "role", + }, + result: []*RouteOpts{ + { + Receiver: "notify-def", + GroupBy: lset("role"), + GroupWait: def.GroupWait, + GroupInterval: def.GroupInterval, + RepeatInterval: def.RepeatInterval, + }, + }, + }, + { + input: model.LabelSet{ + "env": "testing", + "group_by": "role", + }, + result: []*RouteOpts{ + { + Receiver: "notify-testing", + GroupBy: lset("role"), + GroupWait: def.GroupWait, + GroupInterval: def.GroupInterval, + RepeatInterval: def.RepeatInterval, + }, + }, + }, + { + input: model.LabelSet{ + "env": "testing", + "group_by": "role", + "wait": "long", + }, + result: []*RouteOpts{ + { + Receiver: "notify-testing", + GroupBy: lset("role"), + GroupWait: 2 * time.Minute, + GroupInterval: def.GroupInterval, + RepeatInterval: def.RepeatInterval, + }, + }, + }, } for _, test := range tests { From 1d6b6d6ea84e2cbdcaa0185fec9b21f8359cca17 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Fri, 17 Jun 2016 15:10:16 +0200 Subject: [PATCH 3/7] Fix go vet warnings --- inhibit_test.go | 2 +- notify/impl.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/inhibit_test.go b/inhibit_test.go index f212471d38..f2acebfc26 100644 --- a/inhibit_test.go +++ b/inhibit_test.go @@ -124,7 +124,7 @@ func TestInhibitRuleHasEqual(t *testing.T) { } if have := r.hasEqual(c.input); have != c.result { - t.Errorf("Unexpected result %q, expected %q", have, c.result) + t.Errorf("Unexpected result %t, expected %t", have, c.result) } if !reflect.DeepEqual(r.scache, c.initial) { t.Errorf("Cache state unexpectedly changed") diff --git a/notify/impl.go b/notify/impl.go index a4eadfe450..59a1f04e81 100644 --- a/notify/impl.go +++ b/notify/impl.go @@ -563,7 +563,7 @@ func (n *Hipchat) Notify(ctx context.Context, as ...*types.Alert) error { data = n.tmpl.Data(receiver(ctx), groupLabels(ctx), as...) tmplText = tmplText(n.tmpl, data, &err) tmplHTML = tmplHTML(n.tmpl, data, &err) - url = fmt.Sprintf("%sv2/room/%s/notification?auth_token=%s", n.conf.APIURL, n.conf.RoomID, n.conf.AuthToken) + url = fmt.Sprintf("%sv2/room/%d/notification?auth_token=%s", n.conf.APIURL, n.conf.RoomID, n.conf.AuthToken) ) if n.conf.MessageFormat == "html" { From 218c44d6ca9d94e4915d663129eefe909a6c27f9 Mon Sep 17 00:00:00 2001 From: shalom Date: Tue, 21 Jun 2016 10:33:40 -0400 Subject: [PATCH 4/7] Fixing hipchat room number formating to string --- notify/impl.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notify/impl.go b/notify/impl.go index 59a1f04e81..a4eadfe450 100644 --- a/notify/impl.go +++ b/notify/impl.go @@ -563,7 +563,7 @@ func (n *Hipchat) Notify(ctx context.Context, as ...*types.Alert) error { data = n.tmpl.Data(receiver(ctx), groupLabels(ctx), as...) tmplText = tmplText(n.tmpl, data, &err) tmplHTML = tmplHTML(n.tmpl, data, &err) - url = fmt.Sprintf("%sv2/room/%d/notification?auth_token=%s", n.conf.APIURL, n.conf.RoomID, n.conf.AuthToken) + url = fmt.Sprintf("%sv2/room/%s/notification?auth_token=%s", n.conf.APIURL, n.conf.RoomID, n.conf.AuthToken) ) if n.conf.MessageFormat == "html" { From 1a93b01234c5475a883d6df7ae82bc6b202ecc03 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Tue, 21 Jun 2016 16:38:35 +0200 Subject: [PATCH 5/7] api: fix missing silence initialization --- api.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/api.go b/api.go index 6a4a7af617..fde027e42b 100644 --- a/api.go +++ b/api.go @@ -283,14 +283,15 @@ func (api *API) insertAlerts(w http.ResponseWriter, r *http.Request, alerts ...* } func (api *API) addSilence(w http.ResponseWriter, r *http.Request) { - var sil types.Silence - if err := receive(r, &sil); err != nil { + var msil model.Silence + if err := receive(r, &msil); err != nil { respondError(w, apiError{ typ: errorBadData, err: err, }, nil) return } + sil := types.NewSilence(&msil) if sil.CreatedAt.IsZero() { sil.CreatedAt = time.Now() @@ -304,7 +305,7 @@ func (api *API) addSilence(w http.ResponseWriter, r *http.Request) { return } - sid, err := api.silences.Set(&sil) + sid, err := api.silences.Set(sil) if err != nil { respondError(w, apiError{ typ: errorInternal, From d3c2ff7bba9d43a7fd81cf698876b3db5839b66b Mon Sep 17 00:00:00 2001 From: beorn7 Date: Tue, 21 Jun 2016 22:39:47 +0200 Subject: [PATCH 6/7] Fix VERSION --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 17e51c385e..0ea3a944b3 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.1.1 +0.2.0 From ee37ea6965659d7276fa8afeb06d3610cd722d18 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Tue, 21 Jun 2016 16:38:50 +0200 Subject: [PATCH 7/7] provider/boltmem: add in-memory silence cache --- provider/boltmem/boltmem.go | 86 ++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 34 deletions(-) diff --git a/provider/boltmem/boltmem.go b/provider/boltmem/boltmem.go index 58b0a06696..0c3481f615 100644 --- a/provider/boltmem/boltmem.go +++ b/provider/boltmem/boltmem.go @@ -217,6 +217,9 @@ func (a *Alerts) Put(alerts ...*types.Alert) error { type Silences struct { db *bolt.DB mk types.Marker + + mtx sync.RWMutex + cache map[uint64]*types.Silence } // NewSilences creates a new Silences provider. @@ -229,7 +232,15 @@ func NewSilences(path string, mk types.Marker) (*Silences, error) { _, err := tx.CreateBucketIfNotExists(bktSilences) return err }) - return &Silences{db: db, mk: mk}, err + if err != nil { + return nil, err + } + s := &Silences{ + db: db, + mk: mk, + cache: map[uint64]*types.Silence{}, + } + return s, s.initCache() } // Close the silences provider. @@ -261,7 +272,19 @@ func (s *Silences) Mutes(lset model.LabelSet) bool { // All returns all existing silences. func (s *Silences) All() ([]*types.Silence, error) { - var res []*types.Silence + s.mtx.RLock() + defer s.mtx.RUnlock() + + res := make([]*types.Silence, 0, len(s.cache)) + for _, s := range s.cache { + res = append(res, s) + } + return res, nil +} + +func (s *Silences) initCache() error { + s.mtx.Lock() + defer s.mtx.Unlock() err := s.db.View(func(tx *bolt.Tx) error { b := tx.Bucket(bktSilences) @@ -272,23 +295,21 @@ func (s *Silences) All() ([]*types.Silence, error) { if err := json.Unmarshal(v, &ms); err != nil { return err } - ms.ID = binary.BigEndian.Uint64(k) - - if err := json.Unmarshal(v, &ms); err != nil { - return err - } - - res = append(res, types.NewSilence(&ms)) + // The ID is duplicated in the value and always equal + // to the stored key. + s.cache[ms.ID] = types.NewSilence(&ms) } return nil }) - - return res, err + return err } // Set a new silence. func (s *Silences) Set(sil *types.Silence) (uint64, error) { + s.mtx.Lock() + defer s.mtx.Unlock() + var ( uid uint64 err error @@ -312,11 +333,18 @@ func (s *Silences) Set(sil *types.Silence) (uint64, error) { } return b.Put(k, msb) }) - return uid, err + if err != nil { + return 0, err + } + s.cache[uid] = sil + return uid, nil } // Del removes a silence. func (s *Silences) Del(uid uint64) error { + s.mtx.Lock() + defer s.mtx.Unlock() + err := s.db.Update(func(tx *bolt.Tx) error { b := tx.Bucket(bktSilences) @@ -325,33 +353,23 @@ func (s *Silences) Del(uid uint64) error { return b.Delete(k) }) - return err + if err != nil { + return err + } + delete(s.cache, uid) + return nil } // Get a silence associated with a fingerprint. func (s *Silences) Get(uid uint64) (*types.Silence, error) { - var sil *types.Silence - - err := s.db.View(func(tx *bolt.Tx) error { - b := tx.Bucket(bktSilences) - - k := make([]byte, 8) - binary.BigEndian.PutUint64(k, uid) - - v := b.Get(k) - if v == nil { - return provider.ErrNotFound - } - var ms model.Silence - - if err := json.Unmarshal(v, &ms); err != nil { - return err - } - sil = types.NewSilence(&ms) + s.mtx.RLock() + defer s.mtx.RUnlock() - return nil - }) - return sil, err + sil, ok := s.cache[uid] + if !ok { + return nil, provider.ErrNotFound + } + return sil, nil } // NotificationInfo provides information about pending and successful