gosnowflake
gosnowflake copied to clipboard
SNOW-625282: Make logrus dependency optional or drop it all together
The dependency on logrus creates two problems:
- I'm currently getting a lot of logs in a different format than my log parser is expected by using Snowflake.
- Creating a customer logging implementation that doesn't use logrus is basically not possible.
Would it be possible to make the logrus dependency optional or maybe remove it all together?
Any feedback on this? If accept I could start a pr.
hi, thank you for submitting this request with us. we'll take a look - as with other enhancements, can't promise any timeline on the ETTR so thank you for bearing with us !
a PR of course would be very helpful to get things moving ; however i'm not sure if simply removing logrus
would be feasible here or more like a breaking change.
I would also be interested in dropping logging. I don't think it's very common to do arbitrary logging from libraries like this. I think removing the logging and propagating errors to the caller would make more sense. If it's not an error I don't think it needs to be logged.
If however you still want to use logging from this codebase, maybe considering something more flexible like logr which has an extensive list of implementations or create your own slimmed down interface uses can implement themselves (info- and error severity should be enough).
And if that also isn't of interest, maybe you could expose the global logger here so users can overwrite it?
Hi all! We started considering this issue!
I have a strong Java background and in Java we have a wonderful SLF4J library which is de facto a standard for Java logging API. SLF4J is not a logger itself, it is more like an API which is implemented by multiple logging libraries like logback or log4j. Still, this is great, because all libraries use only SLF API and not come with logging backend of their choice. Not sure if this is it, but it seems that in Go similar library is https://github.com/go-logr/logr
Another solution might be to use https://go.dev/blog/slog . Unfortunately, this is brand new thing, which was introduced in Go 1.21. We have to support older go versions for a few couple of quarters (fingers crossed that less than more).
If you are interested in such changes, please add a reaction. For those in favour in going to logr - react with 🚀 For those in favour in going to slog (will take more time) - react with 🎉 For those in favour of something else - please add your suggestions.
Thanks in advance!
Thanks for picking this up!
As I wrote in my previous post I'd rather not have logs at all and I'm not sure any of logr
or slog
solves that. Moving to logr
would allow me to build my own noop-logger or maybe use something like buflogr
but for me I think best would be to expose the enable
field on the defaultLogger
.
And browsing the code now again I see that you seem to have some magic word where setting the level to OFF
would disable the logger that I missed last time: https://github.com/snowflakedb/gosnowflake/blob/5c89d42a335ea41256e87ff71317ef6f8df6a086/log.go#L49-L60
Since I seem to be able to get the global logger with GetLogger
here https://github.com/snowflakedb/gosnowflake/blob/5c89d42a335ea41256e87ff71317ef6f8df6a086/log.go#L413-L415
I guess I should be able to turn it off completely with:
gosnowflake.GetLogger().SetLogLevel("OFF")
Maybe it would be worth documenting that and/or using that example in cmd/logger/logger.go
or add an explicit Disable
method on the logger?
So with my issue solved, I guess that the most flexible way to consider most users log setup and also the one you wrote would be fastest, moving to logr
might be a better bet than slog
as well!