From d61e5608900fe64f215367c8898ff31fdb7efc0b Mon Sep 17 00:00:00 2001 From: Steven Zou Date: Sun, 7 Feb 2021 18:07:02 +0800 Subject: [PATCH] fix(jobservice):wrong depth of job logging - use separate std logger for job, not shared with jobservice std logger - merge and remove useless functions Signed-off-by: Steven Zou fix #14079 --- src/jobservice/logger/base.go | 9 ++++++--- src/jobservice/logger/known_loggers.go | 14 +++++--------- src/jobservice/logger/known_loggers_test.go | 18 +++++++----------- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/jobservice/logger/base.go b/src/jobservice/logger/base.go index fc114826f..01e59ca77 100644 --- a/src/jobservice/logger/base.go +++ b/src/jobservice/logger/base.go @@ -45,11 +45,11 @@ func GetLogger(loggerOptions ...Option) (Interface, error) { // Create backends loggers := make([]Interface, 0) for name, ops := range lOptions.values { - if !IsKnownLogger(name) { + d, o := IsKnownLogger(name) + if !o { return nil, fmt.Errorf("no logger registered for name '%s'", name) } - d := KnownLoggers(name) var ( l Interface ok bool @@ -111,7 +111,7 @@ func GetSweeper(context context.Context, sweeperOptions ...Option) (sweeper.Inte return nil, fmt.Errorf("no sweeper provided for the logger %s", name) } - d := KnownLoggers(name) + d, _ := IsKnownLogger(name) s, err := d.Sweeper(ops...) if err != nil { return nil, err @@ -199,7 +199,10 @@ func Init(ctx context.Context) error { if err != nil { return err } + // Avoid data race issue + // Do not cache this system std logger with name `NameStdOutput` as caching causes conflict with job std logger. + singletons.Delete(NameStdOutput) singletons.Store(systemKeyServiceLogger, lg) jOptions := make([]Option, 0) diff --git a/src/jobservice/logger/known_loggers.go b/src/jobservice/logger/known_loggers.go index fdd60d000..9c046f5b0 100644 --- a/src/jobservice/logger/known_loggers.go +++ b/src/jobservice/logger/known_loggers.go @@ -1,9 +1,10 @@ package logger import ( - "github.com/goharbor/harbor/src/jobservice/logger/backend" "reflect" "strings" + + "github.com/goharbor/harbor/src/jobservice/logger/backend" ) const ( @@ -39,10 +40,10 @@ var knownLoggers = map[string]*Declaration{ } // IsKnownLogger checks if the logger is supported with name. -func IsKnownLogger(name string) bool { - _, ok := knownLoggers[name] +func IsKnownLogger(name string) (*Declaration, bool) { + d, ok := knownLoggers[name] - return ok + return d, ok } // HasSweeper checks if the logger with the name provides a sweeper. @@ -59,11 +60,6 @@ func HasGetter(name string) bool { return ok && d.Getter != nil } -// KnownLoggers return the declaration by the name -func KnownLoggers(name string) *Declaration { - return knownLoggers[name] -} - // All known levels which are supported. var debugLevels = []string{ "DEBUG", diff --git a/src/jobservice/logger/known_loggers_test.go b/src/jobservice/logger/known_loggers_test.go index 1ccfdc444..4387fc6c1 100644 --- a/src/jobservice/logger/known_loggers_test.go +++ b/src/jobservice/logger/known_loggers_test.go @@ -1,20 +1,23 @@ package logger import ( - "github.com/goharbor/harbor/src/jobservice/logger/backend" - "github.com/stretchr/testify/require" "os" "path" "testing" + + "github.com/goharbor/harbor/src/jobservice/logger/backend" + "github.com/stretchr/testify/require" ) // TestKnownLoggers func TestKnownLoggers(t *testing.T) { - b := IsKnownLogger("Unknown") + l, b := IsKnownLogger("Unknown") require.False(t, b) + require.Nil(t, l) - b = IsKnownLogger(NameFile) + l, b = IsKnownLogger(NameFile) require.True(t, b) + require.NotNil(t, l) // no getter b = HasGetter(NameStdOutput) @@ -30,13 +33,6 @@ func TestKnownLoggers(t *testing.T) { b = HasSweeper(NameDB) require.True(t, b) - // unknown logger - l := KnownLoggers("unknown") - require.Nil(t, l) - // known logger - l = KnownLoggers(NameDB) - require.NotNil(t, l) - // unknown level b = IsKnownLevel("unknown") require.False(t, b)