logutil, *: implement log desensitization #3011

Yisaer · 2020-09-22T12:09:23Z

What problem does this PR solve?

close #2852

Implement log desensitization to omit the region key information from the log.

What is changed and how it works?

Check List

Tests

Unit test

Code changes

Has configuration change

Related changes

PR to update pingcap/docs/pingcap/docs-cn:

Release note

Support configuration enable-redact-log to enable log desensitization or not.

Signed-off-by: Song Gao <disxiaofei@163.com>

tools/pd-simulator/simulator/event.go

rleungx · 2020-09-23T03:17:31Z

BTW, I think there are more logs which need to be handled.

cmd/pd-server/main.go

disksing · 2020-09-23T03:07:46Z

server/config/config.go

@@ -144,6 +144,8 @@ type Config struct {
 	Dashboard DashboardConfig `toml:"dashboard" json:"dashboard"`

 	ReplicationMode ReplicationModeConfig `toml:"replication-mode" json:"replication-mode"`
+	// EnableRedactLog indicates that whether redact log, 0 is disable. 1 is enable.
+	EnableRedactLog int32 `toml:"enable-redact-log" json:"enable-redact-log"`


why not use boolean?

To keep the same behavior with tidb

https://github.com/pingcap/tidb/blob/fd0299f58ecc174e5ed8f91660299ebac8fa2b9c/config/config.go#L163-L164

They use int32 because they need to do atomic operations but we don't. I think we don't have to follow them.

pkg/logutil/log_test.go

pkg/logutil/log.go

Yisaer · 2020-09-23T03:19:59Z

BTW, I think there are more logs which need to be handled.

After discussion, only start-key and end-key of the region need to be redacted in the log. @rleungx

server/cluster/cluster.go

rleungx · 2020-09-23T03:25:42Z

BTW, I think there are more logs which need to be handled.

After discussion, only start-key and end-key of the region need to be redacted in the log. @rleungx

Does RegionToHexMeta need to be changed to ??

Signed-off-by: Song Gao <disxiaofei@163.com>

disksing · 2020-09-23T05:19:40Z

pkg/logutil/log.go

+}
+
+// RedactBytesArgIfNeeded receives []byte argument and return omitted information if redact log enabled
+func RedactBytesArgIfNeeded(arg []byte) []byte {


Looks verbose. How about just RedactBytes?

Signed-off-by: Song Gao <disxiaofei@163.com>

HunDunDM

How about using a function like func ZapRedact(key string, val interface{}) zap.Field?

pkg/logutil/log.go

Signed-off-by: Song Gao <disxiaofei@163.com>

Yisaer · 2020-09-24T06:09:13Z

How about using a function like func ZapRedact(key string, val interface{}) zap.Field?

good idea.

server/schedule/checker/merge_checker.go

Signed-off-by: Song Gao <disxiaofei@163.com>

HunDunDM

LGTM

Yisaer · 2020-09-24T12:22:50Z

/merge

ti-srebot · 2020-09-24T12:25:10Z

/run-all-tests

Yisaer · 2020-12-11T06:28:21Z

/cherry-picker

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot · 2020-12-11T06:28:41Z

cherry pick to release-4.0 in PR #3266

* cherry pick #3011 to release-4.0 Signed-off-by: ti-srebot <ti-srebot@pingcap.com> * fix conflict Signed-off-by: Song Gao <disxiaofei@163.com> * fix conflict Signed-off-by: Song Gao <disxiaofei@163.com> Co-authored-by: Song Gao <disxiaofei@163.com> Co-authored-by: Ti Prow Robot <71242396+ti-community-prow-bot@users.noreply.github.com>

Yisaer added 5 commits September 22, 2020 19:20

support log redact

02d0457

Signed-off-by: Song Gao <disxiaofei@163.com>

add test

8fc8bfc

Signed-off-by: Song Gao <disxiaofei@163.com>

add comment

7bd8a31

Signed-off-by: Song Gao <disxiaofei@163.com>

fix nil

a310cc8

Signed-off-by: Song Gao <disxiaofei@163.com>

fix test

74871f5

Signed-off-by: Song Gao <disxiaofei@163.com>

rleungx reviewed Sep 23, 2020

View reviewed changes

tools/pd-simulator/simulator/event.go Outdated Show resolved Hide resolved

disksing reviewed Sep 23, 2020

View reviewed changes

rleungx reviewed Sep 23, 2020

View reviewed changes

server/cluster/cluster.go Outdated Show resolved Hide resolved

nolouch added the component/log Log. label Sep 23, 2020

Yisaer added the component/security Security logic. label Sep 23, 2020

Yisaer added 2 commits September 23, 2020 12:54

address the comment

cda3343

Signed-off-by: Song Gao <disxiaofei@163.com>

address the comment

2b791e4

Signed-off-by: Song Gao <disxiaofei@163.com>

disksing reviewed Sep 23, 2020

View reviewed changes

Yisaer added 3 commits September 23, 2020 13:35

address the comment

e539e4b

Signed-off-by: Song Gao <disxiaofei@163.com>

address the comment

0b8675a

Signed-off-by: Song Gao <disxiaofei@163.com>

fix lint

5be08ce

Signed-off-by: Song Gao <disxiaofei@163.com>

disksing approved these changes Sep 23, 2020

View reviewed changes

ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 23, 2020

fix error

90fd966

Signed-off-by: Song Gao <disxiaofei@163.com>

HunDunDM reviewed Sep 24, 2020

View reviewed changes

pkg/logutil/log.go Outdated Show resolved Hide resolved

pkg/logutil/log.go Show resolved Hide resolved

address the comment

ab179da

Signed-off-by: Song Gao <disxiaofei@163.com>

Yisaer requested review from rleungx and HunDunDM September 24, 2020 06:10

HunDunDM reviewed Sep 24, 2020

View reviewed changes

server/schedule/checker/merge_checker.go Outdated Show resolved Hide resolved

address the comment

cc492b4

Signed-off-by: Song Gao <disxiaofei@163.com>

address the comment

e8b34c4

Signed-off-by: Song Gao <disxiaofei@163.com>

HunDunDM approved these changes Sep 24, 2020

View reviewed changes

ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Sep 24, 2020

ti-srebot approved these changes Sep 24, 2020

View reviewed changes

ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Sep 24, 2020

ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 24, 2020

Merge branch 'master' into implement_log_den

e8670f4

ti-srebot merged commit 941b5fe into tikv:master Sep 24, 2020

Yisaer added the needs-cherry-pick-release-4.0 The PR needs to cherry pick to release-4.0 branch. label Dec 11, 2020

ti-srebot pushed a commit to ti-srebot/pd that referenced this pull request Dec 11, 2020

cherry pick tikv#3011 to release-4.0

670d2a5

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot mentioned this pull request Dec 11, 2020

logutil, *: implement log desensitization (#3011) #3266

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

logutil, *: implement log desensitization #3011

logutil, *: implement log desensitization #3011

Yisaer commented Sep 22, 2020

rleungx commented Sep 23, 2020

disksing Sep 23, 2020

Yisaer Sep 23, 2020

disksing Sep 23, 2020

Yisaer commented Sep 23, 2020

rleungx commented Sep 23, 2020

disksing Sep 23, 2020

Yisaer Sep 23, 2020

HunDunDM left a comment

Yisaer commented Sep 24, 2020

HunDunDM left a comment

Yisaer commented Sep 24, 2020

ti-srebot commented Sep 24, 2020

Yisaer commented Dec 11, 2020

ti-srebot commented Dec 11, 2020

logutil, *: implement log desensitization #3011

logutil, *: implement log desensitization #3011

Conversation

Yisaer commented Sep 22, 2020

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

rleungx commented Sep 23, 2020

disksing Sep 23, 2020

Choose a reason for hiding this comment

Yisaer Sep 23, 2020

Choose a reason for hiding this comment

disksing Sep 23, 2020

Choose a reason for hiding this comment

Yisaer commented Sep 23, 2020

rleungx commented Sep 23, 2020

disksing Sep 23, 2020

Choose a reason for hiding this comment

Yisaer Sep 23, 2020

Choose a reason for hiding this comment

HunDunDM left a comment

Choose a reason for hiding this comment

Yisaer commented Sep 24, 2020

HunDunDM left a comment

Choose a reason for hiding this comment

Yisaer commented Sep 24, 2020

ti-srebot commented Sep 24, 2020

Yisaer commented Dec 11, 2020

ti-srebot commented Dec 11, 2020