Skip to content

log/slog: structured, leveled logging #56345

Closed
@jba

Description

@jba
Contributor

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.

Activity

added this to the Proposal milestone on Oct 20, 2022
jba

jba commented on Oct 20, 2022

@jba
Author
fsouza

fsouza commented on Oct 20, 2022

@fsouza
jba

jba commented on Oct 20, 2022

@jba
Author
moved this to Incoming in Proposalson Oct 20, 2022
mpx

mpx commented on Oct 22, 2022

@mpx
Contributor

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

seankhliao commented on Oct 22, 2022

@seankhliao
Member

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

mpx

mpx commented on Oct 22, 2022

@mpx
deefdragon

deefdragon commented on Oct 24, 2022

@deefdragon

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.

hherman1

hherman1 commented on Oct 24, 2022

@hherman1

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

prochac

prochac commented on Oct 24, 2022

@prochac

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.

deefdragon

deefdragon commented on Oct 24, 2022

@deefdragon

@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

mminklet commented on Oct 25, 2022

@mminklet

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

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @thomasjo@dottedmag@abh@pohly@zellyn

        Issue actions

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