Closed
Description
We propose a new package providing structured logging with levels. Structured logging adds key-value pairs to a human-readable output message to enable fast, accurate processing of large amounts of log data.
See the design doc for details.
Metadata
Metadata
Assignees
Type
Projects
Relationships
Development
No branches or pull requests
Activity
jba commentedon Oct 20, 2022
fsouza commentedon Oct 20, 2022
jba commentedon Oct 20, 2022
mpx commentedon Oct 22, 2022
This is a huge API surface without any real production testing (AIUI). Perhaps it might be better to land it under golang.org/x for some time? Eg, like context, xerrors changes.
seankhliao commentedon Oct 22, 2022
It's available under golang.org/x/exp/slog
mpx commentedon Oct 22, 2022
deefdragon commentedon Oct 24, 2022
I love most of what this does, but I don't support its addition as it stands. Specifically, I have issues with the option to use inline key-value pairs in the log calls. I believe the attributes system alone is fine. Logging does not need the breakage that key-value args like that allow.
The complexity in the documentation around Log should be a warning sign.
The suggestion was that potential problems with key-value misalignment will all be solved by vet checks. As I mentioned in this thread of the discussion, relying on vet should be viewed a warning as to potential problems with the design, not a part of the design itself. Vet should not be a catch-all, and we should do what we can to avoid requiring vet warnings to safely develop go.
An accidentally deleted/missed or added/extra argument in key value logging would offset the keys and values after it. That could easily bog down a logging system trying to index all the new "keys" it is getting. It could also lead to data being exposed in a way that it should not be.
I acknowledge that neither of these examples are all that likely in a well defined system, or somewhere with good practices around code reviewing etc.. But they are possible.
hherman1 commentedon Oct 24, 2022
@deefdragon Doesn't this concern apply to Printf as well? Is the difference the dependency on these logs by external systems..?
prochac commentedon Oct 24, 2022
Based on that the Go standard library is very often being recommended as source of idiomatic code, and this package aspires to be merged as part of it, I would like you explain to me the involvement of context package.
If some object uses logger, isn't it its dependency? Shouldn't we make the dependencies explicit? Isn't this logger smuggling bad practice? If passing the logger by context is idiomatic, is *sql.DB too?
Why has the logger stored context? It violates the most famous sentence from the documentation for context
Logger in context containing its own context inside ...
Frankly, I'm a bit confused.
deefdragon commentedon Oct 24, 2022
@hherman1 The same concern does apply to printf, tho it's not as bad compared to logging. With printf, most statements are consumed as a single chunk, and only consumed locally by the programmer. Being misaligned is easy enough for a human to notice, parse, and correct.
In the case of Sprintf, where it might not be consumed by the programmer, and instead be used as an argument to something, the "testing" during development that is making sure the program starts would likely catch most misalignment.
Being off by one in a log is much harder to catch as it has no real impact in the program's execution. You only notice there is an issue when you have to go through your logs.
mminklet commentedon Oct 25, 2022
I think I share some of @prochac 's concerns regarding context. Maybe I'm being a bit of a luddite, but recommending that the logger is passed around inside context rather than via explicit dependency injection, smells a bit funny to me. Context, from what I have always followed, is for request-scoped information, rather than dependencies. And the more clarity surfacing dependencies the better. IE just assuming the right logger is in context, and getting the default one because it's still getting some logger
943 remaining items