From 4487f26dfe07b899fc3f069fc0ca4fdaa3a69f79 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Fri, 1 Nov 2019 13:52:23 -0700 Subject: [PATCH 01/14] feat: count the API hits for each good route and store in expvar metrics --- pkg/api/middleware/metrics.go | 40 +++++++++ pkg/api/middleware/metrics_test.go | 135 +++++++++++++++++++++++++++++ pkg/api/router.go | 17 ++-- 3 files changed, 186 insertions(+), 6 deletions(-) create mode 100644 pkg/api/middleware/metrics.go create mode 100644 pkg/api/middleware/metrics_test.go diff --git a/pkg/api/middleware/metrics.go b/pkg/api/middleware/metrics.go new file mode 100644 index 00000000..45412754 --- /dev/null +++ b/pkg/api/middleware/metrics.go @@ -0,0 +1,40 @@ +/**************************************************************************** + * Copyright 2019, Optimizely, Inc. and contributors * + * * + * 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 middleware // + +package middleware + +import ( + "expvar" + "net/http" + "strings" + + "github.com/go-chi/chi" +) + +// UpdateMetrics update counts for each URL hit +func UpdateMetrics(counts *expvar.Map) func(http.Handler) http.Handler { + f := func(h http.Handler) http.Handler { + fn := func(w http.ResponseWriter, r *http.Request) { + key := r.Method + "_" + strings.ReplaceAll(chi.RouteContext(r.Context()).RoutePattern(), "/", "_") + counts.Add(key, 1) + h.ServeHTTP(w, r) + } + return http.HandlerFunc(fn) + } + return f +} diff --git a/pkg/api/middleware/metrics_test.go b/pkg/api/middleware/metrics_test.go new file mode 100644 index 00000000..3dec10cd --- /dev/null +++ b/pkg/api/middleware/metrics_test.go @@ -0,0 +1,135 @@ +/**************************************************************************** + * Copyright 2019, Optimizely, Inc. and contributors * + * * + * 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 middleware // + +package middleware + +import ( + "context" + "encoding/json" + "expvar" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-chi/chi" + "github.com/stretchr/testify/suite" +) + +type JSON map[string]interface{} + +var getTestMetrics = func() http.HandlerFunc { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + }) +} + +type RequestMetrics struct { + suite.Suite + rw http.ResponseWriter + req *http.Request + handler http.Handler +} + +func (rm *RequestMetrics) setRoute(metricsKey string) { + + metricsMap := expvar.NewMap(metricsKey) + rm.rw = httptest.NewRecorder() + r := httptest.NewRequest("GET", "/", nil) + + rctx := chi.NewRouteContext() + rctx.RoutePatterns = []string{"/item/{set_item}"} + + rm.req = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) + rm.handler = http.Handler(UpdateMetrics(metricsMap)(getTestMetrics())) + +} + +func (rm RequestMetrics) serveRoute() { + rm.handler.ServeHTTP(rm.rw, rm.req) +} + +func (rm RequestMetrics) serveExpvarRoute() { + expvar.Handler().ServeHTTP(rm.rw, rm.req) +} + +func (rm RequestMetrics) getMetricsMap() JSON { + var expVarMap JSON + err := json.Unmarshal(rm.rw.(*httptest.ResponseRecorder).Body.Bytes(), &expVarMap) + rm.Suite.Nil(err) + + return expVarMap +} + +func (rm RequestMetrics) getCode() int { + return rm.rw.(*httptest.ResponseRecorder).Code +} + +func (suite *RequestMetrics) TestUpdateMetricsHitOnce() { + + var metricsKey = "counter" + + suite.setRoute(metricsKey) + suite.serveRoute() + + suite.Equal(http.StatusOK, suite.getCode(), "Status code differs") + + suite.serveExpvarRoute() + + expVarMap := suite.getMetricsMap() + + counterMap, ok := expVarMap[metricsKey] + suite.True(ok) + + suite.Contains(counterMap, "GET__item_{set_item}") + + m := counterMap.(map[string]interface{}) + + suite.Equal(1.0, m["GET__item_{set_item}"]) + +} + +func (suite *RequestMetrics) TestUpdateMetricsHitMultiple() { + + var metricsKey = "counter1" + const hitNumber = 10.0 + + suite.setRoute(metricsKey) + + for i := 0; i < hitNumber; i++ { + suite.serveRoute() + } + + suite.Equal(http.StatusOK, suite.getCode(), "Status code differs") + + suite.serveExpvarRoute() + + expVarMap := suite.getMetricsMap() + + counterMap, ok := expVarMap[metricsKey] + suite.True(ok) + + suite.Contains(counterMap, "GET__item_{set_item}") + + m := counterMap.(map[string]interface{}) + + suite.Equal(hitNumber, m["GET__item_{set_item}"]) + +} + +func TestRequestMetrics(t *testing.T) { + suite.Run(t, new(RequestMetrics)) +} diff --git a/pkg/api/router.go b/pkg/api/router.go index 7c0bcabe..b9b38e73 100644 --- a/pkg/api/router.go +++ b/pkg/api/router.go @@ -18,6 +18,8 @@ package api import ( + "expvar" + "github.com/optimizely/sidedoor/pkg/api/handlers" "github.com/optimizely/sidedoor/pkg/api/middleware" "github.com/optimizely/sidedoor/pkg/optimizely" @@ -47,27 +49,30 @@ func NewDefaultRouter(optlyCache optimizely.Cache) *chi.Mux { return NewRouter(spec) } +const metricsPrefix = "route_counters" + // NewRouter returns HTTP API router backed by an optimizely.Cache implementation func NewRouter(opt *RouterOptions) *chi.Mux { r := chi.NewRouter() + routeCounts := expvar.NewMap(metricsPrefix) r.Use(render.SetContentType(render.ContentTypeJSON), middleware.SetRequestID) - r.With(chimw.AllowContentType("application/json")).Post("/user-event", opt.userEventAPI.AddUserEvent) + r.With(chimw.AllowContentType("application/json"), middleware.UpdateMetrics(routeCounts)).Post("/user-event", opt.userEventAPI.AddUserEvent) r.Route("/features", func(r chi.Router) { r.Use(opt.middleware.ClientCtx) - r.Get("/", opt.featureAPI.ListFeatures) - r.Get("/{featureKey}", opt.featureAPI.GetFeature) + r.With(middleware.UpdateMetrics(routeCounts)).Get("/", opt.featureAPI.ListFeatures) + r.With(middleware.UpdateMetrics(routeCounts)).Get("/{featureKey}", opt.featureAPI.GetFeature) }) r.Route("/users/{userID}", func(r chi.Router) { r.Use(opt.middleware.ClientCtx, opt.middleware.UserCtx) - r.Post("/events/{eventKey}", opt.userAPI.TrackEvent) + r.With(middleware.UpdateMetrics(routeCounts)).Post("/events/{eventKey}", opt.userAPI.TrackEvent) - r.Get("/features/{featureKey}", opt.userAPI.GetFeature) - r.Post("/features/{featureKey}", opt.userAPI.TrackFeature) + r.With(middleware.UpdateMetrics(routeCounts)).Get("/features/{featureKey}", opt.userAPI.GetFeature) + r.With(middleware.UpdateMetrics(routeCounts)).Post("/features/{featureKey}", opt.userAPI.TrackFeature) }) return r From 2cd946ef758cbf74b0b451e7501f11fb8f1ed728 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Mon, 4 Nov 2019 09:24:21 -0800 Subject: [PATCH 02/14] corrected package comment --- pkg/api/middleware/metrics.go | 1 - pkg/api/middleware/metrics_test.go | 1 - 2 files changed, 2 deletions(-) diff --git a/pkg/api/middleware/metrics.go b/pkg/api/middleware/metrics.go index 45412754..c0ffa4bc 100644 --- a/pkg/api/middleware/metrics.go +++ b/pkg/api/middleware/metrics.go @@ -15,7 +15,6 @@ ***************************************************************************/ // Package middleware // - package middleware import ( diff --git a/pkg/api/middleware/metrics_test.go b/pkg/api/middleware/metrics_test.go index 3dec10cd..d3cee2e9 100644 --- a/pkg/api/middleware/metrics_test.go +++ b/pkg/api/middleware/metrics_test.go @@ -15,7 +15,6 @@ ***************************************************************************/ // Package middleware // - package middleware import ( From db9332a164be5db91c511035fec0faf9bc25a700 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Mon, 4 Nov 2019 09:41:06 -0800 Subject: [PATCH 03/14] moved declaration to pkg level to be compatible with existing tests. --- pkg/api/router.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/api/router.go b/pkg/api/router.go index b9b38e73..4be45442 100644 --- a/pkg/api/router.go +++ b/pkg/api/router.go @@ -51,10 +51,11 @@ func NewDefaultRouter(optlyCache optimizely.Cache) *chi.Mux { const metricsPrefix = "route_counters" +var routeCounts = expvar.NewMap(metricsPrefix) + // NewRouter returns HTTP API router backed by an optimizely.Cache implementation func NewRouter(opt *RouterOptions) *chi.Mux { r := chi.NewRouter() - routeCounts := expvar.NewMap(metricsPrefix) r.Use(render.SetContentType(render.ContentTypeJSON), middleware.SetRequestID) From af77161fbd3e2e35697b6db8961e74ba50462a4b Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Mon, 4 Nov 2019 09:44:55 -0800 Subject: [PATCH 04/14] addressing PR comments --- pkg/api/middleware/metrics.go | 4 ++-- pkg/api/middleware/metrics_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/api/middleware/metrics.go b/pkg/api/middleware/metrics.go index c0ffa4bc..b7517f29 100644 --- a/pkg/api/middleware/metrics.go +++ b/pkg/api/middleware/metrics.go @@ -25,8 +25,8 @@ import ( "github.com/go-chi/chi" ) -// UpdateMetrics update counts for each URL hit -func UpdateMetrics(counts *expvar.Map) func(http.Handler) http.Handler { +// HitCount update counts for each URL hit, key being a combination of a method and route pattern +func HitCount(counts *expvar.Map) func(http.Handler) http.Handler { f := func(h http.Handler) http.Handler { fn := func(w http.ResponseWriter, r *http.Request) { key := r.Method + "_" + strings.ReplaceAll(chi.RouteContext(r.Context()).RoutePattern(), "/", "_") diff --git a/pkg/api/middleware/metrics_test.go b/pkg/api/middleware/metrics_test.go index d3cee2e9..daa4315a 100644 --- a/pkg/api/middleware/metrics_test.go +++ b/pkg/api/middleware/metrics_test.go @@ -53,7 +53,7 @@ func (rm *RequestMetrics) setRoute(metricsKey string) { rctx.RoutePatterns = []string{"/item/{set_item}"} rm.req = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) - rm.handler = http.Handler(UpdateMetrics(metricsMap)(getTestMetrics())) + rm.handler = http.Handler(HitCount(metricsMap)(getTestMetrics())) } From f3fcd902ef263daf8e6b36dc21701e6d9d7d5e4a Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Mon, 4 Nov 2019 09:48:01 -0800 Subject: [PATCH 05/14] addressing PR comments --- pkg/api/router.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/api/router.go b/pkg/api/router.go index 4be45442..5926eefc 100644 --- a/pkg/api/router.go +++ b/pkg/api/router.go @@ -59,21 +59,21 @@ func NewRouter(opt *RouterOptions) *chi.Mux { r.Use(render.SetContentType(render.ContentTypeJSON), middleware.SetRequestID) - r.With(chimw.AllowContentType("application/json"), middleware.UpdateMetrics(routeCounts)).Post("/user-event", opt.userEventAPI.AddUserEvent) + r.With(chimw.AllowContentType("application/json"), middleware.HitCount(routeCounts)).Post("/user-event", opt.userEventAPI.AddUserEvent) r.Route("/features", func(r chi.Router) { r.Use(opt.middleware.ClientCtx) - r.With(middleware.UpdateMetrics(routeCounts)).Get("/", opt.featureAPI.ListFeatures) - r.With(middleware.UpdateMetrics(routeCounts)).Get("/{featureKey}", opt.featureAPI.GetFeature) + r.With(middleware.HitCount(routeCounts)).Get("/", opt.featureAPI.ListFeatures) + r.With(middleware.HitCount(routeCounts)).Get("/{featureKey}", opt.featureAPI.GetFeature) }) r.Route("/users/{userID}", func(r chi.Router) { r.Use(opt.middleware.ClientCtx, opt.middleware.UserCtx) - r.With(middleware.UpdateMetrics(routeCounts)).Post("/events/{eventKey}", opt.userAPI.TrackEvent) + r.With(middleware.HitCount(routeCounts)).Post("/events/{eventKey}", opt.userAPI.TrackEvent) - r.With(middleware.UpdateMetrics(routeCounts)).Get("/features/{featureKey}", opt.userAPI.GetFeature) - r.With(middleware.UpdateMetrics(routeCounts)).Post("/features/{featureKey}", opt.userAPI.TrackFeature) + r.With(middleware.HitCount(routeCounts)).Get("/features/{featureKey}", opt.userAPI.GetFeature) + r.With(middleware.HitCount(routeCounts)).Post("/features/{featureKey}", opt.userAPI.TrackFeature) }) return r From 8bbfc56a6874e2015dbea051595dae25b6541ced Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Tue, 5 Nov 2019 17:58:49 -0800 Subject: [PATCH 06/14] using github.com/go-kit/kit for metrics --- go.mod | 1 + go.sum | 2 + pkg/api/middleware/metrics.go | 82 ++++++++++++++++++++++++++++-- pkg/api/middleware/metrics_test.go | 53 +++++++++++-------- pkg/api/router.go | 20 +++----- 5 files changed, 120 insertions(+), 38 deletions(-) diff --git a/go.mod b/go.mod index 8c71689c..73b9b9a0 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.12 require ( github.com/go-chi/chi v4.0.2+incompatible github.com/go-chi/render v1.0.1 + github.com/go-kit/kit v0.9.0 // indirect github.com/google/uuid v1.1.1 github.com/nsqio/nsq v1.2.0 github.com/optimizely/go-sdk v1.0.0-beta5.0.20191031194604-0f774263df60 diff --git a/go.sum b/go.sum index 9995d47a..d482f5fd 100644 --- a/go.sum +++ b/go.sum @@ -36,6 +36,8 @@ github.com/go-chi/chi v4.0.2+incompatible/go.mod h1:eB3wogJHnLi3x/kFX2A+IbTBlXxm github.com/go-chi/render v1.0.1 h1:4/5tis2cKaNdnv9zFLfXzcquC9HbeZgCnxGnKrltBS8= github.com/go-chi/render v1.0.1/go.mod h1:pq4Rr7HbnsdaeHagklXub+p6Wd16Af5l9koip1OvJns= github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= +github.com/go-kit/kit v0.9.0 h1:wDJmvq38kDhkVxi50ni9ykkdUr1PKgqKOoi01fa0Mdk= +github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE= github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= diff --git a/pkg/api/middleware/metrics.go b/pkg/api/middleware/metrics.go index b7517f29..edfc7897 100644 --- a/pkg/api/middleware/metrics.go +++ b/pkg/api/middleware/metrics.go @@ -18,22 +18,96 @@ package middleware import ( - "expvar" + "context" "net/http" "strings" + "sync" + "time" "github.com/go-chi/chi" + "github.com/go-kit/kit/metrics" + "github.com/go-kit/kit/metrics/expvar" ) -// HitCount update counts for each URL hit, key being a combination of a method and route pattern -func HitCount(counts *expvar.Map) func(http.Handler) http.Handler { +const metricPrefix = "urlPath." + +// Metrics struct contains url hit counts, response time and its histogram +type Metrics struct { + HitCounts metrics.Counter + ResponseTime metrics.Gauge + ResponseTimeHistogram metrics.Histogram +} + +// NewMetrics initialized metrics +func NewMetrics(key string) *Metrics { + + uniqueName := metricPrefix + key + + return &Metrics{ + HitCounts: expvar.NewCounter(uniqueName + ".counts"), + ResponseTime: expvar.NewGauge(uniqueName + ".responseTime"), + ResponseTimeHistogram: expvar.NewHistogram(uniqueName+".responseTimeHist", 50), + } +} + +// MetricsCollection holds the map of metrics by their keys +type MetricsCollection struct { + MetricMap map[string]*Metrics + + mlock sync.Mutex +} + +func NewMetricsCollection() MetricsCollection { + return MetricsCollection{MetricMap: map[string]*Metrics{}} + +} +func (mc MetricsCollection) getMetrics(key string) *Metrics { + mc.mlock.Lock() + defer mc.mlock.Unlock() + if metrics, ok := mc.MetricMap[key]; ok { + return metrics + } + metrics := NewMetrics(key) + mc.MetricMap[key] = metrics + return metrics +} + +// UpdateRouteMetrics update counts, total response time, and response time histogram +// for each URL hit, key being a combination of a method and route pattern +func UpdateRouteMetrics(metrics MetricsCollection) func(http.Handler) http.Handler { + f := func(h http.Handler) http.Handler { fn := func(w http.ResponseWriter, r *http.Request) { + key := r.Method + "_" + strings.ReplaceAll(chi.RouteContext(r.Context()).RoutePattern(), "/", "_") - counts.Add(key, 1) + singleMetric := metrics.getMetrics(key) + + singleMetric.HitCounts.Add(1) + ctx := r.Context() + startTime, ok := ctx.Value("responseTime").(time.Time) + if ok { + defer func() { + endTime := time.Now() + timeDiff := endTime.Sub(startTime).Seconds() + singleMetric.ResponseTime.Add(timeDiff) + singleMetric.ResponseTimeHistogram.Observe(timeDiff) + }() + } + h.ServeHTTP(w, r) } return http.HandlerFunc(fn) } return f } + +// SetTime middleware sets the start time in request context +func SetTime(next http.Handler) http.Handler { + + fn := func(w http.ResponseWriter, r *http.Request) { + + ctx := context.WithValue(r.Context(), "responseTime", time.Now()) + next.ServeHTTP(w, r.WithContext(ctx)) + } + return http.HandlerFunc(fn) +} diff --git a/pkg/api/middleware/metrics_test.go b/pkg/api/middleware/metrics_test.go index daa4315a..ae3c6139 100644 --- a/pkg/api/middleware/metrics_test.go +++ b/pkg/api/middleware/metrics_test.go @@ -43,9 +43,9 @@ type RequestMetrics struct { handler http.Handler } -func (rm *RequestMetrics) setRoute(metricsKey string) { +func (rm *RequestMetrics) SetupTest() { - metricsMap := expvar.NewMap(metricsKey) + metricsMap := NewMetricsCollection() rm.rw = httptest.NewRecorder() r := httptest.NewRequest("GET", "/", nil) @@ -53,7 +53,7 @@ func (rm *RequestMetrics) setRoute(metricsKey string) { rctx.RoutePatterns = []string{"/item/{set_item}"} rm.req = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) - rm.handler = http.Handler(HitCount(metricsMap)(getTestMetrics())) + rm.handler = http.Handler(UpdateRouteMetrics(metricsMap)(getTestMetrics())) } @@ -65,6 +65,10 @@ func (rm RequestMetrics) serveExpvarRoute() { expvar.Handler().ServeHTTP(rm.rw, rm.req) } +func (rm RequestMetrics) serveSetTimehHandler() { + http.Handler(SetTime(getTestMetrics())).ServeHTTP(rm.rw, rm.req) +} + func (rm RequestMetrics) getMetricsMap() JSON { var expVarMap JSON err := json.Unmarshal(rm.rw.(*httptest.ResponseRecorder).Body.Bytes(), &expVarMap) @@ -77,11 +81,11 @@ func (rm RequestMetrics) getCode() int { return rm.rw.(*httptest.ResponseRecorder).Code } -func (suite *RequestMetrics) TestUpdateMetricsHitOnce() { +var sufixList = []string{".counts", ".responseTime", ".responseTimeHist.p50", ".responseTimeHist.p90", ".responseTimeHist.p95", ".responseTimeHist.p99"} - var metricsKey = "counter" +func (suite *RequestMetrics) TestUpdateMetricsHitOnce() { - suite.setRoute(metricsKey) + suite.serveSetTimehHandler() suite.serveRoute() suite.Equal(http.StatusOK, suite.getCode(), "Status code differs") @@ -89,25 +93,19 @@ func (suite *RequestMetrics) TestUpdateMetricsHitOnce() { suite.serveExpvarRoute() expVarMap := suite.getMetricsMap() + for _, item := range sufixList { + expectedKey := metricPrefix + "GET__item_{set_item}" + item + value, ok := expVarMap[expectedKey] + suite.True(ok) - counterMap, ok := expVarMap[metricsKey] - suite.True(ok) - - suite.Contains(counterMap, "GET__item_{set_item}") - - m := counterMap.(map[string]interface{}) - - suite.Equal(1.0, m["GET__item_{set_item}"]) - + suite.NotEqual(0.0, value) + } } func (suite *RequestMetrics) TestUpdateMetricsHitMultiple() { - var metricsKey = "counter1" const hitNumber = 10.0 - suite.setRoute(metricsKey) - for i := 0; i < hitNumber; i++ { suite.serveRoute() } @@ -118,14 +116,25 @@ func (suite *RequestMetrics) TestUpdateMetricsHitMultiple() { expVarMap := suite.getMetricsMap() - counterMap, ok := expVarMap[metricsKey] + expectedKey := metricPrefix + "GET__item_{set_item}.counts" + value, ok := expVarMap[expectedKey] suite.True(ok) - suite.Contains(counterMap, "GET__item_{set_item}") + suite.NotEqual(0.0, value) +} - m := counterMap.(map[string]interface{}) +func (suite *RequestMetrics) TestGetMetrics() { + metricsColl := NewMetricsCollection() + suite.NotNil(metricsColl.MetricMap) + suite.Empty(metricsColl.MetricMap) - suite.Equal(hitNumber, m["GET__item_{set_item}"]) + metricsColl.GetMetrics("some_key") + suite.NotNil(metricsColl.MetricMap) + suite.NotEmpty(metricsColl.MetricMap) + + _, ok := metricsColl.MetricMap["some_key"] + + suite.True(ok) } diff --git a/pkg/api/router.go b/pkg/api/router.go index 5926eefc..3a0b4564 100644 --- a/pkg/api/router.go +++ b/pkg/api/router.go @@ -18,8 +18,6 @@ package api import ( - "expvar" - "github.com/optimizely/sidedoor/pkg/api/handlers" "github.com/optimizely/sidedoor/pkg/api/middleware" "github.com/optimizely/sidedoor/pkg/optimizely" @@ -49,31 +47,29 @@ func NewDefaultRouter(optlyCache optimizely.Cache) *chi.Mux { return NewRouter(spec) } -const metricsPrefix = "route_counters" - -var routeCounts = expvar.NewMap(metricsPrefix) - // NewRouter returns HTTP API router backed by an optimizely.Cache implementation func NewRouter(opt *RouterOptions) *chi.Mux { r := chi.NewRouter() + metricsCollection := middleware.NewMetricsCollection() + r.Use(middleware.SetTime) r.Use(render.SetContentType(render.ContentTypeJSON), middleware.SetRequestID) - r.With(chimw.AllowContentType("application/json"), middleware.HitCount(routeCounts)).Post("/user-event", opt.userEventAPI.AddUserEvent) + r.With(chimw.AllowContentType("application/json"), middleware.UpdateRouteMetrics(metricsCollection)).Post("/user-event", opt.userEventAPI.AddUserEvent) r.Route("/features", func(r chi.Router) { r.Use(opt.middleware.ClientCtx) - r.With(middleware.HitCount(routeCounts)).Get("/", opt.featureAPI.ListFeatures) - r.With(middleware.HitCount(routeCounts)).Get("/{featureKey}", opt.featureAPI.GetFeature) + r.With(middleware.UpdateRouteMetrics(metricsCollection)).Get("/", opt.featureAPI.ListFeatures) + r.With(middleware.UpdateRouteMetrics(metricsCollection)).Get("/{featureKey}", opt.featureAPI.GetFeature) }) r.Route("/users/{userID}", func(r chi.Router) { r.Use(opt.middleware.ClientCtx, opt.middleware.UserCtx) - r.With(middleware.HitCount(routeCounts)).Post("/events/{eventKey}", opt.userAPI.TrackEvent) + r.With(middleware.UpdateRouteMetrics(metricsCollection)).Post("/events/{eventKey}", opt.userAPI.TrackEvent) - r.With(middleware.HitCount(routeCounts)).Get("/features/{featureKey}", opt.userAPI.GetFeature) - r.With(middleware.HitCount(routeCounts)).Post("/features/{featureKey}", opt.userAPI.TrackFeature) + r.With(middleware.UpdateRouteMetrics(metricsCollection)).Get("/features/{featureKey}", opt.userAPI.GetFeature) + r.With(middleware.UpdateRouteMetrics(metricsCollection)).Post("/features/{featureKey}", opt.userAPI.TrackFeature) }) return r From fe037100e556dcffb1907a7526391e5278bece7b Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Tue, 5 Nov 2019 18:00:06 -0800 Subject: [PATCH 07/14] small change --- pkg/api/middleware/metrics_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/api/middleware/metrics_test.go b/pkg/api/middleware/metrics_test.go index ae3c6139..8f674195 100644 --- a/pkg/api/middleware/metrics_test.go +++ b/pkg/api/middleware/metrics_test.go @@ -128,7 +128,7 @@ func (suite *RequestMetrics) TestGetMetrics() { suite.NotNil(metricsColl.MetricMap) suite.Empty(metricsColl.MetricMap) - metricsColl.GetMetrics("some_key") + metricsColl.getMetrics("some_key") suite.NotNil(metricsColl.MetricMap) suite.NotEmpty(metricsColl.MetricMap) From 1f266f31a3864269a23205f0be687ef92ec3d34f Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Tue, 5 Nov 2019 18:17:23 -0800 Subject: [PATCH 08/14] some linter fixes --- pkg/api/middleware/metrics.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/api/middleware/metrics.go b/pkg/api/middleware/metrics.go index edfc7897..4e91a799 100644 --- a/pkg/api/middleware/metrics.go +++ b/pkg/api/middleware/metrics.go @@ -54,9 +54,10 @@ func NewMetrics(key string) *Metrics { type MetricsCollection struct { MetricMap map[string]*Metrics - mlock sync.Mutex + mlock *sync.Mutex } +// NewMetricsCollection initializes metric collection map func NewMetricsCollection() MetricsCollection { return MetricsCollection{MetricMap: map[string]*Metrics{}} @@ -64,23 +65,23 @@ func NewMetricsCollection() MetricsCollection { func (mc MetricsCollection) getMetrics(key string) *Metrics { mc.mlock.Lock() defer mc.mlock.Unlock() - if metrics, ok := mc.MetricMap[key]; ok { - return metrics + if stats, ok := mc.MetricMap[key]; ok { + return stats } - metrics := NewMetrics(key) - mc.MetricMap[key] = metrics - return metrics + stats := NewMetrics(key) + mc.MetricMap[key] = stats + return stats } // UpdateRouteMetrics update counts, total response time, and response time histogram // for each URL hit, key being a combination of a method and route pattern -func UpdateRouteMetrics(metrics MetricsCollection) func(http.Handler) http.Handler { +func UpdateRouteMetrics(stats MetricsCollection) func(http.Handler) http.Handler { f := func(h http.Handler) http.Handler { fn := func(w http.ResponseWriter, r *http.Request) { key := r.Method + "_" + strings.ReplaceAll(chi.RouteContext(r.Context()).RoutePattern(), "/", "_") - singleMetric := metrics.getMetrics(key) + singleMetric := stats.getMetrics(key) singleMetric.HitCounts.Add(1) ctx := r.Context() From a778202c767f79e12d698374d2b8d80171921e8a Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Tue, 5 Nov 2019 19:11:06 -0800 Subject: [PATCH 09/14] adjusting tests and linters --- pkg/api/middleware/metrics.go | 14 +++++++++----- pkg/api/middleware/metrics_test.go | 17 +++++++++-------- pkg/api/router.go | 12 ++++++------ 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/pkg/api/middleware/metrics.go b/pkg/api/middleware/metrics.go index 4e91a799..1b2fe9f0 100644 --- a/pkg/api/middleware/metrics.go +++ b/pkg/api/middleware/metrics.go @@ -31,6 +31,10 @@ import ( const metricPrefix = "urlPath." +type contextString string + +const responseTime = contextString("responseTime") + // Metrics struct contains url hit counts, response time and its histogram type Metrics struct { HitCounts metrics.Counter @@ -54,7 +58,7 @@ func NewMetrics(key string) *Metrics { type MetricsCollection struct { MetricMap map[string]*Metrics - mlock *sync.Mutex + mlock sync.Mutex } // NewMetricsCollection initializes metric collection map @@ -62,7 +66,7 @@ func NewMetricsCollection() MetricsCollection { return MetricsCollection{MetricMap: map[string]*Metrics{}} } -func (mc MetricsCollection) getMetrics(key string) *Metrics { +func (mc *MetricsCollection) getMetrics(key string) *Metrics { mc.mlock.Lock() defer mc.mlock.Unlock() if stats, ok := mc.MetricMap[key]; ok { @@ -75,7 +79,7 @@ func (mc MetricsCollection) getMetrics(key string) *Metrics { // UpdateRouteMetrics update counts, total response time, and response time histogram // for each URL hit, key being a combination of a method and route pattern -func UpdateRouteMetrics(stats MetricsCollection) func(http.Handler) http.Handler { +func UpdateRouteMetrics(stats *MetricsCollection) func(http.Handler) http.Handler { f := func(h http.Handler) http.Handler { fn := func(w http.ResponseWriter, r *http.Request) { @@ -85,7 +89,7 @@ func UpdateRouteMetrics(stats MetricsCollection) func(http.Handler) http.Handler singleMetric.HitCounts.Add(1) ctx := r.Context() - startTime, ok := ctx.Value("responseTime").(time.Time) + startTime, ok := ctx.Value(responseTime).(time.Time) if ok { defer func() { endTime := time.Now() @@ -107,7 +111,7 @@ func SetTime(next http.Handler) http.Handler { fn := func(w http.ResponseWriter, r *http.Request) { - ctx := context.WithValue(r.Context(), "responseTime", time.Now()) + ctx := context.WithValue(r.Context(), responseTime, time.Now()) next.ServeHTTP(w, r.WithContext(ctx)) } return http.HandlerFunc(fn) diff --git a/pkg/api/middleware/metrics_test.go b/pkg/api/middleware/metrics_test.go index 8f674195..1157be72 100644 --- a/pkg/api/middleware/metrics_test.go +++ b/pkg/api/middleware/metrics_test.go @@ -24,6 +24,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/go-chi/chi" "github.com/stretchr/testify/suite" @@ -43,9 +44,10 @@ type RequestMetrics struct { handler http.Handler } +var metricsMap = NewMetricsCollection() + func (rm *RequestMetrics) SetupTest() { - metricsMap := NewMetricsCollection() rm.rw = httptest.NewRecorder() r := httptest.NewRequest("GET", "/", nil) @@ -53,7 +55,8 @@ func (rm *RequestMetrics) SetupTest() { rctx.RoutePatterns = []string{"/item/{set_item}"} rm.req = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) - rm.handler = http.Handler(UpdateRouteMetrics(metricsMap)(getTestMetrics())) + rm.req = r.WithContext(context.WithValue(rm.req.Context(), responseTime, time.Now())) + rm.handler = http.Handler(UpdateRouteMetrics(&metricsMap)(getTestMetrics())) } @@ -61,14 +64,14 @@ func (rm RequestMetrics) serveRoute() { rm.handler.ServeHTTP(rm.rw, rm.req) } -func (rm RequestMetrics) serveExpvarRoute() { - expvar.Handler().ServeHTTP(rm.rw, rm.req) -} - func (rm RequestMetrics) serveSetTimehHandler() { http.Handler(SetTime(getTestMetrics())).ServeHTTP(rm.rw, rm.req) } +func (rm RequestMetrics) serveExpvarRoute() { + expvar.Handler().ServeHTTP(rm.rw, rm.req) +} + func (rm RequestMetrics) getMetricsMap() JSON { var expVarMap JSON err := json.Unmarshal(rm.rw.(*httptest.ResponseRecorder).Body.Bytes(), &expVarMap) @@ -85,11 +88,9 @@ var sufixList = []string{".counts", ".responseTime", ".responseTimeHist.p50", ". func (suite *RequestMetrics) TestUpdateMetricsHitOnce() { - suite.serveSetTimehHandler() suite.serveRoute() suite.Equal(http.StatusOK, suite.getCode(), "Status code differs") - suite.serveExpvarRoute() expVarMap := suite.getMetricsMap() diff --git a/pkg/api/router.go b/pkg/api/router.go index 3a0b4564..9878a277 100644 --- a/pkg/api/router.go +++ b/pkg/api/router.go @@ -55,21 +55,21 @@ func NewRouter(opt *RouterOptions) *chi.Mux { r.Use(middleware.SetTime) r.Use(render.SetContentType(render.ContentTypeJSON), middleware.SetRequestID) - r.With(chimw.AllowContentType("application/json"), middleware.UpdateRouteMetrics(metricsCollection)).Post("/user-event", opt.userEventAPI.AddUserEvent) + r.With(chimw.AllowContentType("application/json"), middleware.UpdateRouteMetrics(&metricsCollection)).Post("/user-event", opt.userEventAPI.AddUserEvent) r.Route("/features", func(r chi.Router) { r.Use(opt.middleware.ClientCtx) - r.With(middleware.UpdateRouteMetrics(metricsCollection)).Get("/", opt.featureAPI.ListFeatures) - r.With(middleware.UpdateRouteMetrics(metricsCollection)).Get("/{featureKey}", opt.featureAPI.GetFeature) + r.With(middleware.UpdateRouteMetrics(&metricsCollection)).Get("/", opt.featureAPI.ListFeatures) + r.With(middleware.UpdateRouteMetrics(&metricsCollection)).Get("/{featureKey}", opt.featureAPI.GetFeature) }) r.Route("/users/{userID}", func(r chi.Router) { r.Use(opt.middleware.ClientCtx, opt.middleware.UserCtx) - r.With(middleware.UpdateRouteMetrics(metricsCollection)).Post("/events/{eventKey}", opt.userAPI.TrackEvent) + r.With(middleware.UpdateRouteMetrics(&metricsCollection)).Post("/events/{eventKey}", opt.userAPI.TrackEvent) - r.With(middleware.UpdateRouteMetrics(metricsCollection)).Get("/features/{featureKey}", opt.userAPI.GetFeature) - r.With(middleware.UpdateRouteMetrics(metricsCollection)).Post("/features/{featureKey}", opt.userAPI.TrackFeature) + r.With(middleware.UpdateRouteMetrics(&metricsCollection)).Get("/features/{featureKey}", opt.userAPI.GetFeature) + r.With(middleware.UpdateRouteMetrics(&metricsCollection)).Post("/features/{featureKey}", opt.userAPI.TrackFeature) }) return r From 117433dff71ac1acdca60f6921bb375e636a2be2 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Wed, 6 Nov 2019 13:42:05 -0800 Subject: [PATCH 10/14] simplifying after review --- pkg/api/middleware/metrics.go | 35 ++++-------------------------- pkg/api/middleware/metrics_test.go | 30 ++++--------------------- pkg/api/router.go | 13 +++++------ 3 files changed, 14 insertions(+), 64 deletions(-) diff --git a/pkg/api/middleware/metrics.go b/pkg/api/middleware/metrics.go index 1b2fe9f0..9405c72d 100644 --- a/pkg/api/middleware/metrics.go +++ b/pkg/api/middleware/metrics.go @@ -20,16 +20,13 @@ package middleware import ( "context" "net/http" - "strings" - "sync" "time" - "github.com/go-chi/chi" "github.com/go-kit/kit/metrics" "github.com/go-kit/kit/metrics/expvar" ) -const metricPrefix = "urlPath." +const metricPrefix = "timers." type contextString string @@ -54,38 +51,14 @@ func NewMetrics(key string) *Metrics { } } -// MetricsCollection holds the map of metrics by their keys -type MetricsCollection struct { - MetricMap map[string]*Metrics - - mlock sync.Mutex -} - -// NewMetricsCollection initializes metric collection map -func NewMetricsCollection() MetricsCollection { - return MetricsCollection{MetricMap: map[string]*Metrics{}} - -} -func (mc *MetricsCollection) getMetrics(key string) *Metrics { - mc.mlock.Lock() - defer mc.mlock.Unlock() - if stats, ok := mc.MetricMap[key]; ok { - return stats - } - stats := NewMetrics(key) - mc.MetricMap[key] = stats - return stats -} - // UpdateRouteMetrics update counts, total response time, and response time histogram // for each URL hit, key being a combination of a method and route pattern -func UpdateRouteMetrics(stats *MetricsCollection) func(http.Handler) http.Handler { +func UpdateRouteMetrics(key string) func(http.Handler) http.Handler { + singleMetric := NewMetrics(key) f := func(h http.Handler) http.Handler { - fn := func(w http.ResponseWriter, r *http.Request) { - key := r.Method + "_" + strings.ReplaceAll(chi.RouteContext(r.Context()).RoutePattern(), "/", "_") - singleMetric := stats.getMetrics(key) + fn := func(w http.ResponseWriter, r *http.Request) { singleMetric.HitCounts.Add(1) ctx := r.Context() diff --git a/pkg/api/middleware/metrics_test.go b/pkg/api/middleware/metrics_test.go index 1157be72..4de0582b 100644 --- a/pkg/api/middleware/metrics_test.go +++ b/pkg/api/middleware/metrics_test.go @@ -26,7 +26,6 @@ import ( "testing" "time" - "github.com/go-chi/chi" "github.com/stretchr/testify/suite" ) @@ -44,19 +43,13 @@ type RequestMetrics struct { handler http.Handler } -var metricsMap = NewMetricsCollection() - func (rm *RequestMetrics) SetupTest() { rm.rw = httptest.NewRecorder() r := httptest.NewRequest("GET", "/", nil) - rctx := chi.NewRouteContext() - rctx.RoutePatterns = []string{"/item/{set_item}"} - - rm.req = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) - rm.req = r.WithContext(context.WithValue(rm.req.Context(), responseTime, time.Now())) - rm.handler = http.Handler(UpdateRouteMetrics(&metricsMap)(getTestMetrics())) + rm.req = r.WithContext(context.WithValue(r.Context(), responseTime, time.Now())) + rm.handler = http.Handler(UpdateRouteMetrics("some_key")(getTestMetrics())) } @@ -95,7 +88,7 @@ func (suite *RequestMetrics) TestUpdateMetricsHitOnce() { expVarMap := suite.getMetricsMap() for _, item := range sufixList { - expectedKey := metricPrefix + "GET__item_{set_item}" + item + expectedKey := metricPrefix + "some_key" + item value, ok := expVarMap[expectedKey] suite.True(ok) @@ -117,28 +110,13 @@ func (suite *RequestMetrics) TestUpdateMetricsHitMultiple() { expVarMap := suite.getMetricsMap() - expectedKey := metricPrefix + "GET__item_{set_item}.counts" + expectedKey := metricPrefix + "some_key.counts" value, ok := expVarMap[expectedKey] suite.True(ok) suite.NotEqual(0.0, value) } -func (suite *RequestMetrics) TestGetMetrics() { - metricsColl := NewMetricsCollection() - suite.NotNil(metricsColl.MetricMap) - suite.Empty(metricsColl.MetricMap) - - metricsColl.getMetrics("some_key") - suite.NotNil(metricsColl.MetricMap) - suite.NotEmpty(metricsColl.MetricMap) - - _, ok := metricsColl.MetricMap["some_key"] - - suite.True(ok) - -} - func TestRequestMetrics(t *testing.T) { suite.Run(t, new(RequestMetrics)) } diff --git a/pkg/api/router.go b/pkg/api/router.go index 9878a277..835e8a52 100644 --- a/pkg/api/router.go +++ b/pkg/api/router.go @@ -50,26 +50,25 @@ func NewDefaultRouter(optlyCache optimizely.Cache) *chi.Mux { // NewRouter returns HTTP API router backed by an optimizely.Cache implementation func NewRouter(opt *RouterOptions) *chi.Mux { r := chi.NewRouter() - metricsCollection := middleware.NewMetricsCollection() r.Use(middleware.SetTime) r.Use(render.SetContentType(render.ContentTypeJSON), middleware.SetRequestID) - r.With(chimw.AllowContentType("application/json"), middleware.UpdateRouteMetrics(&metricsCollection)).Post("/user-event", opt.userEventAPI.AddUserEvent) + r.With(chimw.AllowContentType("application/json"), middleware.UpdateRouteMetrics("user-event")).Post("/user-event", opt.userEventAPI.AddUserEvent) r.Route("/features", func(r chi.Router) { r.Use(opt.middleware.ClientCtx) - r.With(middleware.UpdateRouteMetrics(&metricsCollection)).Get("/", opt.featureAPI.ListFeatures) - r.With(middleware.UpdateRouteMetrics(&metricsCollection)).Get("/{featureKey}", opt.featureAPI.GetFeature) + r.With(middleware.UpdateRouteMetrics("list-features")).Get("/", opt.featureAPI.ListFeatures) + r.With(middleware.UpdateRouteMetrics("get-feature")).Get("/{featureKey}", opt.featureAPI.GetFeature) }) r.Route("/users/{userID}", func(r chi.Router) { r.Use(opt.middleware.ClientCtx, opt.middleware.UserCtx) - r.With(middleware.UpdateRouteMetrics(&metricsCollection)).Post("/events/{eventKey}", opt.userAPI.TrackEvent) + r.With(middleware.UpdateRouteMetrics("track-event")).Post("/events/{eventKey}", opt.userAPI.TrackEvent) - r.With(middleware.UpdateRouteMetrics(&metricsCollection)).Get("/features/{featureKey}", opt.userAPI.GetFeature) - r.With(middleware.UpdateRouteMetrics(&metricsCollection)).Post("/features/{featureKey}", opt.userAPI.TrackFeature) + r.With(middleware.UpdateRouteMetrics("get-user-feature")).Get("/features/{featureKey}", opt.userAPI.GetFeature) + r.With(middleware.UpdateRouteMetrics("track-user-feature")).Post("/features/{featureKey}", opt.userAPI.TrackFeature) }) return r From 82914efa067f5ef725bcc9e51489ff1d7269d3ee Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Wed, 6 Nov 2019 13:44:35 -0800 Subject: [PATCH 11/14] change Response time to Counter --- pkg/api/middleware/metrics.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/api/middleware/metrics.go b/pkg/api/middleware/metrics.go index 9405c72d..fbfcdbf1 100644 --- a/pkg/api/middleware/metrics.go +++ b/pkg/api/middleware/metrics.go @@ -35,7 +35,7 @@ const responseTime = contextString("responseTime") // Metrics struct contains url hit counts, response time and its histogram type Metrics struct { HitCounts metrics.Counter - ResponseTime metrics.Gauge + ResponseTime metrics.Counter ResponseTimeHistogram metrics.Histogram } @@ -46,7 +46,7 @@ func NewMetrics(key string) *Metrics { return &Metrics{ HitCounts: expvar.NewCounter(uniqueName + ".counts"), - ResponseTime: expvar.NewGauge(uniqueName + ".responseTime"), + ResponseTime: expvar.NewCounter(uniqueName + ".responseTime"), ResponseTimeHistogram: expvar.NewHistogram(uniqueName+".responseTimeHist", 50), } } From 723b0065dc34fc4affcc0ed66a9e82265d49cc62 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Wed, 6 Nov 2019 13:48:54 -0800 Subject: [PATCH 12/14] adjusted expvar keys --- pkg/api/middleware/metrics_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/api/middleware/metrics_test.go b/pkg/api/middleware/metrics_test.go index 4de0582b..50962dd8 100644 --- a/pkg/api/middleware/metrics_test.go +++ b/pkg/api/middleware/metrics_test.go @@ -43,13 +43,13 @@ type RequestMetrics struct { handler http.Handler } -func (rm *RequestMetrics) SetupTest() { +func (rm *RequestMetrics) SetupRoute(key string) { rm.rw = httptest.NewRecorder() r := httptest.NewRequest("GET", "/", nil) rm.req = r.WithContext(context.WithValue(r.Context(), responseTime, time.Now())) - rm.handler = http.Handler(UpdateRouteMetrics("some_key")(getTestMetrics())) + rm.handler = http.Handler(UpdateRouteMetrics(key)(getTestMetrics())) } @@ -81,6 +81,8 @@ var sufixList = []string{".counts", ".responseTime", ".responseTimeHist.p50", ". func (suite *RequestMetrics) TestUpdateMetricsHitOnce() { + suite.SetupRoute("some_key") + suite.serveRoute() suite.Equal(http.StatusOK, suite.getCode(), "Status code differs") @@ -100,6 +102,8 @@ func (suite *RequestMetrics) TestUpdateMetricsHitMultiple() { const hitNumber = 10.0 + suite.SetupRoute("different_key") + for i := 0; i < hitNumber; i++ { suite.serveRoute() } @@ -110,7 +114,7 @@ func (suite *RequestMetrics) TestUpdateMetricsHitMultiple() { expVarMap := suite.getMetricsMap() - expectedKey := metricPrefix + "some_key.counts" + expectedKey := metricPrefix + "different_key.counts" value, ok := expVarMap[expectedKey] suite.True(ok) From 5ee181e1a55fcc8f46c9bf1e5f379381ba2ee724 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Wed, 6 Nov 2019 14:49:26 -0800 Subject: [PATCH 13/14] making sure the chi router is defined only one time. --- pkg/api/router_test.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/pkg/api/router_test.go b/pkg/api/router_test.go index 29d8366a..afbbcadc 100644 --- a/pkg/api/router_test.go +++ b/pkg/api/router_test.go @@ -21,6 +21,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "sync" "testing" "github.com/go-chi/chi" @@ -98,18 +99,23 @@ type RouterTestSuite struct { mux *chi.Mux } +var once sync.Once + func (suite *RouterTestSuite) SetupTest() { - testClient := optimizelytest.NewClient() - suite.tc = testClient - - opts := &RouterOptions{ - featureAPI: new(MockFeatureAPI), - userEventAPI: new(MockUserEventAPI), - userAPI: new(MockUserAPI), - middleware: new(MockOptlyMiddleware), - } - suite.mux = NewRouter(opts) + once.Do(func() { + testClient := optimizelytest.NewClient() + suite.tc = testClient + + opts := &RouterOptions{ + featureAPI: new(MockFeatureAPI), + userEventAPI: new(MockUserEventAPI), + userAPI: new(MockUserAPI), + middleware: new(MockOptlyMiddleware), + } + + suite.mux = NewRouter(opts) + }) } func (suite *RouterTestSuite) TestListFeatures() { From be3cbf57df04507f5156c0651b408538e98f0b53 Mon Sep 17 00:00:00 2001 From: Pawel Szczodruch Date: Wed, 6 Nov 2019 16:02:22 -0800 Subject: [PATCH 14/14] small change after PR --- pkg/api/middleware/metrics.go | 4 ++-- pkg/api/middleware/metrics_test.go | 2 +- pkg/api/router.go | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/api/middleware/metrics.go b/pkg/api/middleware/metrics.go index fbfcdbf1..fa499fcd 100644 --- a/pkg/api/middleware/metrics.go +++ b/pkg/api/middleware/metrics.go @@ -51,9 +51,9 @@ func NewMetrics(key string) *Metrics { } } -// UpdateRouteMetrics update counts, total response time, and response time histogram +// Metricize updates counts, total response time, and response time histogram // for each URL hit, key being a combination of a method and route pattern -func UpdateRouteMetrics(key string) func(http.Handler) http.Handler { +func Metricize(key string) func(http.Handler) http.Handler { singleMetric := NewMetrics(key) f := func(h http.Handler) http.Handler { diff --git a/pkg/api/middleware/metrics_test.go b/pkg/api/middleware/metrics_test.go index 50962dd8..e968af7f 100644 --- a/pkg/api/middleware/metrics_test.go +++ b/pkg/api/middleware/metrics_test.go @@ -49,7 +49,7 @@ func (rm *RequestMetrics) SetupRoute(key string) { r := httptest.NewRequest("GET", "/", nil) rm.req = r.WithContext(context.WithValue(r.Context(), responseTime, time.Now())) - rm.handler = http.Handler(UpdateRouteMetrics(key)(getTestMetrics())) + rm.handler = http.Handler(Metricize(key)(getTestMetrics())) } diff --git a/pkg/api/router.go b/pkg/api/router.go index 835e8a52..543a52df 100644 --- a/pkg/api/router.go +++ b/pkg/api/router.go @@ -54,21 +54,21 @@ func NewRouter(opt *RouterOptions) *chi.Mux { r.Use(middleware.SetTime) r.Use(render.SetContentType(render.ContentTypeJSON), middleware.SetRequestID) - r.With(chimw.AllowContentType("application/json"), middleware.UpdateRouteMetrics("user-event")).Post("/user-event", opt.userEventAPI.AddUserEvent) + r.With(chimw.AllowContentType("application/json"), middleware.Metricize("user-event")).Post("/user-event", opt.userEventAPI.AddUserEvent) r.Route("/features", func(r chi.Router) { r.Use(opt.middleware.ClientCtx) - r.With(middleware.UpdateRouteMetrics("list-features")).Get("/", opt.featureAPI.ListFeatures) - r.With(middleware.UpdateRouteMetrics("get-feature")).Get("/{featureKey}", opt.featureAPI.GetFeature) + r.With(middleware.Metricize("list-features")).Get("/", opt.featureAPI.ListFeatures) + r.With(middleware.Metricize("get-feature")).Get("/{featureKey}", opt.featureAPI.GetFeature) }) r.Route("/users/{userID}", func(r chi.Router) { r.Use(opt.middleware.ClientCtx, opt.middleware.UserCtx) - r.With(middleware.UpdateRouteMetrics("track-event")).Post("/events/{eventKey}", opt.userAPI.TrackEvent) + r.With(middleware.Metricize("track-event")).Post("/events/{eventKey}", opt.userAPI.TrackEvent) - r.With(middleware.UpdateRouteMetrics("get-user-feature")).Get("/features/{featureKey}", opt.userAPI.GetFeature) - r.With(middleware.UpdateRouteMetrics("track-user-feature")).Post("/features/{featureKey}", opt.userAPI.TrackFeature) + r.With(middleware.Metricize("get-user-feature")).Get("/features/{featureKey}", opt.userAPI.GetFeature) + r.With(middleware.Metricize("track-user-feature")).Post("/features/{featureKey}", opt.userAPI.TrackFeature) }) return r