7

proposal: log/slog: structured, leveled logging · Issue #56345 · golang/go · Git...

 1 year ago
source link: https://github.com/golang/go/issues/56345
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

proposal: log/slog: structured, leveled logging #56345

jba opened this issue 5 days ago · 10 comments

Comments

Contributor

jba commented 5 days ago

edited

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 (CL review in progress).

fsouza, zephyrtronium, r5sec5cyl, willfaught, AndrewHarrisSPU, carlmjohnson, gilcrest, wdvxdr1123, komuw, rverton, and 4 more reacted with thumbs up emojiseptemhill, fsouza, smlx, r5sec5cyl, gilcrest, hnakamur, and bytheway reacted with hooray emojiericlagergren reacted with eyes emoji All reactions

This comment has been hidden.

This comment has been hidden.

This comment has been hidden.

Contributor

mpx commented 2 days ago

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.

aarzilli, fsouza, flibustenet, prochac, deefdragon, and icholy reacted with thumbs up emoji All reactions

Member

seankhliao commented 2 days ago

It's available under golang.org/x/exp/slog

ysomad reacted with hooray emoji All reactions

Contributor

mpx commented 2 days ago

It's available under golang.org/x/exp/slog

True, but it is unlikely to see much production testing under exp.

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 attribute arguments are processed as follows:

  • If an argument is an Attr, it is used as is.
  • If an argument is a string and this is not the last argument, the following argument is treated as the value and the two are combined into an Attr.
  • Otherwise, the argument is treated as a value with key "!BADKEY".

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.

nussjustin, captainbeardo, icholy, shanna, and urandom reacted with thumbs up emojiicholy reacted with heart emoji All reactions

@deefdragon Doesn't this concern apply to Printf as well? Is the difference the dependency on these logs by external systems..?

prochac commented 2 hours ago

edited

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

Contexts should not be stored inside a struct type, but instead passed to each function that needs it.

Logger in context containing its own context inside ...

Frankly, I'm a bit confused.

@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.

icholy and urandom reacted with thumbs up emoji All reactions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Assignees

No one assigned

Labels
Projects

Status: Incoming

Milestone

Proposal

Development

No branches or pull requests

8 participants

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK