From 7f1e8e22c39a715ff5b9728b7acd8d6a7c3450df Mon Sep 17 00:00:00 2001 From: Chunzhu Li Date: Fri, 29 Nov 2019 16:31:06 +0800 Subject: [PATCH] loader/syncer: fix bug in #355 (#381) * fix situations when there is extra msg in error.Error * add deal for zap.Error and add UT --- pkg/log/log.go | 14 ++++++++-- pkg/log/log_test.go | 67 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 pkg/log/log_test.go diff --git a/pkg/log/log.go b/pkg/log/log.go index 3a1c7de212..bb9d41f32e 100644 --- a/pkg/log/log.go +++ b/pkg/log/log.go @@ -16,7 +16,9 @@ package log import ( "context" "fmt" + "strings" + "github.com/pingcap/errors" pclog "github.com/pingcap/log" "github.com/pingcap/tidb/util/logutil" "go.uber.org/zap" @@ -76,8 +78,16 @@ func (l Logger) WithFields(fields ...zap.Field) Logger { // ErrorFilterContextCanceled wraps Logger.Error() and will filter error log when error is context.Canceled func (l Logger) ErrorFilterContextCanceled(msg string, fields ...zap.Field) { for _, field := range fields { - if field.Key == "error" && field.String == context.Canceled.Error() { - return + switch field.Type { + case zapcore.StringType: + if field.Key == "error" && strings.Contains(field.String, context.Canceled.Error()) { + return + } + case zapcore.ErrorType: + err, ok := field.Interface.(error) + if ok && errors.Cause(err) == context.Canceled { + return + } } } l.Logger.Error(msg, fields...) diff --git a/pkg/log/log_test.go b/pkg/log/log_test.go new file mode 100644 index 0000000000..6fbc453008 --- /dev/null +++ b/pkg/log/log_test.go @@ -0,0 +1,67 @@ +// Copyright 2019 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package log + +import ( + "context" + "testing" + + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest" + + . "github.com/pingcap/check" + "github.com/pingcap/errors" +) + +func TestLog(t *testing.T) { + TestingT(t) +} + +type testLogSuite struct{} + +var _ = Suite(&testLogSuite{}) + +func (s *testLogSuite) TestTestLogger(c *C) { + logger, buffer := makeTestLogger() + logger.Warn("the message", zap.Int("number", 123456), zap.Ints("array", []int{7, 8, 9})) + c.Assert( + buffer.Stripped(), Equals, + `{"$lvl":"WARN","$msg":"the message","number":123456,"array":[7,8,9]}`, + ) + buffer.Reset() + logger.ErrorFilterContextCanceled("the message", zap.Int("number", 123456), + zap.Ints("array", []int{7, 8, 9}), zap.Error(context.Canceled)) + c.Assert(buffer.Stripped(), Equals, "") + buffer.Reset() + logger.ErrorFilterContextCanceled("the message", zap.Int("number", 123456), + zap.Ints("array", []int{7, 8, 9}), ShortError(errors.Annotate(context.Canceled, "extra info"))) + c.Assert(buffer.Stripped(), Equals, "") +} + +// makeTestLogger creates a Logger instance which produces JSON logs. +func makeTestLogger() (Logger, *zaptest.Buffer) { + buffer := new(zaptest.Buffer) + logger := zap.New(zapcore.NewCore( + zapcore.NewJSONEncoder(zapcore.EncoderConfig{ + LevelKey: "$lvl", + MessageKey: "$msg", + EncodeLevel: zapcore.CapitalLevelEncoder, + EncodeDuration: zapcore.StringDurationEncoder, + }), + buffer, + zap.DebugLevel, + )) + return Logger{Logger: logger}, buffer +}