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
draft: logging implementation design #2010
Changes from all commits
3fc7c41
42dbf3f
b29f106
e1b280c
f4827ee
34fdce8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
package logger | ||
|
||
import ( | ||
"fmt" | ||
"os" | ||
|
||
"go.opentelemetry.io/otel/otellog" | ||
) | ||
|
||
var Logger otellog.Logger = otellog.NewDefaultLogger(os.Stdout, otellog.LogLevelInfo) | ||
|
||
func Tracef(format string, args ...interface{}) { | ||
Logger.Log(otellog.LogLevelTrace, printfArgs{format, args}) | ||
} | ||
|
||
func Trace(args ...interface{}) { | ||
Logger.Log(otellog.LogLevelTrace, printArgs(args)) | ||
} | ||
|
||
func Debugf(format string, args ...interface{}) { | ||
Logger.Log(otellog.LogLevelDebug, printfArgs{format, args}) | ||
} | ||
|
||
func Debug(args ...interface{}) { | ||
Logger.Log(otellog.LogLevelDebug, printArgs(args)) | ||
} | ||
|
||
func DebugDeferred(fn func() string) { | ||
Logger.Log(otellog.LogLevelDebug, stringerFunc(fn)) | ||
} | ||
|
||
func Infof(format string, args ...interface{}) { | ||
Logger.Log(otellog.LogLevelInfo, printfArgs{format, args}) | ||
} | ||
|
||
func Info(args ...interface{}) { | ||
Logger.Log(otellog.LogLevelInfo, printArgs(args)) | ||
} | ||
|
||
func Warnf(format string, args ...interface{}) { | ||
Logger.Log(otellog.LogLevelWarn, printfArgs{format, args}) | ||
} | ||
|
||
func Warn(args ...interface{}) { | ||
Logger.Log(otellog.LogLevelWarn, printArgs(args)) | ||
} | ||
|
||
func Errorf(format string, args ...interface{}) { | ||
Logger.Log(otellog.LogLevelError, printfArgs{format, args}) | ||
} | ||
|
||
func Error(args ...interface{}) { | ||
Logger.Log(otellog.LogLevelError, printArgs(args)) | ||
} | ||
|
||
func Fatalf(format string, args ...interface{}) { | ||
Logger.Log(otellog.LogLevelFatal, printfArgs{format, args}) | ||
} | ||
|
||
func Fatal(args ...interface{}) { | ||
Logger.Log(otellog.LogLevelFatal, printArgs(args)) | ||
} | ||
|
||
type printfArgs struct { | ||
format string | ||
args []interface{} | ||
} | ||
|
||
func (p printfArgs) String() string { | ||
return fmt.Sprintf(p.format, p.args...) | ||
} | ||
|
||
type printArgs []interface{} | ||
|
||
func (p printArgs) String() string { | ||
return fmt.Sprint([]interface{}(p)...) | ||
} | ||
|
||
type stringerFunc func() string | ||
|
||
func (sf stringerFunc) String() string { | ||
return sf() | ||
} | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,80 @@ | ||||||||||||
package otellog | ||||||||||||
|
||||||||||||
import ( | ||||||||||||
"fmt" | ||||||||||||
"io" | ||||||||||||
"sync" | ||||||||||||
"time" | ||||||||||||
) | ||||||||||||
|
||||||||||||
type Logger interface { | ||||||||||||
Log(level LogLevel, msg fmt.Stringer) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
What about this interface? Including the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. I think we can add context and attribute to the interface method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Aneurysm9 , @bhautikpip : In structured logging I used to see an hierarchy of values. Like And I do not understand how it supposed to work in this interface. Is the hierarchy of values stored in the context? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
What about this interface? I think just including the |
||||||||||||
} | ||||||||||||
|
||||||||||||
type LogLevel int | ||||||||||||
|
||||||||||||
const ( | ||||||||||||
// LogLevelTrace is used to for fine-grained debugging event and disabled in default configurations. | ||||||||||||
LogLevelTrace LogLevel = iota + 1 | ||||||||||||
|
||||||||||||
// LogLevelDebug is usually only enabled when debugging. | ||||||||||||
LogLevelDebug | ||||||||||||
|
||||||||||||
// LogLevelInfo is used only for informal event indicates that event has happened. | ||||||||||||
LogLevelInfo | ||||||||||||
|
||||||||||||
// LogLevelWarn is non-critical entries that deserve eyes. | ||||||||||||
LogLevelWarn | ||||||||||||
|
||||||||||||
// LogLevelError is used for errors that should definitely be noted. | ||||||||||||
LogLevelError | ||||||||||||
|
||||||||||||
// LogLevelError is used for fatal errors such as application or system crash. | ||||||||||||
LogLevelFatal | ||||||||||||
) | ||||||||||||
|
||||||||||||
func (ll LogLevel) String() string { | ||||||||||||
switch ll { | ||||||||||||
case LogLevelTrace: | ||||||||||||
return "TRACE" | ||||||||||||
case LogLevelDebug: | ||||||||||||
return "DEBUG" | ||||||||||||
case LogLevelInfo: | ||||||||||||
return "INFO" | ||||||||||||
case LogLevelWarn: | ||||||||||||
return "WARN" | ||||||||||||
case LogLevelError: | ||||||||||||
return "ERROR" | ||||||||||||
case LogLevelFatal: | ||||||||||||
return "FATAL" | ||||||||||||
default: | ||||||||||||
return fmt.Sprintf("UNKNOWNLOGLEVEL<%d>", ll) | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
func NewDefaultLogger(w io.Writer, minLogLevel LogLevel) Logger { | ||||||||||||
return &defaultLogger{w: w, minLevel: minLogLevel} | ||||||||||||
} | ||||||||||||
|
||||||||||||
type defaultLogger struct { | ||||||||||||
mu sync.Mutex | ||||||||||||
w io.Writer | ||||||||||||
minLevel LogLevel | ||||||||||||
} | ||||||||||||
|
||||||||||||
func (l *defaultLogger) Log(ll LogLevel, msg fmt.Stringer) { | ||||||||||||
if ll < l.minLevel { | ||||||||||||
return | ||||||||||||
} | ||||||||||||
Comment on lines
+66
to
+68
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit but I think that important: at this point, it is a little too late: if one does I suggest adding |
||||||||||||
|
||||||||||||
l.mu.Lock() | ||||||||||||
defer l.mu.Unlock() | ||||||||||||
_, _ = fmt.Fprintf(l.w, "%s\t[%s]\t%s\n", time.Now().Format(time.RFC3339), ll, msg) | ||||||||||||
} | ||||||||||||
|
||||||||||||
var NullLogger = nullLogger{} | ||||||||||||
|
||||||||||||
type nullLogger struct{} | ||||||||||||
|
||||||||||||
func (nl nullLogger) Log(ll LogLevel, msg fmt.Stringer) { | ||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to provide a mechanism somewhere for the application author to provide an alternate implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I would like it if this closer followed what we do with the
ErrorHandler
interface.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I was thinking to add
SetLogger
API to add custom logging implementation but I will try to follow @MrAlias's suggestion and lookupErrorHandler
interface to maintain implementation parity within SDK.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I was suggesting, in a very terse manner, was that we include a
SetLogger
andGetLogger
function in theotel
package and move theLogger
interface to theotel
package as well. This would mirror what is done with theErrorHandler
. From there, the default implementation could be abstracted away into an internal package.