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

Support for structured logging (using log/slog) #2217

Open
cfiderer opened this issue Dec 11, 2023 · 5 comments
Open

Support for structured logging (using log/slog) #2217

cfiderer opened this issue Dec 11, 2023 · 5 comments
Labels
kind/feature A request for, or a PR adding, new functionality

Comments

@cfiderer
Copy link

As of now, the containers/image library uses the sirupsen/logrus package for logging. This has a major drawback: when performing image activities for several images in different goroutines in parallel, it is not possible to mark or tag the log messages such that they an be attributed to the right activity.

Go 1.21 introduced structured logging (see Structured Logging with slog), allowing logger instances to be used as arguments and attributed to specific activities.

Please consider adding structured loggers to the API calls in the containers/image library.

@mtrmac
Copy link
Collaborator

mtrmac commented Dec 12, 2023

Thanks for reaching out.

At this point, c/image only requires Go 1.19, so this is not yet possible (a migration to Go 1.20 could happen pretty soon, but we are probably going to be stuck on 1.20 support for the supported lifetime of Fedora 38).

Longer-term, yes, logrus is in maintenance mode, and migrating to the standard-library API is the most obvious replacement choice.

  • I think we are fairly unlikely to do a mass-scale logging migration, but requiring new code to use slog would make sense
  • The stable API commitment would make it difficult to add some probably desirable features, like callers submitting non-default Logger value, or even providing a context.Context to every log entry. The API can be extended, but again, that would probably only happen over some time.

@mtrmac mtrmac added the kind/feature A request for, or a PR adding, new functionality label Dec 12, 2023
@cfiderer
Copy link
Author

About the support for older Go versions: the github.com/sagikazarmark/slog-shim package provides a means to migrate to slog before adopting Go 1.21.

@cfiderer
Copy link
Author

Looks as if you are at Go 1.21 now... with #2377

@mtrmac
Copy link
Collaborator

mtrmac commented Apr 26, 2024

First the callers need to be able to handle a library that uses both logging frameworks, something vaguely like https://github.com/mtrmac/skopeo/tree/logrus-deprecate .

It’s… tedious work and for interactive use, the output is not particularly pleasant. For non-debug output progress output, manually using fmt.Sprintf() instead of the structured fields is fairly attractive in some cases.

@cfiderer
Copy link
Author

For Skopeo (and other interactive programs), using fmt.Sprintf() is surely attractive and a lightweight solution.

We use Logrus hooks to transport logging messages into the log/slog environment, so we are (at least) able to collect all log data.

But we use the c/image library in a Kubernetes operator, and we perform several (up to eight) image operations im parallel. In that environment, it is highly desirable to use context-aware logging to separate the sequential activities for each image operation. Furthermore, logging frameworks (like Fluentd) are already collecting structured logs and make them available for queries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature A request for, or a PR adding, new functionality
Projects
None yet
Development

No branches or pull requests

2 participants