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.