Optional enforcement of *Context methods #13
Comments
|
Hi, Thanks for sharing, I really like your ideas! Let's start with implementing the Would you like to make a PR? 😉 |
As described in go-simpler#13, third-party implementations of `slog.Handler` can choose to take values out of a context to supplement the attributes already used. An example of this in my day job is adding trace IDs to enhance serviceability. This change adds the ability to optionally identify `slog` methods that do not take a context and so prevent the ability to take values from that context. Any matches can be resolved by appending `Context` to the method. For example, `slog.Info` becomes `slog.Context`.
As described in go-simpler#13, third-party implementations of `slog.Handler` can choose to take values out of a context to supplement the attributes already used. An example of this in my day job is adding trace IDs to enhance serviceability. This change adds the ability to optionally identify `slog` methods that do not take a context and so prevent the ability to take values from that context. Any matches can be resolved by appending `Context` to the method. For example, `slog.Info` becomes `slog.Context`.
As described in go-simpler#13, third-party implementations of `slog.Handler` can choose to take values out of a context to supplement the attributes already used. An example of this in my day job is adding trace IDs to enhance serviceability. This change adds the ability to optionally identify `slog` methods that do not take a context and so prevent the ability to take values from that context. Any matches can be resolved by appending `Context` to the method. For example, `slog.Info` becomes `slog.Context`.
|
I would indeed :) #14 |
As described in go-simpler#13, third-party implementations of `slog.Handler` can choose to take values out of a context to supplement the attributes already used. An example of this in my day job is adding trace IDs to enhance serviceability. This change adds the ability to optionally identify `slog` methods that do not take a context and so prevent the ability to take values from that context. Any matches can be resolved by appending `Context` to the method. For example, `slog.Info` becomes `slog.Context`.
|
@tmzane Would you be willing to take a PR for my last idea on my OP as well? If yes, any suggestion for the flag name? |
|
I'm not sure about the last one, looks like a really rare case 🤔 Maybe you have an example of such issue in some open source project? I've seen something like this once: msg := fmt.Sprintf(...)
slog.Info(msg, ...)But
|
|
I don't have any open source projects that would need it. Everything I work on day-to-day is unfortunately internal. But perhaps I can explain my rationale. If it's a no at the end, that's ok :) As it stands, I work with lots of team using a variety of logging libraries, e.g. logrus, zap and zerlog. logrus and zap support things like What we want to do is encourage teams to embrace structured logging and move dynamic values into attributes. That's mostly so we can improve our ability to group logs from a wide variety of services in our log aggregation system. It's relatively easy to search for "log.customer_id == 'whatever'" and know you're not going to get false positives. With everything mixed into the message, things get more complicated. As an aside, if we're using |
I meant maybe you've seen it somewhere on github or some other platform, so we can start from something :)
I totally get your rational! In the end, that's the whole point of structured logging. For now, I'm just not sure if this check is common enough and worth a new option. I can see how other checks may be useful in an average project that uses Is there anything else you'd like to report or is it just this case? slog.Info(fmt.Sprintf(...)) |
|
Now that I'm thinking about it, it might look like "message should be a constant value", because if it was dynamic, we wouldn't be able to search for it in a log aggregation system (at least without regexp) 🤔 |
I came across this on golangci/golangci-lint#4133 and it looks to be very close to some of what I wanted to do myself. Given that you have a good foundation here already, I wondered if you'd be open to an additional optional extra.
Right now, the slog API allows methods like
slog.Infoandslog.InfoContext. The stdlib doesn't appear to differentiate between these, but a custom handler might take values from the context and add them as fields, e.g. trace/span IDs. What I'd like to be able to enforce is making sure the context is always specified which means disallowing the methods not suffixed with Context.I suspect this is behaviour that only makes sense with a custom backend, hence the optionality. Is this something you'd be open to?
Another couple of ideas I had that are about consistency and style than API usage:
fmt.Sprintfwhen building log messages in favour of putting interpolated values into attributes instead.The text was updated successfully, but these errors were encountered: