diff --git a/cmd/generic-handlers.go b/cmd/generic-handlers.go index 91c71c178..2ec69b03f 100644 --- a/cmd/generic-handlers.go +++ b/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 = ".." diff --git a/cmd/handler-utils.go b/cmd/handler-utils.go index 046aa4478..42c8b0907 100644 --- a/cmd/handler-utils.go +++ b/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())) + } } } } diff --git a/cmd/http-stats.go b/cmd/http-stats.go index a755c2549..5e3bbbb63 100644 --- a/cmd/http-stats.go +++ b/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 diff --git a/cmd/http-tracer.go b/cmd/http-tracer.go index 03b0f0ce6..f70d8327f 100644 --- a/cmd/http-tracer.go +++ b/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) } diff --git a/cmd/routers.go b/cmd/routers.go index 4509cc88a..ff8c88e3a 100644 --- a/cmd/routers.go +++ b/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. diff --git a/internal/http/request-recorder.go b/internal/http/request-recorder.go index 0a1a8b62e..0e3df647a 100644 --- a/internal/http/request-recorder.go +++ b/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 } diff --git a/internal/http/stats/http-traffic-recorder.go b/internal/http/stats/http-traffic-recorder.go deleted file mode 100644 index 3b774f467..000000000 --- a/internal/http/stats/http-traffic-recorder.go +++ /dev/null @@ -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 . - -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 -} diff --git a/internal/logger/audit.go b/internal/logger/audit.go index 3cdc03b23..5b80024d2 100644 --- a/internal/logger/audit.go +++ b/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)