Fix inconsistent log format #13864
Fix inconsistent log format #13864
Conversation
serathius
force-pushed
the
logs
branch
5 times, most recently
from
1253a9a
to
5da0859
Compare
Codecov Report
@@ Coverage Diff @@
## main #13864 +/- ##
==========================================
- Coverage 72.51% 72.42% -0.10%
==========================================
Files 468 467 -1
Lines 38222 38231 +9
==========================================
- Hits 27718 27689 -29
- Misses 8718 8752 +34
- Partials 1786 1790 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
client/v3/snapshot/v3_snapshot.go
Outdated
| @@ -46,7 +46,7 @@ func hasChecksum(n int64) bool { | |||
| // Etcd <v3.6 will return "" as version. | |||
| func SaveWithVersion(ctx context.Context, lg *zap.Logger, cfg clientv3.Config, dbPath string) (version string, err error) { | |||
| if lg == nil { | |||
| lg = zap.NewExample() | |||
| lg = zap.NewNop() | |||
There was a problem hiding this comment.
Why do you prefer Nop ?
Please add a comment, otherwise it's tempting during debugging to add back 'any logging'.
There was a problem hiding this comment.
This is based on "no logging is better than inconsistent one. Alternatively I can remove the nil check.
| } | ||
|
|
||
| // ExpectFunc returns the first line satisfying the function f. | ||
| func (ep *ExpectProcess) ExpectFunc(f func(string) bool) (string, error) { | ||
| lastLinesBuffer := make([]string, 0) | ||
| i := 0 |
There was a problem hiding this comment.
This seems to be an orthogonal refactor on top of the logs change. What's the goal ?
There was a problem hiding this comment.
This change is needed to write an e2e test that will scan all the logs and check if they all follow a single format.
This fixes problem where logs json changes format of timestamp.
| @@ -88,7 +89,7 @@ type epHealth struct { | |||
|
|
|||
| // epHealthCommandFunc executes the "endpoint-health" command. | |||
| func epHealthCommandFunc(cmd *cobra.Command, args []string) { | |||
| lg, err := zap.NewProduction() | |||
| lg, err := logutil.CreateDefaultZapLogger(zap.InfoLevel) | |||
There was a problem hiding this comment.
why not use etcdClientDebugLevel() here? I see other places all use it.
There was a problem hiding this comment.
Got it, it's in different package client/v3
There was a problem hiding this comment.
etcdClientDebugLevel is only used by etcd client in client/v3 package. The default verbosity for zap.NewProduction and zap.NewExample is info. I didn't want this PR to make any changes as this is a fix that I plan to backport to v3.5
Found that etcd logs have inconsistent format. Logs written before etcd logger is configured will use a zap production configuration. This is a big problem when you try to parse those logs and notice that timestamp stored under
tskey can change it's format fromfloattostring.Example logs:
cc @ahrtr @ptabor