fix: set request ID in tracing context key (#17602)

Since `addCustomerHeaders` middleware was after the `httpTracer`
middleware, the request ID was not set in the http tracing context. By
reordering these middleware functions, the request ID header becomes
available. We also avoid setting the tracing context key again in
`newContext`.

Bonus: All middleware functions are renamed with a "Middleware" suffix
to avoid confusion with http Handler functions.

cmd/auth-handler.go

@@ -595,8 +595,8 @@ func isSupportedS3AuthType(aType authType) bool {
 	return ok
 }
 
-// setAuthHandler to validate authorization header for the incoming request.
+// setAuthMiddleware to validate authorization header for the incoming request.
-func setAuthHandler(h http.Handler) http.Handler {
+func setAuthMiddleware(h http.Handler) http.Handler {
 	// handler for validating incoming authorization headers.
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		tc, ok := r.Context().Value(mcontext.ContextTraceKey).(*mcontext.TraceCtxt)

cmd/crossdomain-xml-handler.go

@@ -30,7 +30,7 @@ const crossDomainXMLEntity = "/crossdomain.xml"
 // When clients request content hosted on a particular source domain and that content make requests
 // directed towards a domain other than its own, the remote domain needs to host a cross-domain
 // policy file that grants access to the source domain, allowing the client to continue the transaction.
-func setCrossDomainPolicy(h http.Handler) http.Handler {
+func setCrossDomainPolicyMiddleware(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// Look for 'crossdomain.xml' in the incoming request.
 		if r.URL.Path == crossDomainXMLEntity {

cmd/crossdomain-xml-handler_test.go

@@ -29,7 +29,7 @@ import (
 func TestCrossXMLHandler(t *testing.T) {
 	// Server initialization.
 	router := mux.NewRouter().SkipClean(true).UseEncodedPath()
-	handler := setCrossDomainPolicy(router)
+	handler := setCrossDomainPolicyMiddleware(router)
 	srv := httptest.NewServer(handler)
 
 	resp, err := http.Get(srv.URL + crossDomainXMLEntity)

cmd/generic-handlers.go

@@ -100,7 +100,7 @@ func isHTTPHeaderSizeTooLarge(header http.Header) bool {
 }
 
 // Limits body and header to specific allowed maximum limits as per S3/MinIO API requirements.
-func setRequestLimitHandler(h http.Handler) http.Handler {
+func setRequestLimitMiddleware(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		tc, ok := r.Context().Value(mcontext.ContextTraceKey).(*mcontext.TraceCtxt)
 
@@ -147,7 +147,7 @@ func guessIsBrowserReq(r *http.Request) bool {
 		globalBrowserEnabled && aType == authTypeAnonymous
 }
 
-func setBrowserRedirectHandler(h http.Handler) http.Handler {
+func setBrowserRedirectMiddleware(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		read := r.Method == http.MethodGet || r.Method == http.MethodHead
 		// Re-direction is handled specifically for browser requests.
@@ -325,7 +325,7 @@ func hasMultipleAuth(r *http.Request) bool {
 
 // requestValidityHandler validates all the incoming paths for
 // any malicious requests.
-func setRequestValidityHandler(h http.Handler) http.Handler {
+func setRequestValidityMiddleware(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		tc, ok := r.Context().Value(mcontext.ContextTraceKey).(*mcontext.TraceCtxt)
 
@@ -425,10 +425,10 @@ func setRequestValidityHandler(h http.Handler) http.Handler {
 	})
 }
 
-// setBucketForwardingHandler middleware forwards the path style requests
+// setBucketForwardingMiddleware middleware forwards the path style requests
 // on a bucket to the right bucket location, bucket to IP configuration
 // is obtained from centralized etcd configuration service.
-func setBucketForwardingHandler(h http.Handler) http.Handler {
+func setBucketForwardingMiddleware(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if globalDNSConfig == nil || !globalBucketFederation ||
 			guessIsHealthCheckReq(r) || guessIsMetricsReq(r) ||
@@ -493,9 +493,9 @@ func setBucketForwardingHandler(h http.Handler) http.Handler {
 	})
 }
 
-// addCustomHeaders adds various HTTP(S) response headers.
+// addCustomHeadersMiddleware adds various HTTP(S) response headers.
 // Security Headers enable various security protections behaviors in the client's browser.
-func addCustomHeaders(h http.Handler) http.Handler {
+func addCustomHeadersMiddleware(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		header := w.Header()
 		header.Set("X-XSS-Protection", "1; mode=block")                                // Prevents against XSS attacks
@@ -539,9 +539,9 @@ func setCriticalErrorHandler(h http.Handler) http.Handler {
 	})
 }
 
-// setUploadForwardingHandler middleware forwards multiparts requests
+// setUploadForwardingMiddleware middleware forwards multiparts requests
 // in a site replication setup to peer that initiated the upload
-func setUploadForwardingHandler(h http.Handler) http.Handler {
+func setUploadForwardingMiddleware(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if !globalSiteReplicationSys.isEnabled() ||
 			guessIsHealthCheckReq(r) || guessIsMetricsReq(r) ||

cmd/generic-handlers_test.go

@@ -155,7 +155,7 @@ func TestSSETLSHandler(t *testing.T) {
 		r.Header = test.Header
 		r.URL = test.URL
 
-		h := setRequestValidityHandler(okHandler)
+		h := setRequestValidityMiddleware(okHandler)
 		h.ServeHTTP(w, r)
 
 		switch {

cmd/http-tracer.go

@@ -65,7 +65,7 @@ func getOpName(name string) (op string) {
 
 // If trace is enabled, execute the request if it is traced by other handlers
 // otherwise, generate a trace event with request information but no response.
-func httpTracer(h http.Handler) http.Handler {
+func httpTracerMiddleware(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// Setup a http request response recorder - this is needed for
 		// http stats requests and audit if enabled.

cmd/routers.go

@@ -41,32 +41,34 @@ func registerDistErasureRouters(router *mux.Router, endpointServerPools Endpoint
 	registerLockRESTHandlers(router)
 }
 
-// List of some generic handlers which are applied for all incoming requests.
+// List of some generic middlewares which are applied for all incoming requests.
-var globalHandlers = []mux.MiddlewareFunc{
+var globalMiddlewares = []mux.MiddlewareFunc{
-	// The generic tracer needs to be the first handler
+	// set x-amz-request-id header and others
-	// to catch all requests returned early by any other handler
+	addCustomHeadersMiddleware,
-	httpTracer,
+	// The generic tracer needs to be the first middleware to catch all requests
-	// Auth handler verifies incoming authorization headers and
+	// returned early by any other middleware (but after the middleware that
-	// routes them accordingly. Client receives a HTTP error for
+	// sets the amz request id).
-	// invalid/unsupported signatures.
+	httpTracerMiddleware,
+	// Auth middleware verifies incoming authorization headers and routes them
+	// accordingly. Client receives a HTTP error for invalid/unsupported
+	// signatures.
 	//
 	// Validates all incoming requests to have a valid date header.
-	setAuthHandler,
+	setAuthMiddleware,
-	// Redirect some pre-defined browser request paths to a static location prefix.
+	// Redirect some pre-defined browser request paths to a static location
-	setBrowserRedirectHandler,
+	// prefix.
-	// Adds 'crossdomain.xml' policy handler to serve legacy flash clients.
+	setBrowserRedirectMiddleware,
-	setCrossDomainPolicy,
+	// Adds 'crossdomain.xml' policy middleware to serve legacy flash clients.
+	setCrossDomainPolicyMiddleware,
 	// Limits all body and header sizes to a maximum fixed limit
-	setRequestLimitHandler,
+	setRequestLimitMiddleware,
 	// Validate all the incoming requests.
-	setRequestValidityHandler,
+	setRequestValidityMiddleware,
-	// set x-amz-request-id header.
+	// Add upload forwarding middleware for site replication
-	addCustomHeaders,
+	setUploadForwardingMiddleware,
-	// Add upload forwarding handler for site replication
+	// Add bucket forwarding middleware
-	setUploadForwardingHandler,
+	setBucketForwardingMiddleware,
-	// Add bucket forwarding handler
+	// Add new middlewares here.
-	setBucketForwardingHandler,
-	// Add new handlers here.
 }
 
 // configureServer handler returns final handler for the http server.
@@ -98,7 +100,7 @@ func configureServerHandler(endpointServerPools EndpointServerPools) (http.Handl
 	// Add API router
 	registerAPIRouter(router)
 
-	router.Use(globalHandlers...)
+	router.Use(globalMiddlewares...)
 
 	return router, nil
 }

cmd/test-utils_test.go

@@ -2070,11 +2070,11 @@ func initTestAPIEndPoints(objLayer ObjectLayer, apiFunctions []string) http.Hand
 	if len(apiFunctions) > 0 {
 		// Iterate the list of API functions requested for and register them in mux HTTP handler.
 		registerAPIFunctions(muxRouter, objLayer, apiFunctions...)
-		muxRouter.Use(globalHandlers...)
+		muxRouter.Use(globalMiddlewares...)
 		return muxRouter
 	}
 	registerAPIRouter(muxRouter)
-	muxRouter.Use(globalHandlers...)
+	muxRouter.Use(globalMiddlewares...)
 	return muxRouter
 }
 

cmd/utils.go

@@ -56,7 +56,6 @@ import (
 	ioutilx "github.com/minio/minio/internal/ioutil"
 	"github.com/minio/minio/internal/logger"
 	"github.com/minio/minio/internal/logger/message/audit"
-	"github.com/minio/minio/internal/mcontext"
 	"github.com/minio/minio/internal/rest"
 	"github.com/minio/mux"
 	"github.com/minio/pkg/certs"
@@ -855,14 +854,7 @@ func newContext(r *http.Request, w http.ResponseWriter, api string) context.Cont
 		VersionID:    strings.TrimSpace(r.Form.Get(xhttp.VersionID)),
 	}
 
-	ctx := context.WithValue(r.Context(),
+	return logger.SetReqInfo(r.Context(), reqInfo)
-		mcontext.ContextTraceKey,
-		&mcontext.TraceCtxt{
-			AmzReqID: reqID,
-		},
-	)
-
-	return logger.SetReqInfo(ctx, reqInfo)
 }
 
 // Used for registering with rest handlers (have a look at registerStorageRESTHandlers for usage example)