From 22b9f435badbd03b44faba0160551ae82ce069e2 Mon Sep 17 00:00:00 2001 From: Krenar Avduli <27018375+mjeshtri@users.noreply.github.com> Date: Wed, 19 Jan 2022 15:32:16 +0100 Subject: [PATCH 1/7] protect global monitor from race conditions ensuring that the monitor is declared a single time by becoming atomic. This approach protects also against race conditions --- ginmetrics/types.go | 11 ++++++++--- ginmetrics/types_test.go | 9 +++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) create mode 100644 ginmetrics/types_test.go diff --git a/ginmetrics/types.go b/ginmetrics/types.go index e9846fc..2e3405f 100644 --- a/ginmetrics/types.go +++ b/ginmetrics/types.go @@ -3,6 +3,8 @@ package ginmetrics import ( "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" + + "sync" ) type MetricType int @@ -19,8 +21,10 @@ const ( ) var ( + once sync.Once + monitor *Monitor + defaultDuration = []float64{0.1, 0.3, 1.2, 5, 10} - monitor *Monitor promTypeHandler = map[MetricType]func(metric *Metric) error{ Counter: counterHandler, @@ -41,14 +45,15 @@ type Monitor struct { // GetMonitor used to get global Monitor object, // this function returns a singleton object. func GetMonitor() *Monitor { - if monitor == nil { + once.Do(func() { monitor = &Monitor{ metricPath: defaultMetricPath, slowTime: defaultSlowTime, reqDuration: defaultDuration, metrics: make(map[string]*Metric), } - } + }) + return monitor } diff --git a/ginmetrics/types_test.go b/ginmetrics/types_test.go new file mode 100644 index 0000000..27dd06b --- /dev/null +++ b/ginmetrics/types_test.go @@ -0,0 +1,9 @@ +package ginmetrics + +import "testing" + +func TestSingletonRacing(t *testing.T) { + for i := 0; i < 10; i++ { + go GetMonitor() + } +} From 20ede8883856e52ae49e865dd483b2177011ae74 Mon Sep 17 00:00:00 2001 From: Krenar Avduli <27018375+mjeshtri@users.noreply.github.com> Date: Wed, 19 Jan 2022 16:13:52 +0100 Subject: [PATCH 2/7] add github actions to automatically run tests on PR or when pushing to master branch --- .github/workflows/run-tests.yml | 17 +++++++++++++++++ Makefile | 2 ++ 2 files changed, 19 insertions(+) create mode 100644 .github/workflows/run-tests.yml create mode 100644 Makefile diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml new file mode 100644 index 0000000..fd784e2 --- /dev/null +++ b/.github/workflows/run-tests.yml @@ -0,0 +1,17 @@ +name: Run tests + +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + + build: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Test + run: make test diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..871ec61 --- /dev/null +++ b/Makefile @@ -0,0 +1,2 @@ +test: + go test -v -race ./... From e2af5def917a9785a186d317a5b73562fe58b168 Mon Sep 17 00:00:00 2001 From: Krenar Avduli <27018375+mjeshtri@users.noreply.github.com> Date: Wed, 19 Jan 2022 16:36:36 +0100 Subject: [PATCH 3/7] add tests status reporting badge --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 7c44c26..b55bd17 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,5 @@ +[![Tests](https://github.com/penglongli/gin-metrics/actions/workflows/go.yml/badge.svg?branch=master)](https://github.com/penglongli/gin-metrics/actions/workflows/run-tests.yml) + # gin-metrics gin-gonic/gin metrics exporter for Prometheus. From f1a43d58653de9fb47fa6452e7fd616f84a9ea11 Mon Sep 17 00:00:00 2001 From: Krenar Avduli <27018375+mjeshtri@users.noreply.github.com> Date: Fri, 21 Jan 2022 10:52:17 +0100 Subject: [PATCH 4/7] setup go --- .github/workflows/run-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index fd784e2..6982a59 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -13,5 +13,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@v2 + - uses: actions/setup-go@v2 + with: + go-version: '^1.14' - name: Test run: make test From 19a6f34244e19fe4ccd3e2e0dd363282a0392771 Mon Sep 17 00:00:00 2001 From: Krenar Avduli <27018375+mjeshtri@users.noreply.github.com> Date: Fri, 21 Jan 2022 12:38:05 +0100 Subject: [PATCH 5/7] wait until all goroutines are finished --- ginmetrics/types_test.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/ginmetrics/types_test.go b/ginmetrics/types_test.go index 27dd06b..b94071a 100644 --- a/ginmetrics/types_test.go +++ b/ginmetrics/types_test.go @@ -1,9 +1,20 @@ package ginmetrics -import "testing" +import ( + "sync" + "testing" +) func TestSingletonRacing(t *testing.T) { - for i := 0; i < 10; i++ { - go GetMonitor() + var wg sync.WaitGroup + nLoops := 1000 + wg.Add(nLoops) + for i := 0; i < nLoops; i++ { + go func() { + GetMonitor() + wg.Done() + }() } + + wg.Wait() } From 86c4d6d4bba316c60fb0c15434163eec8314f292 Mon Sep 17 00:00:00 2001 From: Krenar Avduli <27018375+mjeshtri@users.noreply.github.com> Date: Fri, 21 Jan 2022 12:51:54 +0100 Subject: [PATCH 6/7] make BloomFilter thread safe --- bloom/bloom.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bloom/bloom.go b/bloom/bloom.go index 6703360..c8dd7ad 100644 --- a/bloom/bloom.go +++ b/bloom/bloom.go @@ -4,6 +4,7 @@ import ( // "fmt" "github.com/willf/bitset" + "sync" ) const defaultSize = 2 << 24 @@ -11,6 +12,7 @@ const defaultSize = 2 << 24 var seeds = []uint{7, 11, 13, 31, 37, 61} type BloomFilter struct { + mu sync.Mutex Set *bitset.BitSet Funcs [6]simpleHash } @@ -25,12 +27,18 @@ func NewBloomFilter() *BloomFilter { } func (bf *BloomFilter) Add(value string) { + bf.mu.Lock() + defer bf.mu.Unlock() + for _, f := range bf.Funcs { bf.Set.Set(f.hash(value)) } } func (bf *BloomFilter) Contains(value string) bool { + bf.mu.Lock() + defer bf.mu.Unlock() + if value == "" { return false } From f784a815fb86c88a9fa8585b18e5b727b28552ce Mon Sep 17 00:00:00 2001 From: Krenar Avduli <27018375+mjeshtri@users.noreply.github.com> Date: Fri, 21 Jan 2022 12:52:29 +0100 Subject: [PATCH 7/7] add tests for detecting data racing --- ginmetrics/middleware_test.go | 109 ++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 ginmetrics/middleware_test.go diff --git a/ginmetrics/middleware_test.go b/ginmetrics/middleware_test.go new file mode 100644 index 0000000..775d07f --- /dev/null +++ b/ginmetrics/middleware_test.go @@ -0,0 +1,109 @@ +package ginmetrics_test + +import ( + "fmt" + "net/http" + "net/http/httptest" + "sync" + "testing" + + "github.com/gin-gonic/gin" + + "github.com/penglongli/gin-metrics/ginmetrics" +) + +func init() { + gin.SetMode(gin.ReleaseMode) +} + +func TestMiddlewareSamePort(t *testing.T) { + r := newRouter() + + ts := httptest.NewServer(r) + defer ts.Close() + + var wg sync.WaitGroup + nLoops := 1000 + wg.Add(nLoops) + + for i := 0; i < nLoops; i++ { + go func(t *testing.T, i int) { + res, err := http.Get(ts.URL + fmt.Sprintf("/product/%d", i)) + if err != nil { + t.Errorf("Expected nil, received %s", err.Error()) + } + + if res.StatusCode != http.StatusOK { + t.Errorf("Expected %d, received %d", http.StatusOK, res.StatusCode) + } + wg.Done() + }(t, i) + } + + wg.Wait() +} + +func TestMiddlewareDifferentPort(t *testing.T) { + appRouter, metricsRouter := newRouterSeparateMetrics() + + ts := httptest.NewServer(appRouter) + tms := httptest.NewServer(metricsRouter) + + defer ts.Close() + defer tms.Close() + + var wg sync.WaitGroup + nLoops := 1000 + wg.Add(nLoops) + + for i := 0; i < nLoops; i++ { + go func(t *testing.T, i int) { + res, err := http.Get(ts.URL + fmt.Sprintf("/product/%d", i)) + if err != nil { + t.Errorf("Expected nil, received %s", err.Error()) + } + + if res.StatusCode != http.StatusOK { + t.Errorf("Expected %d, received %d", http.StatusOK, res.StatusCode) + } + wg.Done() + }(t, i) + } + + wg.Wait() +} + +func newRouterSeparateMetrics() (*gin.Engine, *gin.Engine) { + appRouter := gin.New() + metricRouter := gin.Default() + + m := ginmetrics.GetMonitor() + m.UseWithoutExposingEndpoint(appRouter) + m.Expose(metricRouter) + + appRouter.GET("/product/:id", func(ctx *gin.Context) { + ctx.JSON(200, map[string]string{ + "productId": ctx.Param("id"), + }) + }) + + return appRouter, metricRouter +} + +func newRouter() *gin.Engine { + r := gin.New() + + m := ginmetrics.GetMonitor() + m.SetMetricPath("/metrics") + m.SetSlowTime(10) + m.SetDuration([]float64{0.1, 0.3, 1.2, 5, 10}) + m.Use(r) + + r.GET("/product/:id", func(ctx *gin.Context) { + ctx.JSON(200, map[string]string{ + "productId": ctx.Param("id"), + }) + }) + + return r +}