From 1de3bd6911a67e0bedb678b64346099b8854d4c3 Mon Sep 17 00:00:00 2001
From: kannappanr <30541348+kannappanr@users.noreply.github.com>
Date: Fri, 5 Jan 2018 11:24:31 -0800
Subject: [PATCH] Save http trace to a file (#5300)

Save http trace to a file instead of displaying it onto the console.
the environment variable MINIO_HTTP_TRACE will be a filepath instead
of a boolean.

This to handle the scenario where both json and http tracing are
turned on. In that case, both http trace and json output are displayed
on the screen making the json not parsable. Loging this trace onto
a file helps us avoid that scenario.

Fixes #5263
---
 cmd/common-main.go   |  7 ++++++-
 cmd/globals.go       |  4 ++--
 cmd/handler-utils.go |  9 ++++-----
 cmd/signals.go       |  3 +++
 cmd/utils.go         |  8 ++++++++
 cmd/utils_test.go    | 15 +++++++++++++++
 6 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/cmd/common-main.go b/cmd/common-main.go
index 1faaf6cf8..98e0d2356 100644
--- a/cmd/common-main.go
+++ b/cmd/common-main.go
@@ -102,7 +102,12 @@ func handleCommonEnvVars() {
 		globalIsBrowserEnabled = bool(browserFlag)
 	}
 
-	globalHTTPTrace = os.Getenv("MINIO_HTTP_TRACE") != ""
+	traceFile := os.Getenv("MINIO_HTTP_TRACE")
+	if traceFile != "" {
+		var err error
+		globalHTTPTraceFile, err = os.OpenFile(traceFile, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0660)
+		fatalIf(err, "error opening file %s", traceFile)
+	}
 
 	globalDomainName = os.Getenv("MINIO_DOMAIN")
 	if globalDomainName != "" {
diff --git a/cmd/globals.go b/cmd/globals.go
index 889510ed1..4e0b842e7 100644
--- a/cmd/globals.go
+++ b/cmd/globals.go
@@ -123,8 +123,8 @@ var (
 	globalHTTPServerErrorCh = make(chan error)
 	globalOSSignalCh        = make(chan os.Signal, 1)
 
-	// Enable HTTP request/response headers and body logging.
-	globalHTTPTrace bool
+	// File to log HTTP request/response headers and body.
+	globalHTTPTraceFile *os.File
 
 	// List of admin peers.
 	globalAdminPeers = adminPeers{}
diff --git a/cmd/handler-utils.go b/cmd/handler-utils.go
index 7dd137798..24b4ca591 100644
--- a/cmd/handler-utils.go
+++ b/cmd/handler-utils.go
@@ -22,7 +22,6 @@ import (
 	"net"
 	"net/http"
 	"net/url"
-	"os"
 	"strings"
 
 	"github.com/minio/minio/pkg/errors"
@@ -251,18 +250,18 @@ func extractPostPolicyFormValues(form *multipart.Form) (filePart io.ReadCloser,
 
 // Log headers and body.
 func httpTraceAll(f http.HandlerFunc) http.HandlerFunc {
-	if !globalHTTPTrace {
+	if globalHTTPTraceFile == nil {
 		return f
 	}
-	return httptracer.TraceReqHandlerFunc(f, os.Stdout, true)
+	return httptracer.TraceReqHandlerFunc(f, globalHTTPTraceFile, true)
 }
 
 // Log only the headers.
 func httpTraceHdrs(f http.HandlerFunc) http.HandlerFunc {
-	if !globalHTTPTrace {
+	if globalHTTPTraceFile == nil {
 		return f
 	}
-	return httptracer.TraceReqHandlerFunc(f, os.Stdout, false)
+	return httptracer.TraceReqHandlerFunc(f, globalHTTPTraceFile, false)
 }
 
 // Returns "/bucketName/objectName" for path-style or virtual-host-style requests.
diff --git a/cmd/signals.go b/cmd/signals.go
index 59ba40529..9739fcc47 100644
--- a/cmd/signals.go
+++ b/cmd/signals.go
@@ -61,6 +61,7 @@ func handleSignals() {
 
 			exit(err == nil && oerr == nil)
 		case osSignal := <-globalOSSignalCh:
+			stopHTTPTrace()
 			log.Printf("Exiting on signal %v\n", osSignal)
 			exit(stopProcess())
 		case signal := <-globalServiceSignalCh:
@@ -71,12 +72,14 @@ func handleSignals() {
 				log.Println("Restarting on service signal")
 				err := globalHTTPServer.Shutdown()
 				errorIf(err, "Unable to shutdown http server")
+				stopHTTPTrace()
 				rerr := restartProcess()
 				errorIf(rerr, "Unable to restart the server")
 
 				exit(err == nil && rerr == nil)
 			case serviceStop:
 				log.Println("Stopping on service signal")
+				stopHTTPTrace()
 				exit(stopProcess())
 			}
 		}
diff --git a/cmd/utils.go b/cmd/utils.go
index 2171297fe..74b05f642 100644
--- a/cmd/utils.go
+++ b/cmd/utils.go
@@ -37,6 +37,14 @@ import (
 	"github.com/pkg/profile"
 )
 
+// Close Http tracing file.
+func stopHTTPTrace() {
+	if globalHTTPTraceFile != nil {
+		errorIf(globalHTTPTraceFile.Close(), "Unable to close httpTraceFile %s", globalHTTPTraceFile.Name())
+		globalHTTPTraceFile = nil
+	}
+}
+
 // make a copy of http.Header
 func cloneHeader(h http.Header) http.Header {
 	h2 := make(http.Header, len(h))
diff --git a/cmd/utils_test.go b/cmd/utils_test.go
index 9d7980f73..db7dbd6e7 100644
--- a/cmd/utils_test.go
+++ b/cmd/utils_test.go
@@ -19,8 +19,10 @@ package cmd
 import (
 	"encoding/json"
 	"errors"
+	"io/ioutil"
 	"net/http"
 	"net/url"
+	"os"
 	"reflect"
 	"strings"
 	"testing"
@@ -50,6 +52,19 @@ func TestCloneHeader(t *testing.T) {
 	}
 }
 
+// Tests closing http tracing file.
+func TestStopHTTPTrace(t *testing.T) {
+	var err error
+	globalHTTPTraceFile, err = ioutil.TempFile("", "")
+	if err != nil {
+		defer os.Remove(globalHTTPTraceFile.Name())
+		stopHTTPTrace()
+		if globalHTTPTraceFile != nil {
+			t.Errorf("globalHTTPTraceFile is not nil, it is expected to be nil")
+		}
+	}
+}
+
 // Tests maximum object size.
 func TestMaxObjectSize(t *testing.T) {
 	sizes := []struct {