mkabilov · alexeyklyukin · Jan 7, 2019 · Jan 7, 2019 · Jan 7, 2019
diff --git a/cmd/backup/config.yaml b/cmd/backup/config.yaml
@@ -16,4 +16,8 @@ db:
   port: 5432
   database: postgres
   user: postgres
+log:
+  development: false
+  level: info
+  location: true
 
diff --git a/cmd/backup/main.go b/cmd/backup/main.go
@@ -12,9 +12,9 @@ import (
 )
 
 var (
-	configFile = flag.String("config", "config.yaml", "path to the config file")
-	version    = flag.Bool("version", false, "print version information")
-	debug      = flag.Bool("debug", false, "enable debug mode")
+	configFile  = flag.String("config", "config.yaml", "path to the config file")
+	version     = flag.Bool("version", false, "print version information")
+	development = flag.Bool("log-development", false, "enable development logging mode")
 
 	Version  string
 	Revision string
@@ -28,8 +28,8 @@ func buildInfo() string {
 
 func main() {
 	flag.Usage = func() {
-		fmt.Fprintf(os.Stderr, "%s\n", buildInfo())
-		fmt.Fprintf(os.Stderr, "\nUsage:\n")
+		_, _ = fmt.Fprintf(os.Stderr, "%s\n", buildInfo())
+		_, _ = fmt.Fprintf(os.Stderr, "\nUsage:\n")
 		flag.PrintDefaults()
 	}
 
@@ -40,18 +40,18 @@ func main() {
 	}
 
 	if _, err := os.Stat(*configFile); os.IsNotExist(err) {
-		fmt.Fprintf(os.Stderr, "Config file %s does not exist", *configFile)
+		_, _ = fmt.Fprintf(os.Stderr, "Config file %s does not exist", *configFile)
 		os.Exit(1)
 	}
 
-	cfg, err := config.New(*configFile, *debug)
+	cfg, err := config.New(*configFile, *development)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "Could not load config file: %v", err)
+		_, _ = fmt.Fprintf(os.Stderr, "Could not load config file: %v", err)
 		os.Exit(1)
 	}
 	// Initialize the logger after we've resolved the debug flag but before its first usage at cfg.Print
-	if err := logger.InitGlobalLogger(*debug); err != nil {
-		fmt.Fprintf(os.Stderr, "Could not initialize global logger")
+	if err := logger.InitGlobalLogger(cfg.Log); err != nil {
+		_, _ = fmt.Fprintf(os.Stderr, "Could not initialize global logger")
 		os.Exit(1)
 	}
 
@@ -63,7 +63,7 @@ func main() {
 	}
 
 	if err := lb.Run(); err != nil {
-		fmt.Fprintln(os.Stderr, err)
+		_, _ = fmt.Fprintln(os.Stderr, err)
 		os.Exit(1)
 	}
 }
diff --git a/cmd/restore/main.go b/cmd/restore/main.go
@@ -16,7 +16,9 @@ var (
 	pgUser, pgPass, pgHost, pgDbname              *string
 	targetTable, backupDir, tableName, schemaName *string
 	pgPort                                        *uint
-	debug                                         *bool
+	logDevelopment                                *bool
+	logLevel                                      *string
+	logShowLocation                               *bool
 )
 
 func init() {
@@ -31,7 +33,9 @@ func init() {
 	schemaName = flag.String("schema", "public", "Schema name")
 	targetTable = flag.String("target-table", "", "Target table name (optional)")
 	backupDir = flag.String("backup-dir", "", "Backups dir")
-	debug = flag.Bool("debug", false, "Toggle debug mode")
+	logDevelopment = flag.Bool("log-development", false, "Enable development logging mode")
+	logLevel = flag.String("log-level", "", "Set log level")
+	logShowLocation = flag.Bool("log-location", true, "Show log location")
 
 	flag.Parse()
 
@@ -41,12 +45,27 @@ func init() {
 	}
 }
 
+func makeLoggerConfig() *logger.LoggerConfig {
+	lc := logger.DefaultLogConfig()
+	lc.Development = *logDevelopment
+	lc.Location = logShowLocation
+
+	if *logLevel != "" {
+		if err := logger.ValidateLogLevel(*logLevel); err != nil {
+			_, _ = fmt.Fprint(os.Stderr, err)
+		}
+		lc.Level = *logLevel
+	}
+	return lc
+}
+
 func main() {
 
 	tbl := message.NamespacedName{Namespace: *schemaName, Name: *tableName}
+	lc := makeLoggerConfig()
 
-	if err := logger.InitGlobalLogger(*debug, "table to restore", tbl.String()); err != nil {
-		fmt.Fprintf(os.Stderr, "Could not initialize global logger")
+	if err := logger.InitGlobalLogger(lc, "table to restore", tbl.String()); err != nil {
+		_, _ = fmt.Fprintf(os.Stderr, "Could not initialize global logger")
 		os.Exit(1)
 	}
 
@@ -62,7 +81,7 @@ func main() {
 		Host:     *pgHost,
 	}
 
-	r, err := logicalrestore.New(tbl, *backupDir, config, *debug)
+	r, err := logicalrestore.New(tbl, *backupDir, config, lc)
 	if err != nil {
 		logger.G.WithError(err).Fatalf("could not create backup logger")
 	}

diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -3,6 +3,7 @@ package config
 import (
 	"fmt"
 	"os"
+	"strings"
 	"time"
 
 	"github.com/jackc/pgx"
@@ -16,25 +17,25 @@ const (
 )
 
 type Config struct {
-	DB                                     pgx.ConnConfig `yaml:"db"`
-	SlotName                               string         `yaml:"slotname"`
-	PublicationName                        string         `yaml:"publication"`
-	TrackNewTables                         bool           `yaml:"trackNewTables"`
-	DeltasPerFile                          int            `yaml:"deltasPerFile"`
-	BackupThreshold                        int            `yaml:"backupThreshold"`
-	ConcurrentBasebackups                  int            `yaml:"concurrentBasebackups"`
-	InitialBasebackup                      bool           `yaml:"initialBasebackup"`
-	Fsync                                  bool           `yaml:"fsync"`
-	StagingDir                             string         `yaml:"StagingDir"`
-	ArchiveDir                             string         `yaml:"archiveDir"`
-	ForceBasebackupAfterInactivityInterval time.Duration  `yaml:"forceBasebackupAfterInactivityInterval"`
-	ArchiverTimeout                        time.Duration  `yaml:"archiverTimeout"`
-	PrometheusPort                         int            `yaml:"prometheusPort"`
-	Debug                                  bool           `yaml:"debug"`
+	DB                                     pgx.ConnConfig       `yaml:"db"`
+	SlotName                               string               `yaml:"slotname"`
+	PublicationName                        string               `yaml:"publication"`
+	TrackNewTables                         bool                 `yaml:"trackNewTables"`
+	DeltasPerFile                          int                  `yaml:"deltasPerFile"`
+	BackupThreshold                        int                  `yaml:"backupThreshold"`
+	ConcurrentBasebackups                  int                  `yaml:"concurrentBasebackups"`
+	InitialBasebackup                      bool                 `yaml:"initialBasebackup"`
+	Fsync                                  bool                 `yaml:"fsync"`
+	StagingDir                             string               `yaml:"StagingDir"`
+	ArchiveDir                             string               `yaml:"archiveDir"`
+	ForceBasebackupAfterInactivityInterval time.Duration        `yaml:"forceBasebackupAfterInactivityInterval"`
+	ArchiverTimeout                        time.Duration        `yaml:"archiverTimeout"`
+	PrometheusPort                         int                  `yaml:"prometheusPort"`
+	Log                                    *logger.LoggerConfig `yaml:"log"`
 }
 
 // New constructs a new Config instance.
-func New(filename string, debug bool) (*Config, error) {
+func New(filename string, developmentMode bool) (*Config, error) {
 	var cfg Config
 
 	fp, err := os.Open(filename)
@@ -52,8 +53,15 @@ func New(filename string, debug bool) (*Config, error) {
 	if cfg.PrometheusPort == 0 {
 		cfg.PrometheusPort = defaultPrometheusPort
 	}
-	if debug {
-		cfg.Debug = debug
+	if cfg.Log == nil {
+		cfg.Log = logger.DefaultLogConfig()
+	}
+	if developmentMode {
+		cfg.Log.Development = developmentMode
+	}
+
+	if err := logger.ValidateLogLevel(cfg.Log.Level); err != nil {
+		return nil, err
 	}
 
 	return &cfg, nil
@@ -69,7 +77,7 @@ func (c Config) Print() {
 			c.DB.User, c.DB.Host, c.DB.Port, c.DB.Database, c.SlotName, c.PublicationName)},
 		{"Track New Tables", fmt.Sprintf("%t", c.TrackNewTables)},
 		{"Fsync", fmt.Sprintf("%t", c.Fsync)},
-		{"Debug mode", fmt.Sprintf("%t", c.Debug)},
+		{"Log development mode", fmt.Sprintf("%t", c.Log.Development)},
 	}
 
 	if c.StagingDir == "" {
@@ -79,6 +87,12 @@ func (c Config) Print() {
 	if c.ForceBasebackupAfterInactivityInterval > 0 {
 		ops = append(ops, []string{"Force new basebackup of a modified table after inactivity", fmt.Sprintf("%v", c.ForceBasebackupAfterInactivityInterval)})
 	}
+	if c.Log.Level != "" {
+		ops = append(ops, []string{"Log level", strings.ToUpper(c.Log.Level)})
+	}
+	if c.Log.Location != nil {
+		ops = append(ops, []string{"Log includes file location", fmt.Sprintf("%t", *c.Log.Location)})
+	}
 
 	for _, opt := range ops {
 		logger.G.With(opt[0], opt[1]).Info("option")

diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go
@@ -2,13 +2,21 @@ package logger
 
 import (
 	"fmt"
-
 	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
 
 	"github.com/ikitiki/logical_backup/pkg/dbutils"
 	"github.com/ikitiki/logical_backup/pkg/message"
 )
 
+// LoggerConfig describes the logger configuration. It couldn't be part of config
+// due to cyclic imports (also, it is used outside of the scope of config in restore code).
+type LoggerConfig struct {
+	Level       string `yaml:"level"`
+	Location    *bool  `yaml:"location"`
+	Development bool   `yaml:"development"`
+}
+
 // Log is a wrapper around zap.SugarLogger, providing a bunch of With... functions to ease writing log messages.
 // Note that all With... functions return the Log structure, never the underlying SugarLogger, to allow chain-linking them.
 type Log struct {
@@ -19,8 +27,8 @@ type Log struct {
 var G *Log
 
 // InitGlobalLogger initializes the package-level logger. It should be called only once at start of the program.
-func InitGlobalLogger(debug bool, args ...interface{}) (err error) {
-	G, err = NewLogger("global", debug)
+func InitGlobalLogger(cfg *LoggerConfig, args ...interface{}) (err error) {
+	G, err = NewLogger("global", cfg)
 	if err == nil && len(args) > 0 {
 		G = NewLoggerFrom(G, "", args...)
 	}
@@ -47,16 +55,16 @@ func (l *Log) WithOID(oid dbutils.OID) *Log {
 	return &Log{l.With("OID", oid)}
 }
 
-// WithTableName returns a logger with a table namespaced name provided in the logging context.
-func (l *Log) WithTableName(n message.NamespacedName) *Log {
-	return &Log{l.With("table name", n.Sanitize())}
-}
-
 // WithTableNameString returns a logger with a table name string provided in the logging context.
 func (l *Log) WithTableNameString(t string) *Log {
 	return &Log{l.With("table name", t)}
 }
 
+// WithTableName returns a logger with a table namespaced name provided in the logging context.
+func (l *Log) WithTableName(n message.NamespacedName) *Log {
+	return l.WithTableNameString(n.String())
+}
+
 // WithReplicationMessage returns a logger with a replication message provided in the logging context.
 func (l *Log) WithReplicationMessage(message []byte) *Log {
 	return &Log{l.With("message", message)}
@@ -80,23 +88,13 @@ func (l *Log) WithHint(template string, args ...interface{}) *Log {
 	return &Log{l.With("hint", fmt.Sprintf(template, args...))}
 }
 
-// NewLogger creates a new logger with a given name, either a development or production one.
-func NewLogger(name string, development bool) (*Log, error) {
-	var (
-		logger *zap.Logger
-		err    error
-	)
-
-	if development {
-		logger, err = zap.NewDevelopment()
-	} else {
-		logger, err = zap.NewProduction()
+// NewLogger creates a new logger with a given name and configuration
+func NewLogger(name string, cfg *LoggerConfig) (*Log, error) {
+	log, err := newCustomLogger(cfg)
+	if err != nil {
+		return nil, err
 	}
-	if err == nil {
-		return &Log{logger.Sugar().Named(name)}, nil
-	}
-
-	return nil, err
+	return &Log{log.Sugar().Named(name)}, err
 }
 
 // NewLoggerFrom creates a new logger from the existing one, adding a name and, optionally, arbitrary fields with values.
@@ -113,3 +111,63 @@ func NewLoggerFrom(existing *Log, name string, withFields ...interface{}) (resul
 func PrintMessageForDebug(prefix string, msg message.Message, currentLSN dbutils.LSN, log *Log) {
 	log.WithLSN(currentLSN).Debugf(prefix+" %T", msg)
 }
+
+// LevelFromString converts the textual logging level to the level that can be passed to zap.Config
+func LevelFromString(text string) (level zapcore.Level, err error) {
+	err = level.UnmarshalText([]byte(text))
+	return
+}
+
+// ValidateLogLevel verifies that the log level text corresponds to the valid log level
+func ValidateLogLevel(level string) error {
+	if level == "" {
+		return nil
+	}
+	if _, err := LevelFromString(level); err != nil {
+		return fmt.Errorf("%v, valid levels are debug, info, warn, error, fatal, dpanic and panic", err)
+	}
+	return nil
+
+}
+
+// DefaultLogConfig returns a default logging configuration
+func DefaultLogConfig() *LoggerConfig {
+	var (
+		showLocationByDefault = true
+	)
+	return &LoggerConfig{"", &showLocationByDefault, false}
+
+}
+
+// We need to make slight customization to the default zap development and production levels
+// Namely, avoid stack traces for Warn in development (too verbose for our usage of Warn),
+// allow customization of the default level and disable showing lines of code in the log output.
+func newCustomLogger(lc *LoggerConfig) (*zap.Logger, error) {
+	var (
+		cfg zap.Config
+	)
+	opts := make([]zap.Option, 0)
+
+	if lc.Development {
+		cfg = zap.NewDevelopmentConfig()
+	} else {
+		cfg = zap.NewProductionConfig()
+	}
+	// adjust the log level if necessary
+	if lc.Level != "" {
+		lv, err := LevelFromString(lc.Level)
+		if err != nil {
+			return nil, err
+		}
+		cfg.Level = zap.NewAtomicLevelAt(lv)
+	}
+	// disable logging source line numbers if instructed.
+	if lc.Location != nil {
+		cfg.DisableCaller = !*lc.Location
+	}
+	// default development configuration sets stacktraces from WarnLevel, override it here
+	if lc.Development {
+		opts = append(opts, zap.AddStacktrace(zap.ErrorLevel))
+	}
+	return cfg.Build(opts...)
+}
diff --git a/pkg/logicalbackup/logicalbackup.go b/pkg/logicalbackup/logicalbackup.go
@@ -126,7 +126,7 @@ func New(cfg *config.Config) (*LogicalBackup, error) {
 		prom:             prom.New(cfg.PrometheusPort),
 	}
 
-	if l, err := logger.NewLogger("logical backup", cfg.Debug); err != nil {
+	if l, err := logger.NewLogger("logical backup", cfg.Log); err != nil {
 		return nil, fmt.Errorf("could not create backup logger: %v", err)
 	} else {
 		lb.log = l

diff --git a/pkg/logicalrestore/logicalrestore.go b/pkg/logicalrestore/logicalrestore.go
@@ -46,14 +46,14 @@ type LogicalRestore struct {
 }
 
 // New constructs a new LogicalRestore instance.
-func New(tbl message.NamespacedName, dir string, cfg pgx.ConnConfig, debug bool) (*LogicalRestore, error) {
+func New(tbl message.NamespacedName, dir string, cfg pgx.ConnConfig, lc *logger.LoggerConfig) (*LogicalRestore, error) {
 	lr := &LogicalRestore{
 		ctx:            context.Background(),
 		baseDir:        dir,
 		cfg:            cfg,
 		NamespacedName: tbl,
 	}
-	if l, err := logger.NewLogger("logical restore", debug); err != nil {
+	if l, err := logger.NewLogger("logical restore", lc); err != nil {
 		return nil, err
 	} else {
 		lr.log = l