search

utrack / clay

Proto-first minimal server platform for gRPС+REST+Swagger APIs
MIT License
290 stars 37 forks source link

go.mod have two logger system #76

Closed NOMORECOFFEE closed 4 years ago

NOMORECOFFEE commented 4 years ago

github.com/sirupsen/logrus v1.4.2 and github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b

bullgare commented 4 years ago

Maybe it'd be better to use just an interface with default implementation depending only on embedded "log" package, and introduce an option to set logger implementation?

utrack commented 4 years ago

That's right - glog is used by generator, and logrus is used by a sample server.

Getting rid of glog is not an option atm since we use grpc-gateway subgenerators extensively; but we can split sample server to a separate go module. What do you think?

bullgare commented 4 years ago

Can it be an opt-in solution? Like having an interface with only Error method. And default implementation..? I mean, if I use zap in my project, which is configured in the way I need, it's a little bit odd to see other type of messages in my log.

bullgare commented 4 years ago

That's what I'm talking about - https://github.com/utrack/clay/pull/77/files. That's the playground - https://play.golang.org/p/bNHHRh-xv84.

BTW, tests are failing for me with a message

transport/middlewares/mwhttp/closenotifier.go:9:9: undefined: mwhttp.CloseNotifier

@utrack could you please check it?

utrack commented 4 years ago

I know, I mean that this server is a reference implementation - splitting /server to a separate submodule would be better. If you're using your own logger then you'll probably replace the whole server stack.

I'll fix CloseNotifier asap

utrack commented 4 years ago

Maybe not though - are you using ref server with your own code? If someone does it, then it's better to merge #77.

bullgare commented 4 years ago

Well, the original point was about go.mod. Not to pollute it with unnecessary things. And using logrus there does not make much sense as it has default format anyway and 99% will differ from the format for the service using it. It's up to you, guys. I just made a proposal :)