akuity
Standardize on go-logr/logr instead of sirupsen/logrus
Proposed Feature
sirupsen/logrus is in maintenance mode. go-logr/logr is a logging API/wrapper that is agnostic to the actual underlying implementation.
Many go libraries, including sigs.k8s.io/controller-runtime/pkg/log, are starting to standardize on the generic go-logr/logr interface, and expect you to set a logr instance if you want log output. e.g.
package logging
import (
runtimelog "sigs.k8s.io/controller-runtime/pkg/log"
"github.com/go-logr/logr"
)
func init() {
var mylogr logr.Logger
runtimelog.SetLogger(mylogr)
}
NOTE: as I mentioned, go-logr/logr is just an interface, we still need to choose the implementation. This can still be sirupsen/logrus, or we can choose zap, klog. Recommend moving off sirupsen/logrus since it is deprecated, but the first step is to move to the go-logr/logr interface.
Motivation
sirupsen/logrus is no longer maintained
Suggested Implementation
Is this specifically for the logging package or for all logging done throughout Kargo.
Everywhere, but I wanted to take a closer look at this first. (Or you can?) I have no great love for logrus, but one very important feature of it that I don't want to lose is the ability to add key/val pairs to log entries. Not all loggers have that, unfortunately, and I like keeping the logs at least semi-structured.
@webstradev were you interested in working on this?
Yeah I'm happy to take a look at the implications of this.
One other consequence of switching to logr is not having warning, and fatal log level's.
I'll do a bit of digging into the logr docs to see what else might occur.
Would we want to move to logr first (sticking with logrus as the implementation) and then change implementations in the future?
And maybe one other question. Have you considered slog which was introduced in go 1.21?
Have you considered slog which was introduced in go 1.21?"
I'd be open to a cook-off, but would also want to hear @jessesuen's opinion.
FYI I'm not very opinionated on the implementation, i have yet to use slog myself other than a quick mess around but it might be worth considering.
@webstradev Did you get a chance to look at this? I've dug around a little and got a provisional PR, but don't want to step on your toes.
Pulling this into v0.6.0 purely on the basis of being low-hanging fruit.
It can be deferred if necessary.