fix: regression in counting total requests (#17024)

cmd/generic-handlers.go

@@ -36,7 +36,6 @@ import (
 	"github.com/minio/minio/internal/config/dns"
 	"github.com/minio/minio/internal/crypto"
 	xhttp "github.com/minio/minio/internal/http"
-	"github.com/minio/minio/internal/http/stats"
 	"github.com/minio/minio/internal/logger"
 	"github.com/minio/minio/internal/mcontext"
 )
@@ -136,9 +135,8 @@ func setRequestLimitHandler(h http.Handler) http.Handler {
 
 // Reserved bucket.
 const (
-	minioReservedBucket              = "minio"
-	minioReservedBucketPath          = SlashSeparator + minioReservedBucket
-	minioReservedBucketPathWithSlash = SlashSeparator + minioReservedBucket + SlashSeparator
+	minioReservedBucket     = "minio"
+	minioReservedBucketPath = SlashSeparator + minioReservedBucket
 
 	loginPathPrefix = SlashSeparator + "login"
 )
@@ -280,60 +278,6 @@ func parseAmzDateHeader(req *http.Request) (time.Time, APIErrorCode) {
 	return time.Time{}, ErrMissingDateHeader
 }
 
-// splitStr splits a string into n parts, empty strings are added
-// if we are not able to reach n elements
-func splitStr(path, sep string, n int) []string {
-	splits := strings.SplitN(path, sep, n)
-	// Add empty strings if we found elements less than nr
-	for i := n - len(splits); i > 0; i-- {
-		splits = append(splits, "")
-	}
-	return splits
-}
-
-func url2Bucket(p string) (bucket string) {
-	tokens := splitStr(p, SlashSeparator, 3)
-	return tokens[1]
-}
-
-// setHttpStatsHandler sets a http Stats handler to gather HTTP statistics
-func setHTTPStatsHandler(h http.Handler) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// Meters s3 connection stats.
-		meteredRequest := &stats.IncomingTrafficMeter{ReadCloser: r.Body}
-		meteredResponse := &stats.OutgoingTrafficMeter{ResponseWriter: w}
-
-		// Execute the request
-		r.Body = meteredRequest
-		h.ServeHTTP(meteredResponse, r)
-
-		if strings.HasPrefix(r.URL.Path, storageRESTPrefix) ||
-			strings.HasPrefix(r.URL.Path, peerRESTPrefix) ||
-			strings.HasPrefix(r.URL.Path, peerS3Prefix) ||
-			strings.HasPrefix(r.URL.Path, lockRESTPrefix) {
-			globalConnStats.incInputBytes(meteredRequest.BytesRead())
-			globalConnStats.incOutputBytes(meteredResponse.BytesWritten())
-			return
-		}
-		if strings.HasPrefix(r.URL.Path, minioReservedBucketPath) {
-			globalConnStats.incAdminInputBytes(meteredRequest.BytesRead())
-			globalConnStats.incAdminOutputBytes(meteredResponse.BytesWritten())
-			return
-		}
-
-		globalConnStats.incS3InputBytes(meteredRequest.BytesRead())
-		globalConnStats.incS3OutputBytes(meteredResponse.BytesWritten())
-
-		if r.URL != nil {
-			bucket := url2Bucket(r.URL.Path)
-			if bucket != "" && bucket != minioReservedBucket {
-				globalBucketConnStats.incS3InputBytes(bucket, meteredRequest.BytesRead())
-				globalBucketConnStats.incS3OutputBytes(bucket, meteredResponse.BytesWritten())
-			}
-		}
-	})
-}
-
 // Bad path components to be rejected by the path validity handler.
 const (
 	dotdotComponent = ".."

cmd/handler-utils.go

@@ -34,6 +34,7 @@ import (
 	"github.com/minio/minio/internal/handlers"
 	xhttp "github.com/minio/minio/internal/http"
 	"github.com/minio/minio/internal/logger"
+	"github.com/minio/minio/internal/mcontext"
 	xnet "github.com/minio/pkg/net"
 )
 
@@ -357,8 +358,42 @@ func collectAPIStats(api string, f http.HandlerFunc) http.HandlerFunc {
 
 		f.ServeHTTP(w, r)
 
-		if sw, ok := w.(*xhttp.ResponseRecorder); ok {
-			globalHTTPStats.updateStats(api, r, sw)
+		tc, ok := r.Context().Value(mcontext.ContextTraceKey).(*mcontext.TraceCtxt)
+		if !ok {
+			return
+		}
+
+		if tc != nil {
+			if strings.HasPrefix(r.URL.Path, storageRESTPrefix) ||
+				strings.HasPrefix(r.URL.Path, peerRESTPrefix) ||
+				strings.HasPrefix(r.URL.Path, peerS3Prefix) ||
+				strings.HasPrefix(r.URL.Path, lockRESTPrefix) {
+				globalConnStats.incInputBytes(int64(tc.RequestRecorder.Size()))
+				globalConnStats.incOutputBytes(int64(tc.ResponseRecorder.Size()))
+				return
+			}
+
+			if strings.HasPrefix(r.URL.Path, minioReservedBucketPath) {
+				globalConnStats.incAdminInputBytes(int64(tc.RequestRecorder.Size()))
+				globalConnStats.incAdminOutputBytes(int64(tc.ResponseRecorder.Size()))
+				return
+			}
+
+			globalHTTPStats.updateStats(api, tc.ResponseRecorder)
+			globalConnStats.incS3InputBytes(int64(tc.RequestRecorder.Size()))
+			globalConnStats.incS3OutputBytes(int64(tc.ResponseRecorder.Size()))
+
+			resource, err := getResource(r.URL.Path, r.Host, globalDomainNames)
+			if err != nil {
+				logger.LogIf(r.Context(), fmt.Errorf("Unable to get the actual resource in the incoming request: %v", err))
+				return
+			}
+
+			bucket, _ := path2BucketObject(resource)
+			if bucket != "" && bucket != minioReservedBucket {
+				globalBucketConnStats.incS3InputBytes(bucket, int64(tc.RequestRecorder.Size()))
+				globalBucketConnStats.incS3OutputBytes(bucket, int64(tc.ResponseRecorder.Size()))
+			}
 		}
 	}
 }

cmd/http-stats.go

@@ -19,7 +19,6 @@ package cmd
 
 import (
 	"net/http"
-	"strings"
 	"sync"
 	"sync/atomic"
 
@@ -299,12 +298,7 @@ func (st *HTTPStats) toServerHTTPStats() ServerHTTPStats {
 }
 
 // Update statistics from http request and response data
-func (st *HTTPStats) updateStats(api string, r *http.Request, w *xhttp.ResponseRecorder) {
-	// Ignore non S3 requests
-	if strings.HasSuffix(r.URL.Path, minioReservedBucketPathWithSlash) {
-		return
-	}
-
+func (st *HTTPStats) updateStats(api string, w *xhttp.ResponseRecorder) {
 	st.totalS3Requests.Inc(api)
 
 	// Increment the prometheus http request response histogram with appropriate label

cmd/http-tracer.go

@@ -71,40 +71,33 @@ func httpTracer(h http.Handler) http.Handler {
 		// http stats requests and audit if enabled.
 		respRecorder := xhttp.NewResponseRecorder(w)
 
-		if globalTrace.NumSubscribers(madmin.TraceS3|madmin.TraceInternal) == 0 {
-			h.ServeHTTP(respRecorder, r)
-			return
-		}
+		// Setup a http request body recorder
+		reqRecorder := &xhttp.RequestRecorder{Reader: r.Body}
+		r.Body = reqRecorder
 
 		// Create tracing data structure and associate it to the request context
 		tc := mcontext.TraceCtxt{
-			AmzReqID: r.Header.Get(xhttp.AmzRequestID),
+			AmzReqID:         r.Header.Get(xhttp.AmzRequestID),
+			RequestRecorder:  reqRecorder,
+			ResponseRecorder: respRecorder,
 		}
 
-		ctx := context.WithValue(r.Context(), mcontext.ContextTraceKey, &tc)
-		r = r.WithContext(ctx)
-
-		// Setup a http request body recorder
-		reqRecorder := &xhttp.RequestRecorder{Reader: r.Body}
-
-		tc.RequestRecorder = reqRecorder
-		tc.ResponseRecorder = respRecorder
-
-		// Execute call.
-		r.Body = reqRecorder
+		r = r.WithContext(context.WithValue(r.Context(), mcontext.ContextTraceKey, &tc))
 
 		reqStartTime := time.Now().UTC()
 		h.ServeHTTP(respRecorder, r)
 		reqEndTime := time.Now().UTC()
 
+		if globalTrace.NumSubscribers(madmin.TraceS3|madmin.TraceInternal) == 0 {
+			// no subscribers nothing to trace.
+			return
+		}
+
 		tt := madmin.TraceInternal
 		if strings.HasPrefix(tc.FuncName, "s3.") {
 			tt = madmin.TraceS3
 		}
-		// No need to continue if no subscribers for actual type...
-		if globalTrace.NumSubscribers(tt) == 0 {
-			return
-		}
+
 		// Calculate input body size with headers
 		reqHeaders := r.Header.Clone()
 		reqHeaders.Set("Host", r.Host)
@@ -113,7 +106,7 @@ func httpTracer(h http.Handler) http.Handler {
 		} else {
 			reqHeaders.Set("Transfer-Encoding", strings.Join(r.TransferEncoding, ","))
 		}
-		inputBytes := reqRecorder.BodySize()
+		inputBytes := reqRecorder.Size()
 		for k, v := range reqHeaders {
 			inputBytes += len(k) + len(v)
 		}

cmd/routers.go

@@ -58,8 +58,6 @@ var globalHandlers = []mux.MiddlewareFunc{
 	setCrossDomainPolicy,
 	// Limits all body and header sizes to a maximum fixed limit
 	setRequestLimitHandler,
-	// Network statistics
-	setHTTPStatsHandler,
 	// Validate all the incoming requests.
 	setRequestValidityHandler,
 	// set x-amz-request-id header.

internal/http/request-recorder.go

@@ -56,8 +56,8 @@ func (r *RequestRecorder) Read(p []byte) (n int, err error) {
 	return n, err
 }
 
-// BodySize returns the body size if the currently read object
-func (r *RequestRecorder) BodySize() int {
+// Size returns the body size if the currently read object
+func (r *RequestRecorder) Size() int {
 	return r.bytesRead
 }
 

internal/http/stats/http-traffic-recorder.go

@@ -1,66 +0,0 @@
-// Copyright (c) 2015-2021 MinIO, Inc.
-//
-// This file is part of MinIO Object Storage stack
-//
-// This program is free software: you can redistribute it and/or modify
-// it under the terms of the GNU Affero General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-//
-// This program is distributed in the hope that it will be useful
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU Affero General Public License for more details.
-//
-// You should have received a copy of the GNU Affero General Public License
-// along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-package stats
-
-import (
-	"io"
-	"net/http"
-)
-
-// IncomingTrafficMeter counts the incoming bytes from the underlying request.Body.
-type IncomingTrafficMeter struct {
-	countBytes int64
-	io.ReadCloser
-}
-
-// Read calls the underlying Read and counts the transferred bytes.
-func (r *IncomingTrafficMeter) Read(p []byte) (n int, err error) {
-	n, err = r.ReadCloser.Read(p)
-	r.countBytes += int64(n)
-
-	return n, err
-}
-
-// BytesRead returns the number of transferred bytes
-func (r *IncomingTrafficMeter) BytesRead() int64 {
-	return r.countBytes
-}
-
-// OutgoingTrafficMeter counts the outgoing bytes through the responseWriter.
-type OutgoingTrafficMeter struct {
-	countBytes int64
-	// wrapper for underlying http.ResponseWriter.
-	http.ResponseWriter
-}
-
-// Write calls the underlying write and counts the output bytes
-func (w *OutgoingTrafficMeter) Write(p []byte) (n int, err error) {
-	n, err = w.ResponseWriter.Write(p)
-	w.countBytes += int64(n)
-	return n, err
-}
-
-// Flush calls the underlying Flush.
-func (w *OutgoingTrafficMeter) Flush() {
-	w.ResponseWriter.(http.Flusher).Flush()
-}
-
-// BytesWritten returns the number of transferred bytes
-func (w *OutgoingTrafficMeter) BytesWritten() int64 {
-	return w.countBytes
-}

internal/logger/audit.go

@@ -24,11 +24,10 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/minio/minio/internal/http/stats"
 	internalAudit "github.com/minio/minio/internal/logger/message/audit"
+	"github.com/minio/minio/internal/mcontext"
 	"github.com/minio/pkg/logger/message/audit"
 
-	"github.com/klauspost/compress/gzhttp"
 	"github.com/minio/madmin-go/v2"
 	xhttp "github.com/minio/minio/internal/http"
 )
@@ -96,34 +95,13 @@ func AuditLog(ctx context.Context, w http.ResponseWriter, r *http.Request, reqCl
 			headerBytes     int64
 		)
 
-		var st *xhttp.ResponseRecorder
-
-		for {
-			switch v := w.(type) {
-			case *gzhttp.GzipResponseWriter:
-				// the writer may be obscured by gzip response writer
-				w = v.ResponseWriter
-			case *gzhttp.NoGzipResponseWriter:
-				// the writer may be obscured by no-gzip response writer
-				w = v.ResponseWriter
-			case *stats.OutgoingTrafficMeter:
-				// the writer may be obscured by http stats response writer
-				w = v.ResponseWriter
-			case *xhttp.ResponseRecorder:
-				st = v
-				goto exit
-			default:
-				goto exit
-			}
-		}
-	exit:
-
-		if st != nil {
-			statusCode = st.StatusCode
-			timeToResponse = time.Now().UTC().Sub(st.StartTime)
-			timeToFirstByte = st.TimeToFirstByte
-			outputBytes = int64(st.Size())
-			headerBytes = int64(st.HeaderSize())
+		tc, ok := r.Context().Value(mcontext.ContextTraceKey).(*mcontext.TraceCtxt)
+		if ok {
+			statusCode = tc.ResponseRecorder.StatusCode
+			outputBytes = int64(tc.ResponseRecorder.Size())
+			headerBytes = int64(tc.ResponseRecorder.HeaderSize())
+			timeToResponse = time.Now().UTC().Sub(tc.ResponseRecorder.StartTime)
+			timeToFirstByte = tc.ResponseRecorder.TimeToFirstByte
 		}
 
 		entry.AccessKey = reqInfo.Cred.AccessKey
@@ -147,7 +125,7 @@ func AuditLog(ctx context.Context, w http.ResponseWriter, r *http.Request, reqCl
 		entry.API.TimeToResponse = strconv.FormatInt(timeToResponse.Nanoseconds(), 10) + "ns"
 		entry.API.TimeToResponseInNS = strconv.FormatInt(timeToResponse.Nanoseconds(), 10)
 		entry.Tags = reqInfo.GetTagsMap()
-		// ttfb will be recorded only for GET requests, Ignore such cases where ttfb will be empty.
+		// ignore cases for ttfb when its zero.
 		if timeToFirstByte != 0 {
 			entry.API.TimeToFirstByte = strconv.FormatInt(timeToFirstByte.Nanoseconds(), 10) + "ns"
 			entry.API.TimeToFirstByteInNS = strconv.FormatInt(timeToFirstByte.Nanoseconds(), 10)