Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dlog: Fix issue where the fast-logging broke backward compat #33

Merged
merged 3 commits into from
May 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
# v1.2.5 (TBD)

- Bugfix: `dlog`: v1.2.4 introduced a regression that broke existing
external implementors of the `Logger` interface. This has been
fixed; implementations wishing to opt-in to v1.2.4's fast-logger
behavior must now implement a distinct `OptimizedLogger` interface,
which is not protected by the usual compatibility promises.

# v1.2.4 (2021-08-27)

- Feature: `dlog`: Support deferring formatting of log messages to
Expand Down
29 changes: 26 additions & 3 deletions dlog/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package dlog

import (
"context"
"fmt"
"log"
)

Expand Down Expand Up @@ -39,6 +40,9 @@ func getLogger(ctx context.Context) Logger {
//
// You should only really ever call WithLogger from the initial
// process set up (i.e. directly inside your 'main()' function).
//
// If the logger implements OptimizedLogger, then dlog will take
// advantage of that.
func WithLogger(ctx context.Context, logger Logger) context.Context {
return context.WithValue(ctx, loggerContextKey{}, logger)
}
Expand All @@ -61,22 +65,41 @@ func StdLogger(ctx context.Context, level LogLevel) *log.Logger {
return getLogger(ctx).StdLogger(level)
}

func sprintln(args ...interface{}) string {
// Trim the trailing newline; what we care about is that spaces are added in between
// arguments, not that there's a trailing newline. See also: logrus.Entry.sprintlnn
msg := fmt.Sprintln(args...)
return msg[:len(msg)-1]
}

// If you change any of these, you should also change convenience.go.gen and run `make generate`.

func Log(ctx context.Context, lvl LogLevel, args ...interface{}) {
l := getLogger(ctx)
l.Helper()
l.Log(lvl, args...)
if opt, ok := l.(OptimizedLogger); ok {
opt.UnformattedLog(lvl, args...)
} else {
l.Log(lvl, fmt.Sprint(args...))
}
}

func Logln(ctx context.Context, lvl LogLevel, args ...interface{}) {
l := getLogger(ctx)
l.Helper()
l.Logln(lvl, args...)
if opt, ok := l.(OptimizedLogger); ok {
opt.UnformattedLogln(lvl, args...)
} else {
l.Log(lvl, sprintln(args...))
}
}

func Logf(ctx context.Context, lvl LogLevel, format string, args ...interface{}) {
l := getLogger(ctx)
l.Helper()
l.Logf(lvl, format, args...)
if opt, ok := l.(OptimizedLogger); ok {
opt.UnformattedLogf(lvl, format, args...)
} else {
l.Log(lvl, fmt.Sprintf(format, args...))
}
}
127 changes: 106 additions & 21 deletions dlog/convenience.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 18 additions & 5 deletions dlog/convenience.go.gen
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import (
"context"
"fmt"
)
EOT
emit() {
Expand All @@ -30,25 +31,37 @@
func ${lvlAlias}(ctx context.Context, args ...interface{}) {
l := getLogger(ctx)
l.Helper()
l.Log(LogLevel${lvlReal}, args...)
if opt, ok := l.(OptimizedLogger); ok {
opt.UnformattedLog(LogLevel${lvlReal}, args...)
} else {
l.Log(LogLevel${lvlReal}, fmt.Sprint(args...))
}
}
func ${lvlAlias}ln(ctx context.Context, args ...interface{}) {
l := getLogger(ctx)
l.Helper()
l.Logln(LogLevel${lvlReal}, args...)
if opt, ok := l.(OptimizedLogger); ok {
opt.UnformattedLogln(LogLevel${lvlReal}, args...)
} else {
l.Log(LogLevel${lvlReal}, sprintln(args...))
}
}
func ${lvlAlias}f(ctx context.Context, format string, args ...interface{}) {
l := getLogger(ctx)
l.Helper()
l.Logf(LogLevel${lvlReal}, format, args...)
if opt, ok := l.(OptimizedLogger); ok {
opt.UnformattedLogf(LogLevel${lvlReal}, format, args...)
} else {
l.Log(LogLevel${lvlReal}, fmt.Sprintf(format, args...))
}
}
EOT
}
for lvl in "${levels[@]}"; do
emit "$lvl" "$lvl"
done
# Sort the keys of aliases so we're sure the methods are always generated in the same order
mapfile -d '' sorted < <(printf '%s\0' "${!aliases[@]}" | sort -z)
# Sort the keys of aliases so we're sure the methods are always generated in the same order
mapfile -d '' sorted < <(printf '%s\0' "${!aliases[@]}" | LC_COLLATE=C sort -z)
for alvl in "${sorted[@]}"; do
emit "$alvl" "${aliases[${alvl}]}"
done
Expand Down
13 changes: 1 addition & 12 deletions dlog/dlog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,18 +183,7 @@ func (l testLogger) StdLogger(dlog.LogLevel) *log.Logger {
panic("not implemented")
}

func (l testLogger) Logln(level dlog.LogLevel, args ...interface{}) {
msg := fmt.Sprintln(args...)
msg = msg[:len(msg)-1]
l.Log(level, msg)
}

func (l testLogger) Logf(level dlog.LogLevel, format string, args ...interface{}) {
l.Log(level, fmt.Sprintf(format, args...))
}

func (l testLogger) Log(lvl dlog.LogLevel, args ...interface{}) {
msg := fmt.Sprint(args...)
func (l testLogger) Log(lvl dlog.LogLevel, msg string) {
entry := testLogEntry{
level: lvl,
message: msg,
Expand Down
3 changes: 3 additions & 0 deletions dlog/fallback.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ func getFallbackLogger() Logger {
// not passed around correctly. However, the default fallback logger
// is Logrus and will behave reasonably; in order to make using dlog a
// safe "no brainer".
//
// If the logger implements OptimizedLogger, then dlog will take
// advantage of that.
func SetFallbackLogger(l Logger) {
fallbackLoggerMu.Lock()
defer fallbackLoggerMu.Unlock()
Expand Down
Loading