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

consider replacing logrus #2

Open
samuelkarp opened this issue Mar 16, 2023 · 8 comments
Open

consider replacing logrus #2

samuelkarp opened this issue Mar 16, 2023 · 8 comments

Comments

@samuelkarp
Copy link
Member

What is the problem you're trying to solve

logrus is in maintenance mode and is no longer receiving significant development beyond security and bugfixes. Structured logging has continued to evolve in Go and recently the standard-library log/slog package was accepted for inclusion in a future version of Go.

Describe the solution you'd like

With containerd 2.0, we should consider replacing logrus with an alternative. Some possibilities:

Additional context

Replacing logrus has changes for clients of containerd (such as Moby) and plugins like shims. We'd also need to adapt containerd's logging helper (log.G(ctx)) to work with a new logging implementation.

@AkihiroSuda
Copy link
Member

👍 on the stdlib (log/slog)

@knight42
Copy link

👍 on slog as well. FWIW zap is also planning to integrate with slog.

I would like to work on replacing logrus to get familiar with containerd.

@cpuguy83
Copy link
Member

I think the nice thing about slog is is it should (in theory) be useful for library code and then in a binary like containerd or dockerd this can be wrapped in a frontend such as logrus or whatever else.

@thaJeztah
Copy link
Member

Yes; afaik, it was designed to be pluggable, so yes, consumers could still decide to use logrus or anything.

Definitely in favor of using the stdlib interfaces (and context-logger)

@Iceber
Copy link
Member

Iceber commented May 4, 2023

Currently shim's logs can't add a uniform runtime field, I'll try to fix this by containerd/containerd#8374, but the runtime field is still not printed in the dependency packages.

slog supports setting a default logger, and it will be easier for other dependencies to use it once slog is integrated in standard library

@dmcgowan dmcgowan transferred this issue from containerd/containerd Feb 27, 2024
@dmcgowan dmcgowan changed the title consider replacing logrus for 2.0 consider replacing logrus Feb 27, 2024
@dmcgowan
Copy link
Member

Moved this one over to the log repo to continue the discussion. Its not currently in scope for 2.0 but we could consider it here for any version.

@tonistiigi
Copy link
Member

I'd suggest only consider this if there some actual benefits (and no downsides, eg. clunkier ux or more limited wrapping support). In the current comments I haven't seen any listed. In that case, I think there are more meaningful changes to spend time on.

I consider "maintenance mode" a major plus for a logging library. I don't want to spend time porting its potential changes across codebase or syncing hell if components use different versions.

I don't want this to be another "let's refactor all the errors because 🤷‍♂️ " and lose all capability to debug errors because of the removal of stacktraces.

@samuelkarp
Copy link
Member Author

I consider "maintenance mode" a major plus for a logging library.

Just as a note on the purpose of this repository (from the README):

This package is not intended to be used as a standalone logging package outside of the containerd ecosystem and is intended as an interface wrapper around a logging implementation. In the future this package may be replaced with a common go logging interface.

I expect that there will be some advantages of log/slog in the future (including: embedding in standard library means the package will already be there if some dependency uses it, less code for us to maintain, and ecosystem software that consumes the log/slog format). But that doesn't really exist right now, so there's no rush to make changes.

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

No branches or pull requests

8 participants