*: clean up the logging component #23534

SabaPing · 2021-03-25T06:51:58Z

What problem does this PR solve?

Please refer to the meta issue for more information. #24190

The primary goal is to clean up the logging component. TiDB depends on two different third part log components, which are logrus and zap. This creates unnecessary complexity. In addition, there are some other logging flaws in code.

What is changed and how it works?

What's Changed:

Migrate logrus dependency to pingcap/log, which is based on zap.
Remove invalid log format console.
Cleanup gRPC debug log.

Outside users should not perceive relevant changes.

Related changes

Need to cherry-pick to the release branch

Check List

Tests

Unit test
Integration test

Release note

No release note.

SabaPing · 2021-03-25T07:09:26Z

This is the first step for the task "Unify Log Framework".
After this pr being merged, I'll cleanup log code in pd, br and tidb-dashboard, one-by-one.
For more information, please read dev design here in Chinese.

zz-jason · 2021-03-27T10:49:46Z

@SabaPing thanks for your contribution, could you file a PR to discuss the proposal in the community?

Edit (2021/03/29): you can find the proposal template here: https://github.com/pingcap/tidb/blob/master/docs/design/TEMPLATE.md

xhebox · 2021-03-27T10:57:01Z

/cc @xhebox @AilinKid

SabaPing · 2021-03-29T02:16:51Z

@SabaPing thanks for your contribution, could you file a PR to discuss the proposal in the community?

Edit (2021/03/29): you can find the proposal template here: https://github.com/pingcap/tidb/blob/master/docs/design/TEMPLATE.md

Sure I'll finish the proposal in this week.

breezewish · 2021-04-02T01:29:49Z

@SabaPing thanks for your contribution, could you file a PR to discuss the proposal in the community?
Edit (2021/03/29): you can find the proposal template here: https://github.com/pingcap/tidb/blob/master/docs/design/TEMPLATE.md

Sure I'll finish the proposal in this week.

I think you can just post your doc in the sig channel. However remember to make it public. @SabaPing

crazycs520 · 2021-04-09T05:08:09Z

@AilinKid @tangenta PTAL

util/logutil/slow_query_logger.go

go.mod

crazycs520

/LGTM

xhebox

LGTM

xhebox · 2021-04-09T07:31:59Z

/merge

ti-chi-bot · 2021-04-09T07:32:01Z

@xhebox: /merge in this pull request requires 2 /lgtm.

In response to this:

/merge

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

AilinKid · 2021-04-09T10:27:07Z

/LGTM

ti-chi-bot · 2021-04-09T10:27:10Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

AilinKid
xhebox

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

xhebox · 2021-05-31T08:23:47Z

@SabaPing Rabase plz? I think we can merge the PR now.

# Conflicts: # tidb-server/main.go

SabaPing · 2021-05-31T09:25:39Z

@SabaPing Rabase plz? I think we can merge the PR now.

Done rebased

xhebox · 2021-05-31T15:21:49Z

/merge

ti-chi-bot · 2021-05-31T15:21:53Z

This pull request has been accepted and is ready to merge.

Commit hash: 7c7cde3

xhebox · 2021-06-01T02:34:10Z

/merge

breezewish · 2021-06-04T05:33:55Z

tidb-server/main.go

-	if len(os.Getenv("GRPC_DEBUG")) > 0 {
-		grpclog.SetLoggerV2(grpclog.NewLoggerV2WithVerbosity(os.Stderr, os.Stderr, os.Stderr, 999))
-	} else {
-		grpclog.SetLoggerV2(grpclog.NewLoggerV2(io.Discard, io.Discard, os.Stderr))


The bug comes from previously we just Discard these logs @SabaPing

tisonkun · 2021-06-12T01:35:10Z

util/logutil/log.go

-var SlowQueryZapLogger = zaplog.L()
-
-// InitLogger initializes PD's logger.
+// InitLogger delegates to InitZapLogger. Keeping it here for historical reason.
 func InitLogger(cfg *LogConfig) error {


PD still depend on this method, right?

If so, why not we retain InitLogger and drop InitZapLogger? An interface is better to be more abstract.

unify log framework

2375a94

ti-srebot added the first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. label Mar 25, 2021

ti-chi-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 25, 2021

fix typo in comments

9df32c5

github-actions bot added component/config sig/sql-infra SIG: SQL Infra labels Mar 25, 2021

ti-chi-bot requested review from AilinKid and xhebox March 27, 2021 10:57

breezewish requested a review from crazycs520 April 2, 2021 01:27

breezewish added sig/diagnosis SIG: Diagnosis and removed sig/sql-infra SIG: SQL Infra labels Apr 2, 2021

github-actions bot added the sig/sql-infra SIG: SQL Infra label Apr 2, 2021

crazycs520 reviewed Apr 9, 2021

View reviewed changes

util/logutil/slow_query_logger.go Outdated Show resolved Hide resolved

crazycs520 reviewed Apr 9, 2021

View reviewed changes

go.mod Show resolved Hide resolved

fix comments

154ed6f

crazycs520 reviewed Apr 9, 2021

View reviewed changes

xhebox approved these changes Apr 9, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 9, 2021

Merge branch 'master' into unified_logging_framework

866f1fc

ti-chi-bot removed the status/LGT1 Indicates that a PR has LGTM 1. label Apr 9, 2021

SabaPing mentioned this pull request Apr 21, 2021

Proposal: Unify Log Library for Golang Projects #24181

Merged

move proposal to meta issue

a4836af

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 21, 2021

Merge branch 'master' into unified_logging_framework

a995ee0

SabaPing mentioned this pull request Apr 21, 2021

Tracking issue for unifying log library #24190

Open

7 tasks

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 29, 2021

Merge remote-tracking branch 'ups/master' into unified_logging_framework

9eb7128

# Conflicts: # tidb-server/main.go

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 31, 2021

fix build

7c7cde3

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label May 31, 2021

Merge branch 'master' into unified_logging_framework

8d942f7

ti-chi-bot merged commit bbd5b32 into pingcap:master Jun 1, 2021

youjiali1995 mentioned this pull request Jun 3, 2021

Too many grpclogger's logs #25117

Closed

breezewish reviewed Jun 4, 2021

View reviewed changes

tisonkun reviewed Jun 12, 2021

View reviewed changes

SabaPing mentioned this pull request Jun 15, 2021

util/logutil: Remove useless grpc log in production #25454

Merged

This was referenced Aug 16, 2021

util/logutil: Remove useless grpc log in production (#25454) #27238

Merged

util/logutil: Remove useless grpc log in production (#25454) #27239

Merged

bb7133 mentioned this pull request Aug 23, 2021

unsupport log format: console #23344

Closed

qw4990 mentioned this pull request Aug 26, 2021

slow query file requires a maximum storage time and maximum file limit #25716

Closed

youjiali1995 mentioned this pull request Dec 1, 2021

TiDB doesn't log any slow log if log.level >= "error" #30309

Closed

SabaPing mentioned this pull request Dec 15, 2021

TiDB: remove invalid log format pingcap/docs-cn#7963

Merged

13 tasks

This was referenced Dec 15, 2021

TiDB: remove invalid log format (#7963) pingcap/docs-cn#7965

Merged

TiDB: remove invalid log format (#7963) pingcap/docs-cn#7966

Merged

TiDB: remove invalid log format (#7963) pingcap/docs-cn#7967

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: clean up the logging component #23534

*: clean up the logging component #23534

SabaPing commented Mar 25, 2021 •

edited

Loading

SabaPing commented Mar 25, 2021 •

edited

Loading

zz-jason commented Mar 27, 2021 •

edited

Loading

xhebox commented Mar 27, 2021

SabaPing commented Mar 29, 2021

breezewish commented Apr 2, 2021

crazycs520 commented Apr 9, 2021

crazycs520 left a comment

xhebox left a comment

xhebox commented Apr 9, 2021

ti-chi-bot commented Apr 9, 2021

AilinKid commented Apr 9, 2021

ti-chi-bot commented Apr 9, 2021

xhebox commented May 31, 2021

SabaPing commented May 31, 2021

xhebox commented May 31, 2021

ti-chi-bot commented May 31, 2021

xhebox commented Jun 1, 2021

breezewish Jun 4, 2021

tisonkun Jun 12, 2021

tisonkun Jun 12, 2021

*: clean up the logging component #23534

*: clean up the logging component #23534

Conversation

SabaPing commented Mar 25, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

SabaPing commented Mar 25, 2021 • edited Loading

zz-jason commented Mar 27, 2021 • edited Loading

xhebox commented Mar 27, 2021

SabaPing commented Mar 29, 2021

breezewish commented Apr 2, 2021

crazycs520 commented Apr 9, 2021

crazycs520 left a comment

Choose a reason for hiding this comment

xhebox left a comment

Choose a reason for hiding this comment

xhebox commented Apr 9, 2021

ti-chi-bot commented Apr 9, 2021

AilinKid commented Apr 9, 2021

ti-chi-bot commented Apr 9, 2021

xhebox commented May 31, 2021

SabaPing commented May 31, 2021

xhebox commented May 31, 2021

ti-chi-bot commented May 31, 2021

xhebox commented Jun 1, 2021

breezewish Jun 4, 2021

Choose a reason for hiding this comment

tisonkun Jun 12, 2021

Choose a reason for hiding this comment

tisonkun Jun 12, 2021

Choose a reason for hiding this comment

SabaPing commented Mar 25, 2021 •

edited

Loading

SabaPing commented Mar 25, 2021 •

edited

Loading

zz-jason commented Mar 27, 2021 •

edited

Loading