search

containerd / log

Common log interface for containerd repositories and clients
Apache License 2.0
4 stars 3 forks source link

consider replacing logrus #2

Open samuelkarp opened 1 year ago

samuelkarp commented 1 year ago

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 commented 1 year ago

:+1: on the stdlib (log/slog)

knight42 commented 1 year ago

👍 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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Currently shim's logs can't add a uniform runtime field, I'll try to fix this by https://github.com/containerd/containerd/pull/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 commented 4 months ago

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 commented 6 days ago

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.