From 0b3282ac9fb6bee6958899bd1924541dc03e955a Mon Sep 17 00:00:00 2001 From: Krishna Srinivas Date: Sat, 29 Oct 2016 15:04:16 +0530 Subject: [PATCH] Logging: errorIf fatalIf print in the format [file.go:82:funcName()] (#3127) --- cmd/globals.go | 1 - cmd/logger.go | 46 ++++++------------------------------------- cmd/logger_test.go | 27 +++++++------------------ cmd/main.go | 3 --- cmd/namespace-lock.go | 29 ++------------------------- 5 files changed, 15 insertions(+), 91 deletions(-) diff --git a/cmd/globals.go b/cmd/globals.go index 75a7c430f..3a1f4451d 100644 --- a/cmd/globals.go +++ b/cmd/globals.go @@ -42,7 +42,6 @@ const ( var ( globalQuiet = false // Quiet flag set via command line - globalTrace = false // Trace flag set via environment setting. // Add new global flags here. diff --git a/cmd/logger.go b/cmd/logger.go index 17ebb4323..e70251094 100644 --- a/cmd/logger.go +++ b/cmd/logger.go @@ -17,13 +17,9 @@ package cmd import ( - "bufio" - "bytes" "fmt" "path" - "path/filepath" "runtime" - "runtime/debug" "strings" "github.com/Sirupsen/logrus" @@ -46,37 +42,6 @@ type logger struct { // Add new loggers here. } -// Function takes input with the results from runtime.Caller(1). Depending on the boolean. -// This function can either returned a shotFile form or a longFile form. -func funcFromPC(pc uintptr, file string, line int, shortFile bool) string { - var fn, name string - if shortFile { - fn = strings.Replace(file, path.Join(filepath.ToSlash(GOPATH)+"/src/github.com/minio/minio/cmd/")+"/", "", -1) - name = strings.Replace(runtime.FuncForPC(pc).Name(), "github.com/minio/minio/cmd.", "", -1) - } else { - fn = strings.Replace(file, path.Join(filepath.ToSlash(GOPATH)+"/src/")+"/", "", -1) - name = strings.Replace(runtime.FuncForPC(pc).Name(), "github.com/minio/minio/cmd.", "", -1) - } - return fmt.Sprintf("%s [%s:%d]", name, fn, line) -} - -// stackInfo returns printable stack trace. -func stackInfo() string { - // Convert stack-trace bytes to io.Reader. - rawStack := bufio.NewReader(bytes.NewBuffer(debug.Stack())) - // Skip stack trace lines until our real caller. - for i := 0; i <= 4; i++ { - rawStack.ReadLine() - } - - // Read the rest of useful stack trace. - stackBuf := new(bytes.Buffer) - stackBuf.ReadFrom(rawStack) - - // Strip GOPATH of the build system and return. - return strings.Replace(stackBuf.String(), filepath.ToSlash(GOPATH)+"/src/", "", -1) -} - // Get file, line, function name of the caller. func callerLocation() string { pc, file, line, success := runtime.Caller(2) @@ -84,9 +49,10 @@ func callerLocation() string { file = "" line = 0 } - shortFile := true // We are only interested in short file form. - callerLoc := funcFromPC(pc, file, line, shortFile) - return callerLoc + file = path.Base(file) + name := runtime.FuncForPC(pc).Name() + name = strings.TrimPrefix(name, "github.com/minio/minio/cmd.") + return fmt.Sprintf("[%s:%d:%s()]", file, line, name) } // errorIf synonymous with fatalIf but doesn't exit on error != nil @@ -116,8 +82,8 @@ func fatalIf(err error, msg string, data ...interface{}) { "location": location, "cause": err.Error(), } - if globalTrace { - fields["stack"] = "\n" + stackInfo() + if e, ok := err.(*Error); ok { + fields["stack"] = strings.Join(e.Trace(), " ") } log.WithFields(fields).Fatalf(msg, data...) } diff --git a/cmd/logger_test.go b/cmd/logger_test.go index 2f69a740f..ed8896361 100644 --- a/cmd/logger_test.go +++ b/cmd/logger_test.go @@ -20,31 +20,18 @@ import ( "bytes" "encoding/json" "errors" - "os" - "path/filepath" - "runtime" "testing" "github.com/Sirupsen/logrus" ) -// Tests func obtained from process stack counter. -func TestFuncToPC(t *testing.T) { - GOPATH = filepath.ToSlash(os.Getenv("GOPATH")) - pc, file, line, success := runtime.Caller(0) - if !success { - file = "???" - line = 0 - } - shortFile := true // We are only interested in short file form. - cLocation := funcFromPC(pc, file, line, shortFile) - if cLocation != "TestFuncToPC [logger_test.go:34]" { - t.Fatal("Unexpected caller location found", cLocation) - } - shortFile = false // We are not interested in short file form. - cLocation = funcFromPC(pc, file, line, shortFile) - if cLocation != "TestFuncToPC [github.com/minio/minio/cmd/logger_test.go:34]" { - t.Fatal("Unexpected caller location found", cLocation) +// Tests callerLocation. +func TestCallerLocation(t *testing.T) { + currentLocation := func() string { return callerLocation() } + gotLocation := currentLocation() + expectedLocation := "[logger_test.go:31:TestCallerLocation()]" + if gotLocation != expectedLocation { + t.Errorf("expected : %s, got : %s", expectedLocation, gotLocation) } } diff --git a/cmd/main.go b/cmd/main.go index d0eac02be..7d5769b3f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -71,9 +71,6 @@ VERSION: func init() { // Check if minio was compiled using a supported version of Golang. checkGoVersion() - - // Set global trace flag. - globalTrace = os.Getenv("MINIO_TRACE") == "1" } func migrate() { diff --git a/cmd/namespace-lock.go b/cmd/namespace-lock.go index 386ac5104..4ddc61639 100644 --- a/cmd/namespace-lock.go +++ b/cmd/namespace-lock.go @@ -20,7 +20,6 @@ import ( "errors" "net/url" pathutil "path" - "runtime" "sync" "github.com/minio/dsync" @@ -197,19 +196,7 @@ func (n *nsLockMap) unlock(volume, path, opsID string, readLock bool) { func (n *nsLockMap) Lock(volume, path, opsID string) { readLock := false // This is a write lock. - // The caller information of the lock held has been obtained - // here before calling any other function. - - // Fetching the package, function name and the line number of - // the caller from the runtime. - pc, file, line, success := runtime.Caller(1) - if !success { - file = "???" - line = 0 - } - shortFile := true // We are only interested in short file form. - lockLocation := funcFromPC(pc, file, line, shortFile) - + lockLocation := callerLocation() // Useful for debugging n.lock(volume, path, lockLocation, opsID, readLock) } @@ -223,19 +210,7 @@ func (n *nsLockMap) Unlock(volume, path, opsID string) { func (n *nsLockMap) RLock(volume, path, opsID string) { readLock := true - // The caller information of the lock held has been obtained - // here before calling any other function. - - // Fetching the package, function name and the line number of - // the caller from the runtime. - pc, file, line, success := runtime.Caller(1) - if !success { - file = "???" - line = 0 - } - shortFile := true // We are only interested in short file form. - lockLocation := funcFromPC(pc, file, line, shortFile) - + lockLocation := callerLocation() // Useful for debugging n.lock(volume, path, lockLocation, opsID, readLock) }