From bdd19627baa73c5a6ab3972f36917d33dfd5fe71 Mon Sep 17 00:00:00 2001 From: guo-shaoge Date: Tue, 30 Nov 2021 15:37:34 +0800 Subject: [PATCH 1/4] logutil: add testcase for SlowQueryLogger.MaxDays/MaxSize/MaxBackups Signed-off-by: guo-shaoge --- util/logutil/log.go | 4 ++-- util/logutil/log_test.go | 14 ++++++++++++-- util/logutil/slow_query_logger.go | 6 +++--- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/util/logutil/log.go b/util/logutil/log.go index a9e7efa6a0b78..f49f6e7a45c94 100644 --- a/util/logutil/log.go +++ b/util/logutil/log.go @@ -106,7 +106,7 @@ func InitLogger(cfg *LogConfig) error { log.ReplaceGlobals(gl, props) // init dedicated logger for slow query log - SlowQueryLogger, _, err = newSlowQueryLogger(cfg) + SlowQueryLogger, _, _, err = newSlowQueryLogger(cfg) if err != nil { return errors.Trace(err) } @@ -157,7 +157,7 @@ func ReplaceLogger(cfg *LogConfig) error { return errors.Trace(err) } - SlowQueryLogger, _, err = newSlowQueryLogger(cfg) + SlowQueryLogger, _, _, err = newSlowQueryLogger(cfg) if err != nil { return errors.Trace(err) } diff --git a/util/logutil/log_test.go b/util/logutil/log_test.go index d4f671f05dc92..870ca86c0418a 100644 --- a/util/logutil/log_test.go +++ b/util/logutil/log_test.go @@ -116,12 +116,22 @@ func TestGrpcLoggerCreation(t *testing.T) { func TestSlowQueryLoggerCreation(t *testing.T) { level := "warn" - conf := NewLogConfig(level, DefaultLogFormat, "", EmptyFileLogConfig, false) - _, prop, err := newSlowQueryLogger(conf) + fileConf := FileLogConfig{ + log.FileLogConfig{ + Filename: "slow-query.log", + MaxSize: 10, + MaxDays: 10, + MaxBackups: 10, + }, + } + conf := NewLogConfig(level, DefaultLogFormat, "", fileConf, false) + _, prop, sqConf, err := newSlowQueryLogger(conf) // assert after init slow query logger, the original conf is not changed require.Equal(t, conf.Level, level) require.Nil(t, err) require.Equal(t, prop.Level.String(), conf.Level) + // make sure SlowQuery.MaxDays/MaxSize/MaxBackups are same with top config. + require.Equal(t, fileConf.FileLogConfig, sqConf.File) } func TestGlobalLoggerReplace(t *testing.T) { diff --git a/util/logutil/slow_query_logger.go b/util/logutil/slow_query_logger.go index 2588c36131fd9..f420b71afac06 100644 --- a/util/logutil/slow_query_logger.go +++ b/util/logutil/slow_query_logger.go @@ -27,7 +27,7 @@ import ( var _pool = buffer.NewPool() -func newSlowQueryLogger(cfg *LogConfig) (*zap.Logger, *log.ZapProperties, error) { +func newSlowQueryLogger(cfg *LogConfig) (*zap.Logger, *log.ZapProperties, *log.Config, error) { // copy the global log config to slow log config // if the filename of slow log config is empty, slow log will behave the same as global log. @@ -40,7 +40,7 @@ func newSlowQueryLogger(cfg *LogConfig) (*zap.Logger, *log.ZapProperties, error) // create the slow query logger sqLogger, prop, err := log.InitLogger(&sqConfig) if err != nil { - return nil, nil, errors.Trace(err) + return nil, nil, nil, errors.Trace(err) } // replace 2018-12-19-unified-log-format text encoder with slow log encoder @@ -50,7 +50,7 @@ func newSlowQueryLogger(cfg *LogConfig) (*zap.Logger, *log.ZapProperties, error) })) prop.Core = newCore - return sqLogger, prop, nil + return sqLogger, prop, &sqConfig, nil } type slowLogEncoder struct{} From 5cf46fa9e7b2e0ebac734275c97f443fcfef574f Mon Sep 17 00:00:00 2001 From: guo-shaoge Date: Tue, 30 Nov 2021 16:22:41 +0800 Subject: [PATCH 2/4] fix Signed-off-by: guo-shaoge --- util/logutil/log_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/util/logutil/log_test.go b/util/logutil/log_test.go index 870ca86c0418a..99041d4bea863 100644 --- a/util/logutil/log_test.go +++ b/util/logutil/log_test.go @@ -116,22 +116,23 @@ func TestGrpcLoggerCreation(t *testing.T) { func TestSlowQueryLoggerCreation(t *testing.T) { level := "warn" + slowQueryFn := "slow-query.log" fileConf := FileLogConfig{ log.FileLogConfig{ - Filename: "slow-query.log", + Filename: slowQueryFn, MaxSize: 10, MaxDays: 10, MaxBackups: 10, }, } - conf := NewLogConfig(level, DefaultLogFormat, "", fileConf, false) - _, prop, sqConf, err := newSlowQueryLogger(conf) + conf := NewLogConfig(level, DefaultLogFormat, slowQueryFn, fileConf, false) + _, prop, slowQueryConf, err := newSlowQueryLogger(conf) // assert after init slow query logger, the original conf is not changed require.Equal(t, conf.Level, level) require.Nil(t, err) require.Equal(t, prop.Level.String(), conf.Level) // make sure SlowQuery.MaxDays/MaxSize/MaxBackups are same with top config. - require.Equal(t, fileConf.FileLogConfig, sqConf.File) + require.Equal(t, fileConf.FileLogConfig, slowQueryConf.File) } func TestGlobalLoggerReplace(t *testing.T) { From 2333572357432a674d43b8fc1d832c41dae6b317 Mon Sep 17 00:00:00 2001 From: guo-shaoge Date: Wed, 8 Dec 2021 12:23:23 +0800 Subject: [PATCH 3/4] fix by comment Signed-off-by: guo-shaoge --- util/logutil/log.go | 4 ++-- util/logutil/log_test.go | 16 +++++++++++++++ util/logutil/slow_query_logger.go | 34 ++++++++++++++++--------------- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/util/logutil/log.go b/util/logutil/log.go index f49f6e7a45c94..a9e7efa6a0b78 100644 --- a/util/logutil/log.go +++ b/util/logutil/log.go @@ -106,7 +106,7 @@ func InitLogger(cfg *LogConfig) error { log.ReplaceGlobals(gl, props) // init dedicated logger for slow query log - SlowQueryLogger, _, _, err = newSlowQueryLogger(cfg) + SlowQueryLogger, _, err = newSlowQueryLogger(cfg) if err != nil { return errors.Trace(err) } @@ -157,7 +157,7 @@ func ReplaceLogger(cfg *LogConfig) error { return errors.Trace(err) } - SlowQueryLogger, _, _, err = newSlowQueryLogger(cfg) + SlowQueryLogger, _, err = newSlowQueryLogger(cfg) if err != nil { return errors.Trace(err) } diff --git a/util/logutil/log_test.go b/util/logutil/log_test.go index 713c34de77ab5..0d716e0005cf0 100644 --- a/util/logutil/log_test.go +++ b/util/logutil/log_test.go @@ -126,6 +126,22 @@ func TestSlowQueryLoggerCreation(t *testing.T) { // level should be less than WarnLevel which is used by it to log slow query. require.NotEqual(t, conf.Level, prop.Level.String()) require.True(t, prop.Level.Level() <= zapcore.WarnLevel) + + level = "warn" + slowQueryFn := "slow-query.log" + fileConf := FileLogConfig{ + log.FileLogConfig{ + Filename: slowQueryFn, + MaxSize: 10, + MaxDays: 10, + MaxBackups: 10, + }, + } + conf = NewLogConfig(level, DefaultLogFormat, slowQueryFn, fileConf, false) + slowQueryConf := newSlowQueryLogConfig(conf) + // newSlowQueryLogConfig() is called by newSlowQueryLogger(), + // so the SlowQuery.MaxDays/MaxSize/MaxBackups are same with global config. + require.Equal(t, fileConf.FileLogConfig, slowQueryConf.File) } func TestGlobalLoggerReplace(t *testing.T) { diff --git a/util/logutil/slow_query_logger.go b/util/logutil/slow_query_logger.go index b29a5aacf33c5..d5d59f98aface 100644 --- a/util/logutil/slow_query_logger.go +++ b/util/logutil/slow_query_logger.go @@ -27,23 +27,11 @@ import ( var _pool = buffer.NewPool() -func newSlowQueryLogger(cfg *LogConfig) (*zap.Logger, *log.ZapProperties, *log.Config, error) { - - // copy the global log config to slow log config - // if the filename of slow log config is empty, slow log will behave the same as global log. - sqConfig := cfg.Config - // level of the global log config doesn't affect the slow query logger which determines whether to - // log by execution duration. - sqConfig.Level = LogConfig{}.Level - if len(cfg.SlowQueryFile) != 0 { - sqConfig.File = cfg.File - sqConfig.File.Filename = cfg.SlowQueryFile - } - +func newSlowQueryLogger(cfg *LogConfig) (*zap.Logger, *log.ZapProperties, error) { // create the slow query logger - sqLogger, prop, err := log.InitLogger(&sqConfig) + sqLogger, prop, err := log.InitLogger(newSlowQueryLogConfig(cfg)) if err != nil { - return nil, nil, nil, errors.Trace(err) + return nil, nil, errors.Trace(err) } // replace 2018-12-19-unified-log-format text encoder with slow log encoder @@ -53,7 +41,21 @@ func newSlowQueryLogger(cfg *LogConfig) (*zap.Logger, *log.ZapProperties, *log.C })) prop.Core = newCore - return sqLogger, prop, &sqConfig, nil + return sqLogger, prop, nil +} + +func newSlowQueryLogConfig(cfg *LogConfig) *log.Config { + // copy the global log config to slow log config + // if the filename of slow log config is empty, slow log will behave the same as global log. + sqConfig := cfg.Config + // level of the global log config doesn't affect the slow query logger which determines whether to + // log by execution duration. + sqConfig.Level = LogConfig{}.Level + if len(cfg.SlowQueryFile) != 0 { + sqConfig.File = cfg.File + sqConfig.File.Filename = cfg.SlowQueryFile + } + return &sqConfig } type slowLogEncoder struct{} From 15c5f1f585c6e0e9237bb0e73043edbc46502268 Mon Sep 17 00:00:00 2001 From: guo-shaoge Date: Wed, 8 Dec 2021 12:33:13 +0800 Subject: [PATCH 4/4] fix Signed-off-by: guo-shaoge --- util/logutil/log_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/util/logutil/log_test.go b/util/logutil/log_test.go index 0d716e0005cf0..69eb731dee8c6 100644 --- a/util/logutil/log_test.go +++ b/util/logutil/log_test.go @@ -139,8 +139,7 @@ func TestSlowQueryLoggerCreation(t *testing.T) { } conf = NewLogConfig(level, DefaultLogFormat, slowQueryFn, fileConf, false) slowQueryConf := newSlowQueryLogConfig(conf) - // newSlowQueryLogConfig() is called by newSlowQueryLogger(), - // so the SlowQuery.MaxDays/MaxSize/MaxBackups are same with global config. + // slowQueryConf.MaxDays/MaxSize/MaxBackups should be same with global config. require.Equal(t, fileConf.FileLogConfig, slowQueryConf.File) }