Repository: kubernetes/klog
Pull request #79 (2019-07-31 02:45:55)

Add trace message output
**What this PR does / why we need it**:

Add trace message output when severity is fatalLog and -logtostderr has been specified.

When -logtostderr is specified, I called klog.Fatal("fatal msg"), only
```
F0731 10:36:27.954792   63349 main.go:30] fatal msg
```
will be output on the console, and no trace information will be output on the console. But more often, I need that. 

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:

_Please confirm that if this PR changes any image versions, then that's the sole change this PR makes._

**Release note**:
<!--  Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. If no release note is required, just write "NONE".
-->
```release-note
klog now always prints a trace of all goroutines on stderr on klog.Fatal(...). Previously this wasn't the case, the stack trace was omitted when the -logtostderr flag was used. (#79)
```


Patch:
diff --git a/klog.go b/klog.go
index 2520ebd..a273a4a 100644
--- a/klog.go
+++ b/klog.go
@@ -827,14 +827,12 @@ func (l *loggingT) output(s severity, buf *buffer, file string, line int, alsoTo
 			os.Exit(1)
 		}
 		// Dump all goroutine stacks before exiting.
-		// First, make sure we see the trace for the current goroutine on standard error.
-		// If -logtostderr has been specified, the loop below will do that anyway
-		// as the first stack in the full dump.
-		if !l.toStderr {
-			os.Stderr.Write(stacks(false))
+		trace := stacks(true)
+		// Write the stack trace for all goroutines to the stderr.
+		if l.toStderr || l.alsoToStderr || s >= l.stderrThreshold.get() || alsoToStderr {
+			os.Stderr.Write(trace)
 		}
 		// Write the stack trace for all goroutines to the files.
-		trace := stacks(true)
 		logExitFunc = func(error) {} // If we get a write error, we'll still exit below.
 		for log := fatalLog; log >= infoLog; log-- {
 			if f := l.file[log]; f != nil { // Can be nil if -logtostderr is set.


Test patch:
diff --git a/integration_tests/klog_test.go b/integration_tests/klog_test.go
index de4ba4b..ef70fb6 100644
--- a/integration_tests/klog_test.go
+++ b/integration_tests/klog_test.go
@@ -82,10 +82,9 @@ func TestDestinationsWithDifferentFlags(t *testing.T) {
 		notExpectedInDir map[int]res
 	}{
 		"default flags": {
-			// Everything, EXCEPT the trace on fatal, goes to stderr
+			// Everything, including the trace on fatal, goes to stderr
 
-			expectedOnStderr:    res{infoLogRE, warningLogRE, errorLogRE, fatalLogRE},
-			notExpectedOnStderr: res{stackTraceRE},
+			expectedOnStderr: allLogREs,
 		},
 		"everything disabled": {
 			// Nothing, including the trace on fatal, is showing anywhere
@@ -104,12 +103,11 @@ func TestDestinationsWithDifferentFlags(t *testing.T) {
 			notExpectedOnStderr: res{infoLogRE},
 		},
 		"with logtostderr only": {
-			// Everything, EXCEPT the trace on fatal, goes to stderr
+			// Everything, including the trace on fatal, goes to stderr
 
 			flags: []string{"-logtostderr=true", "-alsologtostderr=false", "-stderrthreshold=1000"},
 
-			expectedOnStderr:    res{infoLogRE, warningLogRE, errorLogRE, fatalLogRE},
-			notExpectedOnStderr: res{stackTraceRE},
+			expectedOnStderr: allLogREs,
 		},
 		"with log file only": {
 			// Everything, including the trace on fatal, goes to the single log file
@@ -135,14 +133,13 @@ func TestDestinationsWithDifferentFlags(t *testing.T) {
 			notExpectedInDir:    defaultNotExpectedInDirREs,
 		},
 		"with log dir and logtostderr": {
-			// Everything, EXCEPT the trace on fatal, goes to stderr. The -log_dir is
+			// Everything, including the trace on fatal, goes to stderr. The -log_dir is
 			// ignored, nothing goes to the log files in the log dir.
 
 			logdir: true,
 			flags:  []string{"-logtostderr=true", "-alsologtostderr=false", "-stderrthreshold=1000"},
 
-			expectedOnStderr:    res{infoLogRE, warningLogRE, errorLogRE, fatalLogRE},
-			notExpectedOnStderr: res{stackTraceRE},
+			expectedOnStderr: allLogREs,
 		},
 		"with log file and log dir": {
 			// Everything, including the trace on fatal, goes to the single log file.
diff --git a/klog_test.go b/klog_test.go
index 76a780c..c9ca16a 100644
--- a/klog_test.go
+++ b/klog_test.go
@@ -482,7 +482,7 @@ func TestOpenAppendOnStart(t *testing.T) {
 	// Logging agagin should open the file again with O_APPEND instead of O_TRUNC
 	Info(y)
 	// ensure we wrote what we expected
-	logging.flushAll()
+	logging.lockAndFlushAll()
 	b, err = ioutil.ReadFile(logging.logFile)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -491,7 +491,7 @@ func TestOpenAppendOnStart(t *testing.T) {
 		t.Fatalf("got %s, missing expected Info log: %s", string(b), y)
 	}
 	// The initial log message should be preserved across create calls.
-	logging.flushAll()
+	logging.lockAndFlushAll()
 	b, err = ioutil.ReadFile(logging.logFile)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)