[fix] no log print when panic (#1413)

.github/workflows/golangci-lint.yml

@@ -9,5 +9,5 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.48.0
+          version: v1.51.2
           args: --timeout=5m --skip-dirs='api,test,.*/controller/(v3|v4)$,.*/bootstrap$,examples,integration' --enable gofmt,revive,gocyclo,goimports --skip-files=.*_test.go$

pkg/log/config.go

@@ -27,14 +27,13 @@ type Config struct {
 	LogRotateSize  int
 	LogBackupCount int
 	// days
-	LogBackupAge int
-	CallerSkip   int
-	NoTime       bool // if true, not record time
-	NoLevel      bool // if true, not record level
-	NoCaller     bool // if true, not record caller
-	// Event driven
-	FlushFunc   func()
-	RecoverFunc func(r interface{})
+	LogBackupAge   int
+	CallerSkip     int
+	NoTime         bool // if true, not record time
+	NoLevel        bool // if true, not record level
+	NoCaller       bool // if true, not record caller
+	ReplaceGlobals bool
+	RedirectStdLog bool
 }
 
 func (cfg Config) WithCallerSkip(s int) Config {
@@ -57,17 +56,12 @@ func (cfg Config) WithNoLevel(b bool) Config {
 	return cfg
 }
 
-func (cfg Config) WithNoCaller(b bool) Config {
-	cfg.NoCaller = b
+func (cfg Config) WithReplaceGlobals(b bool) Config {
+	cfg.ReplaceGlobals = b
 	return cfg
 }
 
-func (cfg Config) WithExitFunc(f func()) Config {
-	cfg.FlushFunc = f
-	return cfg
-}
-
-func (cfg Config) WithRecoverFunc(f func(itf interface{})) Config {
-	cfg.RecoverFunc = f
+func (cfg Config) WithRedirectStdLog(b bool) Config {
+	cfg.RedirectStdLog = b
 	return cfg
 }

pkg/log/log.go

@@ -25,29 +25,37 @@ import (
 )
 
 const (
-	globalCallerSkip = 2
-	defaultLogLevel  = "DEBUG"
+	globalCallerSkip        = 2
+	globalRecoverCallerSkip = 4
+	defaultLogLevel         = "DEBUG"
 )
 
 var (
-	Configure = DefaultConfig()
-	Logger    = NewLogger(Configure)
+	flushFunc   = func() {}
+	recoverFunc = func(r interface{}) {}
+	Logger      = NewLogger(DefaultConfig())
 )
 
 func Init(cfg Config) {
-	Configure = cfg
-	Logger = NewLogger(cfg.WithCallerSkip(globalCallerSkip))
+	logger := NewZapLogger(cfg.
+		WithCallerSkip(cfg.CallerSkip + globalCallerSkip).
+		WithReplaceGlobals(true).
+		WithRedirectStdLog(true))
+	flushFunc = logger.Sync
+	recoverFunc = func(r interface{}) {
+		logger.Recover(r, cfg.CallerSkip+globalRecoverCallerSkip)
+	}
+	Logger = logger
 }
 
 func NewLogger(cfg Config) openlog.Logger {
-	return NewZapLogger(cfg)
+	return NewZapLogger(cfg.WithCallerSkip(cfg.CallerSkip + globalCallerSkip))
 }
 
 func DefaultConfig() Config {
 	return Config{
 		LoggerLevel:   defaultLogLevel,
 		LogFormatText: true,
-		CallerSkip:    globalCallerSkip,
 	}
 }
 
@@ -72,9 +80,7 @@ func Fatal(msg string, err error) {
 }
 
 func Flush() {
-	if Configure.FlushFunc != nil {
-		Configure.FlushFunc()
-	}
+	flushFunc()
 }
 
 func NilOrWarn(start time.Time, message string) {
@@ -105,9 +111,7 @@ func InfoOrWarn(start time.Time, message string) {
 
 // Panic is a function can only be called in defer function.
 func Panic(r interface{}) {
-	if Configure.RecoverFunc != nil {
-		Configure.RecoverFunc(r)
-	}
+	recoverFunc(r)
 }
 
 // Recover is a function call recover() and print the stack in log

pkg/log/log_test.go

@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 log_test
+
+import (
+	"testing"
+
+	"github.com/apache/servicecomb-service-center/pkg/log"
+)
+
+func TestRecover(t *testing.T) {
+	defer log.Recover()
+	log.Init(log.DefaultConfig())
+	panic("test")
+}

pkg/log/zap_logger.go

@@ -158,17 +158,11 @@ func (l *ZapLogger) Recover(r interface{}, callerSkip int) {
 		e.Caller.TrimmedPath(),
 		r,
 		e.Stack)
-	err := StderrSyncer.Sync() // sync immediately, for server may exit abnormally
-	if err != nil {
-		log.Println(err)
-	}
+	_ = StderrSyncer.Sync() // sync immediately, for server may exit abnormally
 	if err := l.zapLogger.Core().With([]zap.Field{zap.Reflect("recover", r)}).Write(e, nil); err != nil {
 		fmt.Fprintf(StderrSyncer, "%s\tERROR\t%v\n", time.Now().Format("2006-01-02T15:04:05.000Z0700"), err)
 		fmt.Fprintln(StderrSyncer, util.BytesToStringWithNoCopy(debug.Stack()))
-		err = StderrSyncer.Sync()
-		if err != nil {
-			log.Println(err)
-		}
+		_ = StderrSyncer.Sync()
 		return
 	}
 }
@@ -195,22 +189,16 @@ func NewZapLogger(cfg Config) *ZapLogger {
 		opts = append(opts, zap.AddCaller(), zap.AddCallerSkip(cfg.CallerSkip))
 	}
 	l := zap.New(toZapConfig(cfg), opts...)
-	// zap internal log
-	_ = zap.ReplaceGlobals(l)
-	// golang log
-	_ = zap.RedirectStdLog(l)
+	if cfg.ReplaceGlobals {
+		_ = zap.ReplaceGlobals(l)
+	}
+	if cfg.RedirectStdLog {
+		_ = zap.RedirectStdLog(l)
+	}
 	logger := &ZapLogger{
 		Config:    cfg,
 		zapLogger: l,
 		zapSugar:  l.Sugar(),
 	}
-	if cfg.FlushFunc == nil {
-		cfg.FlushFunc = logger.Sync
-	}
-	if cfg.RecoverFunc == nil {
-		cfg.RecoverFunc = func(r interface{}) {
-			logger.Recover(r, 3)
-		}
-	}
 	return logger
 }

pkg/metrics/calculator.go

@@ -69,8 +69,8 @@ func metricCounterOf(details *Details, m []*dto.Metric) {
 
 func metricSummaryOf(details *Details, m []*dto.Metric) {
 	var (
-		count uint64  = 0
-		sum   float64 = 0
+		count uint64
+		sum   float64
 	)
 	for _, d := range m {
 		count += d.GetSummary().GetSampleCount()
@@ -87,8 +87,8 @@ func metricSummaryOf(details *Details, m []*dto.Metric) {
 
 func metricHistogramOf(details *Details, m []*dto.Metric) {
 	var (
-		count uint64  = 0
-		sum   float64 = 0
+		count uint64
+		sum   float64
 	)
 	for _, d := range m {
 		count += d.GetHistogram().GetSampleCount()

syncer/service/replicator/resource/instance.go

@@ -56,8 +56,6 @@ type instance struct {
 	cur     *pb.MicroServiceInstance
 
 	manager metadataManager
-
-	defaultFailHandler
 }
 
 func (i *instance) loadInput() error {