From a3ef049df93e9d4b4070382c00ac4475dbff3a3c Mon Sep 17 00:00:00 2001 From: Giovanni Bajo Date: Mon, 15 Dec 2014 20:20:33 +0100 Subject: [PATCH 1/4] Avoid extra quotes where not strictly necessary. It's not necessary to enclose in quotes every single string value in log2met format; when using basic words, it's possible to not quote it (as heroku does for its own logging). This keeps the logs easier on the human eye. --- text_formatter.go | 17 +++++++++++++++-- text_formatter_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 text_formatter_test.go diff --git a/text_formatter.go b/text_formatter.go index fc0a408..ee34b50 100644 --- a/text_formatter.go +++ b/text_formatter.go @@ -3,6 +3,7 @@ package logrus import ( "bytes" "fmt" + "regexp" "sort" "strings" "time" @@ -19,11 +20,13 @@ const ( var ( baseTimestamp time.Time isTerminal bool + noQuoteNeeded *regexp.Regexp ) func init() { baseTimestamp = time.Now() isTerminal = IsTerminal() + noQuoteNeeded, _ = regexp.Compile("^[a-zA-Z0-9.-]*$") } func miniTS() int { @@ -87,8 +90,18 @@ func printColored(b *bytes.Buffer, entry *Entry, keys []string) { func (f *TextFormatter) appendKeyValue(b *bytes.Buffer, key, value interface{}) { switch value.(type) { - case string, error: - fmt.Fprintf(b, "%v=%q ", key, value) + case string: + if noQuoteNeeded.MatchString(value.(string)) { + fmt.Fprintf(b, "%v=%s ", key, value) + } else { + fmt.Fprintf(b, "%v=%q ", key, value) + } + case error: + if noQuoteNeeded.MatchString(value.(error).Error()) { + fmt.Fprintf(b, "%v=%s ", key, value) + } else { + fmt.Fprintf(b, "%v=%q ", key, value) + } default: fmt.Fprintf(b, "%v=%v ", key, value) } diff --git a/text_formatter_test.go b/text_formatter_test.go new file mode 100644 index 0000000..4e956c9 --- /dev/null +++ b/text_formatter_test.go @@ -0,0 +1,33 @@ +package logrus + +import ( + "bytes" + "errors" + + "testing" +) + +func TestQuoting(t *testing.T) { + tf := new(TextFormatter) + + checkQuoting := func(q bool, value interface{}) { + b, _ := tf.Format(WithField("test", value)) + idx := bytes.LastIndex(b, []byte{'='}) + cont := bytes.Contains(b[idx:], []byte{'"'}) + if cont != q { + if q { + t.Errorf("quoting expected for: %#v", value) + } else { + t.Errorf("quoting not expected for: %#v", value) + } + } + } + + checkQuoting(false, "abcd") + checkQuoting(false, "v1.0") + checkQuoting(true, "/foobar") + checkQuoting(true, "x y") + checkQuoting(true, "x,y") + checkQuoting(false, errors.New("invalid")) + checkQuoting(true, errors.New("invalid argument")) +} From 98ee5434ef02856cc9289f546c38d12a3244fe15 Mon Sep 17 00:00:00 2001 From: Giovanni Bajo Date: Thu, 18 Dec 2014 07:59:41 +0100 Subject: [PATCH 2/4] Make the test more robust --- text_formatter_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/text_formatter_test.go b/text_formatter_test.go index 4e956c9..f604f1b 100644 --- a/text_formatter_test.go +++ b/text_formatter_test.go @@ -8,12 +8,12 @@ import ( ) func TestQuoting(t *testing.T) { - tf := new(TextFormatter) + tf := &TextFormatter{DisableColors: true} checkQuoting := func(q bool, value interface{}) { b, _ := tf.Format(WithField("test", value)) - idx := bytes.LastIndex(b, []byte{'='}) - cont := bytes.Contains(b[idx:], []byte{'"'}) + idx := bytes.Index(b, ([]byte)("test=")) + cont := bytes.Contains(b[idx+5:], []byte{'"'}) if cont != q { if q { t.Errorf("quoting expected for: %#v", value) From 15b296befc8fa20d93c34f6ca2ba934444d27749 Mon Sep 17 00:00:00 2001 From: Giovanni Bajo Date: Thu, 18 Dec 2014 15:09:01 +0100 Subject: [PATCH 3/4] Avoid using regexp --- text_formatter.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/text_formatter.go b/text_formatter.go index ee34b50..658df9b 100644 --- a/text_formatter.go +++ b/text_formatter.go @@ -26,7 +26,6 @@ var ( func init() { baseTimestamp = time.Now() isTerminal = IsTerminal() - noQuoteNeeded, _ = regexp.Compile("^[a-zA-Z0-9.-]*$") } func miniTS() int { @@ -88,16 +87,28 @@ func printColored(b *bytes.Buffer, entry *Entry, keys []string) { } } +func needsQuoting(text string) bool { + for _, ch := range text { + if !((ch >= 'a' && ch <= 'z') || + (ch >= 'A' && ch <= 'Z') || + (ch >= '0' && ch < '9') || + ch == '-' || ch == '.') { + return false + } + } + return true +} + func (f *TextFormatter) appendKeyValue(b *bytes.Buffer, key, value interface{}) { switch value.(type) { case string: - if noQuoteNeeded.MatchString(value.(string)) { + if needsQuoting(value.(string)) { fmt.Fprintf(b, "%v=%s ", key, value) } else { fmt.Fprintf(b, "%v=%q ", key, value) } case error: - if noQuoteNeeded.MatchString(value.(error).Error()) { + if needsQuoting(value.(error).Error()) { fmt.Fprintf(b, "%v=%s ", key, value) } else { fmt.Fprintf(b, "%v=%q ", key, value) From e5c1364122680f75bf4615a880a4afae08ccef53 Mon Sep 17 00:00:00 2001 From: Giovanni Bajo Date: Thu, 18 Dec 2014 15:14:24 +0100 Subject: [PATCH 4/4] Fix test to adjust for missing quotes --- logrus_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/logrus_test.go b/logrus_test.go index 15157d1..0682281 100644 --- a/logrus_test.go +++ b/logrus_test.go @@ -44,8 +44,12 @@ func LogAndAssertText(t *testing.T, log func(*Logger), assertions func(fields ma } kvArr := strings.Split(kv, "=") key := strings.TrimSpace(kvArr[0]) - val, err := strconv.Unquote(kvArr[1]) - assert.NoError(t, err) + val := kvArr[1] + if kvArr[1][0] == '"' { + var err error + val, err = strconv.Unquote(val) + assert.NoError(t, err) + } fields[key] = val } assertions(fields)