responses to code review

- field rename to be more properly generic
 - drop rewrite of main.main
This commit is contained in:
Dave Clendenan 2016-12-06 12:53:21 -08:00
parent d8fd23467c
commit 88dd8df1f8
7 changed files with 50 additions and 49 deletions

View File

@ -75,7 +75,10 @@ time="2015-03-26T01:27:38-04:00" level=fatal method=arcticcreatures.migrate msg=
``` ```
Note that this does add measurable overhead - the cost will depend on the of Go, but is Note that this does add measurable overhead - the cost will depend on the of Go, but is
between 20 and 40% in recent tests with 1.6 and 1.7. You can validate this in your between 20 and 40% in recent tests with 1.6 and 1.7. You can validate this in your
environment via benchmarks: ```go test -bench=.*CallerTracing``` environment via benchmarks:
```
go test -bench=.*CallerTracing
```
#### Example #### Example

View File

@ -138,11 +138,7 @@ func getCaller() (method string) {
// If the caller isn't part of this package, we're done // If the caller isn't part of this package, we're done
if pkg != logrusPackage { if pkg != logrusPackage {
if fullFuncName == "main.main" { return fullFuncName
return "main"
} else {
return fullFuncName
}
} }
} }

View File

@ -1,6 +1,8 @@
package logrus package logrus
import "time" import (
"time"
)
// DefaultTimestampFormat is YYYY-mm-DDTHH:MM:SS-TZ // DefaultTimestampFormat is YYYY-mm-DDTHH:MM:SS-TZ
const DefaultTimestampFormat = time.RFC3339 const DefaultTimestampFormat = time.RFC3339
@ -19,7 +21,7 @@ type Formatter interface {
Format(*Entry) ([]byte, error) Format(*Entry) ([]byte, error)
} }
// This is to not silently overwrite `time`, `msg`, `method` and `level` fields when // This is to not silently overwrite `time`, `msg`, `func` and `level` fields when
// dumping it. If this code wasn't there doing: // dumping it. If this code wasn't there doing:
// //
// logrus.WithField("level", 1).Info("hello") // logrus.WithField("level", 1).Info("hello")
@ -44,10 +46,10 @@ func prefixFieldClashes(data Fields, reportCaller bool) {
data["fields.level"] = l data["fields.level"] = l
} }
// If reportCaller is not set, 'method' will not conflict. // If reportCaller is not set, 'func' will not conflict.
if reportCaller { if reportCaller {
if l, ok := data["method"]; ok { if l, ok := data["func"]; ok {
data["fields.method"] = l data["fields.func"] = l
} }
} }
} }

View File

@ -9,10 +9,10 @@ type fieldKey string
type FieldMap map[fieldKey]string type FieldMap map[fieldKey]string
const ( const (
FieldKeyMsg = "msg" FieldKeyMsg = "msg"
FieldKeyLevel = "level" FieldKeyLevel = "level"
FieldKeyTime = "time" FieldKeyTime = "time"
FieldKeyCaller = "method" FieldKeyFunc = "func"
) )
func (f FieldMap) resolve(key fieldKey) string { func (f FieldMap) resolve(key fieldKey) string {
@ -34,10 +34,10 @@ type JSONFormatter struct {
// As an example: // As an example:
// formatter := &JSONFormatter{ // formatter := &JSONFormatter{
// FieldMap: FieldMap{ // FieldMap: FieldMap{
// FieldKeyTime: "@timestamp", // FieldKeyTime: "@timestamp",
// FieldKeyLevel: "@level", // FieldKeyLevel: "@level",
// FieldKeyMsg: "@message", // FieldKeyMsg: "@message",
// FieldKeyCaller: "@caller", // FieldKeyFunc: "@caller",
// }, // },
// } // }
FieldMap FieldMap FieldMap FieldMap
@ -69,7 +69,7 @@ func (f *JSONFormatter) Format(entry *Entry) ([]byte, error) {
data[f.FieldMap.resolve(FieldKeyMsg)] = entry.Message data[f.FieldMap.resolve(FieldKeyMsg)] = entry.Message
data[f.FieldMap.resolve(FieldKeyLevel)] = entry.Level.String() data[f.FieldMap.resolve(FieldKeyLevel)] = entry.Level.String()
if entry.HasCaller() { if entry.HasCaller() {
data[f.FieldMap.resolve(FieldKeyCaller)] = entry.Caller data[f.FieldMap.resolve(FieldKeyFunc)] = entry.Caller
} }
serialized, err := json.Marshal(data) serialized, err := json.Marshal(data)
if err != nil { if err != nil {

View File

@ -174,7 +174,7 @@ func TestFieldDoesNotClashWithCaller(t *testing.T) {
SetReportCaller(false) SetReportCaller(false)
formatter := &JSONFormatter{} formatter := &JSONFormatter{}
b, err := formatter.Format(WithField("method", "howdy pardner")) b, err := formatter.Format(WithField("func", "howdy pardner"))
if err != nil { if err != nil {
t.Fatal("Unable to format entry: ", err) t.Fatal("Unable to format entry: ", err)
} }
@ -185,15 +185,15 @@ func TestFieldDoesNotClashWithCaller(t *testing.T) {
t.Fatal("Unable to unmarshal formatted entry: ", err) t.Fatal("Unable to unmarshal formatted entry: ", err)
} }
if entry["method"] != "howdy pardner" { if entry["func"] != "howdy pardner" {
t.Fatal("method field replaced when ReportCaller=false") t.Fatal("func field replaced when ReportCaller=false")
} }
} }
func TestFieldClashWithCaller(t *testing.T) { func TestFieldClashWithCaller(t *testing.T) {
SetReportCaller(true) SetReportCaller(true)
formatter := &JSONFormatter{} formatter := &JSONFormatter{}
e := WithField("method", "howdy pardner") e := WithField("func", "howdy pardner")
e.Caller = "somefunc" e.Caller = "somefunc"
b, err := formatter.Format(e) b, err := formatter.Format(e)
if err != nil { if err != nil {
@ -206,14 +206,14 @@ func TestFieldClashWithCaller(t *testing.T) {
t.Fatal("Unable to unmarshal formatted entry: ", err) t.Fatal("Unable to unmarshal formatted entry: ", err)
} }
if entry["fields.method"] != "howdy pardner" { if entry["fields.func"] != "howdy pardner" {
t.Fatalf("fields.method not set to original method field when ReportCaller=true (got '%s')", t.Fatalf("fields.func not set to original func field when ReportCaller=true (got '%s')",
entry["fields.method"]) entry["fields.func"])
} }
if entry["method"] != "somefunc" { if entry["func"] != "somefunc" {
t.Fatalf("method not set as expected when ReportCaller=true (got '%s')", t.Fatalf("func not set as expected when ReportCaller=true (got '%s')",
entry["method"]) entry["func"])
} }
SetReportCaller(false) // return to default value SetReportCaller(false) // return to default value

View File

@ -57,7 +57,7 @@ func LogAndAssertText(t *testing.T, log func(*Logger), assertions func(fields ma
assertions(fields) assertions(fields)
} }
// TestReportCaller verifies that when ReportCaller is set, the 'method' field // TestReportCaller verifies that when ReportCaller is set, the 'func' field
// is added, and when it is unset it is not set or modified // is added, and when it is unset it is not set or modified
func TestReportCaller(t *testing.T) { func TestReportCaller(t *testing.T) {
LogAndAssertJSON(t, func(log *Logger) { LogAndAssertJSON(t, func(log *Logger) {
@ -66,7 +66,7 @@ func TestReportCaller(t *testing.T) {
}, func(fields Fields) { }, func(fields Fields) {
assert.Equal(t, "testNoCaller", fields["msg"]) assert.Equal(t, "testNoCaller", fields["msg"])
assert.Equal(t, "info", fields["level"]) assert.Equal(t, "info", fields["level"])
assert.Equal(t, nil, fields["method"]) assert.Equal(t, nil, fields["func"])
}) })
LogAndAssertJSON(t, func(log *Logger) { LogAndAssertJSON(t, func(log *Logger) {
@ -75,7 +75,7 @@ func TestReportCaller(t *testing.T) {
}, func(fields Fields) { }, func(fields Fields) {
assert.Equal(t, "testWithCaller", fields["msg"]) assert.Equal(t, "testWithCaller", fields["msg"])
assert.Equal(t, "info", fields["level"]) assert.Equal(t, "info", fields["level"])
assert.Equal(t, "testing.tRunner", fields["method"]) assert.Equal(t, "testing.tRunner", fields["func"])
}) })
} }
@ -279,36 +279,36 @@ func TestNestedLoggingReportsCorrectCaller(t *testing.T) {
err := json.Unmarshal(buffer.Bytes(), &fields) err := json.Unmarshal(buffer.Bytes(), &fields)
assert.NoError(t, err, "should have decoded first message") assert.NoError(t, err, "should have decoded first message")
assert.Equal(t, len(fields), 5, "should have msg/time/level/method/context fields") assert.Equal(t, len(fields), 5, "should have msg/time/level/func/context fields")
assert.Equal(t, "looks delicious", fields["msg"]) assert.Equal(t, "looks delicious", fields["msg"])
assert.Equal(t, "eating raw fish", fields["context"]) assert.Equal(t, "eating raw fish", fields["context"])
assert.Equal(t, "testing.tRunner", fields["method"]) assert.Equal(t, "testing.tRunner", fields["func"])
buffer.Reset() buffer.Reset()
logger.WithFields(Fields{ logger.WithFields(Fields{
"foo": "a", "Clyde": "Stubblefield",
}).WithFields(Fields{ }).WithFields(Fields{
"bar": "b", "Jab'o": "Starks",
}).WithFields(Fields{ }).WithFields(Fields{
"baz": "c", "uri": "https://www.youtube.com/watch?v=V5DTznu-9v0",
}).WithFields(Fields{ }).WithFields(Fields{
"method": "man", "func": "y drummer",
}).WithFields(Fields{ }).WithFields(Fields{
"clan": "Wu Tang", "James": "Brown",
}).Print("omg it is!") }).Print("The hardest workin' man in show business")
err = json.Unmarshal(buffer.Bytes(), &fields) err = json.Unmarshal(buffer.Bytes(), &fields)
assert.NoError(t, err, "should have decoded second message") assert.NoError(t, err, "should have decoded second message")
assert.Equal(t, 10, len(fields), "should have all builtin fields plus foo,bar,baz,...") assert.Equal(t, 10, len(fields), "should have all builtin fields plus foo,bar,baz,...")
assert.Equal(t, "omg it is!", fields["msg"]) assert.Equal(t, "Stubblefield", fields["Clyde"])
assert.Equal(t, "a", fields["foo"]) assert.Equal(t, "Starks", fields["Jab'o"])
assert.Equal(t, "b", fields["bar"]) assert.Equal(t, "https://www.youtube.com/watch?v=V5DTznu-9v0", fields["uri"])
assert.Equal(t, "c", fields["baz"]) assert.Equal(t, "y drummer", fields["fields.func"])
assert.Equal(t, "man", fields["fields.method"]) assert.Equal(t, "Brown", fields["James"])
assert.Equal(t, "Wu Tang", fields["clan"]) assert.Equal(t, "The hardest workin' man in show business", fields["msg"])
assert.Nil(t, fields["fields.msg"], "should not have prefixed previous `msg` entry") assert.Nil(t, fields["fields.msg"], "should not have prefixed previous `msg` entry")
assert.Equal(t, "testing.tRunner", fields["method"]) assert.Equal(t, "testing.tRunner", fields["func"])
logger.ReportCaller = false // return to default value logger.ReportCaller = false // return to default value
} }

View File

@ -89,7 +89,7 @@ func (f *TextFormatter) Format(entry *Entry) ([]byte, error) {
} }
f.appendKeyValue(b, "level", entry.Level.String()) f.appendKeyValue(b, "level", entry.Level.String())
if entry.HasCaller() { if entry.HasCaller() {
f.appendKeyValue(b, "method", entry.Caller) f.appendKeyValue(b, "func", entry.Caller)
} }
if entry.Message != "" { if entry.Message != "" {
f.appendKeyValue(b, "msg", entry.Message) f.appendKeyValue(b, "msg", entry.Message)