Skip to content

Commit 59a20b8

Browse files
slokslok
committed
Fix tests after the recorder change
Signed-off-by: Xabier Larrakoetxea <[email protected]>
1 parent 7af7311 commit 59a20b8

File tree

7 files changed

+121
-34
lines changed

7 files changed

+121
-34
lines changed

‎internal/mocks/metrics/Recorder.go‎

Lines changed: 10 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎middleware/gin/gin_test.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/stretchr/testify/mock"
1111

1212
mmetrics "github.com/slok/go-http-metrics/internal/mocks/metrics"
13+
"github.com/slok/go-http-metrics/metrics"
1314
"github.com/slok/go-http-metrics/middleware"
1415
ginmiddleware "github.com/slok/go-http-metrics/middleware/gin"
1516
)
@@ -28,6 +29,7 @@ func TestMiddlewareIntegration(t *testing.T) {
2829
req *http.Request
2930
config middleware.Config
3031
expHandlerID string
32+
expService string
3133
expMethod string
3234
expStatusCode string
3335
}{
@@ -47,10 +49,20 @@ func TestMiddlewareIntegration(t *testing.T) {
4749

4850
// Mocks.
4951
mr := &mmetrics.Recorder{}
50-
mr.On("ObserveHTTPRequestDuration", mock.Anything, test.expHandlerID, mock.Anything, test.expMethod, test.expStatusCode).Once()
51-
mr.On("ObserveHTTPResponseSize", mock.Anything, test.expHandlerID, mock.Anything, test.expMethod, test.expStatusCode).Once()
52-
mr.On("AddInflightRequests", mock.Anything, test.expHandlerID, 1).Once()
53-
mr.On("AddInflightRequests", mock.Anything, test.expHandlerID, -1).Once()
52+
expHTTPReqProps := metrics.HTTPReqProperties{
53+
ID: test.expHandlerID,
54+
Service: test.expService,
55+
Method: test.expMethod,
56+
Code: test.expStatusCode,
57+
}
58+
expHTTPProps := metrics.HTTPProperties{
59+
ID: test.expHandlerID,
60+
Service: test.expService,
61+
}
62+
mr.On("ObserveHTTPRequestDuration", mock.Anything, expHTTPReqProps, mock.Anything).Once()
63+
mr.On("ObserveHTTPResponseSize", mock.Anything, expHTTPReqProps, mock.Anything).Once()
64+
mr.On("AddInflightRequests", mock.Anything, expHTTPProps, 1).Once()
65+
mr.On("AddInflightRequests", mock.Anything, expHTTPProps, -1).Once()
5466

5567
// Create our instance with the middleware.
5668
mdlw := middleware.New(middleware.Config{Recorder: mr})

‎middleware/gorestful/gorestful_test.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/stretchr/testify/mock"
1111

1212
mmetrics "github.com/slok/go-http-metrics/internal/mocks/metrics"
13+
"github.com/slok/go-http-metrics/metrics"
1314
"github.com/slok/go-http-metrics/middleware"
1415
gorestfulmiddleware "github.com/slok/go-http-metrics/middleware/gorestful"
1516
)
@@ -28,6 +29,7 @@ func TestMiddlewareIntegration(t *testing.T) {
2829
req *http.Request
2930
config middleware.Config
3031
expHandlerID string
32+
expService string
3133
expMethod string
3234
expStatusCode string
3335
}{
@@ -47,10 +49,20 @@ func TestMiddlewareIntegration(t *testing.T) {
4749

4850
// Mocks.
4951
mr := &mmetrics.Recorder{}
50-
mr.On("ObserveHTTPRequestDuration", mock.Anything, test.expHandlerID, mock.Anything, test.expMethod, test.expStatusCode).Once()
51-
mr.On("ObserveHTTPResponseSize", mock.Anything, test.expHandlerID, mock.Anything, test.expMethod, test.expStatusCode).Once()
52-
mr.On("AddInflightRequests", mock.Anything, test.expHandlerID, 1).Once()
53-
mr.On("AddInflightRequests", mock.Anything, test.expHandlerID, -1).Once()
52+
expHTTPReqProps := metrics.HTTPReqProperties{
53+
ID: test.expHandlerID,
54+
Service: test.expService,
55+
Method: test.expMethod,
56+
Code: test.expStatusCode,
57+
}
58+
expHTTPProps := metrics.HTTPProperties{
59+
ID: test.expHandlerID,
60+
Service: test.expService,
61+
}
62+
mr.On("ObserveHTTPRequestDuration", mock.Anything, expHTTPReqProps, mock.Anything).Once()
63+
mr.On("ObserveHTTPResponseSize", mock.Anything, expHTTPReqProps, mock.Anything).Once()
64+
mr.On("AddInflightRequests", mock.Anything, expHTTPProps, 1).Once()
65+
mr.On("AddInflightRequests", mock.Anything, expHTTPProps, -1).Once()
5466

5567
// Create our instance with the middleware.
5668
mdlw := middleware.New(middleware.Config{Recorder: mr})

‎middleware/httprouter/httprouter_test.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/stretchr/testify/mock"
1111

1212
mmetrics "github.com/slok/go-http-metrics/internal/mocks/metrics"
13+
"github.com/slok/go-http-metrics/metrics"
1314
"github.com/slok/go-http-metrics/middleware"
1415
httproutermiddleware "github.com/slok/go-http-metrics/middleware/httprouter"
1516
)
@@ -28,6 +29,7 @@ func TestMiddlewareIntegration(t *testing.T) {
2829
req *http.Request
2930
config middleware.Config
3031
expHandlerID string
32+
expService string
3133
expMethod string
3234
expStatusCode string
3335
}{
@@ -47,10 +49,20 @@ func TestMiddlewareIntegration(t *testing.T) {
4749

4850
// Mocks.
4951
mr := &mmetrics.Recorder{}
50-
mr.On("ObserveHTTPRequestDuration", mock.Anything, test.expHandlerID, mock.Anything, test.expMethod, test.expStatusCode).Once()
51-
mr.On("ObserveHTTPResponseSize", mock.Anything, test.expHandlerID, mock.Anything, test.expMethod, test.expStatusCode).Once()
52-
mr.On("AddInflightRequests", mock.Anything, test.expHandlerID, 1).Once()
53-
mr.On("AddInflightRequests", mock.Anything, test.expHandlerID, -1).Once()
52+
expHTTPReqProps := metrics.HTTPReqProperties{
53+
ID: test.expHandlerID,
54+
Service: test.expService,
55+
Method: test.expMethod,
56+
Code: test.expStatusCode,
57+
}
58+
expHTTPProps := metrics.HTTPProperties{
59+
ID: test.expHandlerID,
60+
Service: test.expService,
61+
}
62+
mr.On("ObserveHTTPRequestDuration", mock.Anything, expHTTPReqProps, mock.Anything).Once()
63+
mr.On("ObserveHTTPResponseSize", mock.Anything, expHTTPReqProps, mock.Anything).Once()
64+
mr.On("AddInflightRequests", mock.Anything, expHTTPProps, 1).Once()
65+
mr.On("AddInflightRequests", mock.Anything, expHTTPProps, -1).Once()
5466

5567
// Create our instance with the middleware.
5668
mdlw := middleware.New(middleware.Config{Recorder: mr})

‎middleware/middleware.go‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ import (
1717
type Config struct {
1818
// Recorder is the way the metrics will be recorder in the different backends.
1919
Recorder metrics.Recorder
20+
// Service is an optional identifier for the metrics, this can be useful if
21+
// a same service has multiple servers (e.g API, metrics and healthchecks).
22+
Service string
2023
// GroupedStatus will group the status label in the form of `\dxx`, for example,
2124
// 200, 201, and 203 will have the label `code="2xx"`. This impacts on the cardinality
2225
// of the metrics and also improves the performance of queries that are grouped by
@@ -86,8 +89,12 @@ func (m *middleware) Handler(handlerID string, h http.Handler) http.Handler {
8689

8790
// Measure inflights if required.
8891
if !m.cfg.DisableMeasureInflight {
89-
m.cfg.Recorder.AddInflightRequests(r.Context(), hid, 1)
90-
defer m.cfg.Recorder.AddInflightRequests(r.Context(), hid, -1)
92+
props := metrics.HTTPProperties{
93+
Service: m.cfg.Service,
94+
ID: hid,
95+
}
96+
m.cfg.Recorder.AddInflightRequests(r.Context(), props, 1)
97+
defer m.cfg.Recorder.AddInflightRequests(r.Context(), props, -1)
9198
}
9299

93100
// Start the timer and when finishing measure the duration.
@@ -105,11 +112,17 @@ func (m *middleware) Handler(handlerID string, h http.Handler) http.Handler {
105112
code = strconv.Itoa(wi.statusCode)
106113
}
107114

108-
m.cfg.Recorder.ObserveHTTPRequestDuration(r.Context(), hid, duration, r.Method, code)
115+
props := metrics.HTTPReqProperties{
116+
Service: m.cfg.Service,
117+
ID: hid,
118+
Method: r.Method,
119+
Code: code,
120+
}
121+
m.cfg.Recorder.ObserveHTTPRequestDuration(r.Context(), props, duration)
109122

110123
// Measure size of response if required.
111124
if !m.cfg.DisableMeasureSize {
112-
m.cfg.Recorder.ObserveHTTPResponseSize(r.Context(), hid, int64(wi.bytesWritten), r.Method, code)
125+
m.cfg.Recorder.ObserveHTTPResponseSize(r.Context(), props, int64(wi.bytesWritten))
113126
}
114127

115128
}()

‎middleware/middleware_test.go‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ func TestMiddlewareHandler(t *testing.T) {
2727
req *http.Request
2828
config middleware.Config
2929
expHandlerID string
30+
expService string
3031
expMethod string
3132
expSize int64
3233
expStatusCode string
@@ -37,6 +38,7 @@ func TestMiddlewareHandler(t *testing.T) {
3738
body: "Я бэтмен",
3839
req: httptest.NewRequest(http.MethodGet, "/test", nil),
3940
expHandlerID: "/test",
41+
expService: "",
4042
expSize: 15,
4143
expMethod: http.MethodGet,
4244
expStatusCode: "202",
@@ -48,6 +50,7 @@ func TestMiddlewareHandler(t *testing.T) {
4850
statusCode: http.StatusTeapot,
4951
req: httptest.NewRequest(http.MethodPost, "/test", nil),
5052
expHandlerID: "customID",
53+
expService: "",
5154
expSize: 10,
5255
expMethod: http.MethodPost,
5356
expStatusCode: "418",
@@ -58,19 +61,42 @@ func TestMiddlewareHandler(t *testing.T) {
5861
statusCode: http.StatusGatewayTimeout,
5962
req: httptest.NewRequest(http.MethodPatch, "/test", nil),
6063
expHandlerID: "/test",
64+
expService: "",
6165
expMethod: http.MethodPatch,
6266
expStatusCode: "5xx",
6367
},
68+
{
69+
name: "Using the service middleware option should set the service on the metrics.",
70+
config: middleware.Config{Service: "Yoda"},
71+
statusCode: http.StatusContinue,
72+
body: "May the force be with you",
73+
req: httptest.NewRequest(http.MethodGet, "/test", nil),
74+
expHandlerID: "/test",
75+
expService: "Yoda",
76+
expSize: 25,
77+
expMethod: http.MethodGet,
78+
expStatusCode: "100",
79+
},
6480
}
6581

6682
for _, test := range tests {
6783
t.Run(test.name, func(t *testing.T) {
6884
// Mocks.
6985
mr := &mmetrics.Recorder{}
70-
mr.On("ObserveHTTPRequestDuration", mock.Anything, test.expHandlerID, mock.Anything, test.expMethod, test.expStatusCode).Once()
71-
mr.On("ObserveHTTPResponseSize", mock.Anything, test.expHandlerID, test.expSize, test.expMethod, test.expStatusCode).Once()
72-
mr.On("AddInflightRequests", mock.Anything, test.expHandlerID, 1).Once()
73-
mr.On("AddInflightRequests", mock.Anything, test.expHandlerID, -1).Once()
86+
expHTTPReqProps := metrics.HTTPReqProperties{
87+
ID: test.expHandlerID,
88+
Service: test.expService,
89+
Method: test.expMethod,
90+
Code: test.expStatusCode,
91+
}
92+
expHTTPProps := metrics.HTTPProperties{
93+
ID: test.expHandlerID,
94+
Service: test.expService,
95+
}
96+
mr.On("ObserveHTTPRequestDuration", mock.Anything, expHTTPReqProps, mock.Anything).Once()
97+
mr.On("ObserveHTTPResponseSize", mock.Anything, expHTTPReqProps, test.expSize).Once()
98+
mr.On("AddInflightRequests", mock.Anything, expHTTPProps, 1).Once()
99+
mr.On("AddInflightRequests", mock.Anything, expHTTPProps, -1).Once()
74100

75101
// Make the request.
76102
test.config.Recorder = mr

‎middleware/negroni/negroni_test.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/urfave/negroni"
1111

1212
mmetrics "github.com/slok/go-http-metrics/internal/mocks/metrics"
13+
"github.com/slok/go-http-metrics/metrics"
1314
"github.com/slok/go-http-metrics/middleware"
1415
negronimiddleware "github.com/slok/go-http-metrics/middleware/negroni"
1516
)
@@ -28,6 +29,7 @@ func TestMiddlewareIntegration(t *testing.T) {
2829
req *http.Request
2930
config middleware.Config
3031
expHandlerID string
32+
expService string
3133
expMethod string
3234
expStatusCode string
3335
}{
@@ -47,10 +49,20 @@ func TestMiddlewareIntegration(t *testing.T) {
4749

4850
// Mocks.
4951
mr := &mmetrics.Recorder{}
50-
mr.On("ObserveHTTPRequestDuration", mock.Anything, test.expHandlerID, mock.Anything, test.expMethod, test.expStatusCode).Once()
51-
mr.On("ObserveHTTPResponseSize", mock.Anything, test.expHandlerID, mock.Anything, test.expMethod, test.expStatusCode).Once()
52-
mr.On("AddInflightRequests", mock.Anything, test.expHandlerID, 1).Once()
53-
mr.On("AddInflightRequests", mock.Anything, test.expHandlerID, -1).Once()
52+
expHTTPReqProps := metrics.HTTPReqProperties{
53+
ID: test.expHandlerID,
54+
Service: test.expService,
55+
Method: test.expMethod,
56+
Code: test.expStatusCode,
57+
}
58+
expHTTPProps := metrics.HTTPProperties{
59+
ID: test.expHandlerID,
60+
Service: test.expService,
61+
}
62+
mr.On("ObserveHTTPRequestDuration", mock.Anything, expHTTPReqProps, mock.Anything).Once()
63+
mr.On("ObserveHTTPResponseSize", mock.Anything, expHTTPReqProps, mock.Anything).Once()
64+
mr.On("AddInflightRequests", mock.Anything, expHTTPProps, 1).Once()
65+
mr.On("AddInflightRequests", mock.Anything, expHTTPProps, -1).Once()
5466

5567
// Create our negroni instance with the middleware.
5668
mdlw := middleware.New(middleware.Config{Recorder: mr})

0 commit comments

Comments
 (0)