Skip to content

Commit

Permalink
Merge #131529
Browse files Browse the repository at this point in the history
131529: util/log/logconfig: allow `format` for stderr log sink r=xinhaoz a=xinhaoz

Previously in #113532 we fixed the stderr sink log format to `crdb-v2-tty`. This was deemed to be a regression, as the stderr sink could previously be set to any available format. This commit reverts the restriction.

The following are still true after this commit:
- `format` in `file-defaults` is not propagated to `format` of the stderr sink. To set `stderr` format, one must explicitly define it in the `stderr` sink section of the log config.
- `format-options` in `file-defaults` are only applied to stderr if the format of the stderr sink and that in file-defaults are compatible.

Epic: none
Fixes: #128665

Release note (ops change): Users may set the log format for the stderr sink by setting the `format` field in the logging config under the `stderr` sink section.

Co-authored-by: Xin Hao Zhang <[email protected]>
  • Loading branch information
craig[bot] and xinhaoz committed Sep 27, 2024
2 parents f395f6a + ccd3609 commit 946ebda
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 14 deletions.
3 changes: 0 additions & 3 deletions pkg/util/log/logconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ import (
const DefaultFileFormat = `crdb-v2`

// 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
Expand Down
38 changes: 35 additions & 3 deletions pkg/util/log/logconfig/testdata/validate
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ 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.
# Check that file-defaults format options are transferred to stderr if using the same format (crdb-v2).
yaml
file-defaults:
format: crdb-v2
Expand All @@ -276,7 +276,38 @@ capture-stray-errors:
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.
# Check that file-defaults format options are transferred to stderr if stderr is using the same format (json).
yaml
file-defaults:
format: json
format-options: {datetime-format: rfc3339, datetime-timezone: America/New_York}
sinks:
stderr:
format: json
----
sinks:
file-groups:
default:
channels: {INFO: all}
filter: INFO
format: json
format-options:
datetime-format: rfc3339
datetime-timezone: America/New_York
stderr:
filter: NONE
format: json
format-options:
datetime-format: rfc3339
datetime-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 stderr is NOT using the same format
# as file-defaults.
# Note that here, we are using the default stderr format crdb-v2-tty.
yaml
file-defaults:
format: json
Expand Down Expand Up @@ -323,7 +354,7 @@ capture-stray-errors:
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
# Check that stderr can accept formats other than crdb-v2-tty.
yaml
sinks:
stderr:
Expand All @@ -336,6 +367,7 @@ sinks:
filter: INFO
stderr:
filter: NONE
format: crdb-v1-tty
capture-stray-errors:
enable: true
dir: /default-dir
Expand Down
22 changes: 14 additions & 8 deletions pkg/util/log/logconfig/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,17 +177,12 @@ func (c *Config) Validate(defaultLogDir *string) (resErr error) {
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,
// FileDefaults if FileDefaults specifies the same format as the stderr sink. 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") &&
!canShareFormatOptions(*c.FileDefaults.CommonSinkConfig.Format, *c.Sinks.Stderr.Format) &&
!stdErrFormatOptionsOriginallySet {
c.Sinks.Stderr.CommonSinkConfig.FormatOptions = map[string]string{}
}
Expand Down Expand Up @@ -538,3 +533,14 @@ func propagateDefaults(target, source interface{}) {
}
}
}

// canShareFormatOptions returns true if f1 and f2 can share format options.
// See log.FormatParsers for full list of supported formats.
// Examples:
//
// canShareFormatOptions("json", "json") => true
// canShareFormatOptions("crdb-v2", "crdb-v2-tty") => true
// canShareFormatOptions("crdb-v2", "json") => false
func canShareFormatOptions(f1, f2 string) bool {
return strings.HasPrefix(f1, f2) || strings.HasPrefix(f2, f1)
}

0 comments on commit 946ebda

Please sign in to comment.