Commit b4c159dc authored by Kamil Trzciński's avatar Kamil Trzciński 🔴

Merge branch 'fix/race-on-error-metric-collector' into 'master'

Fix race in helpers/prometheus/log_hook.go: Fire() method

See merge request !463
parents 6df9cd60 e27af8a7
package prometheus
import (
"sync/atomic"
"github.com/Sirupsen/logrus"
"github.com/prometheus/client_golang/prometheus"
)
......@@ -8,7 +10,7 @@ import (
var numErrorsDesc = prometheus.NewDesc("ci_runner_errors", "The number of catched errors.", []string{"level"}, nil)
type LogHook struct {
errorsNumber map[logrus.Level]float64
errorsNumber map[logrus.Level]*int64
}
func (lh *LogHook) Levels() []logrus.Level {
......@@ -21,7 +23,7 @@ func (lh *LogHook) Levels() []logrus.Level {
}
func (lh *LogHook) Fire(entry *logrus.Entry) error {
lh.errorsNumber[entry.Level]++
atomic.AddInt64(lh.errorsNumber[entry.Level], 1)
return nil
}
......@@ -30,7 +32,8 @@ func (lh *LogHook) Describe(ch chan<- *prometheus.Desc) {
}
func (lh *LogHook) Collect(ch chan<- prometheus.Metric) {
for level, number := range lh.errorsNumber {
for _, level := range lh.Levels() {
number := float64(atomic.LoadInt64(lh.errorsNumber[level]))
ch <- prometheus.MustNewConstMetric(numErrorsDesc, prometheus.CounterValue, number, level.String())
}
}
......@@ -39,9 +42,9 @@ func NewLogHook() LogHook {
lh := LogHook{}
levels := lh.Levels()
lh.errorsNumber = make(map[logrus.Level]float64, len(levels))
lh.errorsNumber = make(map[logrus.Level]*int64, len(levels))
for _, level := range levels {
lh.errorsNumber[level] = 0
lh.errorsNumber[level] = new(int64)
}
return lh
......
package prometheus
import (
"testing"
"github.com/Sirupsen/logrus"
"github.com/prometheus/client_golang/prometheus"
"github.com/stretchr/testify/assert"
)
func callFireConcurrent(t *testing.T, lh *LogHook, repeats int, finish chan bool) {
for i := 0; i < repeats; i++ {
lh.Fire(&logrus.Entry{
Level: logrus.ErrorLevel,
})
finish <- true
}
}
func TestConcurrentFireCall(t *testing.T) {
lh := NewLogHook()
finish := make(chan bool)
times := 5
repeats := 100
total := times * repeats
for i := 0; i < times; i++ {
go callFireConcurrent(t, &lh, repeats, finish)
}
finished := 0
for {
if finished >= total {
break
}
<-finish
finished++
}
assert.Equal(t, total, *lh.errorsNumber[logrus.ErrorLevel], "Should fire log_hook N times")
}
func callCollectConcurrent(t *testing.T, lh *LogHook, repeats int, ch chan<- prometheus.Metric, finish chan bool) {
for i := 0; i < repeats; i++ {
lh.Collect(ch)
finish <- true
}
}
func TestCouncurrentFireCallWithCollect(t *testing.T) {
lh := NewLogHook()
finish := make(chan bool)
ch := make(chan prometheus.Metric)
times := 5
repeats := 100
total := times * repeats * 2
go func() {
for {
<-ch
}
}()
for i := 0; i < times; i++ {
go callFireConcurrent(t, &lh, repeats, finish)
go callCollectConcurrent(t, &lh, repeats, ch, finish)
}
finished := 0
for {
if finished >= total {
break
}
<-finish
finished++
}
assert.Equal(t, total/2, *lh.errorsNumber[logrus.ErrorLevel], "Should fire log_hook N times")
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment