diff --git a/klog.go b/klog.go index 04895647..f292496a 100644 --- a/klog.go +++ b/klog.go @@ -62,8 +62,11 @@ // (legacy behavior). To filter logs by severity when // -logtostderr=true, set -legacy_stderr_threshold_behavior=false // and use -stderrthreshold. -// With -legacy_stderr_threshold_behavior=true, -// -stderrthreshold has no effect. +// With -legacy_stderr_threshold_behavior=true (the current default), +// -stderrthreshold has no effect. A deprecation warning is printed +// if -stderrthreshold is set while legacy mode is active. +// The default for -legacy_stderr_threshold_behavior will change +// to false in a future release. // // The following flags always have no effect: // -alsologtostderr, -alsologtostderrthreshold, and -log_dir. @@ -77,8 +80,11 @@ // -stderrthreshold=ERROR // Log events at or above this severity are logged to standard // error as well as to files. When -logtostderr=true, this flag -// has no effect unless -legacy_stderr_threshold_behavior=false. +// is currently ignored unless -legacy_stderr_threshold_behavior=false. +// A deprecation warning is printed when this flag is set while +// legacy mode is active. The default will change in a future release. // -legacy_stderr_threshold_behavior=true +// Deprecated: the default will change to false in a future release. // If true, -stderrthreshold is ignored when -logtostderr=true // (legacy behavior). If false, -stderrthreshold is honored even // when -logtostderr=true, allowing severity-based filtering. @@ -175,6 +181,23 @@ func (s *severityValue) Set(value string) error { return nil } +// stderrThresholdValue wraps severityValue and records whether the flag +// was explicitly set by the user. +type stderrThresholdValue struct { + severityValue + explicitlySet *bool +} + +func (s *stderrThresholdValue) Set(value string) error { + if err := s.severityValue.Set(value); err != nil { + return err + } + if s.explicitlySet != nil { + *s.explicitlySet = true + } + return nil +} + // OutputStats tracks the number of output lines and bytes written. type OutputStats struct { lines int64 @@ -425,8 +448,11 @@ var commandLine flag.FlagSet // init sets up the defaults and creates command line flags. func init() { // Initialize severity thresholds - logging.stderrThreshold = severityValue{ - Severity: severity.ErrorLog, // Default stderrThreshold is ERROR. + logging.stderrThreshold = stderrThresholdValue{ + severityValue: severityValue{ + Severity: severity.ErrorLog, // Default stderrThreshold is ERROR. + }, + explicitlySet: &logging.stderrThresholdExplicitlySet, } logging.alsologtostderrthreshold = severityValue{ Severity: severity.InfoLog, // Default alsologtostderrthreshold is INFO (to maintain backward compatibility). @@ -440,13 +466,13 @@ func init() { "If the value is 0, the maximum file size is unlimited.") commandLine.BoolVar(&logging.toStderr, "logtostderr", true, "log to standard error instead of files") commandLine.BoolVar(&logging.alsoToStderr, "alsologtostderr", false, "log to standard error as well as files (no effect when -logtostderr=true)") - commandLine.BoolVar(&logging.legacyStderrThresholdBehavior, "legacy_stderr_threshold_behavior", true, "If true, stderrthreshold is ignored when logtostderr=true (legacy behavior). If false, stderrthreshold is honored even when logtostderr=true") + commandLine.BoolVar(&logging.legacyStderrThresholdBehavior, "legacy_stderr_threshold_behavior", true, "If true, stderrthreshold is ignored when logtostderr=true (legacy behavior, deprecated — will default to false in a future release). If false, stderrthreshold is honored even when logtostderr=true") commandLine.Var(&logging.verbosity, "v", "number for the log level verbosity") commandLine.BoolVar(&logging.addDirHeader, "add_dir_header", false, "If true, adds the file directory to the header of the log messages") commandLine.BoolVar(&logging.skipHeaders, "skip_headers", false, "If true, avoid header prefixes in the log messages") commandLine.BoolVar(&logging.oneOutput, "one_output", false, "If true, only write logs to their native severity level (vs also writing to each lower severity level; no effect when -logtostderr=true)") commandLine.BoolVar(&logging.skipLogHeaders, "skip_log_headers", false, "If true, avoid headers when opening log files (no effect when -logtostderr=true)") - commandLine.Var(&logging.stderrThreshold, "stderrthreshold", "logs at or above this threshold go to stderr when writing to files and stderr (no effect when -logtostderr=true or -alsologtostderr=true unless -legacy_stderr_threshold_behavior=false)") + commandLine.Var(&logging.stderrThreshold, "stderrthreshold", "logs at or above this threshold go to stderr when writing to files and stderr (currently ignored when -logtostderr=true unless -legacy_stderr_threshold_behavior=false; the default will change in a future release)") commandLine.Var(&logging.alsologtostderrthreshold, "alsologtostderrthreshold", "logs at or above this threshold go to stderr when -alsologtostderr=true (no effect when -logtostderr=true)") commandLine.Var(&logging.vmodule, "vmodule", "comma-separated list of pattern=N settings for file-filtered logging") commandLine.Var(&logging.traceLocation, "log_backtrace_at", "when logging hits line file:N, emit a stack trace") @@ -497,7 +523,7 @@ type settings struct { legacyStderrThresholdBehavior bool // The -legacy_stderr_threshold_behavior flag. // Level flag. Handled atomically. - stderrThreshold severityValue // The -stderrthreshold flag. + stderrThreshold stderrThresholdValue // The -stderrthreshold flag. alsologtostderrthreshold severityValue // The -alsologtostderrthreshold flag. // Access to all of the following fields must be protected via a mutex. @@ -575,6 +601,12 @@ type loggingT struct { // in settingsT which need a mutex lock. mu sync.Mutex + // stderrThresholdExplicitlySet tracks whether -stderrthreshold was + // passed on the command line or set programmatically. + stderrThresholdExplicitlySet bool + // legacyDeprecationWarned guards the one-time deprecation warning. + legacyDeprecationWarned sync.Once + // pcs is used in V to avoid an allocation when computing the caller's PC. pcs [1]uintptr // vmap is a cache of the V Level for each V() call site, identified by PC. @@ -917,7 +949,15 @@ func (l *loggingT) output(s severity.Severity, logger *logWriter, buf *buffer.Bu // When logging to stderr only, check if we should filter by severity. // This is controlled by the legacy_stderr_threshold_behavior flag. if l.legacyStderrThresholdBehavior { - // Legacy behavior: always write to stderr, ignore stderrthreshold + if logging.stderrThresholdExplicitlySet { + logging.legacyDeprecationWarned.Do(func() { + fmt.Fprintf(os.Stderr, "WARNING: -stderrthreshold is set but ignored because "+ + "-legacy_stderr_threshold_behavior=true (the current default). "+ + "Set -legacy_stderr_threshold_behavior=false to honor -stderrthreshold "+ + "when -logtostderr=true. The default for -legacy_stderr_threshold_behavior "+ + "will change to false in a future klog release.\n") + }) + } os.Stderr.Write(data) } else { // New behavior: honor stderrthreshold even when logtostderr=true diff --git a/klog_test.go b/klog_test.go index 3fd7bc15..2ba18a34 100644 --- a/klog_test.go +++ b/klog_test.go @@ -811,8 +811,8 @@ func BenchmarkLogs(b *testing.B) { require.NoError(b, logging.verbosity.Set("0")) logging.toStderr = false logging.alsoToStderr = false - logging.stderrThreshold = severityValue{ - Severity: severity.FatalLog, + logging.stderrThreshold = stderrThresholdValue{ + severityValue: severityValue{Severity: severity.FatalLog}, } logging.logFile = testFile.Name() logging.swap([severity.NumSeverity]io.Writer{nil, nil, nil, nil}) @@ -847,8 +847,8 @@ func BenchmarkFlush(b *testing.B) { require.NoError(b, logging.verbosity.Set("0")) logging.toStderr = false logging.alsoToStderr = false - logging.stderrThreshold = severityValue{ - Severity: severity.FatalLog, + logging.stderrThreshold = stderrThresholdValue{ + severityValue: severityValue{Severity: severity.FatalLog}, } logging.logFile = testFile.Name() logging.swap([severity.NumSeverity]io.Writer{nil, nil, nil, nil}) @@ -947,7 +947,7 @@ func TestCommandLine(t *testing.T) { -alsologtostderrthreshold value logs at or above this threshold go to stderr when -alsologtostderr=true (no effect when -logtostderr=true) -legacy_stderr_threshold_behavior - If true, stderrthreshold is ignored when logtostderr=true (legacy behavior). If false, stderrthreshold is honored even when logtostderr=true (default true) + If true, stderrthreshold is ignored when logtostderr=true (legacy behavior, deprecated — will default to false in a future release). If false, stderrthreshold is honored even when logtostderr=true (default true) -log_backtrace_at value when logging hits line file:N, emit a stack trace -log_dir string @@ -965,7 +965,7 @@ func TestCommandLine(t *testing.T) { -skip_log_headers If true, avoid headers when opening log files (no effect when -logtostderr=true) -stderrthreshold value - logs at or above this threshold go to stderr when writing to files and stderr (no effect when -logtostderr=true or -alsologtostderr=true unless -legacy_stderr_threshold_behavior=false) (default 2) + logs at or above this threshold go to stderr when writing to files and stderr (currently ignored when -logtostderr=true unless -legacy_stderr_threshold_behavior=false; the default will change in a future release) (default 2) -v value number for the log level verbosity -vmodule value @@ -1298,8 +1298,8 @@ func createTestValueOfLoggingT() *loggingT { l := new(loggingT) l.toStderr = true l.alsoToStderr = false - l.stderrThreshold = severityValue{ - Severity: severity.ErrorLog, + l.stderrThreshold = stderrThresholdValue{ + severityValue: severityValue{Severity: severity.ErrorLog}, } l.verbosity = Level(0) l.skipHeaders = false diff --git a/stderr_threshold_test.go b/stderr_threshold_test.go index 9405d7f0..877d8299 100644 --- a/stderr_threshold_test.go +++ b/stderr_threshold_test.go @@ -23,6 +23,7 @@ import ( "io" "os" "strings" + "sync" "testing" "k8s.io/klog/v2/internal/buffer" @@ -349,3 +350,71 @@ func TestNewFlagParsing(t *testing.T) { }) } } + +func TestLegacyBehaviorDeprecationWarning(t *testing.T) { + defer CaptureState().Restore() + + tests := []struct { + name string + legacy bool + setThreshold bool + expectWarning bool + }{ + { + name: "warning when legacy=true and stderrthreshold explicitly set", + legacy: true, + setThreshold: true, + expectWarning: true, + }, + { + name: "no warning when legacy=true but stderrthreshold not set", + legacy: true, + setThreshold: false, + expectWarning: false, + }, + { + name: "no warning when legacy=false", + legacy: false, + setThreshold: true, + expectWarning: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + state := CaptureState() + defer state.Restore() + + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + + logging.mu.Lock() + logging.toStderr = true + logging.legacyStderrThresholdBehavior = tt.legacy + logging.legacyDeprecationWarned = sync.Once{} + logging.stderrThreshold.severityValue.Set("ERROR") + logging.stderrThresholdExplicitlySet = tt.setThreshold + logging.mu.Unlock() + + buf := buffer.GetBuffer() + buf.WriteString("deprecation test message\n") + logging.output(severity.InfoLog, nil, buf, 0, "test.go", 1, false) + + w.Close() + var stderrBuf bytes.Buffer + io.Copy(&stderrBuf, r) + os.Stderr = oldStderr + + stderrContent := stderrBuf.String() + hasWarning := strings.Contains(stderrContent, "WARNING: -stderrthreshold is set but ignored") + + if tt.expectWarning && !hasWarning { + t.Errorf("expected deprecation warning but not found.\nStderr: %q", stderrContent) + } + if !tt.expectWarning && hasWarning { + t.Errorf("did not expect deprecation warning but found it.\nStderr: %q", stderrContent) + } + }) + } +}