Skip to content

Commit

Permalink
*: unify logger & lint ./lib
Browse files Browse the repository at this point in the history
Signed-off-by: xhe <xw897002528@gmail.com>
  • Loading branch information
xhebox committed Sep 27, 2022
1 parent e0cfc70 commit 86de17f
Show file tree
Hide file tree
Showing 14 changed files with 222 additions and 99 deletions.
9 changes: 9 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,21 @@ run:
issues-exit-code: 1

linters-settings:
errcheck:
exclude-functions:
- io.WriteString
- (*go.uber.org/zap/buffer.Buffer).WriteString
- (*go.uber.org/zap/buffer.Buffer).WriteByte

issues:
exclude-rules:
- path: _test\.go
linters:
- gosec
- path: util/security/tls.go
linters:
- gosec
text: "G402:"

linters:
enable:
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ cmd_%:

lint: ./bin/golangci-lint
$(GOBIN)/golangci-lint run
cd lib && $(GOBIN)/golangci-lint run

test: ./bin/gocovmerge
rm -f .cover.*
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ require (
go.uber.org/zap v1.23.0
golang.org/x/exp v0.0.0-20220907003533-145caa8ea1d0
google.golang.org/grpc v1.49.0
gopkg.in/natefinch/lumberjack.v2 v2.0.0
)

require (
Expand Down Expand Up @@ -121,7 +122,6 @@ require (
golang.org/x/time v0.0.0-20220722155302-e5dcc9cfc0b9 // indirect
google.golang.org/genproto v0.0.0-20220822174746-9e6da59bd2fc // indirect
google.golang.org/protobuf v1.28.1 // indirect
gopkg.in/natefinch/lumberjack.v2 v2.0.0 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
sigs.k8s.io/yaml v1.2.0 // indirect
Expand Down
15 changes: 8 additions & 7 deletions lib/cli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ package cli
import (
"net/http"

"github.com/pingcap/TiProxy/lib/config"
"github.com/pingcap/TiProxy/lib/util/cmd"
"github.com/spf13/cobra"
"go.uber.org/zap"
)

func GetRootCmd() *cobra.Command {
Expand All @@ -34,12 +35,12 @@ func GetRootCmd() *cobra.Command {
logLevel := rootCmd.PersistentFlags().String("log_level", "info", "log level")
rootCmd.PersistentFlags().Bool("indent", true, "whether indent the returned json")
rootCmd.PersistentPreRunE = func(_ *cobra.Command, _ []string) error {
zapcfg := zap.NewDevelopmentConfig()
zapcfg.Encoding = *logEncoder
if level, err := zap.ParseAtomicLevel(*logLevel); err == nil {
zapcfg.Level = level
}
logger, err := zapcfg.Build()
logger, _, _, err := cmd.BuildLogger(&config.Log{
Encoder: *logEncoder,
LogOnline: config.LogOnline{
Level: *logLevel,
},
})
if err != nil {
return err
}
Expand Down
5 changes: 2 additions & 3 deletions lib/cli/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"bytes"
"encoding/json"
"fmt"
"io/ioutil"
"net/http"
"os"
"path/filepath"
Expand All @@ -34,7 +33,7 @@ const (
)

func listAllFiles(dir, ext string) ([]string, error) {
infos, err := ioutil.ReadDir(dir)
infos, err := os.ReadDir(dir)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -178,7 +177,7 @@ func GetNamespaceCmd(ctx *Context) *cobra.Command {
}

for _, nFile := range nFiles {
fileData, err := ioutil.ReadFile(nFile)
fileData, err := os.ReadFile(nFile)
if err != nil {
return err
}
Expand Down
3 changes: 1 addition & 2 deletions lib/cli/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"
"fmt"
"io"
"io/ioutil"
"math/rand"
"net/http"

Expand Down Expand Up @@ -50,7 +49,7 @@ func doRequest(ctx context.Context, bctx *Context, method string, url string, rd
if err != nil {
return "", err
}
resb, _ := ioutil.ReadAll(res.Body)
resb, _ := io.ReadAll(res.Body)
res.Body.Close()

switch res.StatusCode {
Expand Down
1 change: 1 addition & 0 deletions lib/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ require (
github.com/pkg/errors v0.9.1 // indirect
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1 // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
gopkg.in/natefinch/lumberjack.v2 v2.0.0
)
15 changes: 0 additions & 15 deletions lib/util/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,12 @@ import (
"fmt"
"os"
"os/signal"
"sync"
"syscall"

"github.com/pingcap/log"
"github.com/spf13/cobra"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
)

var registerEncoders sync.Once

func RunRootCommand(rootCmd *cobra.Command) {
registerEncoders.Do(func() {
zap.RegisterEncoder("tidb", func(cfg zapcore.EncoderConfig) (zapcore.Encoder, error) {
return log.NewTextEncoder(&log.Config{})
})
zap.RegisterEncoder("newtidb", func(cfg zapcore.EncoderConfig) (zapcore.Encoder, error) {
return NewTiDBEncoder(cfg), nil
})
})

ctx, cancel := context.WithCancel(context.Background())
go func() {
sc := make(chan os.Signal, 1)
Expand Down
14 changes: 0 additions & 14 deletions lib/util/cmd/encoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,20 +148,6 @@ func (e *tidbEncoder) EncodeEntry(ent zapcore.Entry, fields []zapcore.Field) (*b
}

/* map encoder part */
func (f *tidbEncoder) needDoubleQuotes(s string) bool {
for i := 0; i < len(s); {
b := s[i]
if b <= 0x20 {
return true
}
switch b {
case '\\', '"', '[', ']', '=':
return true
}
i++
}
return false
}
func (f *tidbEncoder) safeAddString(s string) {
needQuotes := false
outerloop:
Expand Down
50 changes: 25 additions & 25 deletions pkg/manager/logger/builder.go → lib/util/cmd/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,41 +12,25 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package logger
package cmd

import (
"sync"

"github.com/pingcap/TiProxy/lib/config"
"github.com/pingcap/TiProxy/lib/util/cmd"
"github.com/pingcap/log"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
)

var registerEncoders sync.Once

func buildEncoder(cfg *config.Log) (zapcore.Encoder, error) {
zapcfg := zap.NewDevelopmentConfig()
zapcfg.Encoding = cfg.Encoder
zapcfg.DisableStacktrace = true
encoder, err := log.NewTextEncoder(&log.Config{})
if err != nil {
return nil, err
encfg := zap.NewProductionEncoderConfig()
switch cfg.Encoder {
case "tidb":
return log.NewTextEncoder(&log.Config{})
case "newtidb":
fallthrough
default:
return NewTiDBEncoder(encfg), nil
}
registerEncoders.Do(func() {
if err = zap.RegisterEncoder("tidb", func(cfg zapcore.EncoderConfig) (zapcore.Encoder, error) {
return encoder, nil
}); err != nil {
return
}
if err = zap.RegisterEncoder("newtidb", func(cfg zapcore.EncoderConfig) (zapcore.Encoder, error) {
return cmd.NewTiDBEncoder(cfg), nil
}); err != nil {
return
}
})
return encoder, err
}

func buildLevel(cfg *config.Log) (zap.AtomicLevel, error) {
Expand All @@ -60,3 +44,19 @@ func buildSyncer(cfg *config.Log) (*AtomicWriteSyncer, error) {
}
return syncer, nil
}

func BuildLogger(cfg *config.Log) (*zap.Logger, *AtomicWriteSyncer, zap.AtomicLevel, error) {
level, err := buildLevel(cfg)
if err != nil {
return nil, nil, level, err
}
encoder, err := buildEncoder(cfg)
if err != nil {
return nil, nil, level, err
}
syncer, err := buildSyncer(cfg)
if err != nil {
return nil, nil, level, err
}
return zap.New(zapcore.NewCore(encoder, syncer, level), zap.ErrorOutput(syncer), zap.AddStacktrace(zapcore.FatalLevel)), syncer, level, nil
}
144 changes: 144 additions & 0 deletions lib/util/cmd/syncer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
// Copyright 2022 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,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package cmd

import (
"os"
"sync"

"github.com/pingcap/TiProxy/lib/config"
"github.com/pingcap/TiProxy/lib/util/errors"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"gopkg.in/natefinch/lumberjack.v2"
)

const (
defaultLogMaxSize = 300 // MB
)

var _ closableSyncer = (*rotateLogger)(nil)
var _ closableSyncer = (*stdoutLogger)(nil)
var _ zapcore.WriteSyncer = (*AtomicWriteSyncer)(nil)

// Wrap the syncers as closableSyncer because lumberjack.Logger needs to be closed.
type closableSyncer interface {
zapcore.WriteSyncer
Close() error
}

type rotateLogger struct {
*lumberjack.Logger
}

func (lg *rotateLogger) Sync() error {
return nil
}

type stdoutLogger struct {
zapcore.WriteSyncer
}

func (lg *stdoutLogger) Close() error {
return nil
}

// AtomicWriteSyncer is a WriteSyncer that can be updated online.
type AtomicWriteSyncer struct {
sync.RWMutex
output closableSyncer
}

// Rebuild creates a new output and replaces the current one.
func (ws *AtomicWriteSyncer) Rebuild(cfg *config.LogOnline) error {
var output closableSyncer
if len(cfg.LogFile.Filename) > 0 {
fileLogger, err := initFileLog(&cfg.LogFile)
if err != nil {
return err
}
output = &rotateLogger{fileLogger}
} else {
stdLogger, _, err := zap.Open([]string{"stdout"}...)
if err != nil {
return err
}
output = &stdoutLogger{stdLogger}
}
return ws.setOutput(output)
}

// Write implements WriteSyncer.Write().
func (ws *AtomicWriteSyncer) Write(p []byte) (n int, err error) {
ws.RLock()
if ws.output != nil {
n, err = ws.output.Write(p)
}
ws.RUnlock()
return
}

// Sync implements WriteSyncer.Sync().
func (ws *AtomicWriteSyncer) Sync() error {
var err error
ws.RLock()
if ws.output != nil {
err = ws.output.Sync()
}
ws.RUnlock()
return err
}

func (ws *AtomicWriteSyncer) setOutput(output closableSyncer) error {
var err error
ws.Lock()
if ws.output != nil {
err = ws.output.Close()
}
ws.output = output
ws.Unlock()
return err
}

// Close closes logger.
func (ws *AtomicWriteSyncer) Close() error {
var err error
ws.Lock()
if ws.output != nil {
err = ws.output.Close()
ws.output = nil
}
ws.Unlock()
return err
}

// initFileLog initializes file based logging options.
func initFileLog(cfg *config.LogFile) (*lumberjack.Logger, error) {
if st, err := os.Stat(cfg.Filename); err == nil {
if st.IsDir() {
return nil, errors.New("can't use directory as log file name")
}
}
if cfg.MaxSize == 0 {
cfg.MaxSize = defaultLogMaxSize
}
return &lumberjack.Logger{
Filename: cfg.Filename,
MaxSize: cfg.MaxSize,
MaxBackups: cfg.MaxBackups,
MaxAge: cfg.MaxDays,
LocalTime: true,
}, nil
}
2 changes: 1 addition & 1 deletion lib/util/errors/stacktrace.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func formatFrame(s fmt.State, fr runtime.Frame, verb rune) {
}
}

// stacktrace only stores the pointers, infomation will be queried as need.
// stacktrace only stores the pointers, information will be queried as need.
type stacktrace []uintptr

// Format formats the stack of Frames according to the fmt.Formatter interface.
Expand Down
Loading

0 comments on commit 86de17f

Please sign in to comment.