feat: fix stderrthreshold not honored when logtostderr is set (#212) + two new flags

examples/stderr_threshold_fix/main.go

@@ -0,0 +1,37 @@
+// Example demonstrating the new stderr threshold behavior
+package main
+
+import (
+	"flag"
+
+	"k8s.io/klog/v2"
+)
+
+func main() {
+	klog.InitFlags(nil)
+	flag.Parse()
+
+	klog.Info("This is an INFO message")
+	klog.Warning("This is a WARNING message")
+	klog.Error("This is an ERROR message")
+
+	klog.Flush()
+}
+
+// Run examples:
+//
+// 1. Legacy behavior (default) - all logs to stderr:
+//    go run main.go -logtostderr=true -stderrthreshold=ERROR
+//    Result: All three messages appear
+//
+// 2. New behavior - filter by severity:
+//    go run main.go -logtostderr=true -legacy_stderr_threshold_behavior=false -stderrthreshold=ERROR
+//    Result: Only ERROR message appears
+//
+// 3. New behavior - show WARNING and above:
+//    go run main.go -logtostderr=true -legacy_stderr_threshold_behavior=false -stderrthreshold=WARNING
+//    Result: WARNING and ERROR messages appear
+//
+// 4. Using alsologtostderrthreshold with file logging:
+//    go run main.go -logtostderr=false -alsologtostderr=true -alsologtostderrthreshold=ERROR -log_dir=/tmp/logs
+//    Result: All logs in files, only ERROR to stderr

klog.go

@@ -58,15 +58,30 @@
 //
 //		-logtostderr=true
 //			Logs are written to standard error instead of to files.
-//	             This shortcuts most of the usual output routing:
-//	             -alsologtostderr, -stderrthreshold and -log_dir have no
-//	             effect and output redirection at runtime with SetOutput is
-//	             ignored.
+//	             By default, all logs are written regardless of severity
+//	             (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.
+//
+//	             The following flags always have no effect:
+//	             -alsologtostderr, -alsologtostderrthreshold, and -log_dir.
+//	             Output redirection at runtime with SetOutput is also ignored.
 //		-alsologtostderr=false
 //			Logs are written to standard error as well as to files.
+//		-alsologtostderrthreshold=INFO
+//			Log events at or above this severity are logged to standard
+//			error when -alsologtostderr=true (no effect when -logtostderr=true).
+//			Default is INFO to maintain backward compatibility.
 //		-stderrthreshold=ERROR
 //			Log events at or above this severity are logged to standard
-//			error as well as to files.
+//			error as well as to files. When -logtostderr=true, this flag
+//			has no effect unless -legacy_stderr_threshold_behavior=false.
+//		-legacy_stderr_threshold_behavior=true
+//			If true, -stderrthreshold is ignored when -logtostderr=true
+//			(legacy behavior). If false, -stderrthreshold is honored even
+//			when -logtostderr=true, allowing severity-based filtering.
 //		-log_dir=""
 //			Log files will be written to this directory instead of the
 //			default temporary directory.
@@ -156,7 +171,7 @@ func (s *severityValue) Set(value string) error {
 		}
 		threshold = severity.Severity(v)
 	}
-	logging.stderrThreshold.set(threshold)
+	s.set(threshold)
 	return nil
 }
 
@@ -409,23 +424,30 @@ 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.alsologtostderrthreshold = severityValue{
+		Severity: severity.InfoLog, // Default alsologtostderrthreshold is INFO (to maintain backward compatibility).
+	}
+	logging.setVState(0, nil, false)
+
 	commandLine.StringVar(&logging.logDir, "log_dir", "", "If non-empty, write log files in this directory (no effect when -logtostderr=true)")
 	commandLine.StringVar(&logging.logFile, "log_file", "", "If non-empty, use this log file (no effect when -logtostderr=true)")
 	commandLine.Uint64Var(&logging.logFileMaxSizeMB, "log_file_max_size", 1800,
 		"Defines the maximum size a log file can grow to (no effect when -logtostderr=true). Unit is megabytes. "+
 			"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)")
-	logging.setVState(0, nil, false)
+	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.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)")
-	logging.stderrThreshold = severityValue{
-		Severity: severity.ErrorLog, // Default stderrThreshold is ERROR.
-	}
-	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)")
+	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.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")
 
@@ -470,11 +492,13 @@ type settings struct {
 	// Boolean flags. Not handled atomically because the flag.Value interface
 	// does not let us avoid the =true, and that shorthand is necessary for
 	// compatibility. TODO: does this matter enough to fix? Seems unlikely.
-	toStderr     bool // The -logtostderr flag.
-	alsoToStderr bool // The -alsologtostderr flag.
+	toStderr                      bool // The -logtostderr flag.
+	alsoToStderr                  bool // The -alsologtostderr flag.
+	legacyStderrThresholdBehavior bool // The -legacy_stderr_threshold_behavior flag.
 
 	// Level flag. Handled atomically.
-	stderrThreshold severityValue // The -stderrthreshold flag.
+	stderrThreshold          severityValue // The -stderrthreshold flag.
+	alsologtostderrthreshold severityValue // The -alsologtostderrthreshold flag.
 
 	// Access to all of the following fields must be protected via a mutex.
 
@@ -890,9 +914,25 @@ func (l *loggingT) output(s severity.Severity, logger *logWriter, buf *buffer.Bu
 			}
 		}
 	} else if l.toStderr {
-		os.Stderr.Write(data)
+		// 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
+			os.Stderr.Write(data)
+		} else {
+			// New behavior: honor stderrthreshold even when logtostderr=true
+			if s >= l.stderrThreshold.get() {
+				os.Stderr.Write(data)
+			}
+		}
 	} else {
-		if alsoToStderr || l.alsoToStderr || s >= l.stderrThreshold.get() {
+		// Write to stderr if any of these conditions are met:
+		// - alsoToStderr is set (legacy behavior)
+		// - alsologtostderr is set and severity meets alsologtostderrthreshold
+		// - alsologtostderr is not set and severity meets stderrThreshold
+		if alsoToStderr ||
+			(l.alsoToStderr && s >= l.alsologtostderrthreshold.get()) ||
+			(!l.alsoToStderr && s >= l.stderrThreshold.get()) {
 			os.Stderr.Write(data)
 		}
 

klog_test.go

@@ -944,6 +944,10 @@ func TestCommandLine(t *testing.T) {
     	If true, adds the file directory to the header of the log messages
   -alsologtostderr
     	log to standard error as well as files (no effect when -logtostderr=true)
+  -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)
   -log_backtrace_at value
     	when logging hits line file:N, emit a stack trace
   -log_dir string
@@ -961,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) (default 2)
+    	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)
   -v value
     	number for the log level verbosity
   -vmodule value
@@ -1949,19 +1953,21 @@ func checkLogrEntryCorrectCaller(t *testing.T, wantFile string, wantLine int, en
 
 // existedFlag contains all existed flag, without KlogPrefix
 var existedFlag = map[string]struct{}{
-	"log_dir":           {},
-	"add_dir_header":    {},
-	"alsologtostderr":   {},
-	"log_backtrace_at":  {},
-	"log_file":          {},
-	"log_file_max_size": {},
-	"logtostderr":       {},
-	"one_output":        {},
-	"skip_headers":      {},
-	"skip_log_headers":  {},
-	"stderrthreshold":   {},
-	"v":                 {},
-	"vmodule":           {},
+	"log_dir":                          {},
+	"add_dir_header":                   {},
+	"alsologtostderr":                  {},
+	"alsologtostderrthreshold":         {},
+	"legacy_stderr_threshold_behavior": {},
+	"log_backtrace_at":                 {},
+	"log_file":                         {},
+	"log_file_max_size":                {},
+	"logtostderr":                      {},
+	"one_output":                       {},
+	"skip_headers":                     {},
+	"skip_log_headers":                 {},
+	"stderrthreshold":                  {},
+	"v":                                {},
+	"vmodule":                          {},
 }
 
 // KlogPrefix define new flag prefix