Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

*: add json log format support #808

Merged
merged 3 commits into from
Jul 17, 2020
Merged

*: add json log format support #808

merged 3 commits into from
Jul 17, 2020

Conversation

csuzhangxc
Copy link
Member

What problem does this PR solve?

add json log format support, see internal PRM-327.

What is changed and how it works?

add log-format flag for some binaries.

Check List

Tests

  • Manual test (add detailed scripts or steps below)
    • run binary with/without --log-format=json

      [2020/07/17 16:13:11.774 +08:00] [INFO] [printer.go:54] ["Welcome to dm-master"] ["Release Version"=] ["Git Commit Hash"=4f36c144fde76685ebf82d81161bccea326b73b7] ["Git Branch"=json-log] ["UTC Build Time"="2020-07-17 08:11:24"] ["Go Version"="go version go1.13.4 linux/amd64"]
      
      {"level":"INFO","time":"2020/07/17 16:14:08.839 +08:00","caller":"printer.go:54","message":"Welcome to dm-master","Release Version":"","Git Commit Hash":"4f36c144fde76685ebf82d81161bccea326b73b7","Git Branch":"json-log","UTC Build Time":"2020-07-17 08:11:24","Go Version":"go version go1.13.4 linux/amd64"}
      

Code changes

  • Has exported function/method change
  • Has exported variable/fields change

Related changes

  • Need to cherry-pick to the release branch

@csuzhangxc csuzhangxc added priority/normal Minor change, requires approval from ≥1 primary reviewer status/PTAL This PR is ready for review. Add this label back after committing new changes type/enhancement Performance improvement or refactoring needs-cherry-pick-release-1.0 This PR should be cherry-picked to release-1.0. Remove this label after cherry-picked to release-1.0 labels Jul 17, 2020
@csuzhangxc csuzhangxc added this to the v2.0.0 RC milestone Jul 17, 2020
@lance6716
Copy link
Collaborator

do we need to verify user input?

@csuzhangxc
Copy link
Member Author

csuzhangxc commented Jul 17, 2020

do we need to verify user input?

@lance6716 do you mean the value of --log-format? the log pkg will verify it, an example

panic: unsupport log format: abc

goroutine 1 [running]:
github.com/pingcap/log.NewTextEncoder(0xc00005e360, 0xc00010c748, 0x4)
        /home/zhangxc/gopath/pkg/mod/github.com/pingcap/log@v0.0.0-20200511115504-543df19646ad/zap_text_encoder.go:168 +0x3fb
github.com/pingcap/log.newZapTextEncoder(...)
        /home/zhangxc/gopath/pkg/mod/github.com/pingcap/log@v0.0.0-20200511115504-543df19646ad/config.go:78
github.com/pingcap/log.InitLoggerWithWriteSyncer(0xc00005e360, 0x22ea920, 0xc0001e8ed0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xc0004d3c18, 0x40cad8)
        /home/zhangxc/gopath/pkg/mod/github.com/pingcap/log@v0.0.0-20200511115504-543df19646ad/log.go:52 +0xc0
github.com/pingcap/log.InitLogger(0xc00005e360, 0x0, 0x0, 0x0, 0x1a5dc1b, 0xc00064a140, 0x0, 0x0)
        /home/zhangxc/gopath/pkg/mod/github.com/pingcap/log@v0.0.0-20200511115504-543df19646ad/log.go:42 +0xb0
github.com/pingcap/dm/pkg/log.InitLogger(0xc0004d3f08, 0x4, 0x1f41f1e)
        /home/zhangxc/gopath/src/github.com/csuzhangxc/dm/pkg/log/log.go:112 +0x141
main.main()
        /home/zhangxc/gopath/src/github.com/csuzhangxc/dm/cmd/dm-master/main.go:47 +0x2b5

Copy link
Collaborator

@lance6716 lance6716 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lance6716 lance6716 added status/LGT1 One reviewer already commented LGTM and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Jul 17, 2020
Copy link
Collaborator

@GMHDBJD GMHDBJD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@GMHDBJD GMHDBJD merged commit 008f1e9 into pingcap:master Jul 17, 2020
@ti-srebot
Copy link

cherry pick to release-1.0 failed

@csuzhangxc csuzhangxc deleted the json-log branch July 17, 2020 10:16
@GMHDBJD GMHDBJD added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Jul 17, 2020
csuzhangxc added a commit to csuzhangxc/dm that referenced this pull request Jul 17, 2020
csuzhangxc added a commit to csuzhangxc/dm that referenced this pull request Jul 17, 2020
@csuzhangxc csuzhangxc added already-cherry-pick-1.0 The related PR is already cherry-picked to release-1.0. Add this label once the PR is cherry-picked and removed needs-cherry-pick-release-1.0 This PR should be cherry-picked to release-1.0. Remove this label after cherry-picked to release-1.0 labels Jul 17, 2020
GMHDBJD pushed a commit that referenced this pull request Jul 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
already-cherry-pick-1.0 The related PR is already cherry-picked to release-1.0. Add this label once the PR is cherry-picked priority/normal Minor change, requires approval from ≥1 primary reviewer status/LGT2 Two reviewers already commented LGTM, ready for merge type/enhancement Performance improvement or refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants