From 27632ca6ec42efaf21bdcc2d247d777d78e380f2 Mon Sep 17 00:00:00 2001 From: Anis Elleuch Date: Thu, 30 Apr 2020 19:27:19 +0100 Subject: [PATCH] audit: Merge ResponseWriter with RecordAPIStats (#9496) ResponseWriter & RecordAPIStats has similar role, merge them. This commit will also fix wrong auditing for STS and Web and others since they are using ResponseWriter instead of the RecordAPIStats. --- cmd/handler-utils.go | 4 +-- cmd/http-stats.go | 8 ++--- cmd/http-tracer.go | 3 +- cmd/http/stats/http-traffic-recorder.go | 43 ------------------------- cmd/logger/audit.go | 19 +++++------ 5 files changed, 17 insertions(+), 60 deletions(-) diff --git a/cmd/handler-utils.go b/cmd/handler-utils.go index 2e4332b20..099e69a48 100644 --- a/cmd/handler-utils.go +++ b/cmd/handler-utils.go @@ -36,8 +36,6 @@ import ( "github.com/minio/minio/pkg/bucket/object/tagging" "github.com/minio/minio/pkg/handlers" "github.com/minio/minio/pkg/madmin" - - stats "github.com/minio/minio/cmd/http/stats" ) const ( @@ -366,7 +364,7 @@ func collectAPIStats(api string, f http.HandlerFunc) http.HandlerFunc { globalHTTPStats.currentS3Requests.Inc(api) defer globalHTTPStats.currentS3Requests.Dec(api) - statsWriter := stats.NewRecordAPIStats(w) + statsWriter := logger.NewResponseWriter(w) f.ServeHTTP(statsWriter, r) diff --git a/cmd/http-stats.go b/cmd/http-stats.go index 20e78c969..206bd5372 100644 --- a/cmd/http-stats.go +++ b/cmd/http-stats.go @@ -23,7 +23,7 @@ import ( "sync" "time" - stats "github.com/minio/minio/cmd/http/stats" + "github.com/minio/minio/cmd/logger" "github.com/prometheus/client_golang/prometheus" "go.uber.org/atomic" ) @@ -167,13 +167,13 @@ func (st *HTTPStats) toServerHTTPStats() ServerHTTPStats { } // Update statistics from http request and response data -func (st *HTTPStats) updateStats(api string, r *http.Request, w *stats.RecordAPIStats, durationSecs float64) { +func (st *HTTPStats) updateStats(api string, r *http.Request, w *logger.ResponseWriter, durationSecs float64) { // A successful request has a 2xx response code - successReq := (w.RespStatusCode >= 200 && w.RespStatusCode < 300) + successReq := (w.StatusCode >= 200 && w.StatusCode < 300) if !strings.HasSuffix(r.URL.Path, prometheusMetricsPath) { st.totalS3Requests.Inc(api) - if !successReq && w.RespStatusCode != 0 { + if !successReq && w.StatusCode != 0 { st.totalS3Errors.Inc(api) } } diff --git a/cmd/http-tracer.go b/cmd/http-tracer.go index 004afd046..01ac76a26 100644 --- a/cmd/http-tracer.go +++ b/cmd/http-tracer.go @@ -123,7 +123,8 @@ func Trace(f http.HandlerFunc, logBody bool, w http.ResponseWriter, r *http.Requ } rw := logger.NewResponseWriter(w) - rw.LogBody = logBody + rw.LogErrBody = true + rw.LogAllBody = logBody f(rw, r) rq := trace.RequestInfo{ diff --git a/cmd/http/stats/http-traffic-recorder.go b/cmd/http/stats/http-traffic-recorder.go index 8cb70608c..f554524f7 100644 --- a/cmd/http/stats/http-traffic-recorder.go +++ b/cmd/http/stats/http-traffic-recorder.go @@ -19,7 +19,6 @@ package stats import ( "io" "net/http" - "time" ) // IncomingTrafficMeter counts the incoming bytes from the underlying request.Body. @@ -63,45 +62,3 @@ func (w *OutgoingTrafficMeter) Flush() { func (w OutgoingTrafficMeter) BytesCount() int { return w.countBytes } - -// RecordAPIStats is a response writer which stores -// information of the underlying http response. -type RecordAPIStats struct { - http.ResponseWriter - TTFB time.Duration // TimeToFirstByte. - StartTime time.Time - RespStatusCode int - - firstByteRead bool -} - -// NewRecordAPIStats creates a new response writer with -// start time set to the function call time. -func NewRecordAPIStats(w http.ResponseWriter) *RecordAPIStats { - return &RecordAPIStats{ - ResponseWriter: w, - StartTime: time.Now().UTC(), - } -} - -// WriteHeader calls the underlying WriteHeader -// and records the response status code. -func (r *RecordAPIStats) WriteHeader(i int) { - r.RespStatusCode = i - r.ResponseWriter.WriteHeader(i) -} - -// Write calls the underlying Write and updates TTFB and other info -func (r *RecordAPIStats) Write(p []byte) (n int, err error) { - if !r.firstByteRead { - r.TTFB = time.Now().UTC().Sub(r.StartTime) - r.firstByteRead = true - } - n, err = r.ResponseWriter.Write(p) - return -} - -// Flush calls the underlying Flush. -func (r *RecordAPIStats) Flush() { - r.ResponseWriter.(http.Flusher).Flush() -} diff --git a/cmd/logger/audit.go b/cmd/logger/audit.go index 6e309288d..d3539b8b6 100644 --- a/cmd/logger/audit.go +++ b/cmd/logger/audit.go @@ -26,16 +26,17 @@ import ( "github.com/gorilla/mux" "github.com/minio/minio/cmd/logger/message/audit" - - stats "github.com/minio/minio/cmd/http/stats" ) // ResponseWriter - is a wrapper to trap the http response status code. type ResponseWriter struct { http.ResponseWriter StatusCode int - // Response body should be logged - LogBody bool + // Log body of 4xx or 5xx responses + LogErrBody bool + // Log body of all responses + LogAllBody bool + TimeToFirstByte time.Duration StartTime time.Time // number of bytes written @@ -69,7 +70,7 @@ func (lrw *ResponseWriter) Write(p []byte) (int, error) { lrw.writeHeaders(&lrw.headers, http.StatusOK, lrw.Header()) lrw.headersLogged = true } - if lrw.StatusCode >= http.StatusBadRequest || lrw.LogBody { + if (lrw.LogErrBody && lrw.StatusCode >= http.StatusBadRequest) || lrw.LogAllBody { // Always logging error responses. lrw.body.Write(p) } @@ -96,7 +97,7 @@ var BodyPlaceHolder = []byte("") func (lrw *ResponseWriter) Body() []byte { // If there was an error response or body logging is enabled // then we return the body contents - if lrw.StatusCode >= http.StatusBadRequest || lrw.LogBody { + if (lrw.LogErrBody && lrw.StatusCode >= http.StatusBadRequest) || lrw.LogAllBody { return lrw.body.Bytes() } // ... otherwise we return the place holder @@ -145,11 +146,11 @@ func AuditLog(w http.ResponseWriter, r *http.Request, api string, reqClaims map[ timeToFirstByte time.Duration ) - st, ok := w.(*stats.RecordAPIStats) + st, ok := w.(*ResponseWriter) if ok { - statusCode = st.RespStatusCode + statusCode = st.StatusCode timeToResponse = time.Now().UTC().Sub(st.StartTime) - timeToFirstByte = st.TTFB + timeToFirstByte = st.TimeToFirstByte } vars := mux.Vars(r)