From dccee86141bd74c7306f1a1962b668b5530b27ba Mon Sep 17 00:00:00 2001 From: Tony Chen Date: Fri, 27 May 2022 16:29:07 +0800 Subject: [PATCH] log: remove component logger to use global logger (#2061) * remove component logger to user global logger --- config/config.go | 22 ++++------ config/config_test.go | 5 --- config/options.go | 6 +-- config/reader.go | 4 +- contrib/config/apollo/README.md | 5 +-- contrib/config/apollo/apollo.go | 28 ++---------- contrib/config/apollo/watcher.go | 18 ++------ contrib/registry/discovery/README.md | 5 +-- contrib/registry/discovery/discovery.go | 43 ++++++------------- contrib/registry/discovery/impl_registrar.go | 11 ++--- log/global.go | 42 ++++++++++-------- log/value.go | 4 ++ middleware/recovery/recovery.go | 10 ++--- middleware/recovery/recovery_test.go | 3 +- transport/grpc/client.go | 8 +--- transport/grpc/client_test.go | 11 ----- transport/grpc/resolver/discovery/builder.go | 11 ----- .../grpc/resolver/discovery/builder_test.go | 19 -------- transport/grpc/resolver/discovery/resolver.go | 17 ++++---- .../grpc/resolver/discovery/resolver_test.go | 4 -- transport/grpc/server.go | 11 ++--- transport/grpc/server_test.go | 30 ------------- transport/http/resolver.go | 14 +++--- transport/http/server.go | 11 ++--- 24 files changed, 96 insertions(+), 246 deletions(-) diff --git a/config/config.go b/config/config.go index fa8aa430b..126a63791 100644 --- a/config/config.go +++ b/config/config.go @@ -7,13 +7,12 @@ import ( "sync" "time" - "github.com/go-kratos/kratos/v2/log" - // init encoding _ "github.com/go-kratos/kratos/v2/encoding/json" _ "github.com/go-kratos/kratos/v2/encoding/proto" _ "github.com/go-kratos/kratos/v2/encoding/xml" _ "github.com/go-kratos/kratos/v2/encoding/yaml" + "github.com/go-kratos/kratos/v2/log" ) var ( @@ -43,13 +42,11 @@ type config struct { cached sync.Map observers sync.Map watchers []Watcher - log *log.Helper } // New new a config with options. func New(opts ...Option) Config { o := options{ - logger: log.GetLogger(), decoder: defaultDecoder, resolver: defaultResolver, } @@ -59,7 +56,6 @@ func New(opts ...Option) Config { return &config{ opts: o, reader: newReader(o), - log: log.NewHelper(o.logger), } } @@ -67,20 +63,20 @@ func (c *config) watch(w Watcher) { for { kvs, err := w.Next() if errors.Is(err, context.Canceled) { - c.log.Infof("watcher's ctx cancel : %v", err) + log.Infof("watcher's ctx cancel : %v", err) return } if err != nil { time.Sleep(time.Second) - c.log.Errorf("failed to watch next config: %v", err) + log.Errorf("failed to watch next config: %v", err) continue } if err := c.reader.Merge(kvs...); err != nil { - c.log.Errorf("failed to merge next config: %v", err) + log.Errorf("failed to merge next config: %v", err) continue } if err := c.reader.Resolve(); err != nil { - c.log.Errorf("failed to resolve next config: %v", err) + log.Errorf("failed to resolve next config: %v", err) continue } c.cached.Range(func(key, value interface{}) bool { @@ -104,22 +100,22 @@ func (c *config) Load() error { return err } for _, v := range kvs { - c.log.Debugf("config loaded: %s format: %s", v.Key, v.Format) + log.Debugf("config loaded: %s format: %s", v.Key, v.Format) } if err = c.reader.Merge(kvs...); err != nil { - c.log.Errorf("failed to merge config source: %v", err) + log.Errorf("failed to merge config source: %v", err) return err } w, err := src.Watch() if err != nil { - c.log.Errorf("failed to watch config source: %v", err) + log.Errorf("failed to watch config source: %v", err) return err } c.watchers = append(c.watchers, w) go c.watch(w) } if err := c.reader.Resolve(); err != nil { - c.log.Errorf("failed to resolve config source: %v", err) + log.Errorf("failed to resolve config source: %v", err) return err } return nil diff --git a/config/config_test.go b/config/config_test.go index d08a00d50..5699dec16 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -4,8 +4,6 @@ import ( "errors" "reflect" "testing" - - "github.com/go-kratos/kratos/v2/log" ) const ( @@ -123,7 +121,6 @@ func TestConfig(t *testing.T) { WithSource(newTestJSONSource(_testJSON)), WithDecoder(defaultDecoder), WithResolver(defaultResolver), - WithLogger(log.GetLogger()), ) err = c.Close() if err != nil { @@ -135,12 +132,10 @@ func TestConfig(t *testing.T) { sources: []Source{jSource}, decoder: defaultDecoder, resolver: defaultResolver, - logger: log.GetLogger(), } cf := &config{} cf.opts = opts cf.reader = newReader(opts) - cf.log = log.NewHelper(opts.logger) err = cf.Load() if err != nil { diff --git a/config/options.go b/config/options.go index 0f584a701..e89a6fcad 100644 --- a/config/options.go +++ b/config/options.go @@ -22,7 +22,6 @@ type options struct { sources []Source decoder Decoder resolver Resolver - logger log.Logger } // WithSource with config source. @@ -51,10 +50,9 @@ func WithResolver(r Resolver) Option { } // WithLogger with config logger. +// Deprecated: use global logger instead. func WithLogger(l log.Logger) Option { - return func(o *options) { - o.logger = l - } + return func(o *options) {} } // defaultDecoder decode config from source KeyValue diff --git a/config/reader.go b/config/reader.go index 221ac6ce5..6858a0f64 100644 --- a/config/reader.go +++ b/config/reader.go @@ -47,11 +47,11 @@ func (r *reader) Merge(kvs ...*KeyValue) error { for _, kv := range kvs { next := make(map[string]interface{}) if err := r.opts.decoder(kv, next); err != nil { - _ = log.GetLogger().Log(log.LevelError, fmt.Sprintf("config decode error, err: %v, key: %s, value: %s", err, kv.Key, string(kv.Value))) + log.Errorf("Failed to config decode error: %v key: %s value: %s", err, kv.Key, string(kv.Value)) return err } if err := mergo.Map(&merged, convertMap(next), mergo.WithOverride); err != nil { - _ = log.GetLogger().Log(log.LevelError, fmt.Sprintf("config merge error, err: %v, key: %s, value: %s", err, kv.Key, string(kv.Value))) + log.Errorf("Failed to config merge error: %v key: %s value: %s", err, kv.Key, string(kv.Value)) return err } } diff --git a/contrib/config/apollo/README.md b/contrib/config/apollo/README.md index c07b67ec4..c03cff0fd 100644 --- a/contrib/config/apollo/README.md +++ b/contrib/config/apollo/README.md @@ -55,9 +55,6 @@ func WithEnableBackup() Option // specify apollo endpoint, such as http://localhost:8080 func WithEndpoint(endpoint string) Option -// inject a logger to debug -func WithLogger(logger log.Logger) Option - // namespaces to load, comma to separate. func WithNamespace(name string) Option @@ -101,4 +98,4 @@ config := map[string]interface{}{ } } _ = config -``` \ No newline at end of file +``` diff --git a/contrib/config/apollo/apollo.go b/contrib/config/apollo/apollo.go index 1e496f812..0d3b975e6 100644 --- a/contrib/config/apollo/apollo.go +++ b/contrib/config/apollo/apollo.go @@ -1,7 +1,6 @@ package apollo import ( - "fmt" "strings" "github.com/go-kratos/kratos/v2/config" @@ -28,8 +27,6 @@ type options struct { namespace string isBackupConfig bool backupPath string - - logger log.Logger } // WithAppID with apollo config app id @@ -88,19 +85,8 @@ func WithBackupPath(backupPath string) Option { } } -// WithLogger use custom logger to replace default logger. -func WithLogger(logger log.Logger) Option { - return func(o *options) { - if logger != nil { - o.logger = logger - } - } -} - func NewSource(opts ...Option) config.Source { - op := options{ - logger: log.GetLogger(), - } + op := options{} for _, o := range opts { o(&op) } @@ -167,10 +153,7 @@ func resolve(key string, value interface{}, target map[string]interface{}) { // current exists, then check existing value type, if it's not map // that means duplicate keys, and at least one is not map instance. if cursor, ok = v.(map[string]interface{}); !ok { - _ = log.GetLogger().Log(log.LevelWarn, - "msg", - fmt.Sprintf("duplicate key: %v\n", strings.Join(keys[:i+1], ".")), - ) + log.Warnf("duplicate key: %v\n", strings.Join(keys[:i+1], ".")) break } } @@ -202,10 +185,7 @@ func (e *apollo) load() []*config.KeyValue { codec := encoding.GetCodec(f) val, err := codec.Marshal(next) if err != nil { - _ = e.opt.logger.Log(log.LevelWarn, - "msg", - fmt.Sprintf("apollo could not handle namespace %s: %v", ns, err), - ) + log.Warnf("apollo could not handle namespace %s: %v", ns, err) continue } @@ -224,7 +204,7 @@ func (e *apollo) Load() (kv []*config.KeyValue, err error) { } func (e *apollo) Watch() (config.Watcher, error) { - w, err := newWatcher(e, e.opt.logger) + w, err := newWatcher(e) if err != nil { return nil, err } diff --git a/contrib/config/apollo/watcher.go b/contrib/config/apollo/watcher.go index 2cda69ef2..cd6a41785 100644 --- a/contrib/config/apollo/watcher.go +++ b/contrib/config/apollo/watcher.go @@ -2,7 +2,6 @@ package apollo import ( "context" - "fmt" "github.com/go-kratos/kratos/v2/config" "github.com/go-kratos/kratos/v2/encoding" @@ -17,8 +16,7 @@ type watcher struct { } type customChangeListener struct { - in chan<- []*config.KeyValue - logger log.Logger + in chan<- []*config.KeyValue } func (c *customChangeListener) onChange(namespace string, changes map[string]*storage.ConfigChange) []*config.KeyValue { @@ -33,10 +31,7 @@ func (c *customChangeListener) onChange(namespace string, changes map[string]*st codec := encoding.GetCodec(f) val, err := codec.Marshal(next) if err != nil { - _ = c.logger.Log(log.LevelWarn, - "msg", - fmt.Sprintf("apollo could not handle namespace %s: %v", namespace, err), - ) + log.Warnf("apollo could not handle namespace %s: %v", namespace, err) return nil } kv = append(kv, &config.KeyValue{ @@ -59,15 +54,10 @@ func (c *customChangeListener) OnChange(changeEvent *storage.ChangeEvent) { func (c *customChangeListener) OnNewestChange(changeEvent *storage.FullChangeEvent) {} -func newWatcher(a *apollo, logger log.Logger) (config.Watcher, error) { - if logger == nil { - logger = log.GetLogger() - } - +func newWatcher(a *apollo) (config.Watcher, error) { changeCh := make(chan []*config.KeyValue) - listener := &customChangeListener{in: changeCh, logger: logger} + listener := &customChangeListener{in: changeCh} a.client.AddChangeListener(listener) - return &watcher{ out: changeCh, cancelFn: func() { diff --git a/contrib/registry/discovery/README.md b/contrib/registry/discovery/README.md index b220b6293..46c46e2d0 100644 --- a/contrib/registry/discovery/README.md +++ b/contrib/registry/discovery/README.md @@ -14,9 +14,6 @@ import ( ) func main() { - logger := log.NewStdLogger(os.Stdout) - logger = log.With(logger, "service", "example.registry.discovery") - // initialize a registry r := discovery.New(&discovery.Config{ Nodes: []string{"0.0.0.0:7171"}, @@ -24,7 +21,7 @@ func main() { Region: "sh1", Zone: "zone1", Host: "hostname", - }, logger) + }) // construct srv instance // ... diff --git a/contrib/registry/discovery/discovery.go b/contrib/registry/discovery/discovery.go index 9c85c0ab2..f6df56dc0 100644 --- a/contrib/registry/discovery/discovery.go +++ b/contrib/registry/discovery/discovery.go @@ -5,16 +5,14 @@ import ( "fmt" "math/rand" "net/url" - "os" "strconv" "sync" "sync/atomic" "time" + "github.com/go-kratos/kratos/v2/log" "github.com/go-resty/resty/v2" "github.com/pkg/errors" - - "github.com/go-kratos/kratos/v2/log" ) type Discovery struct { @@ -32,8 +30,6 @@ type Discovery struct { registry map[string]struct{} lastHost string cancelPolls context.CancelFunc - - logger log.Logger } type appInfo struct { @@ -44,15 +40,7 @@ type appInfo struct { // New construct a Discovery instance which implements registry.Registrar, // registry.Discovery and registry.Watcher. -func New(c *Config, logger log.Logger) *Discovery { - if logger == nil { - logger = log.NewStdLogger(os.Stdout) - logger = log.With(logger, - "registry.pluginName", "Discovery", - "ts", log.DefaultTimestamp, - "caller", log.DefaultCaller, - ) - } +func New(c *Config) *Discovery { if c == nil { c = new(Config) } @@ -66,7 +54,6 @@ func New(c *Config, logger log.Logger) *Discovery { cancelFunc: cancel, apps: map[string]*appInfo{}, registry: map[string]struct{}{}, - logger: logger, } d.httpClient = resty.New(). @@ -94,10 +81,6 @@ func (d *Discovery) Close() error { return nil } -func (d *Discovery) Logger() *log.Helper { - return log.NewHelper(d.logger) -} - // selfProc start a goroutine to refresh Discovery self registration information. func (d *Discovery) selfProc(resolver *Resolve, event <-chan struct{}) { for { @@ -178,7 +161,7 @@ func (d *Discovery) resolveBuild(appID string) *Resolve { } } - d.Logger().Debugf("Discovery: AddWatch(%s) already watch(%v)", appID, ok) + log.Debugf("Discovery: AddWatch(%s) already watch(%v)", appID, ok) d.once.Do(func() { go d.serverProc() }) @@ -186,7 +169,7 @@ func (d *Discovery) resolveBuild(appID string) *Resolve { } func (d *Discovery) serverProc() { - defer d.Logger().Debug("Discovery serverProc quit") + defer log.Debug("Discovery serverProc quit") var ( retry int @@ -242,8 +225,6 @@ func (d *Discovery) switchNode() { // renew an instance with Discovery func (d *Discovery) renew(ctx context.Context, ins *discoveryInstance) (err error) { - // d.Logger().Debug("Discovery:renew renew calling") - d.mutex.RLock() c := d.config d.mutex.RUnlock() @@ -262,7 +243,7 @@ func (d *Discovery) renew(ctx context.Context, ins *discoveryInstance) (err erro SetResult(&res). Post(uri); err != nil { d.switchNode() - d.Logger().Errorf("Discovery: renew client.Get(%v) env(%s) appid(%s) hostname(%s) error(%v)", + log.Errorf("Discovery: renew client.Get(%v) env(%s) appid(%s) hostname(%s) error(%v)", uri, c.Env, ins.AppID, c.Host, err) return } @@ -276,7 +257,7 @@ func (d *Discovery) renew(ctx context.Context, ins *discoveryInstance) (err erro return } - d.Logger().Errorf( + log.Errorf( "Discovery: renew client.Get(%v) env(%s) appid(%s) hostname(%s) code(%v)", uri, c.Env, ins.AppID, c.Host, res.Code, ) @@ -304,7 +285,7 @@ func (d *Discovery) cancel(ins *discoveryInstance) (err error) { SetResult(&res). Post(uri); err != nil { d.switchNode() - d.Logger().Errorf("Discovery cancel client.Get(%v) env(%s) appid(%s) hostname(%s) error(%v)", + log.Errorf("Discovery cancel client.Get(%v) env(%s) appid(%s) hostname(%s) error(%v)", uri, config.Env, ins.AppID, config.Host, err) return } @@ -315,7 +296,7 @@ func (d *Discovery) cancel(ins *discoveryInstance) (err error) { return nil } - d.Logger().Warnf("Discovery cancel client.Get(%v) env(%s) appid(%s) hostname(%s) code(%v)", + log.Warnf("Discovery cancel client.Get(%v) env(%s) appid(%s) hostname(%s) code(%v)", uri, config.Env, ins.AppID, config.Host, res.Code) err = fmt.Errorf("ErrorCode: %d", res.Code) return @@ -407,13 +388,13 @@ func (d *Discovery) polls(ctx context.Context) (apps map[string]*disInstancesInf SetQueryParamsFromValues(p). SetResult(res).Get(uri); err != nil { d.switchNode() - d.Logger().Errorf("Discovery: client.Get(%s) error(%+v)", reqURI, err) + log.Errorf("Discovery: client.Get(%s) error(%+v)", reqURI, err) return nil, err } if res.Code != _codeOK { if res.Code != _codeNotModified { - d.Logger().Errorf("Discovery: client.Get(%s) get error code(%d)", reqURI, res.Code) + log.Errorf("Discovery: client.Get(%s) get error code(%d)", reqURI, res.Code) } err = fmt.Errorf("discovery.polls failed ErrCode: %d", res.Code) return @@ -422,12 +403,12 @@ func (d *Discovery) polls(ctx context.Context) (apps map[string]*disInstancesInf for _, app := range res.Data { if app.LastTs == 0 { err = ErrServerError - d.Logger().Errorf("Discovery: client.Get(%s) latest_timestamp is 0, instances:(%+v)", reqURI, res.Data) + log.Errorf("Discovery: client.Get(%s) latest_timestamp is 0, instances:(%+v)", reqURI, res.Data) return } } - d.Logger().Debugf("Discovery: successfully polls(%s) instances (%+v)", reqURI, res.Data) + log.Debugf("Discovery: successfully polls(%s) instances (%+v)", reqURI, res.Data) apps = res.Data return } diff --git a/contrib/registry/discovery/impl_registrar.go b/contrib/registry/discovery/impl_registrar.go index f044d8b6c..c5590f57f 100644 --- a/contrib/registry/discovery/impl_registrar.go +++ b/contrib/registry/discovery/impl_registrar.go @@ -9,6 +9,7 @@ import ( "github.com/pkg/errors" + "github.com/go-kratos/kratos/v2/log" "github.com/go-kratos/kratos/v2/registry" ) @@ -43,7 +44,7 @@ func (d *Discovery) Register(ctx context.Context, service *registry.ServiceInsta // renew the current register_service go func() { - defer d.Logger().Warn("Discovery:register_service goroutine quit") + defer log.Warn("Discovery:register_service goroutine quit") ticker := time.NewTicker(_registerGap) defer ticker.Stop() for { @@ -70,7 +71,7 @@ func (d *Discovery) register(ctx context.Context, ins *discoveryInstance) (err e var metadata []byte if ins.Metadata != nil { if metadata, err = json.Marshal(ins.Metadata); err != nil { - d.Logger().Errorf( + log.Errorf( "Discovery:register instance Marshal metadata(%v) failed!error(%v)", ins.Metadata, err, ) } @@ -102,18 +103,18 @@ func (d *Discovery) register(ctx context.Context, ins *discoveryInstance) (err e SetResult(&res). Post(uri); err != nil { d.switchNode() - d.Logger().Errorf("Discovery: register client.Get(%s) zone(%s) env(%s) appid(%s) addrs(%v) error(%v)", + log.Errorf("Discovery: register client.Get(%s) zone(%s) env(%s) appid(%s) addrs(%v) error(%v)", uri+"?"+p.Encode(), c.Zone, c.Env, ins.AppID, ins.Addrs, err) return } if res.Code != 0 { err = fmt.Errorf("ErrorCode: %d", res.Code) - d.Logger().Errorf("Discovery: register client.Get(%v) env(%s) appid(%s) addrs(%v) code(%v)", + log.Errorf("Discovery: register client.Get(%v) env(%s) appid(%s) addrs(%v) code(%v)", uri, c.Env, ins.AppID, ins.Addrs, res.Code) } - d.Logger().Infof( + log.Infof( "Discovery: register client.Get(%v) env(%s) appid(%s) addrs(%s) success\n", uri, c.Env, ins.AppID, ins.Addrs, ) diff --git a/log/global.go b/log/global.go index a44d0b768..6dbc722a8 100644 --- a/log/global.go +++ b/log/global.go @@ -2,6 +2,8 @@ package log import ( "context" + "fmt" + "os" "sync" ) @@ -13,7 +15,6 @@ var global = &loggerAppliance{} type loggerAppliance struct { lock sync.Mutex Logger - helper *Helper } func init() { @@ -24,7 +25,6 @@ func (a *loggerAppliance) SetLogger(in Logger) { a.lock.Lock() defer a.lock.Unlock() a.Logger = in - a.helper = NewHelper(a.Logger) } func (a *loggerAppliance) GetLogger() Logger { @@ -44,84 +44,88 @@ func GetLogger() Logger { // Log Print log by level and keyvals. func Log(level Level, keyvals ...interface{}) { - global.helper.Log(level, keyvals...) + _ = global.Log(level, keyvals...) } +// Context with context logger. func Context(ctx context.Context) *Helper { - return global.helper.WithContext(ctx) + return NewHelper(WithContext(ctx, global)) } // Debug logs a message at debug level. func Debug(a ...interface{}) { - global.helper.Debug(a...) + _ = global.Log(LevelDebug, DefaultMessageKey, fmt.Sprint(a...)) } // Debugf logs a message at debug level. func Debugf(format string, a ...interface{}) { - global.helper.Debugf(format, a...) + _ = global.Log(LevelDebug, DefaultMessageKey, fmt.Sprintf(format, a...)) } // Debugw logs a message at debug level. func Debugw(keyvals ...interface{}) { - global.helper.Debugw(keyvals...) + _ = global.Log(LevelDebug, keyvals...) } // Info logs a message at info level. func Info(a ...interface{}) { - global.helper.Info(a...) + _ = global.Log(LevelInfo, DefaultMessageKey, fmt.Sprint(a...)) } // Infof logs a message at info level. func Infof(format string, a ...interface{}) { - global.helper.Infof(format, a...) + _ = global.Log(LevelInfo, DefaultMessageKey, fmt.Sprintf(format, a...)) } // Infow logs a message at info level. func Infow(keyvals ...interface{}) { - global.helper.Infow(keyvals...) + _ = global.Log(LevelInfo, keyvals...) } // Warn logs a message at warn level. func Warn(a ...interface{}) { - global.helper.Warn(a...) + _ = global.Log(LevelWarn, DefaultMessageKey, fmt.Sprint(a...)) } // Warnf logs a message at warnf level. func Warnf(format string, a ...interface{}) { - global.helper.Warnf(format, a...) + _ = global.Log(LevelWarn, DefaultMessageKey, fmt.Sprintf(format, a...)) } // Warnw logs a message at warnf level. func Warnw(keyvals ...interface{}) { - global.helper.Warnw(keyvals...) + _ = global.Log(LevelWarn, keyvals...) } // Error logs a message at error level. func Error(a ...interface{}) { - global.helper.Error(a...) + _ = global.Log(LevelError, DefaultMessageKey, fmt.Sprint(a...)) } // Errorf logs a message at error level. func Errorf(format string, a ...interface{}) { - global.helper.Errorf(format, a...) + _ = global.Log(LevelError, DefaultMessageKey, fmt.Sprintf(format, a...)) } // Errorw logs a message at error level. func Errorw(keyvals ...interface{}) { - global.helper.Errorw(keyvals...) + _ = global.Log(LevelError, keyvals...) } // Fatal logs a message at fatal level. func Fatal(a ...interface{}) { - global.helper.Fatal(a...) + _ = global.Log(LevelFatal, DefaultMessageKey, fmt.Sprint(a...)) + os.Exit(1) } // Fatalf logs a message at fatal level. func Fatalf(format string, a ...interface{}) { - global.helper.Fatalf(format, a...) + _ = global.Log(LevelFatal, DefaultMessageKey, fmt.Sprintf(format, a...)) + os.Exit(1) } // Fatalw logs a message at fatal level. func Fatalw(keyvals ...interface{}) { - global.helper.Fatalw(keyvals...) + _ = global.Log(LevelFatal, keyvals...) + os.Exit(1) } diff --git a/log/value.go b/log/value.go index 8154ae934..36782f0dd 100644 --- a/log/value.go +++ b/log/value.go @@ -40,6 +40,10 @@ func Caller(depth int) Valuer { d++ _, file, line, _ = runtime.Caller(d) } + if strings.LastIndex(file, "/log/global.go") > 0 { + d++ + _, file, line, _ = runtime.Caller(d) + } idx := strings.LastIndexByte(file, '/') return file[idx+1:] + ":" + strconv.Itoa(line) } diff --git a/middleware/recovery/recovery.go b/middleware/recovery/recovery.go index 298d75319..dfa7cb7fd 100644 --- a/middleware/recovery/recovery.go +++ b/middleware/recovery/recovery.go @@ -20,7 +20,6 @@ type Option func(*options) type options struct { handler HandlerFunc - logger log.Logger } // WithHandler with recovery handler. @@ -31,16 +30,14 @@ func WithHandler(h HandlerFunc) Option { } // WithLogger with recovery logger. +// Deprecated: use global logger instead. func WithLogger(logger log.Logger) Option { - return func(o *options) { - o.logger = logger - } + return func(o *options) {} } // Recovery is a server middleware that recovers from any panics. func Recovery(opts ...Option) middleware.Middleware { op := options{ - logger: log.GetLogger(), handler: func(ctx context.Context, req, err interface{}) error { return ErrUnknownRequest }, @@ -48,7 +45,6 @@ func Recovery(opts ...Option) middleware.Middleware { for _, o := range opts { o(&op) } - logger := log.NewHelper(op.logger) return func(handler middleware.Handler) middleware.Handler { return func(ctx context.Context, req interface{}) (reply interface{}, err error) { defer func() { @@ -56,7 +52,7 @@ func Recovery(opts ...Option) middleware.Middleware { buf := make([]byte, 64<<10) //nolint:gomnd n := runtime.Stack(buf, false) buf = buf[:n] - logger.WithContext(ctx).Errorf("%v: %+v\n%s\n", rerr, req, buf) + log.Context(ctx).Errorf("%v: %+v\n%s\n", rerr, req, buf) err = op.handler(ctx, req, rerr) } diff --git a/middleware/recovery/recovery_test.go b/middleware/recovery/recovery_test.go index aa77ab9f4..cf4a7fbb3 100644 --- a/middleware/recovery/recovery_test.go +++ b/middleware/recovery/recovery_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/go-kratos/kratos/v2/errors" - "github.com/go-kratos/kratos/v2/log" ) func TestOnce(t *testing.T) { @@ -19,7 +18,7 @@ func TestOnce(t *testing.T) { next := func(ctx context.Context, req interface{}) (interface{}, error) { panic("panic reason") } - _, e := Recovery(WithLogger(log.GetLogger()))(next)(context.Background(), "panic") + _, e := Recovery()(next)(context.Background(), "panic") t.Logf("succ and reason is %v", e) } diff --git a/transport/grpc/client.go b/transport/grpc/client.go index 538e43437..a5174b738 100644 --- a/transport/grpc/client.go +++ b/transport/grpc/client.go @@ -90,10 +90,9 @@ func WithFilter(filters ...selector.Filter) ClientOption { } // WithLogger with logger +// Deprecated: use global logger instead. func WithLogger(log log.Logger) ClientOption { - return func(o *clientOptions) { - o.logger = log - } + return func(o *clientOptions) {} } // clientOptions is gRPC Client @@ -107,7 +106,6 @@ type clientOptions struct { grpcOpts []grpc.DialOption balancerName string filters []selector.Filter - logger log.Logger } // Dial returns a GRPC connection. @@ -124,7 +122,6 @@ func dial(ctx context.Context, insecure bool, opts ...ClientOption) (*grpc.Clien options := clientOptions{ timeout: 2000 * time.Millisecond, balancerName: wrr.Name, - logger: log.GetLogger(), } for _, o := range opts { o(&options) @@ -145,7 +142,6 @@ func dial(ctx context.Context, insecure bool, opts ...ClientOption) (*grpc.Clien discovery.NewBuilder( options.discovery, discovery.WithInsecure(insecure), - discovery.WithLogger(options.logger), ))) } if insecure { diff --git a/transport/grpc/client_test.go b/transport/grpc/client_test.go index 1443b9b84..d2540a7db 100644 --- a/transport/grpc/client_test.go +++ b/transport/grpc/client_test.go @@ -7,7 +7,6 @@ import ( "testing" "time" - "github.com/go-kratos/kratos/v2/log" "github.com/go-kratos/kratos/v2/middleware" "github.com/go-kratos/kratos/v2/registry" "google.golang.org/grpc" @@ -70,15 +69,6 @@ func TestWithTLSConfig(t *testing.T) { } } -func TestWithLogger(t *testing.T) { - o := &clientOptions{} - v := log.DefaultLogger - WithLogger(v)(o) - if !reflect.DeepEqual(v, o.logger) { - t.Errorf("expect %v but got %v", v, o.logger) - } -} - func EmptyMiddleware() middleware.Middleware { return func(handler middleware.Handler) middleware.Handler { return func(ctx context.Context, req interface{}) (reply interface{}, err error) { @@ -145,7 +135,6 @@ func TestDialConn(t *testing.T) { true, WithDiscovery(&mockRegistry{}), WithTimeout(10*time.Second), - WithLogger(log.DefaultLogger), WithEndpoint("abc"), WithMiddleware(EmptyMiddleware()), ) diff --git a/transport/grpc/resolver/discovery/builder.go b/transport/grpc/resolver/discovery/builder.go index 5c98e5192..7ce777176 100644 --- a/transport/grpc/resolver/discovery/builder.go +++ b/transport/grpc/resolver/discovery/builder.go @@ -6,7 +6,6 @@ import ( "strings" "time" - "github.com/go-kratos/kratos/v2/log" "github.com/go-kratos/kratos/v2/registry" "google.golang.org/grpc/resolver" @@ -17,13 +16,6 @@ const name = "discovery" // Option is builder option. type Option func(o *builder) -// WithLogger with builder logger. -func WithLogger(logger log.Logger) Option { - return func(b *builder) { - b.logger = logger - } -} - // WithTimeout with timeout option. func WithTimeout(timeout time.Duration) Option { return func(b *builder) { @@ -47,7 +39,6 @@ func DisableDebugLog() Option { type builder struct { discoverer registry.Discovery - logger log.Logger timeout time.Duration insecure bool debugLogDisabled bool @@ -57,7 +48,6 @@ type builder struct { func NewBuilder(d registry.Discovery, opts ...Option) resolver.Builder { b := &builder{ discoverer: d, - logger: log.GetLogger(), timeout: time.Second * 10, insecure: false, debugLogDisabled: false, @@ -93,7 +83,6 @@ func (b *builder) Build(target resolver.Target, cc resolver.ClientConn, opts res cc: cc, ctx: ctx, cancel: cancel, - log: log.NewHelper(b.logger), insecure: b.insecure, debugLogDisabled: b.debugLogDisabled, } diff --git a/transport/grpc/resolver/discovery/builder_test.go b/transport/grpc/resolver/discovery/builder_test.go index e1f2d2868..7a06abfcd 100644 --- a/transport/grpc/resolver/discovery/builder_test.go +++ b/transport/grpc/resolver/discovery/builder_test.go @@ -6,30 +6,11 @@ import ( "testing" "time" - "github.com/go-kratos/kratos/v2/log" "github.com/go-kratos/kratos/v2/registry" "google.golang.org/grpc/resolver" "google.golang.org/grpc/serviceconfig" ) -type mockLogger struct { - level log.Level - key string - val string -} - -func (l *mockLogger) Log(level log.Level, keyvals ...interface{}) error { - l.level = level - l.key = keyvals[0].(string) - l.val = keyvals[1].(string) - return nil -} - -func TestWithLogger(t *testing.T) { - b := &builder{} - WithLogger(&mockLogger{})(b) -} - func TestWithInsecure(t *testing.T) { b := &builder{} WithInsecure(true)(b) diff --git a/transport/grpc/resolver/discovery/resolver.go b/transport/grpc/resolver/discovery/resolver.go index 79d5da1cd..770fa8013 100644 --- a/transport/grpc/resolver/discovery/resolver.go +++ b/transport/grpc/resolver/discovery/resolver.go @@ -15,9 +15,8 @@ import ( ) type discoveryResolver struct { - w registry.Watcher - cc resolver.ClientConn - log *log.Helper + w registry.Watcher + cc resolver.ClientConn ctx context.Context cancel context.CancelFunc @@ -38,7 +37,7 @@ func (r *discoveryResolver) watch() { if errors.Is(err, context.Canceled) { return } - r.log.Errorf("[resolver] Failed to watch discovery endpoint: %v", err) + log.Errorf("[resolver] Failed to watch discovery endpoint: %v", err) time.Sleep(time.Second) continue } @@ -52,7 +51,7 @@ func (r *discoveryResolver) update(ins []*registry.ServiceInstance) { for _, in := range ins { endpoint, err := endpoint.ParseEndpoint(in.Endpoints, "grpc", !r.insecure) if err != nil { - r.log.Errorf("[resolver] Failed to parse discovery endpoint: %v", err) + log.Errorf("[resolver] Failed to parse discovery endpoint: %v", err) continue } if endpoint == "" { @@ -72,17 +71,17 @@ func (r *discoveryResolver) update(ins []*registry.ServiceInstance) { addrs = append(addrs, addr) } if len(addrs) == 0 { - r.log.Warnf("[resolver] Zero endpoint found,refused to write, instances: %v", ins) + log.Warnf("[resolver] Zero endpoint found,refused to write, instances: %v", ins) return } err := r.cc.UpdateState(resolver.State{Addresses: addrs}) if err != nil { - r.log.Errorf("[resolver] failed to update state: %s", err) + log.Errorf("[resolver] failed to update state: %s", err) } if !r.debugLogDisabled { b, _ := json.Marshal(ins) - r.log.Infof("[resolver] update instances: %s", b) + log.Infof("[resolver] update instances: %s", b) } } @@ -90,7 +89,7 @@ func (r *discoveryResolver) Close() { r.cancel() err := r.w.Stop() if err != nil { - r.log.Errorf("[resolver] failed to watch top: %s", err) + log.Errorf("[resolver] failed to watch top: %s", err) } } diff --git a/transport/grpc/resolver/discovery/resolver_test.go b/transport/grpc/resolver/discovery/resolver_test.go index be8878993..1d196382b 100644 --- a/transport/grpc/resolver/discovery/resolver_test.go +++ b/transport/grpc/resolver/discovery/resolver_test.go @@ -7,7 +7,6 @@ import ( "testing" "time" - "github.com/go-kratos/kratos/v2/log" "github.com/go-kratos/kratos/v2/registry" "google.golang.org/grpc/resolver" ) @@ -56,7 +55,6 @@ func TestWatch(t *testing.T) { r := &discoveryResolver{ w: &testWatch{}, cc: &testClientConn{te: t}, - log: log.NewHelper(log.GetLogger()), ctx: ctx, cancel: cancel, insecure: false, @@ -75,7 +73,6 @@ func TestWatchError(t *testing.T) { r := &discoveryResolver{ w: &testWatch{err: errors.New("bad")}, cc: &testClientConn{te: t}, - log: log.NewHelper(log.GetLogger()), ctx: ctx, cancel: cancel, } @@ -93,7 +90,6 @@ func TestWatchContextCancel(t *testing.T) { r := &discoveryResolver{ w: &testWatch{err: context.Canceled}, cc: &testClientConn{te: t}, - log: log.NewHelper(log.GetLogger()), ctx: ctx, cancel: cancel, } diff --git a/transport/grpc/server.go b/transport/grpc/server.go index 1ca635515..8ec76884a 100644 --- a/transport/grpc/server.go +++ b/transport/grpc/server.go @@ -54,10 +54,9 @@ func Timeout(timeout time.Duration) ServerOption { } // Logger with server logger. +// Deprecated: use global logger instead. func Logger(logger log.Logger) ServerOption { - return func(s *Server) { - s.log = log.NewHelper(logger) - } + return func(s *Server) {} } // Middleware with server middleware. @@ -113,7 +112,6 @@ type Server struct { address string endpoint *url.URL timeout time.Duration - log *log.Helper middleware []middleware.Middleware unaryInts []grpc.UnaryServerInterceptor streamInts []grpc.StreamServerInterceptor @@ -130,7 +128,6 @@ func NewServer(opts ...ServerOption) *Server { address: ":0", timeout: 1 * time.Second, health: health.NewServer(), - log: log.NewHelper(log.GetLogger()), } for _, o := range opts { o(srv) @@ -184,7 +181,7 @@ func (s *Server) Start(ctx context.Context) error { return s.err } s.baseCtx = ctx - s.log.Infof("[gRPC] server listening on: %s", s.lis.Addr().String()) + log.Infof("[gRPC] server listening on: %s", s.lis.Addr().String()) s.health.Resume() return s.Serve(s.lis) } @@ -193,7 +190,7 @@ func (s *Server) Start(ctx context.Context) error { func (s *Server) Stop(ctx context.Context) error { s.health.Shutdown() s.GracefulStop() - s.log.Info("[gRPC] server stopping") + log.Info("[gRPC] server stopping") return nil } diff --git a/transport/grpc/server_test.go b/transport/grpc/server_test.go index 99a22ffb8..5dd779f37 100644 --- a/transport/grpc/server_test.go +++ b/transport/grpc/server_test.go @@ -13,7 +13,6 @@ import ( "github.com/go-kratos/kratos/v2/errors" pb "github.com/go-kratos/kratos/v2/internal/testdata/helloworld" - "github.com/go-kratos/kratos/v2/log" "github.com/go-kratos/kratos/v2/middleware" "github.com/go-kratos/kratos/v2/transport" @@ -157,35 +156,6 @@ func TestMiddleware(t *testing.T) { } } -type mockLogger struct { - level log.Level - key string - val string -} - -func (l *mockLogger) Log(level log.Level, keyvals ...interface{}) error { - l.level = level - l.key = keyvals[0].(string) - l.val = keyvals[1].(string) - return nil -} - -func TestLogger(t *testing.T) { - o := &Server{} - v := &mockLogger{} - Logger(v)(o) - o.log.Log(log.LevelWarn, "foo", "bar") - if !reflect.DeepEqual("foo", v.key) { - t.Errorf("expect %s, got %s", "foo", v.key) - } - if !reflect.DeepEqual("bar", v.val) { - t.Errorf("expect %s, got %s", "bar", v.val) - } - if !reflect.DeepEqual(log.LevelWarn, v.level) { - t.Errorf("expect %s, got %s", log.LevelWarn, v.level) - } -} - func TestTLSConfig(t *testing.T) { o := &Server{} v := &tls.Config{} diff --git a/transport/http/resolver.go b/transport/http/resolver.go index 6bb1c3fff..1a4ba81c5 100644 --- a/transport/http/resolver.go +++ b/transport/http/resolver.go @@ -44,7 +44,6 @@ type resolver struct { target *Target watcher registry.Watcher - logger *log.Helper insecure bool } @@ -57,7 +56,6 @@ func newResolver(ctx context.Context, discovery registry.Discovery, target *Targ r := &resolver{ target: target, watcher: watcher, - logger: log.NewHelper(log.GetLogger()), rebalancer: rebalancer, insecure: insecure, } @@ -81,15 +79,15 @@ func newResolver(ctx context.Context, discovery registry.Discovery, target *Targ if err != nil { stopErr := watcher.Stop() if stopErr != nil { - r.logger.Errorf("failed to http client watch stop: %v, error: %+v", target, stopErr) + log.Errorf("failed to http client watch stop: %v, error: %+v", target, stopErr) } return nil, err } case <-ctx.Done(): - r.logger.Errorf("http client watch service %v reaching context deadline!", target) + log.Errorf("http client watch service %v reaching context deadline!", target) stopErr := watcher.Stop() if stopErr != nil { - r.logger.Errorf("failed to http client watch stop: %v, error: %+v", target, stopErr) + log.Errorf("failed to http client watch stop: %v, error: %+v", target, stopErr) } return nil, ctx.Err() } @@ -101,7 +99,7 @@ func newResolver(ctx context.Context, discovery registry.Discovery, target *Targ if errors.Is(err, context.Canceled) { return } - r.logger.Errorf("http client watch service %v got unexpected error:=%v", target, err) + log.Errorf("http client watch service %v got unexpected error:=%v", target, err) time.Sleep(time.Second) continue } @@ -116,7 +114,7 @@ func (r *resolver) update(services []*registry.ServiceInstance) bool { for _, ins := range services { ept, err := endpoint.ParseEndpoint(ins.Endpoints, "http", !r.insecure) if err != nil { - r.logger.Errorf("Failed to parse (%v) discovery endpoint: %v error %v", r.target, ins.Endpoints, err) + log.Errorf("Failed to parse (%v) discovery endpoint: %v error %v", r.target, ins.Endpoints, err) continue } if ept == "" { @@ -125,7 +123,7 @@ func (r *resolver) update(services []*registry.ServiceInstance) bool { nodes = append(nodes, selector.NewNode("http", ept, ins)) } if len(nodes) == 0 { - r.logger.Warnf("[http resolver]Zero endpoint found,refused to write,set: %s ins: %v", r.target.Endpoint, nodes) + log.Warnf("[http resolver]Zero endpoint found,refused to write,set: %s ins: %v", r.target.Endpoint, nodes) return false } r.rebalancer.Apply(nodes) diff --git a/transport/http/server.go b/transport/http/server.go index c9a0597f2..58f3060de 100644 --- a/transport/http/server.go +++ b/transport/http/server.go @@ -49,10 +49,9 @@ func Timeout(timeout time.Duration) ServerOption { } // Logger with server logger. +// Deprecated: use global logger instead. func Logger(logger log.Logger) ServerOption { - return func(s *Server) { - s.log = log.NewHelper(logger) - } + return func(s *Server) {} } // Middleware with service middleware option. @@ -130,7 +129,6 @@ type Server struct { ene EncodeErrorFunc strictSlash bool router *mux.Router - log *log.Helper } // NewServer creates an HTTP server by options. @@ -143,7 +141,6 @@ func NewServer(opts ...ServerOption) *Server { enc: DefaultResponseEncoder, ene: DefaultErrorEncoder, strictSlash: true, - log: log.NewHelper(log.GetLogger()), } for _, o := range opts { o(srv) @@ -243,7 +240,7 @@ func (s *Server) Start(ctx context.Context) error { s.BaseContext = func(net.Listener) context.Context { return ctx } - s.log.Infof("[HTTP] server listening on: %s", s.lis.Addr().String()) + log.Infof("[HTTP] server listening on: %s", s.lis.Addr().String()) var err error if s.tlsConf != nil { err = s.ServeTLS(s.lis, "", "") @@ -258,7 +255,7 @@ func (s *Server) Start(ctx context.Context) error { // Stop stop the HTTP server. func (s *Server) Stop(ctx context.Context) error { - s.log.Info("[HTTP] server stopping") + log.Info("[HTTP] server stopping") return s.Shutdown(ctx) }