Feature Request: Replace glog with Zap logger

[EmadMokhtar]

Feature Description

Overview

Vitess is using glog as its logger. glog package isn't under development "as stated in the README" plus its original version is maintained by Google, and it's a closed-source.

The repository contains an open source version of the log package used inside Google. The master copy of the source lives inside Google, not here. The code in this repo is for export only and is not itself under development. Feature requests will be ignored.

Motivation

Unmaintained package

Replacing an unmaintained package with one is maintained and well-known in the golang community.

Package	Stars	Last commit	OSS
glog	3.2K	20 Aug. 2021	❌
zap	17.3K	28 Oct. 2022	✅

Faster logger

Plus point (which is a marketing point) zap logger is blazingly fast 😉

Verbosity flag

One of my colleague is working on a feature that needs to import vitess libraries. He's building a CLI where -v flag is needed. He stumbled across a problem because the glog package already defines the -v flag. So he can't import Vitess libs and define -v flag.

Use Case(s)

• Use suggered logger for development and unsugered logger for production. Suggered logger i s better for debugging, and it has better readability.
• Avoid the -v flag collision.

[EmadMokhtar]

In case the proposal approved, I can start working on this feature.

[rohit-nayak-ps]

@EmadMokhtar ,thanks for reporting this and offering to work on it. We are going to discuss it internally and hope to get back to you in a week or two.

[deepthi]

Related: https://github.com/vitessio/vitess/issues/11614 Should keep that in mind.

[EmadMokhtar]

Thanks for the comment. I'll do my research on what is the memory friendly logger in Go.

[ejortegau]

Related: https://github.com/planetscale/log

[EmadMokhtar]

Related: #11614 Should keep that in mind. I ran a benchmark tests for multiple logger and AFAIK zap “Production” logger is good choice.

https://github.com/EmadMokhtar/golang-benchmark

[ejortegau]

Planetscale's log is a wrapper for zap, @EmadMokhtar. Coupled with noglog, it can make for an easy migration path, I think.

[rohit-nayak-ps]

@EmadMokhtar @ejortegau, just repeating here what Deepthi said in Vitess Slack: we all think it is a good idea to get this done now. Thanks for this initiative.

It will be good if you can implement this in parts. Since this change will impact a lot of code, doing this in multiple PRs will help get any review feedback earlier in the cycle. We can also start merging and testing sooner in main.

You can take a call on what works best: using PlanetScale's log or some other approach and share it on this issue with your thoughts/decisions.

[EmadMokhtar]

After the merge of #11960 the next step is to make the structure logging as the default logging in Vitess. This will help us in deprecating the glog package and using PlanetScale logger as standard logging in Vitess.

Risks

• Not all components are migrated to PlanetScale logger. So I'm not sure about the dependency of these components and glog package.

[dbussink]

I know it's a bit late, but I think we should at this point also seriously consider https://pkg.go.dev/golang.org/x/exp/slog. Afaik it's aimed to be part of the standard library in the future as well with the goal of having something more standardized across different Go projects.

We might want to change the PlanetScale logger then to wrap exp/slog instead of zap then though to make the migration easier.