pkg/util/log: selectively apply file-defaults.format-options to stderr

Fixes: #113321

The stderr sink in the log config is only allowed to use the
`crdb-v2-tty` format, and therefore, only `format-options` supported by
the `crdb-v2-tty` format should be applied to the stderr sink.

Unfortunately, a bug in the log config parse & validation code didn't
follow this rule. The configuration that's part of the `file-defaults`
is propagated to the stderr sink, and this included the
`format-options`, even if they were only relevant to another log format
type (e.g. `json`). This caused an error when trying to apply the
options to the stderr log sink on startup, e.g.:
```
ERROR: unknown format option: "datetime-format"
```

To solve this problem, we should only propagate the `format-options`
used in `file-defaults` to the stderr sink's config IFF the
`file-defaults` format is of a `crdb-v2` variety. Since the stderr sink
also uses the `crdb-v2-tty` format, we can only be sure that the
`format-options` used in `file-defaults` is supported by the stderr sink
if the `format` used in `file-defaults` is also part of `crdb-v2`.

However, if `format-options` is explicitly defined within the
`sinks.stderr` config, we need to be careful not to overwrite them with
those defined in `file-defaults`.

This patch accomplishes fixes for all these issues, and adds new tests
to cover all these scenarios.

Release note (bug fix): A bug in the log config code prevented users
from setting the `datetime-format` and `datetime-timezone` log format
options (set via the `format-options` structure) within their log
config. Specifically, when users tried to use these options in
`file-defaults` with any `json` type log format, the log config was
previously unable to be parsed due to validation errors.

This was because the `file-defaults.format-options` were propagated to
the `sinks.stderr.format-options`. `sinks.stderr` only supports a format
of `crdb-v2-tty`. Therefore, the incorrectly propagated
`format-options`, which are only supported by the `json` log format,
were identified as not being supported when validating `sinks.stderr`.

With this patch, the `file-defaults.format-options` are only propagated
to `sinks.stderr.format-options` if both of these conditions are true:
1. `file-defaults.format` is one of `crdb-v2` or `crdb-v2-tty`.
2. `sinks.stderr.format-options` are not explicitly set in the log
   config.

pkg/cli/interactive_tests/test_log_flags.tcl

@@ -77,7 +77,6 @@ end_test
 
 start_test "Check that the log flag is properly recognized for non-server commands"
 send "$argv debug reset-quorum 123 --log='sinks: {stderr: {format: json }}'\r"
-eexpect "\"severity\":\"ERROR\""
 eexpect "connection to server failed"
 eexpect ":/# "
 end_test

pkg/util/log/logconfig/config.go

@@ -29,8 +29,10 @@ import (
 // specified in a configuration.
 const DefaultFileFormat = `crdb-v2`
 
-// DefaultStderrFormat is the entry format for stderr sinks
-// when not specified in a configuration.
+// DefaultStderrFormat is the entry format for stderr sinks.
+// NB: The format for stderr is always set to `crdb-v2-tty`,
+// and cannot be changed. We enforce this in the validation step.
+// See: https://www.cockroachlabs.com/docs/stable/configure-logs#output-to-stderr
 const DefaultStderrFormat = `crdb-v2-tty`
 
 // DefaultFluentFormat is the entry format for fluent sinks

pkg/util/log/logconfig/testdata/validate

@@ -254,6 +254,93 @@ capture-stray-errors:
   dir: /default-dir
   max-group-size: 100MiB
 
+# Check that file-defaults format options are transferred to stderr if using a crdb-v2 format.
+yaml
+file-defaults:
+  format: crdb-v2
+  format-options: {timezone: america/new_york}
+----
+sinks:
+  file-groups:
+    default:
+      channels: {INFO: all}
+      filter: INFO
+      format-options:
+        timezone: america/new_york
+  stderr:
+    filter: NONE
+    format-options:
+      timezone: america/new_york
+capture-stray-errors:
+  enable: true
+  dir: /default-dir
+  max-group-size: 100MiB
+
+# Check that file-defaults format options are NOT transferred to stderr if NOT using a crdb-v2 format.
+yaml
+file-defaults:
+  format: json
+  format-options: {datetime-format: rfc3339, datetime-timezone: America/New_York}
+----
+sinks:
+  file-groups:
+    default:
+      channels: {INFO: all}
+      filter: INFO
+      format: json
+      format-options:
+        datetime-format: rfc3339
+        datetime-timezone: America/New_York
+  stderr:
+    filter: NONE
+capture-stray-errors:
+  enable: true
+  dir: /default-dir
+  max-group-size: 100MiB
+
+# Check that file-defaults format options do NOT overwrite format-options if explicitly defined in stderr.
+yaml
+file-defaults:
+  format: crdb-v2
+  format-options: {timezone: america/new_york}
+sinks:
+  stderr:
+    format-options: {timezone: america/chicago}
+----
+sinks:
+  file-groups:
+    default:
+      channels: {INFO: all}
+      filter: INFO
+      format-options:
+        timezone: america/new_york
+  stderr:
+    filter: NONE
+    format-options:
+      timezone: america/chicago
+capture-stray-errors:
+  enable: true
+  dir: /default-dir
+  max-group-size: 100MiB
+
+# Check that stderr always uses crdb-v2-tty format, even if we try to set it to an invalid format
+yaml
+sinks:
+  stderr:
+    format: crdb-v1-tty
+----
+sinks:
+  file-groups:
+    default:
+      channels: {INFO: all}
+      filter: INFO
+  stderr:
+    filter: NONE
+capture-stray-errors:
+  enable: true
+  dir: /default-dir
+  max-group-size: 100MiB
+
 # Check that NONE filter elides files.
 yaml
 file-defaults: {filter: NONE}

pkg/util/log/logconfig/validate.go

@@ -170,11 +170,28 @@ func (c *Config) Validate(defaultLogDir *string) (resErr error) {
 	if c.Sinks.Stderr.Filter == logpb.Severity_UNKNOWN {
 		c.Sinks.Stderr.Filter = logpb.Severity_NONE
 	}
+	// We need to know if format-options were specifically defined on the stderr sink later on,
+	// since this information is lost once propagateCommonDefaults is called.
+	stdErrFormatOptionsOriginallySet := len(c.Sinks.Stderr.FormatOptions) > 0
 	propagateCommonDefaults(&c.Sinks.Stderr.CommonSinkConfig, c.FileDefaults.CommonSinkConfig)
 	if c.Sinks.Stderr.Auditable != nil && *c.Sinks.Stderr.Auditable {
 		c.Sinks.Stderr.Criticality = &bt
 	}
 	c.Sinks.Stderr.Auditable = nil
+	// The format parameter for stderr is set to `crdb-v2-tty` and cannot be changed.
+	// See docs: https://www.cockroachlabs.com/docs/stable/configure-logs#output-to-stderr
+	if *c.Sinks.Stderr.Format != DefaultStderrFormat {
+		f := DefaultStderrFormat
+		c.Sinks.Stderr.Format = &f
+	}
+	// FormatOptions are format-specific. We should only copy them over to StdErr from
+	// FileDefaults if FileDefaults is also making use of a crdb-v2 format. Otherwise,
+	// we are likely to error when trying to apply an unsupported format option.
+	if c.FileDefaults.CommonSinkConfig.Format != nil &&
+		!strings.Contains(*c.FileDefaults.CommonSinkConfig.Format, "v2") &&
+		!stdErrFormatOptionsOriginallySet {
+		c.Sinks.Stderr.CommonSinkConfig.FormatOptions = map[string]string{}
+	}
 	if err := c.ValidateCommonSinkConfig(c.Sinks.Stderr.CommonSinkConfig); err != nil {
 		fmt.Fprintf(&errBuf, "stderr sink: %v\n", err)
 	}