roachprod: redirect CRDB logs utility

Currently, CRDB log is configured, by roachtest, to log to a file to catch any
logs written to it during a roachtest run. This is usually from a shared test
util that uses the CRDB log. The file sink on the CRDB logger will log program
arguments by default, but this can leak sensitive information.

This PR introduces a log redirect that uses the CRDB log interceptor
functionality instead of using a file sink. This way we can avoid logging the
program arguments.

Epic: None
Release note: None

pkg/BUILD.bazel

@@ -310,6 +310,7 @@ ALL_TESTS = [
     "//pkg/roachprod/cloud:cloud_test",
     "//pkg/roachprod/config:config_test",
     "//pkg/roachprod/install:install_test",
+    "//pkg/roachprod/logger:logger_test",
     "//pkg/roachprod/opentelemetry:opentelemetry_test",
     "//pkg/roachprod/prometheus:prometheus_test",
     "//pkg/roachprod/promhelperclient:promhelperclient_test",
@@ -1634,6 +1635,7 @@ GO_TARGETS = [
     "//pkg/roachprod/install:install_test",
     "//pkg/roachprod/lock:lock",
     "//pkg/roachprod/logger:logger",
+    "//pkg/roachprod/logger:logger_test",
     "//pkg/roachprod/opentelemetry:opentelemetry",
     "//pkg/roachprod/opentelemetry:opentelemetry_test",
     "//pkg/roachprod/prometheus:prometheus",

pkg/roachprod/logger/BUILD.bazel

@@ -1,12 +1,27 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "logger",
-    srcs = ["log.go"],
+    srcs = [
+        "log.go",
+        "log_redirect.go",
+    ],
     importpath = "github.com/cockroachdb/cockroach/pkg/roachprod/logger",
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/util/log",
+        "//pkg/util/log/logconfig",
+        "//pkg/util/log/logpb",
         "//pkg/util/syncutil",
     ],
 )
+
+go_test(
+    name = "logger_test",
+    srcs = ["log_redirect_test.go"],
+    embed = [":logger"],
+    deps = [
+        "//pkg/util/log",
+        "@com_github_stretchr_testify//require",
+    ],
+)

pkg/roachprod/logger/log_redirect.go

@@ -0,0 +1,93 @@
+// Copyright 2024 The Cockroach Authors.
+//
+// Use of this software is governed by the CockroachDB Software License
+// included in the /LICENSE file.
+
+package logger
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"io"
+
+	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logconfig"
+	"github.com/cockroachdb/cockroach/pkg/util/log/logpb"
+	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
+)
+
+type logRedirect struct {
+	syncutil.Mutex
+	logger          *Logger
+	cancelIntercept func()
+	configured      bool
+}
+
+var logRedirectInst = &logRedirect{}
+
+// InitCRDBLogConfig sets up an interceptor for the CockroachDB log in order to
+// redirect logs to a roachprod logger. This is necessary as the CockroachDB log
+// is used in some test utilities shared between roachtest and CockroachDB.
+// Generally, CockroachDB logs should not be used explicitly in roachtest.
+func InitCRDBLogConfig(logger *Logger) {
+	logRedirectInst.Lock()
+	defer logRedirectInst.Unlock()
+	if logRedirectInst.configured {
+		panic("internal error: CRDB log interceptor already configured")
+	}
+	if logger == nil {
+		panic("internal error: specified logger is nil")
+	}
+	logConf := logconfig.DefaultStderrConfig()
+	logConf.Sinks.Stderr.Filter = logpb.Severity_FATAL
+	// Disable logging to a file. File sinks write the application arguments to
+	// the log by default (see: log_entry.go), and it is best to avoid logging
+	// the roachtest arguments as it may contain sensitive information.
+	if err := logConf.Validate(nil); err != nil {
+		panic(fmt.Errorf("internal error: could not validate CRDB log config: %w", err))
+	}
+	if _, err := log.ApplyConfig(logConf, nil /* fileSinkMetricsForDir */, nil /* fatalOnLogStall */); err != nil {
+		panic(fmt.Errorf("internal error: could not apply CRDB log config: %w", err))
+	}
+	logRedirectInst.logger = logger
+	logRedirectInst.cancelIntercept = log.InterceptWith(context.Background(), logRedirectInst)
+	logRedirectInst.configured = true
+}
+
+// TestingCRDBLogConfig is meant to be used in unit tests to reset the CRDB log,
+// it's interceptor, and the redirect log config. This is necessary to avoid
+// leaking state between tests, that test the logging.
+func TestingCRDBLogConfig(logger *Logger) {
+	logRedirectInst.Lock()
+	defer logRedirectInst.Unlock()
+	if logRedirectInst.cancelIntercept != nil {
+		logRedirectInst.cancelIntercept()
+	}
+	log.TestingResetActive()
+	logRedirectInst = &logRedirect{}
+	InitCRDBLogConfig(logger)
+}
+
+// Intercept intercepts CockroachDB log entries and redirects it to the
+// appropriate roachtest test logger or stderr.
+func (i *logRedirect) Intercept(logData []byte) {
+	var entry logpb.Entry
+	if err := json.Unmarshal(logData, &entry); err != nil {
+		i.logger.Errorf("failed to unmarshal intercepted log entry: %v", err)
+	}
+	l := i.logger
+	if l != nil && !l.Closed() {
+		if entry.Severity == logpb.Severity_ERROR || entry.Severity == logpb.Severity_FATAL {
+			i.writeLog(l.Stderr, entry)
+			return
+		}
+		i.writeLog(l.Stdout, entry)
+	}
+}
+
+func (i *logRedirect) writeLog(dst io.Writer, entry logpb.Entry) {
+	if err := log.FormatLegacyEntry(entry, dst); err != nil {
+		i.logger.Errorf("could not format and write CRDB log entry: %v", err)
+	}
+}

pkg/roachprod/logger/log_redirect_test.go

@@ -0,0 +1,59 @@
+// Copyright 2024 The Cockroach Authors.
+//
+// Use of this software is governed by the CockroachDB Software License
+// included in the /LICENSE file.
+
+package logger
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/stretchr/testify/require"
+)
+
+type mockLogger struct {
+	logger *Logger
+	writer *mockWriter
+}
+
+type mockWriter struct {
+	lines []string
+}
+
+func (w *mockWriter) Write(p []byte) (n int, err error) {
+	w.lines = append(w.lines, string(p))
+	return len(p), nil
+}
+
+func newMockLogger(t *testing.T) *mockLogger {
+	writer := &mockWriter{}
+	logConf := Config{Stdout: writer, Stderr: writer}
+	l, err := logConf.NewLogger("" /* path */)
+	require.NoError(t, err)
+	return &mockLogger{logger: l, writer: writer}
+}
+
+func requireLine(t *testing.T, l *mockLogger, line string) {
+	t.Helper()
+	found := false
+	for _, logLine := range l.writer.lines {
+		if strings.Contains(logLine, line) {
+			found = true
+			break
+		}
+	}
+	require.True(t, found, "expected line not found: %s", line)
+}
+
+func TestLogRedirect(t *testing.T) {
+	l := newMockLogger(t)
+	TestingCRDBLogConfig(l.logger)
+	ctx := context.Background()
+
+	log.Infof(ctx, "[simple test]")
+	requireLine(t, l, "[simple test]")
+	require.Equal(t, 1, len(l.writer.lines))
+}