Skip to content
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

switch logger to golang.org/x/exp/slog #96

Closed
komuw opened this issue Mar 4, 2023 · 2 comments
Closed

switch logger to golang.org/x/exp/slog #96

komuw opened this issue Mar 4, 2023 · 2 comments

Comments

@komuw
Copy link

komuw commented Mar 4, 2023

Currently weaver is using a custom logger; https://github.com/ServiceWeaver/weaver/blob/main/logger.go, https://github.com/ServiceWeaver/weaver/tree/main/runtime/logging

There's a proposal to bring structured logging to the Go standard library. That proposal looks likely to be accepted; It was marked as;

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

2days ago: golang/go#56345 (comment)

An implementation of that proposal already exists at; https://pkg.go.dev/golang.org/x/exp/slog

Since weaver is just starting, I think it should switch to golang.org/x/exp/slog early on. It does not have to do so right away, we can wait a few more days to see if the exp/slog proposal is accepted upstream

@komuw komuw changed the title switch logger to https://pkg.go.dev/golang.org/x/exp/slog switch logger to golang.org/x/exp/slog Mar 4, 2023
@spetrovic77
Copy link
Contributor

Thanks for reporting.

Our logger API is based on the slog api. However, our logger needs to intercept all the logs, in order to store them in the appropriate logs storage (e.g., Google Cloud Logging, local files). So we can't use the slog package directly.

@spetrovic77 spetrovic77 closed this as not planned Won't fix, can't repro, duplicate, stale Mar 5, 2023
@flowchartsman
Copy link

flowchartsman commented Mar 6, 2023

Could you please provide a bit more detail on why this was closed? Specifically, I think it could help to clarify how slog integration is not possible. From where I sit, the necessity of intercepting all logs does not preclude using slog, and I have an internal service library implementation that does exactly this via a common logging interface, while also providing access to creating new handlers through a well-defined interface layer. What's to prevent ServiceWeaver from adopting a similar approach? If you enumerate the difficulties or concerns, it could serve as a useful starting point for a PR from someone with experience integrating the slog package. At the very least, this issue could serve as a guideline for exactly why slog can't be used, and could be linked from the documentation to prevent any further issues or PR requests.

Given that slog is intended to serve as a stdlib-standard interface to structured logging, would you consider reopening this issue until the proposal lands? Maybe change the issue title to something like "Investigate integrating the proposed slog package"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants