Logging interface #82
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
January 31, 2019 13:51
The seelog logger had various drawbacks:
- not easily user configurable (e.g. can't disable, can't write to a
file, can't write to stderr, can't write to json, etc)
- uses global seelog instance which can conflict w/ other packages
or user's logger
The replacement logger uses a simple Logger interface:
type Logger interface {
Log(level LogLevel, msg fmt.Stringer)
}
Rather than a separate method for each log level, the level is passed
as an argument. This makes it easy/possible to add more log levels in
the future. The lowest log level is now "debug" instead of seelog's
"trace" (seemed mildly confusing to have a log method named "trace" in
a cloud tracing package).
We use a fmt.Stringer so any log message serialization can be deferred
until the message is actually written.
There is a default Logger implementation the user can use via
NewDefaultLogger(io.Writer, LogLevel) to create a logger that writes
log messages of at least a certain log level to a specified io.Writer.
The logger defaults to a stdout writer of min level "info".
The "xraylog" package is the user visible logging API. It contains
"xray" in the package name so it doesn't annoyingly look like every
other "logger" package (e.g. to goimports). The user can set a new
logger using xray.SetLogger().
The actual logging instance and logging wrapper methods are in an
internal "logger" package hidden from the user. There are
Info(...interface{}) and Infof(string, ...interface{}) varieties for
each log level. In addition there is a DebugDeferred(func() string)
takes an arbitrary function (which won't get called unless the log
message is actually logged).
|
Hi - can someone work with me on this? I think it resolves all issues I know about with the logger. |
|
Hi @muirrn, I am looking at it right now and will you let you about my feedback ASAP. |
luluzhao
suggested changes
Feb 5, 2019
luluzhao
suggested changes
Feb 5, 2019
|
Hi @muirrn , I am currently working on this PR. Will post an update soon. |
yogiraj07
reviewed
Feb 8, 2019
Use a mutex to synchronize calls to underling io.Writer. This avoids log message interleaving and potentially worse race conditions, depending on what the writer is. Also tweak the invalid LogLevel String() to be UNKNOWNLOGLEVEL instead of just UNKNOWN so it is easier to understand what is unknown if it ever shows up.
|
Hi @muirrn , Best, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #15.
Replace seelog with a custom logger interface. This change is backwards compatible except that the existing LogLevel and LogFormat config fields no longer have any effect. Users will have to reset their min log level if they weren't using the default of "info".
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.