Closed udf2457 closed 9 months ago
I agree 👍 Pinging @MDr164 as he was the one who mainly helped abstract this
Technically go-logr, which I implemented before for structured logging purposes, is just an interface definition which tries to implement a common standard for logger (e.g. logrus, which was used before go-logr). We can now go two routes. Keep go-logr and ditch logrus, as go-logr also allows using log/slog as one possible implementation or go full log/slog.
Benefits of using go-logr: People can plug in their own custom logger with minimal effort as long as it implements the interface defined by go-logr.
Benefits of using log/slog: We fully rely on stdlib but other will have to accept the fact that we use log/slog or write custom handlers for it if they need extra functionality.
I personally don't mind either route here but being a go-logr user quite some time, before log/slog existed, I do have my own preference of course.
I would say there is an increasing evidence from recent years in preferring stdlib to third-party.
This is perhaps particularly the case with Go where we are blessed with the good fortune of such a comprehensive stdlib unlike, say Rust where you have to farm out to third-parties for everything.
First there is the supply-chain security aspect which in recent years has become a very real concern. The less moving parts the better.
Second, we again know from recent years, third-party repos have habit of becoming "yesterday's story", either becoming explicit abandonware through project archiving, or implicit through core maintainers loosing interest and just letting it fester. It is the unfortunate side of OSS that continued maintenance is effectively wholly reliant on the goodwill of others.
@rdimitrov via #485 and aided by others has done a lot of work bring together a more sensible and simplified approach to go-tuf
. One of the fundamental aspects mentioned in the opening post of #485 was "regarding the overall maintainability and usability of the current implementation". Thus I would argue that moving as much as possible to stdlib is very much in keeping with the maintainability vision.
I agree with reducing technical debt as much as possible but given that go-logr is still not an implementation but rather just a common interface I would treat it differently here. The idea was to be able to provide your own logger if you for example include go-tuf as a library in a bigger project and we didn't want to make assumptions about other peoples choice of a logger. Given that go-logr is mostly an interface, there is not much technical debt going on here. Don't get me wrong, I don't have any objections but I also do not see much benefit of replacing go-logr. What we could do however is to ditch the logrus implementation and opt for log/slog as our default.
As I am not the sole decision maker I would recommend getting more opinions from the other maintainers on this matter.
I don't have a very strong preference on this one, but I prefer more to keep the interface and switch to using the standard library instead of logrus.
I like that it's a nice balance between allowing users to tailor the library to their existing logging choice but also dropping the 3rd part dependency we have with logrus (not that it's bad, but I do relate to what @udf2457 is saying about reusing as much as possible from the std library).
Of course we can always revisit this if we get enough signals for it, but for the time being the suggestion from @MDr164 seems good enough.
@kommendorkapten - do you have any thoughts on that in relation to how it is used in sigstore-go?
I would first like to say that go-tuf
does not depend on go-logr
. We define our own logging interface. This interface is a sub-set of the interface that go-logr
defines.
The go.mod
containes references to go-logr
yes, but those are all comming from either tests programs or unit tests.
As the interface is very small, it's easy to provide whatever logger the client is using, possibly by wrapping it in a small adapter.
@rdimitrov sigstore-go
does not use any logging, it's all done by returning an error
.
@kommendorkapten - thanks for your feedback 👍 I completely missed to check where we actually use the loggers and you're right - we already have a pretty good go.mod file where:
Perhaps as an action item from this issue, I'll be happy if we move from logrus to log/slog for the tests so we can trim this even more, but other than that it seems that what we have now is pretty lightweight 👍
In any case @udf2457 thanks for bringing this up. I think we all agree that having less 3rd dependencies is always better, so such discussions are quite helpful.
Oh right, totally forgot that we switched to a subset of go-logr defined directly in this repo. In this case I agree and we can use the existing log/slog implementation of go-logr (which also satisfies the subset of course). Should I take care of switching this around or is someone already looking into it?
yeah, please do that @MDr164 I don't think anyone is looking into this.
log/slog
introduced in Go 1.21 brings structured logging that was previously the main reason to use a third-party library.Reducing dependencies on third-party libraries is always a good thing IMHO. :wink: