grafana / k6

A modern load testing tool, using Go and JavaScript - https://k6.io
GNU Affero General Public License v3.0
26.04k stars 1.27k forks source link

Switch log library to Zap #216

Closed liclac closed 1 year ago

liclac commented 7 years ago

https://github.com/uber-go/zap

According to my tests, it is significantly faster than logrus, to the point where logging in hot paths would even be okay.

thinkerou commented 4 years ago

@liclac @na-- this switch still needed? I recently am learning k6 and want to contribute some code for more learn it. thanks!

mstoykov commented 4 years ago

I don't think I have ever noticed logrus in any of my profiling of k6.

And while more speed has never hurt anyone, in a lot of places the problem with logging is that it's done using the global logger instead of local one, which will require a lot more refactoring. I think we have made big strides fixing this in the last year and maybe we are nearly there, as me grepping in new-schedulers branch gives me

$ rg 'logrus\.(Info|Warn|Error|Log|Print)\w*\('
stats/cloud/collector.go
133:            logrus.Warn("K6CLOUD_TOKEN is deprecated and will be removed. Use K6_CLOUD_TOKEN instead.")

lib/executor/execution_config_shortcuts.go
121:                    logrus.Warnf(
128:                    logrus.Warnf("`stages` was explicitly set to an empty value, running the script with 1 iteration in 1 VU")
132:                    logrus.Warnf("`execution` was explicitly set to an empty value, running the script with 1 iteration in 1 VU")

Although I am not certain I am grepping for everything possible :) Edit - if I add With I found way more - mostly in the collector/output code where I would expect them :)

After this.. changing logging libraries could possibly be an interesting question, although again it will need to be profiled and see if it actually even have a difference to make :)

I myself haven't used zap so I can't give any personal opinion on whether there are other concerns around moving to it ...

liclac commented 4 years ago

When I wrote this, there weren't any log calls in the VU hot path at all; I don't know if that's changed, but I was curious if we could get it fast enough to be able to log errors and warnings in there - plus debug logging if it's turned on, using Checked Entries: https://godoc.org/go.uber.org/zap#Logger.Check

thinkerou commented 4 years ago

@MStoykov @liclac thank you reply, got it.

na-- commented 1 year ago

Closing this in favor of https://github.com/grafana/k6/issues/2958