-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Introduce structured logging #2020
Comments
I'd like to work on this, but we should reach a consensus first. I'm not a huge fan of the current logging strategy, because:
The ideal logger interface should be owned by Dex. Logur provides a template for that (both map[string]interface{} and key-value based). Another nice logging library/abstraction is go-kit/log. It's also key-value based, but it's not easy to decouple from the code with an interface. As for the implementation: Uber's Zap seems to be the most popular choice these days. |
Thank you Mark. It feels like logs is one of our weak spots.
|
I came up with a few different options: Option 1 The simplest interface with key-value pair support. type Logger interface {
Debug(msg string, kvs ...interface{})
Info(msg string, kvs ...interface{})
Warn(msg string, kvs ...interface{})
Error(msg string, kvs ...interface{})
} Usage: logger.Info("Something happened", "in", "storage") Option 2 A bit more complex interface with some level of type (and argument count) safety. type Logger interface {
Debug(msg string, kvs ...KeyValue)
Info(msg string, kvs ...KeyValue)
Warn(msg string, kvs ...KeyValue)
Error(msg string, kvs ...KeyValue)
}
type KeyValue struct {
Key string
Value interface{}
}
func KV(key string, value interface{}) KeyValue {
return KeyValue{
Key: key,
Value: value,
}
} Usage: logger.Info("Something happened", log.KV("at", "storage")) Option 3 Zap's API without directly importing Zap in the rest of the application. Has a bit higher maintenance cost. type Logger interface {
Debug(msg string, fields ...Field)
Info(msg string, fields ...Field)
Warn(msg string, fields ...Field)
Error(msg string, fields ...Field)
With(fields ...Field) Logger
}
type Field = zap.Field
func String(key string, value string) Field {
return zap.String(key, value)
} Usage: logger.Info("Something happened", log.String("at", "storage")) Option 4: Just use Zap as is. Generally, I'm not a huge fan of hard-wiring dependencies into my applications, but Go lacks the consensus on a logger interface and Zap's API is type safe and fast. Going with one or the other doesn't mean we have to set that in stone for ever. For example, we could go with option 1 and move to one of the Zap solutions later. The goal is to have some sort of structured logging and to improve developer experience. (But if we can avoid shooting us in the leg or dead ends, that would be great as well) |
This has been long open and while I'd really like to make progress here, the fact that there is an effort to make a standard library structured logger kept me back in the last couple months. We need to make a decision here: go with Zap or wait and hope that Go 1.21 will have a structured logger. |
Yep, I saw that. Though it seems there are lots of concerns raised, so it may not be the final version yet. |
Is your feature request related to a problem?
Logs are currently formatted strings with a single message. If someone were to extract any information it would require parsing the log messages.
Describe the solution you'd like to see
Introduce structured logging. Probably replace the logger interface with this: https://github.com/logur/logur
Describe alternatives you've considered
I don't see an alternative: the current logging strategy is kinda useless IMHO.
Additional context
I noticed this when working on the run group in the serve command: log messages contain server names, listen addresses.
The text was updated successfully, but these errors were encountered: