From f663668ba61773470734392af18c92b9dbc49eeb Mon Sep 17 00:00:00 2001 From: Douglas Danger Manley Date: Wed, 20 Jul 2022 11:17:24 -0400 Subject: [PATCH] Add config for the gorestful middleware One of the benefits of using gorestful is its fancy routes, which let you specify "path parameters" such as "/users/{id}". This change adds a "Config" structure for the gorestful middleware that allows you to use the gorestful route path and not the actual request path. For backward compatibility, this introduces the "HandlerWithConfig" function, and the existing "Handler" function calls "HandlerWithConfig" with the default "Config". The example has been updated to use "HandlerWithConfig". --- middleware/gorestful/example_test.go | 2 +- middleware/gorestful/gorestful.go | 26 ++++-- middleware/gorestful/gorestful_test.go | 108 ++++++++++++++++++++++++- 3 files changed, 126 insertions(+), 10 deletions(-) diff --git a/middleware/gorestful/example_test.go b/middleware/gorestful/example_test.go index ccccbbc..f7541b7 100644 --- a/middleware/gorestful/example_test.go +++ b/middleware/gorestful/example_test.go @@ -24,7 +24,7 @@ func Example_gorestfulMiddleware() { c := gorestful.NewContainer() // Add the middleware for all routes. - c.Filter(gorestfulmiddleware.Handler("", mdlw)) + c.Filter(gorestfulmiddleware.HandlerWithConfig("", mdlw, gorestfulmiddleware.Config{})) // Add our handler, ws := &gorestful.WebService{} diff --git a/middleware/gorestful/gorestful.go b/middleware/gorestful/gorestful.go index 4cfe76e..e4ce99b 100644 --- a/middleware/gorestful/gorestful.go +++ b/middleware/gorestful/gorestful.go @@ -9,10 +9,20 @@ import ( "github.com/slok/go-http-metrics/middleware" ) -// Handler returns a gorestful measuring middleware. +// Config is the configuration for the gorestful middleware. +type Config struct { + UseRoutePath bool // When true, aggregate requests by their route path. For example, "/users/{id}" instead of "/users/1", "/users/2", etc. +} + +// Handler returns a gorestful measuring middleware with the default config. func Handler(handlerID string, m middleware.Middleware) gorestful.FilterFunction { + return HandlerWithConfig(handlerID, m, Config{}) +} + +// HandlerWithConfig returns a gorestful measuring middleware. +func HandlerWithConfig(handlerID string, m middleware.Middleware, config Config) gorestful.FilterFunction { return func(req *gorestful.Request, resp *gorestful.Response, chain *gorestful.FilterChain) { - r := &reporter{req: req, resp: resp} + r := &reporter{req: req, resp: resp, config: config} m.Measure(handlerID, r, func() { chain.ProcessFilter(req, resp) }) @@ -20,15 +30,21 @@ func Handler(handlerID string, m middleware.Middleware) gorestful.FilterFunction } type reporter struct { - req *gorestful.Request - resp *gorestful.Response + req *gorestful.Request + resp *gorestful.Response + config Config } func (r *reporter) Method() string { return r.req.Request.Method } func (r *reporter) Context() context.Context { return r.req.Request.Context() } -func (r *reporter) URLPath() string { return r.req.Request.URL.Path } +func (r *reporter) URLPath() string { + if r.config.UseRoutePath { + return r.req.SelectedRoutePath() + } + return r.req.Request.URL.Path +} func (r *reporter) StatusCode() int { return r.resp.StatusCode() } diff --git a/middleware/gorestful/gorestful_test.go b/middleware/gorestful/gorestful_test.go index f94482e..959e022 100644 --- a/middleware/gorestful/gorestful_test.go +++ b/middleware/gorestful/gorestful_test.go @@ -19,8 +19,9 @@ import ( func TestMiddleware(t *testing.T) { tests := map[string]struct { - handlerID string - config middleware.Config + handlerID string // This is the name of the handler. If empty, the path will be used. + route string // This is the route path to use go-restful. + config gorestfulmiddleware.Config // This is the go-restful middleware config. req func() *http.Request mock func(m *mmetrics.Recorder) handler func() gorestful.RouteFunction @@ -28,6 +29,8 @@ func TestMiddleware(t *testing.T) { expRespBody string }{ "A default HTTP middleware should call the recorder to measure.": { + route: "/test", + config: gorestfulmiddleware.Config{}, req: func() *http.Request { return httptest.NewRequest(http.MethodPost, "/test", nil) }, @@ -57,6 +60,103 @@ func TestMiddleware(t *testing.T) { expRespCode: 202, expRespBody: "test1", }, + "The handler ID overrides the path.": { + handlerID: "my-handler", + route: "/test/{id}", + req: func() *http.Request { + return httptest.NewRequest(http.MethodPost, "/test/1", nil) + }, + mock: func(m *mmetrics.Recorder) { + expHTTPReqProps := metrics.HTTPReqProperties{ + ID: "my-handler", + Service: "", + Method: "POST", + Code: "202", + } + m.On("ObserveHTTPRequestDuration", mock.Anything, expHTTPReqProps, mock.Anything).Once() + m.On("ObserveHTTPResponseSize", mock.Anything, expHTTPReqProps, int64(5)).Once() + + expHTTPProps := metrics.HTTPProperties{ + ID: "my-handler", + Service: "", + } + m.On("AddInflightRequests", mock.Anything, expHTTPProps, 1).Once() + m.On("AddInflightRequests", mock.Anything, expHTTPProps, -1).Once() + }, + handler: func() gorestful.RouteFunction { + return gorestful.RouteFunction(func(_ *gorestful.Request, resp *gorestful.Response) { + resp.WriteHeader(202) + resp.Write([]byte("test1")) // nolint: errcheck + }) + }, + expRespCode: 202, + expRespBody: "test1", + }, + "The full path is used by default.": { + route: "/test/{id}", + req: func() *http.Request { + return httptest.NewRequest(http.MethodPost, "/test/1", nil) + }, + mock: func(m *mmetrics.Recorder) { + expHTTPReqProps := metrics.HTTPReqProperties{ + ID: "/test/1", + Service: "", + Method: "POST", + Code: "202", + } + m.On("ObserveHTTPRequestDuration", mock.Anything, expHTTPReqProps, mock.Anything).Once() + m.On("ObserveHTTPResponseSize", mock.Anything, expHTTPReqProps, int64(5)).Once() + + expHTTPProps := metrics.HTTPProperties{ + ID: "/test/1", + Service: "", + } + m.On("AddInflightRequests", mock.Anything, expHTTPProps, 1).Once() + m.On("AddInflightRequests", mock.Anything, expHTTPProps, -1).Once() + }, + handler: func() gorestful.RouteFunction { + return gorestful.RouteFunction(func(_ *gorestful.Request, resp *gorestful.Response) { + resp.WriteHeader(202) + resp.Write([]byte("test1")) // nolint: errcheck + }) + }, + expRespCode: 202, + expRespBody: "test1", + }, + "The route path is used when desired.": { + route: "/test/{id}", + config: gorestfulmiddleware.Config{ + UseRoutePath: true, + }, + req: func() *http.Request { + return httptest.NewRequest(http.MethodPost, "/test/1", nil) + }, + mock: func(m *mmetrics.Recorder) { + expHTTPReqProps := metrics.HTTPReqProperties{ + ID: "/test/{id}", + Service: "", + Method: "POST", + Code: "202", + } + m.On("ObserveHTTPRequestDuration", mock.Anything, expHTTPReqProps, mock.Anything).Once() + m.On("ObserveHTTPResponseSize", mock.Anything, expHTTPReqProps, int64(5)).Once() + + expHTTPProps := metrics.HTTPProperties{ + ID: "/test/{id}", + Service: "", + } + m.On("AddInflightRequests", mock.Anything, expHTTPProps, 1).Once() + m.On("AddInflightRequests", mock.Anything, expHTTPProps, -1).Once() + }, + handler: func() gorestful.RouteFunction { + return gorestful.RouteFunction(func(_ *gorestful.Request, resp *gorestful.Response) { + resp.WriteHeader(202) + resp.Write([]byte("test1")) // nolint: errcheck + }) + }, + expRespCode: 202, + expRespBody: "test1", + }, } for name, test := range tests { @@ -71,11 +171,11 @@ func TestMiddleware(t *testing.T) { // Create our instance with the middleware. mdlw := middleware.New(middleware.Config{Recorder: mr}) c := gorestful.NewContainer() - c.Filter(gorestfulmiddleware.Handler(test.handlerID, mdlw)) + c.Filter(gorestfulmiddleware.HandlerWithConfig(test.handlerID, mdlw, test.config)) ws := &gorestful.WebService{} ws.Produces(gorestful.MIME_JSON) req := test.req() - ws.Route(ws.Method(req.Method).Path(req.URL.Path).To(test.handler())) + ws.Route(ws.Method(req.Method).Path(test.route).To(test.handler())) c.Add(ws) // Make the request.