MM-23131 Include HTTP status code in the metrics (#14240)

* ResponseWriter wrapper to get status code

For our metrics, we need the status code returned by a request
so this wrapper includes a new method StatusCode() that includes
the desired code

* Shadow the responsewriter variable in the handlers

In order to avoid confusion to people deciding what variable to use.
I've also changed the tests to reflect this change and added a new
one that checks the Flush method works

einterfaces/metrics.go

@@ -48,7 +48,7 @@ type MetricsInterface interface {
 	IncrementPostsSearchCounter()
 	ObservePostsSearchDuration(elapsed float64)
 	ObserveStoreMethodDuration(method, success string, elapsed float64)
-	ObserveApiEndpointDuration(endpoint, method string, elapsed float64)
+	ObserveApiEndpointDuration(endpoint, method, statusCode string, elapsed float64)
 	IncrementPostIndexCounter()
 	IncrementUserIndexCounter()
 	IncrementChannelIndexCounter()

einterfaces/mocks/MetricsInterface.go

@@ -171,9 +171,9 @@ func (_m *MetricsInterface) IncrementWebsocketEvent(eventType string) {
 	_m.Called(eventType)
 }
 
-// ObserveApiEndpointDuration provides a mock function with given fields: endpoint, method, elapsed
-func (_m *MetricsInterface) ObserveApiEndpointDuration(endpoint string, method string, elapsed float64) {
-	_m.Called(endpoint, method, elapsed)
+// ObserveApiEndpointDuration provides a mock function with given fields: endpoint, method, statusCode, elapsed
+func (_m *MetricsInterface) ObserveApiEndpointDuration(endpoint string, method string, statusCode string, elapsed float64) {
+	_m.Called(endpoint, method, statusCode, elapsed)
 }
 
 // ObserveClusterRequestDuration provides a mock function with given fields: elapsed

web/handlers.go

@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"reflect"
 	"runtime"
+	"strconv"
 	"strings"
 	"time"
 
@@ -79,6 +80,7 @@ type Handler struct {
 }
 
 func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	w = newWrappedWriter(w)
 	now := time.Now()
 
 	requestID := model.NewId()
@@ -257,8 +259,9 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 		if r.URL.Path != model.API_URL_SUFFIX+"/websocket" {
 			elapsed := float64(time.Since(now)) / float64(time.Second)
+			statusCode := strconv.Itoa(w.(*responseWriterWrapper).StatusCode())
 			c.App.Metrics().ObserveHttpRequestDuration(elapsed)
-			c.App.Metrics().ObserveApiEndpointDuration(h.HandlerName, r.Method, elapsed)
+			c.App.Metrics().ObserveApiEndpointDuration(h.HandlerName, r.Method, statusCode, elapsed)
 		}
 	}
 }

web/response_writer_wrapper.go

@@ -0,0 +1,62 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package web
+
+import (
+	"bufio"
+	"errors"
+	"net"
+	"net/http"
+)
+
+type responseWriterWrapper struct {
+	http.ResponseWriter
+	statusCode        int
+	statusCodeWritten bool
+	hijacker          http.Hijacker
+	flusher           http.Flusher
+}
+
+func newWrappedWriter(original http.ResponseWriter) *responseWriterWrapper {
+	hijacker, _ := original.(http.Hijacker)
+	flusher, _ := original.(http.Flusher)
+	return &responseWriterWrapper{
+		ResponseWriter:    original,
+		statusCodeWritten: false,
+		hijacker:          hijacker,
+		flusher:           flusher,
+	}
+}
+
+func (rw *responseWriterWrapper) StatusCode() int {
+	return rw.statusCode
+}
+
+func (rw *responseWriterWrapper) WriteHeader(statusCode int) {
+	rw.statusCode = statusCode
+	rw.statusCodeWritten = true
+	rw.ResponseWriter.WriteHeader(statusCode)
+}
+
+func (rw *responseWriterWrapper) Write(data []byte) (int, error) {
+	if !rw.statusCodeWritten {
+		rw.statusCode = http.StatusOK
+	}
+	return rw.ResponseWriter.Write(data)
+}
+
+// Using as embedded makes the ResponseWrite be stored as interface and that way
+// it loses the access to the implementation for Hijack or Flush
+func (rw *responseWriterWrapper) Hijack() (net.Conn, *bufio.ReadWriter, error) {
+	if rw.hijacker == nil {
+		return nil, nil, errors.New("Hijacker interface not supported by the wrapped ResponseWriter")
+	}
+	return rw.hijacker.Hijack()
+}
+
+func (rw *responseWriterWrapper) Flush() {
+	if rw.flusher != nil {
+		rw.flusher.Flush()
+	}
+}

web/response_writer_wrapper_test.go

@@ -0,0 +1,87 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package web
+
+import (
+	"bufio"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+type TestHandler struct {
+	TestFunc func(w http.ResponseWriter, r *http.Request)
+}
+
+func (h *TestHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	h.TestFunc(w, r)
+}
+
+type responseRecorderHijack struct {
+	httptest.ResponseRecorder
+}
+
+func (r *responseRecorderHijack) Hijack() (net.Conn, *bufio.ReadWriter, error) {
+	r.WriteHeader(http.StatusOK)
+	return nil, nil, nil
+}
+
+func newResponseWithHijack(original *httptest.ResponseRecorder) *responseRecorderHijack {
+	return &responseRecorderHijack{*original}
+}
+
+func TestStatusCodeIsAccessible(t *testing.T) {
+	resp := newWrappedWriter(httptest.NewRecorder())
+	req := httptest.NewRequest("GET", "/api/v4/test", nil)
+	handler := TestHandler{func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusBadRequest)
+	}}
+	handler.ServeHTTP(resp, req)
+	assert.Equal(t, http.StatusBadRequest, resp.StatusCode())
+}
+
+func TestStatusCodeShouldBe200IfNotHeaderWritten(t *testing.T) {
+	resp := newWrappedWriter(httptest.NewRecorder())
+	req := httptest.NewRequest("GET", "/api/v4/test", nil)
+	handler := TestHandler{func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte{})
+	}}
+	handler.ServeHTTP(resp, req)
+	assert.Equal(t, http.StatusOK, resp.StatusCode())
+}
+
+func TestForUnsupportedHijack(t *testing.T) {
+	resp := newWrappedWriter(httptest.NewRecorder())
+	req := httptest.NewRequest("GET", "/api/v4/test", nil)
+	handler := TestHandler{func(w http.ResponseWriter, r *http.Request) {
+		_, _, err := w.(*responseWriterWrapper).Hijack()
+		assert.NotNil(t, err)
+		assert.Equal(t, "Hijacker interface not supported by the wrapped ResponseWriter", err.Error())
+	}}
+	handler.ServeHTTP(resp, req)
+}
+
+func TestForSupportedHijack(t *testing.T) {
+	resp := newWrappedWriter(newResponseWithHijack(httptest.NewRecorder()))
+	req := httptest.NewRequest("GET", "/api/v4/test", nil)
+	handler := TestHandler{func(w http.ResponseWriter, r *http.Request) {
+		_, _, err := w.(*responseWriterWrapper).Hijack()
+		assert.Nil(t, err)
+	}}
+	handler.ServeHTTP(resp, req)
+}
+
+func TestForSupportedFlush(t *testing.T) {
+	resp := newWrappedWriter(httptest.NewRecorder())
+	req := httptest.NewRequest("GET", "/api/v4/test", nil)
+	handler := TestHandler{func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte{})
+		w.(*responseWriterWrapper).Flush()
+	}}
+	handler.ServeHTTP(resp, req)
+	assert.Equal(t, http.StatusOK, resp.StatusCode())
+}