Repository: pingcap/dumpling
Pull request #23 (2020-01-08 16:05:57)

log: replace the old logger with `zap`

codecov-io: # [Codecov](https://codecov.io/gh/pingcap/dumpling/pull/23?src=pr&el=h1) Report
> Merging [#23](https://codecov.io/gh/pingcap/dumpling/pull/23?src=pr&el=desc) into [master](https://codecov.io/gh/pingcap/dumpling/commit/3e953cee2a6cc6af289ece6322f335f803ecefda?src=pr&el=desc) will **increase** coverage by `1.68%`.
> The diff coverage is `65%`.


```diff
@@            Coverage Diff             @@
##           master      #23      +/-   ##
==========================================
+ Coverage   42.05%   43.74%   +1.68%     
==========================================
  Files          12       12              
  Lines         642      647       +5     
==========================================
+ Hits          270      283      +13     
+ Misses        339      331       -8     
  Partials       33       33
```


Patch:
diff --git a/cmd/dumpling/main.go b/cmd/dumpling/main.go
index 559d3b9..eaace85 100644
--- a/cmd/dumpling/main.go
+++ b/cmd/dumpling/main.go
@@ -22,6 +22,8 @@ import (
 
 	"github.com/pingcap/dumpling/v4/cli"
 	"github.com/pingcap/dumpling/v4/export"
+	"github.com/pingcap/dumpling/v4/log"
+	"go.uber.org/zap"
 )
 
 var (
@@ -33,6 +35,7 @@ var (
 	threads   int
 	outputDir string
 	fileSize  uint64
+	logLevel  string
 )
 
 func init() {
@@ -59,6 +62,8 @@ func init() {
 
 	flag.StringVar(&outputDir, "output", defaultOutputDir, "Output directory")
 	flag.StringVar(&outputDir, "o", defaultOutputDir, "Output directory")
+
+	flag.StringVar(&logLevel, "loglevel", "info", "Log level {debug|info|warn|error|dpanic|panic|fatal}")
 }
 
 var defaultOutputDir = timestampDirName()
@@ -71,6 +76,12 @@ func main() {
 	flag.Parse()
 	println(cli.LongVersion())
 
+	err := log.InitAppLogger(&log.Config{Level: logLevel})
+	if err != nil {
+		_, _ = fmt.Fprintf(os.Stderr, "initialize logger failed: %s", err.Error())
+		os.Exit(1)
+	}
+
 	conf := export.DefaultConfig()
 	conf.Database = database
 	conf.Host = host
@@ -81,9 +92,9 @@ func main() {
 	conf.FileSize = fileSize
 	conf.OutputDirPath = outputDir
 
-	err := export.Dump(conf)
+	err = export.Dump(conf)
 	if err != nil {
-		fmt.Println(err)
+		log.Zap().Error("dump failed", zap.String("error", err.Error()))
 		os.Exit(1)
 	}
 	return
diff --git a/go.mod b/go.mod
index 7a42d6f..dc2dc9f 100644
--- a/go.mod
+++ b/go.mod
@@ -5,6 +5,9 @@ require (
 	github.com/coreos/go-semver v0.3.0
 	github.com/go-sql-driver/mysql v1.4.1
 	github.com/pingcap/check v0.0.0-20191107115940-caf2b9e6ccf4
+	github.com/pingcap/errors v0.11.0
+	github.com/pingcap/log v0.0.0-20191012051959-b742a5d432e9
+	go.uber.org/zap v1.12.0
 	golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e
 	google.golang.org/appengine v1.6.5 // indirect
 )
diff --git a/v4/export/config.go b/v4/export/config.go
index 6a0930f..2ea2ed2 100644
--- a/v4/export/config.go
+++ b/v4/export/config.go
@@ -6,6 +6,8 @@ import (
 	"strings"
 
 	"github.com/coreos/go-semver/semver"
+	"github.com/pingcap/dumpling/v4/log"
+	"go.uber.org/zap"
 )
 
 type Config struct {
@@ -16,7 +18,6 @@ type Config struct {
 	Password string
 	Threads  int
 
-	Logger        Logger
 	FileSize      uint64
 	OutputDirPath string
 	ServerInfo    ServerInfo
@@ -33,7 +34,6 @@ func DefaultConfig() *Config {
 		Port:          3306,
 		Password:      "",
 		Threads:       4,
-		Logger:        &DummyLogger{},
 		FileSize:      UnspecifiedSize,
 		OutputDirPath: ".",
 		ServerInfo:    ServerInfoUnknown,
@@ -63,6 +63,7 @@ var versionRegex = regexp.MustCompile(`^\d+\.\d+\.\d+([0-9A-Za-z-]+(\.[0-9A-Za-z
 var tidbVersionRegex = regexp.MustCompile(`v\d+\.\d+\.\d+([0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?`)
 
 func ParseServerInfo(src string) ServerInfo {
+	log.Zap().Debug("parse server info", zap.String("server info string", src))
 	lowerCase := strings.ToLower(src)
 	serverInfo := ServerInfo{}
 	if strings.Contains(lowerCase, "tidb") {
@@ -75,6 +76,9 @@ func ParseServerInfo(src string) ServerInfo {
 		serverInfo.ServerType = ServerTypeUnknown
 	}
 
+	log.Zap().Info("detect server type",
+		zap.String("type", serverInfo.ServerType.String()))
+
 	var versionStr string
 	if serverInfo.ServerType == ServerTypeTiDB {
 		versionStr = tidbVersionRegex.FindString(src)[1:]
@@ -85,13 +89,25 @@ func ParseServerInfo(src string) ServerInfo {
 	var err error
 	serverInfo.ServerVersion, err = semver.NewVersion(versionStr)
 	if err != nil {
+		log.Zap().Warn("fail to parse version",
+			zap.String("version", versionStr))
 		return serverInfo
 	}
+
+	log.Zap().Info("detect server version",
+		zap.String("version", serverInfo.ServerVersion.String()))
 	return serverInfo
 }
 
 type ServerType int8
 
+func (s ServerType) String() string {
+	if s >= ServerTypeAll {
+		return ""
+	}
+	return serverTypeString[s]
+}
+
 const (
 	ServerTypeUnknown = iota
 	ServerTypeMySQL
@@ -101,6 +117,16 @@ const (
 	ServerTypeAll
 )
 
+var serverTypeString []string
+
+func init() {
+	serverTypeString = make([]string, ServerTypeAll)
+	serverTypeString[ServerTypeUnknown] = "Unknown"
+	serverTypeString[ServerTypeMySQL] = "MySQL"
+	serverTypeString[ServerTypeMariaDB] = "MariaDB"
+	serverTypeString[ServerTypeTiDB] = "TiDB"
+}
+
 type databaseName = string
 
 type tableName = string
diff --git a/v4/export/prepare.go b/v4/export/prepare.go
index a21c503..4f97bb4 100644
--- a/v4/export/prepare.go
+++ b/v4/export/prepare.go
@@ -4,6 +4,8 @@ import (
 	"database/sql"
 	"errors"
 	"strings"
+
+	"github.com/pingcap/dumpling/v4/log"
 )
 
 func detectServerInfo(db *sql.DB) (ServerInfo, error) {
@@ -23,6 +25,7 @@ func prepareDumpingDatabases(conf *Config, db *sql.DB) ([]string, error) {
 }
 
 func listAllTables(db *sql.DB, databaseNames []string) (DatabaseTables, error) {
+	log.Zap().Debug("list all the tables")
 	dbTables := DatabaseTables{}
 	for _, dbName := range databaseNames {
 		err := UseDatabase(db, dbName)
diff --git a/v4/export/writer.go b/v4/export/writer.go
index f2588d3..a633df4 100644
--- a/v4/export/writer.go
+++ b/v4/export/writer.go
@@ -31,7 +31,7 @@ func (f *SimpleWriter) WriteDatabaseMeta(ctx context.Context, db, createSQL stri
 		target:  db,
 		metaSQL: createSQL,
 	}
-	return WriteMeta(meta, fsStringWriter, f.cfg)
+	return WriteMeta(meta, fsStringWriter)
 }
 
 func (f *SimpleWriter) WriteTableMeta(ctx context.Context, db, table, createSQL string) error {
@@ -41,21 +41,21 @@ func (f *SimpleWriter) WriteTableMeta(ctx context.Context, db, table, createSQL
 		target:  table,
 		metaSQL: createSQL,
 	}
-	return WriteMeta(meta, fsStringWriter, f.cfg)
+	return WriteMeta(meta, fsStringWriter)
 }
 
 func (f *SimpleWriter) WriteTableData(ctx context.Context, ir TableDataIR) error {
 	if f.cfg.FileSize == UnspecifiedSize {
 		fileName := path.Join(f.cfg.OutputDirPath, fmt.Sprintf("%s.%s.sql", ir.DatabaseName(), ir.TableName()))
 		fsStringWriter := NewFileSystemStringWriter(fileName, true)
-		return WriteInsert(ir, fsStringWriter, f.cfg)
+		return WriteInsert(ir, fsStringWriter)
 	}
 
 	ir = splitTableDataIntoChunks(ir, f.cfg.FileSize)
 	for chunkCount := 0; ; /* loop */ chunkCount += 1 {
 		fileName := path.Join(f.cfg.OutputDirPath, fmt.Sprintf("%s.%s.%3d.sql", ir.DatabaseName(), ir.TableName(), chunkCount))
 		fsStringWriter := newInterceptStringWriter(NewFileSystemStringWriter(fileName, true))
-		err := WriteInsert(ir, fsStringWriter, f.cfg)
+		err := WriteInsert(ir, fsStringWriter)
 		if err != nil {
 			return err
 		}
diff --git a/v4/export/writer_util.go b/v4/export/writer_util.go
index 1db6634..b838be8 100644
--- a/v4/export/writer_util.go
+++ b/v4/export/writer_util.go
@@ -4,55 +4,56 @@ import (
 	"fmt"
 	"io"
 	"strings"
+
+	"github.com/pingcap/dumpling/v4/log"
+	"go.uber.org/zap"
 )
 
-func WriteMeta(meta MetaIR, w io.StringWriter, cfg *Config) error {
-	log := cfg.Logger
-	log.Debug("start dumping meta data for target %s", meta.TargetName())
+func WriteMeta(meta MetaIR, w io.StringWriter) error {
+	log.Zap().Debug("start dumping meta data", zap.String("target", meta.TargetName()))
 
 	specCmtIter := meta.SpecialComments()
 	for specCmtIter.HasNext() {
-		if err := write(w, fmt.Sprintf("%s\n", specCmtIter.Next()), log); err != nil {
+		if err := write(w, fmt.Sprintf("%s\n", specCmtIter.Next())); err != nil {
 			return err
 		}
 	}
 
-	if err := write(w, fmt.Sprintf("%s;\n", meta.MetaSQL()), log); err != nil {
+	if err := write(w, fmt.Sprintf("%s;\n", meta.MetaSQL())); err != nil {
 		return err
 	}
 
-	log.Debug("finish dumping meta data for target %s", meta.TargetName())
+	log.Zap().Debug("finish dumping meta data", zap.String("target", meta.TargetName()))
 	return nil
 }
 
-func WriteInsert(tblIR TableDataIR, w io.StringWriter, cfg *Config) error {
-	log := cfg.Logger
+func WriteInsert(tblIR TableDataIR, w io.StringWriter) error {
 	rowIter := tblIR.Rows()
 	if !rowIter.HasNext() {
 		return nil
 	}
 
-	log.Debug("start dumping for table %s", tblIR.TableName())
+	log.Zap().Debug("start dumping for table", zap.String("table", tblIR.TableName()))
 	specCmtIter := tblIR.SpecialComments()
 	for specCmtIter.HasNext() {
-		if err := write(w, fmt.Sprintf("%s\n", specCmtIter.Next()), log); err != nil {
+		if err := write(w, fmt.Sprintf("%s\n", specCmtIter.Next())); err != nil {
 			return err
 		}
 	}
 
 	tblName := wrapBackTicks(tblIR.TableName())
-	if err := write(w, fmt.Sprintf("INSERT INTO %s VALUES\n", tblName), log); err != nil {
+	if err := write(w, fmt.Sprintf("INSERT INTO %s VALUES\n", tblName)); err != nil {
 		return err
 	}
 
 	for rowIter.HasNext() {
 		row := MakeRowReceiver(tblIR.ColumnTypes())
 		if err := rowIter.Next(row); err != nil {
-			log.Error("scanning from sql.Row failed, error: %s", err.Error())
+			log.Zap().Error("scanning from sql.Row failed", zap.String("error", err.Error()))
 			return err
 		}
 
-		if err := write(w, row.ToString(), log); err != nil {
+		if err := write(w, row.ToString()); err != nil {
 			return err
 		}
 
@@ -62,18 +63,20 @@ func WriteInsert(tblIR TableDataIR, w io.StringWriter, cfg *Config) error {
 		} else {
 			splitter = ";"
 		}
-		if err := write(w, fmt.Sprintf("%s\n", splitter), log); err != nil {
+		if err := write(w, fmt.Sprintf("%s\n", splitter)); err != nil {
 			return err
 		}
 	}
-	log.Debug("finish dumping for table %s", tblIR.TableName())
+	log.Zap().Debug("finish dumping for table", zap.String("table", tblIR.TableName()))
 	return nil
 }
 
-func write(writer io.StringWriter, str string, logger Logger) error {
+func write(writer io.StringWriter, str string) error {
 	_, err := writer.WriteString(str)
-	if err != nil && logger != nil {
-		logger.Error("writing failed, string: `%s`, error: %s", str, err.Error())
+	if err != nil {
+		log.Zap().Error("writing failed",
+			zap.String("string", str),
+			zap.String("error", err.Error()))
 	}
 	return err
 }
diff --git a/v4/log/log.go b/v4/log/log.go
new file mode 100644
index 0000000..83466d8
--- /dev/null
+++ b/v4/log/log.go
@@ -0,0 +1,59 @@
+package log
+
+import (
+	"github.com/pingcap/errors"
+	pclog "github.com/pingcap/log"
+	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
+)
+
+var (
+	appLogger = Logger{zap.NewNop()}
+	appLevel  = zap.NewAtomicLevel()
+)
+
+type Logger struct {
+	*zap.Logger
+}
+
+// Zap returns the global logger.
+func Zap() Logger {
+	return appLogger
+}
+
+// Config serializes log related config in toml/json.
+type Config struct {
+	// Log level.
+	// One of "debug", "info", "warn", "error", "dpanic", "panic", and "fatal".
+	Level string `toml:"level" json:"level"`
+	// Log filename, leave empty to disable file log.
+	File string `toml:"file" json:"file"`
+	// Max size for a single file, in MB.
+	FileMaxSize int `toml:"max-size" json:"max-size"`
+	// Max log keep days, default is never deleting.
+	FileMaxDays int `toml:"max-days" json:"max-days"`
+	// Maximum number of old log files to retain.
+	FileMaxBackups int `toml:"max-backups" json:"max-backups"`
+}
+
+func InitAppLogger(cfg *Config) error {
+	logger, props, err := pclog.InitLogger(&pclog.Config{
+		Level: cfg.Level,
+		File: pclog.FileLogConfig{
+			Filename:   cfg.File,
+			MaxSize:    cfg.FileMaxSize,
+			MaxDays:    cfg.FileMaxDays,
+			MaxBackups: cfg.FileMaxBackups,
+		},
+	})
+	if err != nil {
+		return errors.WithStack(err)
+	}
+	appLogger = Logger{logger}
+	appLevel = props.Level
+	return nil
+}
+
+func ChangeAppLogLevel(level zapcore.Level) {
+	appLevel.SetLevel(level)
+}


Test patch:
diff --git a/v4/export/test_util.go b/v4/export/test_util.go
index 3851310..e937855 100644
--- a/v4/export/test_util.go
+++ b/v4/export/test_util.go
@@ -130,25 +130,3 @@ func newMockTableIR(databaseName, tableName string, data [][]driver.Value, speci
 		colTypes: colTypes,
 	}
 }
-
-type DummyLogger struct{}
-
-func (d *DummyLogger) Debug(format string, args ...interface{}) {
-	fmt.Printf(format, args...)
-	fmt.Println()
-}
-
-func (d *DummyLogger) Info(format string, args ...interface{}) {
-	fmt.Printf(format, args...)
-	fmt.Println()
-}
-
-func (d *DummyLogger) Warn(format string, args ...interface{}) {
-	fmt.Printf(format, args...)
-	fmt.Println()
-}
-
-func (d *DummyLogger) Error(format string, args ...interface{}) {
-	fmt.Printf(format, args...)
-	fmt.Println()
-}
diff --git a/v4/export/writer_util_test.go b/v4/export/writer_util_test.go
index bdc0434..281171d 100644
--- a/v4/export/writer_util_test.go
+++ b/v4/export/writer_util_test.go
@@ -21,7 +21,6 @@ type testUtilSuite struct {
 
 func (s *testUtilSuite) SetUpSuite(c *C) {
 	s.mockCfg = &Config{
-		Logger:   &DummyLogger{},
 		FileSize: UnspecifiedSize,
 	}
 }
@@ -34,7 +33,7 @@ func (s *testUtilSuite) TestWriteMeta(c *C) {
 	meta := newMockMetaIR("t1", createTableStmt, specCmts)
 	strCollector := &mockStringCollector{}
 
-	err := WriteMeta(meta, strCollector, s.mockCfg)
+	err := WriteMeta(meta, strCollector)
 	c.Assert(err, IsNil)
 	expected := "/*!40103 SET TIME_ZONE='+00:00' */;\n" +
 		"CREATE TABLE `t1` (\n" +
@@ -58,7 +57,7 @@ func (s *testUtilSuite) TestWriteInsert(c *C) {
 	tableIR := newMockTableIR("test", "employee", data, specCmts, colTypes)
 	strCollector := &mockStringCollector{}
 
-	err := WriteInsert(tableIR, strCollector, s.mockCfg)
+	err := WriteInsert(tableIR, strCollector)
 	c.Assert(err, IsNil)
 	expected := "/*!40101 SET NAMES binary*/;\n" +
 		"/*!40014 SET FOREIGN_KEY_CHECKS=0*/;\n" +
@@ -85,7 +84,7 @@ func (s *testUtilSuite) TestSQLDataTypes(c *C) {
 		tableIR := newMockTableIR("test", "t", tableData, nil, colType)
 		strCollector := &mockStringCollector{}
 
-		err := WriteInsert(tableIR, strCollector, s.mockCfg)
+		err := WriteInsert(tableIR, strCollector)
 		c.Assert(err, IsNil)
 		lines := strings.Split(strCollector.buf, "\n")
 		c.Assert(len(lines), Equals, 3)
@@ -99,7 +98,7 @@ func (s *testUtilSuite) TestWrite(c *C) {
 	exp := []string{"test", "loooooooooooooooooooong", "poison_error"}
 
 	for i, s := range src {
-		err := write(mocksw, s, nil)
+		err := write(mocksw, s)
 		if err != nil {
 			c.Assert(err.Error(), Equals, exp[i])
 		} else {
@@ -107,6 +106,6 @@ func (s *testUtilSuite) TestWrite(c *C) {
 			c.Assert(mocksw.buf, Equals, exp[i])
 		}
 	}
-	err := write(mocksw, "test", nil)
+	err := write(mocksw, "test")
 	c.Assert(err, IsNil)
 }