Skip to content

Commit

Permalink
logging: avoid '%!F(MISSING)' in logs
Browse files Browse the repository at this point in the history
When only passing a single string into a Printf-style function, any %-format
specs in it will be filled accordingly; but there will be no arguments to use.

So now, we'll avoid calling the `{Debug,Info,Warn,Error}f` functions when we
lack arguments.

Signed-off-by: Stephan Renatus <stephan@styra.com>
  • Loading branch information
srenatus committed Jan 29, 2024
1 parent 3acc0b9 commit d7e8842
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 0 deletions.
16 changes: 16 additions & 0 deletions logging/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,21 +126,37 @@ func (l *StandardLogger) GetLevel() Level {

// Debug logs at debug level
func (l *StandardLogger) Debug(fmt string, a ...interface{}) {
if len(a) == 0 {
l.logger.WithFields(l.getFields()).Debug(fmt)
return
}
l.logger.WithFields(l.getFields()).Debugf(fmt, a...)
}

// Info logs at info level
func (l *StandardLogger) Info(fmt string, a ...interface{}) {
if len(a) == 0 {
l.logger.WithFields(l.getFields()).Info(fmt)
return
}
l.logger.WithFields(l.getFields()).Infof(fmt, a...)
}

// Error logs at error level
func (l *StandardLogger) Error(fmt string, a ...interface{}) {
if len(a) == 0 {
l.logger.WithFields(l.getFields()).Error(fmt)
return
}
l.logger.WithFields(l.getFields()).Errorf(fmt, a...)
}

// Warn logs at warn level
func (l *StandardLogger) Warn(fmt string, a ...interface{}) {
if len(a) == 0 {
l.logger.WithFields(l.getFields()).Warn(fmt)
return
}
l.logger.WithFields(l.getFields()).Warnf(fmt, a...)
}

Expand Down
33 changes: 33 additions & 0 deletions logging/logging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"crypto/rand"
"net/url"
"strings"
"testing"

Expand Down Expand Up @@ -45,6 +46,38 @@ func TestCaptureWarningWithErrorSet(t *testing.T) {
}
}

func TestNoFormattingForSingleString(t *testing.T) {
buf := bytes.Buffer{}
logger := New()
logger.SetOutput(&buf)
logger.SetLevel(Debug)

// NOTE(sr): This construction is somewhat realistic: If we fed logger.Error()
// a format string but no args, the golang linters would yell. The indirection
// taken here is enough to not trigger linters.
x := url.PathEscape("/foo/bar/bar")
logger.Debug(x)
logger.Info(x)
logger.Warn(x)
logger.Error(x)

exp := `"%2Ffoo%2Fbar%2Fbar"`
expected := []string{
`level=error msg=` + exp,
`level=warning msg=` + exp,
`level=info msg=` + exp,
`level=debug msg=` + exp,
}
for _, exp := range expected {
if !strings.Contains(buf.String(), exp) {
t.Errorf("expected string %q not found in logs", exp)
}
}
if t.Failed() {
t.Logf("actual output:\n%s", buf.String())
}
}

func TestWithFieldsOverrides(t *testing.T) {
logger := New().
WithFields(map[string]interface{}{"context": "contextvalue"}).
Expand Down

0 comments on commit d7e8842

Please sign in to comment.