Shall we have a logger interface in the runtime pkg?

[souleb]

We would like to have your opinion on the PR: cosi-project/runtime#22

I copy our slack discussion here:

soulé BA 15 h 14
hello 👋

Artem Chernyshev 15 h 18
Hello 😃

soulé BA 15 h 19
Wanted to say sorry for commenting a pr and leaving it unfinished
15 h 19
so just wanted to clarify my thoughts
15 h 19
I understand your views with the sugaredLogger key-val variadic param
15 h 20
but if can have a simple interface
15 h 20
we can have a logging package that we will be able to inject through every cosi pkg
15 h 20
we could add a few helper function for dealing with the key-vals
15 h 21
what do you think?

Artem Chernyshev 15 h 28
question is if it really worth using the interface.
we have two run modes there:

1. As a separate service which is controlled using gRPC -- in this case it doesn't really matter what logger we use inside.
2. As part of the other service -- like it's used in Talos right now.
   So if we imagine use cases for the framework: if someone decides to use it as part of the existing system, he can just wrap his logger into zap.Logger and pass it into the framework.
   Otherwise it's either we implement a simple interface that limits Zap usage, or the interface becomes too complicated and that makes implementing custom logger troublesome (modifié)

soulé BA 15 h 33
on the second use case
15 h 35
limiting usage is a bad move you think? What I see is if it’s successful we will have many contributor and giving them a simple interface, a way of logging is an asset. And if we decide for some reason to change the logger, we will be able to do it easily

Artem Chernyshev 15 h 56
What I see is if it’s successful we will have many contributor and giving them a simple interface
If we decide to make it more flexible and have an ability to swap the logger inside the framework, we can abstract that in a bit different way.
zap.Logger is able to write logs to several outputs, so any specific user implementation can implement that output interface.
Actually it just needs to implement io.Writer.
And we can have a function that can help users construct that link between zap and other log destinations.
As of custom controllers... I think we can hide zap fields under type definition and then it will become possible to swap it with something else later on, like:
logger.Info("msg", logging.Error())
In the end I'm also a bit torn about what is the best solution there. And I don't quite like either (modifié)

Artem Chernyshev 16 h 09
you know, maybe it's better to ask that in cosi-project discussions instead, to get other people opinion on that topic as well
16 h 10
https://github.com/cosi-project/community/discussions this one I mean :visage_légèrement_souriant:

soulé BA 16 h 35
Good idea. Let's copy this thread over there
👍
1