-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
server: fix over redacted log lines #137915
base: master
Are you sure you want to change the base?
server: fix over redacted log lines #137915
Conversation
Previosuly, few log lines were getting redacted assuming that it contains sensitive information. Support team was facing challenges during dignostics. To address this, this patch address few of the log lines which were overly redacted. Epic: CRDB-37533 Part of: CRDB-44885 Release note: None
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @aa-joshi)
pkg/server/migration.go
line 147 at r1 (raw file):
} log.Infof(ctx, "active cluster version setting is now %s (up from %s)", redact.Safe(newCV.PrettyPrint()), redact.Safe(prevCV.PrettyPrint()))
I think we can directly pass newCV
and prevCV
without Safe
or PrettyPrint
. Because the underlying type implements SafeFormatter
It was getting redacted because PrettyPrint()
returns string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @arjunmahishi)
pkg/server/migration.go
line 147 at r1 (raw file):
Previously, arjunmahishi (Arjun Mahishi) wrote…
I think we can directly pass
newCV
andprevCV
withoutSafe
orPrettyPrint
. Because the underlying type implementsSafeFormatter
It was getting redacted because
PrettyPrint()
returnsstring
I believe we need to wrap in PrettyPrint because it has formatting logic based on v.IsFence()
ref:
cockroach/pkg/roachpb/version.go
Line 115 in ac3c5f2
func (v Version) PrettyPrint() string { |
Previously, aa-joshi (Akshay Joshi) wrote…
Then I think we can change PrettyPrint and make it return |
Previosuly, few log lines were getting redacted assuming that it contains sensitive information. Support team was facing challenges during dignostics. To address this, this patch address few of the log lines which were overly redacted.
Epic: CRDB-37533
Part of: CRDB-44885
Release note: None