Skip to content
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

util/logs: request redactable logs by default #53263

Merged
merged 3 commits into from
Aug 24, 2020

Conversation

knz
Copy link
Contributor

@knz knz commented Aug 22, 2020

First two commits from #53313

This changes the default value for the --redactable-logs flag to
true. This makes server produce redactable logs by default.

Note that --redactable-logs only triggers redactable logs if the
caller calls SetupRedactionAndStderrRedirects(). This is done by
e.g. the server code. This means that even with this patch,
unit tests continue to produce non-redactable logs.

This commit also adds telemetry for the enablement of redactable
markers, as server.logging.redactable_logs.{enabled,disabled}.

Fixes #51834.

Release note (cli change): The cockroach start, start-single-node
and demo command now enable --redactable-logs by default.
This causes log files to become redactable, so that
cockroach debug merge-log --redact or cockroach debug zip --redact
can remove sensitive information out of log files.
(Reminder: cockroach debug zip --redact only affects log files;
other items collected by the command can still contain sensitive information)

@knz knz requested review from bdarnell and tbg August 22, 2020 08:52
@knz knz requested a review from a team as a code owner August 22, 2020 08:52
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz knz force-pushed the 20200822-redactable-on-default branch 2 times, most recently from 56d00b1 to db0e815 Compare August 24, 2020 10:29
@knz knz requested review from irfansharif and removed request for bdarnell August 24, 2020 10:29
knz added 3 commits August 24, 2020 12:43
Release note: None
The demo code was trying a complicated dance with the flag structs to
disable logging. This was not necessary - a simple adjustment of the
advertised store config is sufficient to reach the desired effect.

Additionally, this commit provides a hint to the user if they want
to enable redaction markers but there is no logging dir yet.

Release note: None
This changes the default value for the `--redactable-logs` flag to
`true`. This makes server produce redactable logs by default.

Note that `--redactable-logs` only triggers redactable logs if the
caller calls `SetupRedactionAndStderrRedirects()`. This is done by
e.g. the server code. This means that even with this patch,
*unit tests* continue to produce non-redactable logs.

This commit also adds telemetry for the enablement of redactable
markers, as `server.logging.redactable_logs.{enabled,disabled}`.

Release note (cli change): The `cockroach start` and `start-single-node`
commands now enable `--redactable-logs` by default. It is also
enabled by default in `cockroach demo` if `--log-dir` is passed.
This causes log files to become redactable, so that `cockroach debug
merge-log --redact` or `cockroach debug zip --redact` can remove
sensitive information out of log files. (Reminder: `cockroach debug
zip --redact` only affects *log files*; other items collected by the
command can still contain sensitive information)
@knz knz force-pushed the 20200822-redactable-on-default branch from db0e815 to 25b99aa Compare August 24, 2020 10:49
Copy link
Contributor

@irfansharif irfansharif left a comment

Choose a reason for hiding this comment

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

Last commit LGTM.

@knz
Copy link
Contributor Author

knz commented Aug 24, 2020

thanks!

The prefix two commits have been approved separately by Rohan.

bors r=irfansharif

@craig
Copy link
Contributor

craig bot commented Aug 24, 2020

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 24, 2020

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Aug 24, 2020

Build succeeded:

@craig craig bot merged commit 620f0a4 into cockroachdb:master Aug 24, 2020
@knz knz deleted the 20200822-redactable-on-default branch August 25, 2020 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

log: instrument telemetry when PII redaction is enabled
3 participants