Skip to content

Commit

Permalink
*: clean up the logging component (#23534)
Browse files Browse the repository at this point in the history
  • Loading branch information
SabaPing authored Jun 1, 2021
1 parent a80047c commit bbd5b32
Show file tree
Hide file tree
Showing 10 changed files with 141 additions and 427 deletions.
2 changes: 1 addition & 1 deletion config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ func (b *nullableBool) UnmarshalJSON(data []byte) error {
type Log struct {
// Log level.
Level string `toml:"level" json:"level"`
// Log format. one of json, text, or console.
// Log format, one of json or text.
Format string `toml:"format" json:"format"`
// Disable automatic timestamps in output. Deprecated: use EnableTimestamp instead.
DisableTimestamp nullableBool `toml:"disable-timestamp" json:"disable-timestamp"`
Expand Down
2 changes: 1 addition & 1 deletion config/config.toml.example
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ enable-enum-length-limit = true
# Log level: debug, info, warn, error, fatal.
level = "info"

# Log format, one of json, text, console.
# Log format, one of json or text.
format = "text"

# Enable automatic timestamps in log output, if not set, it will be defaulted to true.
Expand Down
5 changes: 2 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ require (
github.com/prometheus/common v0.9.1
github.com/rivo/uniseg v0.2.0 // indirect
github.com/shirou/gopsutil v3.21.2+incompatible
github.com/sirupsen/logrus v1.6.0
github.com/sirupsen/logrus v1.6.0 // indirect
github.com/soheilhy/cmux v0.1.4
github.com/tiancaiamao/appdash v0.0.0-20181126055449-889f96f722a2
github.com/tikv/pd v1.1.0-beta.0.20210323121136-78679e5e209d
Expand All @@ -77,9 +77,8 @@ require (
golang.org/x/tools v0.1.0
google.golang.org/grpc v1.27.1
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
gopkg.in/natefinch/lumberjack.v2 v2.0.0
gopkg.in/yaml.v2 v2.4.0 // indirect
honnef.co/go/tools v0.1.4 // indirect
honnef.co/go/tools v0.2.0 // indirect
modernc.org/mathutil v1.2.2 // indirect
sourcegraph.com/sourcegraph/appdash v0.0.0-20190731080439-ebfcffb1b5c0
sourcegraph.com/sourcegraph/appdash-data v0.0.0-20151005221446-73f23eafcf67
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -928,8 +928,8 @@ honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWh
honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
honnef.co/go/tools v0.1.4 h1:SadWOkti5uVN1FAMgxn165+Mw00fuQKyk4Gyn/inxNQ=
honnef.co/go/tools v0.1.4/go.mod h1:NgwopIslSNH47DimFoV78dnkksY2EFtX0ajyb3K/las=
honnef.co/go/tools v0.2.0 h1:ws8AfbgTX3oIczLPNPCu5166oBg9ST2vNs0rcht+mDE=
honnef.co/go/tools v0.2.0/go.mod h1:lPVVZ2BS5TfnjLyizF7o7hv7j9/L+8cZY2hLyjP9cGY=
k8s.io/klog v1.0.0/go.mod h1:4Bi6QPql/J/LkTDqv7R/cd3hPo4k2DG6Ptcz060Ez5I=
modernc.org/mathutil v1.2.2 h1:+yFk8hBprV+4c0U9GjFtL+dV3N8hOJ8JCituQcMShFY=
modernc.org/mathutil v1.2.2/go.mod h1:mZW8CKdRPY1v87qxC/wUdX5O1qDzXMP5TH3wjfpga6E=
Expand Down
29 changes: 11 additions & 18 deletions server/http_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/pingcap/failpoint"
"github.com/pingcap/kvproto/pkg/kvrpcpb"
"github.com/pingcap/kvproto/pkg/metapb"
"github.com/pingcap/log"
"github.com/pingcap/parser/model"
"github.com/pingcap/parser/terror"
"github.com/pingcap/tidb/config"
Expand Down Expand Up @@ -61,7 +62,6 @@ import (
"github.com/pingcap/tidb/util/gcutil"
"github.com/pingcap/tidb/util/logutil"
"github.com/pingcap/tidb/util/pdapi"
log "github.com/sirupsen/logrus"
"go.uber.org/zap"
)

Expand Down Expand Up @@ -625,13 +625,6 @@ func (h settingsHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
return
}

l, err1 := log.ParseLevel(levelStr)
if err1 != nil {
writeError(w, err1)
return
}
log.SetLevel(l)

config.GetGlobalConfig().Log.Level = levelStr
}
if generalLog := req.Form.Get("tidb_general_log"); generalLog != "" {
Expand Down Expand Up @@ -1090,7 +1083,7 @@ func (h ddlResignOwnerHandler) ServeHTTP(w http.ResponseWriter, req *http.Reques

err := h.resignDDLOwner()
if err != nil {
log.Error(err)
log.Error("failed to resign DDL owner", zap.Error(err))
writeError(w, err)
return
}
Expand Down Expand Up @@ -1134,7 +1127,7 @@ func (h tableHandler) addScatterSchedule(startKey, endKey []byte, name string) e
return err
}
if err := resp.Body.Close(); err != nil {
log.Error(err)
log.Error("failed to close response body", zap.Error(err))
}
return nil
}
Expand All @@ -1154,7 +1147,7 @@ func (h tableHandler) deleteScatterSchedule(name string) error {
return err
}
if err := resp.Body.Close(); err != nil {
log.Error(err)
log.Error("failed to close response body", zap.Error(err))
}
return nil
}
Expand Down Expand Up @@ -1664,14 +1657,14 @@ func (h serverInfoHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
do, err := session.GetDomain(h.Store)
if err != nil {
writeError(w, errors.New("create session error"))
log.Error(err)
log.Error("failed to get session domain", zap.Error(err))
return
}
info := serverInfo{}
info.ServerInfo, err = infosync.GetServerInfo()
if err != nil {
writeError(w, err)
log.Error(err)
log.Error("failed to get server info", zap.Error(err))
return
}
info.IsOwner = do.DDL().OwnerManager().IsOwner()
Expand All @@ -1694,22 +1687,22 @@ func (h allServerInfoHandler) ServeHTTP(w http.ResponseWriter, req *http.Request
do, err := session.GetDomain(h.Store)
if err != nil {
writeError(w, errors.New("create session error"))
log.Error(err)
log.Error("failed to get session domain", zap.Error(err))
return
}
ctx := context.Background()
allServersInfo, err := infosync.GetAllServerInfo(ctx)
if err != nil {
writeError(w, errors.New("ddl server information not found"))
log.Error(err)
log.Error("failed to get all server info", zap.Error(err))
return
}
ctx, cancel := context.WithTimeout(ctx, 3*time.Second)
ownerID, err := do.DDL().OwnerManager().GetOwnerID(ctx)
cancel()
if err != nil {
writeError(w, errors.New("ddl server information not found"))
log.Error(err)
log.Error("failed to get owner id", zap.Error(err))
return
}
allVersionsMap := map[infosync.ServerVersionInfo]struct{}{}
Expand Down Expand Up @@ -1905,13 +1898,13 @@ func (h ddlHookHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {

dom, err := session.GetDomain(h.store)
if err != nil {
log.Error(err)
log.Error("failed to get session domain", zap.Error(err))
writeError(w, err)
}

newCallbackFunc, err := ddl.GetCustomizedHook(req.FormValue("ddl_hook"))
if err != nil {
log.Error(err)
log.Error("failed to get customized hook", zap.Error(err))
writeError(w, err)
}
callback := newCallbackFunc(dom)
Expand Down
11 changes: 4 additions & 7 deletions server/http_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import (
. "github.com/pingcap/check"
"github.com/pingcap/failpoint"
"github.com/pingcap/kvproto/pkg/kvrpcpb"
zaplog "github.com/pingcap/log"
"github.com/pingcap/log"
"github.com/pingcap/parser/model"
"github.com/pingcap/parser/mysql"
"github.com/pingcap/tidb/config"
Expand All @@ -57,7 +57,6 @@ import (
"github.com/pingcap/tidb/util/codec"
"github.com/pingcap/tidb/util/rowcodec"
"github.com/pingcap/tidb/util/versioninfo"
log "github.com/sirupsen/logrus"
"go.uber.org/zap"
)

Expand Down Expand Up @@ -1155,8 +1154,7 @@ func (ts *HTTPHandlerTestSuite) TestPostSettings(c *C) {
resp, err := ts.formStatus("/settings", form)
c.Assert(err, IsNil)
c.Assert(resp.StatusCode, Equals, http.StatusOK)
c.Assert(log.GetLevel(), Equals, log.ErrorLevel)
c.Assert(zaplog.GetLevel(), Equals, zap.ErrorLevel)
c.Assert(log.GetLevel(), Equals, zap.ErrorLevel)
c.Assert(config.GetGlobalConfig().Log.Level, Equals, "error")
c.Assert(variable.ProcessGeneralLog.Load(), IsTrue)
val, err := variable.GetGlobalSystemVar(se.GetSessionVars(), variable.TiDBEnableAsyncCommit)
Expand All @@ -1175,8 +1173,7 @@ func (ts *HTTPHandlerTestSuite) TestPostSettings(c *C) {
c.Assert(err, IsNil)
c.Assert(resp.StatusCode, Equals, http.StatusOK)
c.Assert(variable.ProcessGeneralLog.Load(), IsFalse)
c.Assert(log.GetLevel(), Equals, log.FatalLevel)
c.Assert(zaplog.GetLevel(), Equals, zap.FatalLevel)
c.Assert(log.GetLevel(), Equals, zap.FatalLevel)
c.Assert(config.GetGlobalConfig().Log.Level, Equals, "fatal")
val, err = variable.GetGlobalSystemVar(se.GetSessionVars(), variable.TiDBEnableAsyncCommit)
c.Assert(err, IsNil)
Expand Down Expand Up @@ -1244,7 +1241,7 @@ func (ts *HTTPHandlerTestSuite) TestPprof(c *C) {
}
time.Sleep(time.Millisecond * 10)
}
zaplog.Fatal("failed to get profile for %d retries in every 10 ms", zap.Int("retryTime", retryTime))
log.Fatal("failed to get profile for %d retries in every 10 ms", zap.Int("retryTime", retryTime))
}

func (ts *HTTPHandlerTestSuite) TestServerInfo(c *C) {
Expand Down
10 changes: 0 additions & 10 deletions tidb-server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"context"
"flag"
"fmt"
"io"
"os"
"runtime"
"strconv"
Expand Down Expand Up @@ -69,7 +68,6 @@ import (
pd "github.com/tikv/pd/client"
"go.uber.org/automaxprocs/maxprocs"
"go.uber.org/zap"
"google.golang.org/grpc/grpclog"
)

// Flag Names
Expand Down Expand Up @@ -588,14 +586,6 @@ func setupLog() {
err := logutil.InitZapLogger(cfg.Log.ToLogConfig())
terror.MustNil(err)

err = logutil.InitLogger(cfg.Log.ToLogConfig())
terror.MustNil(err)

if len(os.Getenv("GRPC_DEBUG")) > 0 {
grpclog.SetLoggerV2(grpclog.NewLoggerV2WithVerbosity(os.Stderr, os.Stderr, os.Stderr, 999))
} else {
grpclog.SetLoggerV2(grpclog.NewLoggerV2(io.Discard, io.Discard, os.Stderr))
}
// trigger internal http(s) client init.
util.InternalHTTPClient()
}
Expand Down
Loading

0 comments on commit bbd5b32

Please sign in to comment.