Skip to content

Commit

Permalink
util/logs: request redactable logs by default
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
knz committed Aug 24, 2020
1 parent 120169c commit 25b99aa
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 2 deletions.
4 changes: 2 additions & 2 deletions pkg/cli/interactive_tests/test_missing_log_output.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ eexpect ":/# "
# is only reported on the logger which is writing first after stderr
# is has been broken, and that may be the secondary logger.
send "tail -F `find logs/db/logs -type l`\r"
eexpect "error: write /dev/stderr: broken pipe"
eexpect "error: write */dev/stderr*: broken pipe"
interrupt
eexpect ":/# "
end_test
Expand Down Expand Up @@ -89,7 +89,7 @@ eexpect "CockroachDB node starting"
system "($argv sql --insecure -e \"select crdb_internal.force_panic('helloworld')\" || true)&"
# Check the panic is reported on the server's stderr
eexpect "a SQL panic has occurred"
eexpect "panic: helloworld"
eexpect "panic: *helloworld"
eexpect "stack trace"
eexpect ":/# "
# Check the panic is reported on the server log file
Expand Down
17 changes: 17 additions & 0 deletions pkg/cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/server"
"github.com/cockroachdb/cockroach/pkg/server/status"
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/storage"
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
Expand Down Expand Up @@ -1044,6 +1045,15 @@ func setupAndInitializeLoggingAndProfiling(
return nil, err
}

// Enable redactable logs if no other instruction given by the
// user. We do this only if we have a logging directory, because
// it's not safe to enable redaction markers on stderr.
if rl := fl.Lookup(logflags.RedactableLogsName); !rl.Changed {
if err := rl.Value.Set("true"); err != nil {
return nil, err
}
}

// Start the log file GC daemon to remove files that make the log
// directory too large.
log.StartGCDaemon(ctx)
Expand All @@ -1067,6 +1077,13 @@ func setupAndInitializeLoggingAndProfiling(
return nil, err
}

// Record redaction usage for telemetry.
if log.RedactableLogsEnabled() {
telemetry.Count("server.logging.redactable_logs.enabled")
} else {
telemetry.Count("server.logging.redactable_logs.disabled")
}

// We want to be careful to still produce useful debug dumps if the
// server configuration has disabled logging to files.
outputDirectory := "."
Expand Down
9 changes: 9 additions & 0 deletions pkg/util/log/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,12 @@ func TestingResetActive() {
defer logging.mu.Unlock()
logging.mu.active = false
}

// RedactableLogsEnabled reports whether redaction markers were
// actually enabled for the main logger. This is used for flag
// telemetry; a better API would be needed for more fine grained
// information, as each different logger may have a different
// redaction setting.
func RedactableLogsEnabled() bool {
return mainLog.redactableLogs.Get()
}

0 comments on commit 25b99aa

Please sign in to comment.