Skip to content

*: redact arguments for Error#20436

Merged
SunRunAway merged 19 commits intopingcap:masterfrom
SunRunAway:logredact2
Oct 21, 2020
Merged

*: redact arguments for Error#20436
SunRunAway merged 19 commits intopingcap:masterfrom
SunRunAway:logredact2

Conversation

@SunRunAway
Copy link
Contributor

@SunRunAway SunRunAway commented Oct 13, 2020 •
edited
Loading

What problem does this PR solve?

Issue Number: part of #18566

Problem Summary:

What is changed and how it works?

Proposal: xxx

What's Changed:

  • changes in "github.com/pingcap/errors" Redact arguments for Error errors#27
    • redact errors in Error.GenWithStackByArgs & Error.FastGenByArgs
  • changes in "github.com/pingcap/parser" *: redact arguments for Error parser#1051
    • define mysql.Message to bind redactArgs which means which arguments of the error need to be redacted with each error message
    • MySQLErrName should be defined with mysql.Message.
    • ErrClass.NewStd considers redactArgs
    • All errors uses ErrClass.NewStd or mysql.Message to consider error redaction
  • changes in "github.com/pingcap/tidb" *: redact arguments for Error #20436
    • inherit ErrClass.NewStd
    • All errors uses ErrClass.NewStd or mysql.Message to consider error redaction
      How it Works:

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn: To be added

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)
Case 1
mysql> create table t (a int, unique key (a));
Query OK, 0 rows affected (0.00 sec)

mysql> insert into t values (1),(1);
ERROR 1062 (23000): Duplicate entry '1' for key 'a'
mysql> set @@session.tidb_redact_log=1;
Query OK, 0 rows affected (0.00 sec)

mysql> insert into t values (1),(1);
ERROR 1062 (23000): Duplicate entry '?' for key '?'

log:

[2020/10/21 14:40:05.368 +08:00] [INFO] [tidb.go:222] ["rollbackTxn for ddl/autocommit failed"]
[2020/10/21 14:40:05.368 +08:00] [INFO] [conn.go:800] ["command dispatched failed"] [conn=1] [connInfo="id:1, addr:127.0.0.1:53367 status:10, collation:utf8_general_ci, user:root"] [command=Query] [status="inTxn:0, autocommit:1"] [sql="insert into t values (1),(1)"] [txn_mode=OPTIMISTIC] [err="[kv:1062]Duplicate entry '1' for key 'a'"]
[2020/10/21 14:40:14.104 +08:00] [INFO] [tidb.go:222] ["rollbackTxn for ddl/autocommit failed"]
[2020/10/21 14:40:14.104 +08:00] [INFO] [conn.go:800] ["command dispatched failed"] [conn=1] [connInfo="id:1, addr:127.0.0.1:53367 status:10, collation:utf8_general_ci, user:root"] [command=Query] [status="inTxn:0, autocommit:1"] [sql="insert into t values ( ? ) , ( ? )"] [txn_mode=OPTIMISTIC] [err="[kv:1062]Duplicate entry '?' for key '?'"]
Case 2

Client 1:

mysql> set @@global.tidb_redact_log=1;
Query OK, 0 rows affected (0.00 sec)

Then start client 2:

mysql> insert into t values (1),(1);
ERROR 1062 (23000): Duplicate entry '?' for key '?'

log:

[2020/10/21 14:41:20.513 +08:00] [INFO] [set.go:216] ["set global var"] [conn=3] [name=tidb_redact_log] [val=1]
[2020/10/21 14:41:23.552 +08:00] [INFO] [tidb.go:222] ["rollbackTxn for ddl/autocommit failed"]
[2020/10/21 14:41:23.552 +08:00] [INFO] [conn.go:800] ["command dispatched failed"] [conn=4] [connInfo="id:4, addr:127.0.0.1:53636 status:10, collation:utf8_general_ci, user:root"] [command=Query] [status="inTxn:0, autocommit:1"] [sql="insert into t values ( ? ) , ( ? )"] [txn_mode=OPTIMISTIC] [err="[kv:1062]Duplicate entry '?' for key '?'"]

Release note

  • Support variable tidb_redact_log to control showing the sensitive message in the log.

@sre-bot
Copy link
Contributor

sre-bot commented Oct 13, 2020

No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md

@SunRunAway SunRunAway requested a review from crazycs520 October 14, 2020 11:26
@sre-bot
Copy link
Contributor

sre-bot commented Oct 16, 2020

No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md

@sre-bot
Copy link
Contributor

sre-bot commented Oct 16, 2020

No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md

@SunRunAway SunRunAway requested a review from ichn-hu October 16, 2020 11:12
@sre-bot
Copy link
Contributor

sre-bot commented Oct 16, 2020

No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md

@sre-bot
Copy link
Contributor

sre-bot commented Oct 19, 2020

No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md

@sre-bot
Copy link
Contributor

sre-bot commented Oct 19, 2020

No release note, Please follow https://github.com/pingcap/community/blob/master/contributors/release-note-checker.md

@SunRunAway SunRunAway marked this pull request as ready for review October 19, 2020 10:53
@SunRunAway SunRunAway requested review from a team as code owners October 19, 2020 10:53
@SunRunAway SunRunAway requested review from XuHuaiyu and removed request for a team October 19, 2020 10:53
ichn-hu
ichn-hu previously approved these changes Oct 20, 2020
View reviewed changes
Copy link
Contributor

@ichn-hu ichn-hu left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Oct 20, 2020
ti-srebot
ti-srebot previously approved these changes Oct 20, 2020
View reviewed changes
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Oct 20, 2020
@SunRunAway SunRunAway dismissed stale reviews from ti-srebot and ichn-hu via 2b62917 October 21, 2020 04:06
@SunRunAway
Copy link
Contributor Author

SunRunAway commented Oct 21, 2020

/run-all-tests parser=pr/1051

1 similar comment
@bb7133
Copy link
Member

bb7133 commented Oct 21, 2020

/run-all-tests parser=pr/1051

@SunRunAway
Copy link
Contributor Author

SunRunAway commented Oct 21, 2020

/run-unit-test

@SunRunAway
Copy link
Contributor Author

SunRunAway commented Oct 21, 2020

/run-all-tests

@SunRunAway
Copy link
Contributor Author

SunRunAway commented Oct 21, 2020

/run-sqllogic-test-1

@SunRunAway SunRunAway merged commit 2f067c0 into pingcap:master Oct 21, 2020
@SunRunAway SunRunAway deleted the logredact2 branch October 21, 2020 06:44
@SunRunAway SunRunAway mentioned this pull request Oct 22, 2020
Closed
@lonng lonng mentioned this pull request Nov 10, 2020
Merged
@wshwsh12 wshwsh12 mentioned this pull request Nov 12, 2020
Merged
@lonng lonng mentioned this pull request Nov 13, 2020
Closed
ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Nov 17, 2020
@SunRunAway @ti-srebot
cherry pick pingcap#20436 to release-4.0
9333dcf 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Nov 17, 2020
Closed
@ti-srebot
Copy link
Contributor

ti-srebot commented Nov 17, 2020

cherry pick to release-4.0 in PR #21106

SunRunAway added a commit to SunRunAway/tidb that referenced this pull request Nov 24, 2020
@SunRunAway
*: redact arguments for Error (pingcap#20436)
c423159
@SunRunAway SunRunAway mentioned this pull request Nov 24, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crazycs520 crazycs520 crazycs520 approved these changes

@ichn-hu ichn-hu ichn-hu left review comments

@ti-srebot ti-srebot ti-srebot left review comments

@XuHuaiyu XuHuaiyu Awaiting requested review from XuHuaiyu

Assignees

No one assigned

Labels

component/config component/expression sig/execution SIG execution sig/sql-infra SIG: SQL Infra status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@SunRunAway @sre-bot @ichn-hu @bb7133 @ti-srebot @crazycs520 @lonng