From 9bd621140da3dd3ea38f9687b85e6d71e99cdfcd Mon Sep 17 00:00:00 2001 From: Pierluigi Lenoci Date: Wed, 22 Apr 2026 11:48:12 +0200 Subject: [PATCH] feat: add deprecation warning for legacy_stderr_threshold_behavior Add a one-time deprecation warning when -stderrthreshold is explicitly set but silently ignored because -legacy_stderr_threshold_behavior=true (the current default). This alerts users that their threshold setting has no effect and guides them to opt in to the corrected behavior. The warning is emitted via fmt.Fprintf to stderr exactly once per process lifetime, only when both conditions are met: 1. -legacy_stderr_threshold_behavior=true (the default) 2. -stderrthreshold was explicitly set by the user Flag descriptions and package documentation are updated to mark the legacy behavior as deprecated, with a note that the default will change to false in a future release. This is the first step in a deprecation timeline: - This release: deprecation warning when legacy mode ignores threshold - Future release: flip legacy_stderr_threshold_behavior default to false Ref: kubernetes-sigs/metrics-server#1782 Ref: #436 Signed-off-by: Pierluigi Lenoci --- klog.go | 58 +++++++++++++++++++++++++++------ klog_test.go | 16 +++++----- stderr_threshold_test.go | 69 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 17 deletions(-) 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) + } + }) + } +}