cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
29.96k stars 3.79k forks source link

roachtest: consolidate logging #131412

Open herkolategan opened 2 days ago

herkolategan commented 2 days ago

There are currently several instances in roachtest where logging is not standardised. The CRDB logger github.com/cockroachdb/cockroach/pkg/util/log ends up being used in a few places [1].

An effort was made in the past to isolate these logs to a file [2] to avoid cluttering stderr on CI. However, this also had the unintended side effect of writing possibly sensitive information, such as the roachtest program arguments, directly to the log file.

In essence this issue should remove all explicit references to the CRDB log, and instead use the appropriate test logger. Unfortunately it is difficult to completely remove all calls to the CRDB log, since both roachtest and cockroach uses a shared set of testutils. Many of the testutils make CRDB log calls [3]. This leaves us with either enforcing no use of the CRDB log in testutils or no use of testutils in roachtest. Alternatively we could redirect logs from the CRDB log to the correct roachtest test log. This will also alleviate logs from 3rd party libraries that end up redirecting through the CRDB log, which automatically sets up a redirect on init [4].

[1] https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/roachtest/tests/restore.go#L1153 [2] https://github.com/cockroachdb/cockroach/pull/110523 [3] https://github.com/cockroachdb/cockroach/blob/master/pkg/testutils/soon.go#L78 [4] https://github.com/cockroachdb/cockroach/blob/master/pkg/util/log/log_bridge.go#L62

Jira issue: CRDB-42538

blathers-crl[bot] commented 2 days ago

cc @cockroachdb/test-eng