pingcap / log Goto Github PK

View Code? Open in Web Editor NEW

17.0 99.0 29.0 83 KB

License: Apache License 2.0

Go 100.00%

log's People

Contributors

Stargazers

Watchers

log's Issues

CallerSkip is not set correctly

#29 moved zap.AddCallerSkip(1) to the L() function. However, the global logger may be replaced after first call to L().

As a result, some complements that depend on this log library print wrong file name and line number.
tiflow-master

tidb-server

new sub logger using `with` can not get right log format

what we do

func (s *testLogSuite) TestContextLogger(c *C)  {
	l, _, err := zaplog.InitLogger(&zaplog.Config{DisableStacktrace:true})
	if err != nil {
		panic(err)
	}
	l2 := l.With(zap.String("c", "c1"))
	l2.Error("", zap.String("k", "v"), zap.String("k2", "v2"))
}

expect

[2019/03/04 23:00:05.943 +08:00] [ERROR] [log_test.go:209] [c=c1] [k=v] [k2=v2]

but got

[2019/03/04 23:00:05.943 +08:00] [ERROR] [log_test.go:209],c=c1 [k=v] [k2=v2]

could we write normal log into stderr?

As https://github.com/pingcap/log/blob/master/log.go#L53, if we don't customize cfg.File, normal log will be written into stdout, which pollutes the stdout of applications in the downstream.

I see this library distinguishes normal log and error log, and they are designed to be written into different places. So I propose two solutions:

write all log into stderr by default (may be incompatible)
keep the default behavior and add an option to allow normal log to be written into stderr (currently only support file path)

Integrate SkyWalking-eyes to check license headers in CI

This is a mirror issue from pingcap/tidb#26956 to bring the benefit to pingcap/log.

There is no need to sync

See here:

log/log.go

Line 127 in 8f1e84a

return S().Sync()

// Sync flushes any buffered log entries.
func Sync() error {
	err := L().Sync()
	if err != nil {
		return err
	}
	return S().Sync()
}

这里没有必要调用S().Sync()，因为 *SugaredLogger.Sync() 的实现如下：

There is no need to call S().Sync() because *SugaredLogger.Sync() which is implemented as:

type SugaredLogger struct {
	base *Logger
}

// Sync flushes any buffered log entries.
func (s *SugaredLogger) Sync() error {
	return s.base.Sync()
}

因此，这和已经调用的L().Sync()等价，SugaredLogger没有独立的Sync()方法，它依赖于Logger的Sync()。

It's equivalent to L().Sync(), which has been called.

panic on unknown log format

What did you do?

Use --log-format=xml.

What did you expect to see?

Either an XML formatted log or an error saying only text and json are supported

What did you see instead?

[dvaneeden@dve-carbon test_db]$ tiup br backup full -s local:///tmp/tidb_backup_003 --log-format=xml
Starting component `br`: /home/dvaneeden/.tiup/components/br/v5.0.1/br backup full -s local:///tmp/tidb_backup_003 --log-format=xml
Detail BR log in /tmp/br.log.2021-05-04T16.48.04+0200 
panic: unsupport log format: xml

goroutine 1 [running]:
github.com/pingcap/log.NewTextEncoder(0xc0008383c0, 0xc0008441e0, 0x4)
	github.com/pingcap/[email protected]/zap_text_encoder.go:168 +0x3fb
github.com/pingcap/log.newZapTextEncoder(...)
	github.com/pingcap/[email protected]/config.go:78
github.com/pingcap/log.InitLoggerWithWriteSyncer(0xc0008383c0, 0x3a4b860, 0xc00084e0b0, 0x0, 0x0, 0x0, 0xc0006bba30, 0x1, 0x1, 0x37)
	github.com/pingcap/[email protected]/log.go:63 +0xc0
github.com/pingcap/log.InitLogger(0xc0008383c0, 0x0, 0x0, 0x0, 0x1, 0xc0007b3380, 0x37, 0x0)
	github.com/pingcap/[email protected]/log.go:53 +0xa7
main.Init.func1()
	github.com/pingcap/br@/cmd/br/cmd.go:115 +0x260
sync.(*Once).doSlow(0x57d9a78, 0xc0006bbb80)
	sync/once.go:66 +0xe3
sync.(*Once).Do(...)
	sync/once.go:57
main.Init(0xc000840dc0, 0x0, 0x0)
	github.com/pingcap/br@/cmd/br/cmd.go:89 +0xbc
main.NewBackupCommand.func1(0xc000840dc0, 0xc00084a2d0, 0x0, 0x3, 0x0, 0x0)
	github.com/pingcap/br@/cmd/br/backup.go:74 +0x2f
github.com/spf13/cobra.(*Command).execute(0xc000840dc0, 0xc0000f2150, 0x3, 0x3, 0xc000840dc0, 0xc0000f2150)
	github.com/spf13/[email protected]/command.go:821 +0x56b
github.com/spf13/cobra.(*Command).ExecuteC(0xc000205340, 0xc0006bbe38, 0x3, 0x3)
	github.com/spf13/[email protected]/command.go:950 +0x349
github.com/spf13/cobra.(*Command).Execute(...)
	github.com/spf13/[email protected]/command.go:887
main.main()
	github.com/pingcap/br@/cmd/br/main.go:56 +0x35c
Error: run `/home/dvaneeden/.tiup/components/br/v5.0.1/br` (wd:/home/dvaneeden/.tiup/data/SWSD09N) failed: exit status 2

What version of BR and TiDB/TiKV/PD are you using?

[dvaneeden@dve-carbon test_db]$ tiup br --version
Starting component `br`: /home/dvaneeden/.tiup/components/br/v5.0.1/br --version
Release Version: v5.0.1
Git Commit Hash: b3bd5495369244cfb1d3f2c49763a7fdb55d4ecd
Git Branch: heads/refs/tags/v5.0.1
Go Version: go1.13
UTC Build Time: 2021-04-23 06:02:57
Race Enabled: false

Improvements:

Add documentation for --log-format
List possible choices in --help output
Return a more helpful error message instead of a panic
Maybe use "text" as fallback?

log.With shouldn't add caller skip

log/global.go

Lines 66 to 70 in ca23291

 // With creates a child logger and adds structured context to it. 

 // Fields added to the child don't affect the parent, and vice versa. 

 func With(fields ...zap.Field) *zap.Logger { 

 return L().WithOptions(zap.AddCallerSkip(1)).With(fields...) 

 }

log.With shouldn't add caller skip, since it just returns a new logger without calling any function to print logs.

log/global.go

Lines 27 to 29 in 1b12a1f

 func Debug(msg string, fields ...zap.Field) { 

 L().WithOptions(zap.AddCallerSkip(1)).Debug(msg, fields...) 

 }

pprof

	// With creates a child logger and adds structured context to it.
	// Fields added to the child don't affect the parent, and vice versa.
	func With(fields ...zap.Field) *zap.Logger {
	return L().WithOptions(zap.AddCallerSkip(1)).With(fields...)
	}

	func Debug(msg string, fields ...zap.Field) {
	L().WithOptions(zap.AddCallerSkip(1)).Debug(msg, fields...)
	}

pingcap / log Goto Github PK

log's People

Contributors

Stargazers

Watchers

Forkers

log's Issues

what we do

expect

but got

Recommend Projects

Recommend Topics

Recommend Org