Skip to content

Commit

Permalink
Merge pull request kubernetes#141 from aanm/do-not-repeat-messages
Browse files Browse the repository at this point in the history
klog: do not repeat messages on all severity flushSyncWriter
  • Loading branch information
k8s-ci-robot authored Jul 22, 2020
2 parents b5c3182 + 0917ddb commit db8b2b2
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 12 deletions.
31 changes: 31 additions & 0 deletions integration_tests/klog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ var (
2: {stackTraceRE, fatalLogRE, errorLogRE},
3: {stackTraceRE, fatalLogRE},
}
expectedOneOutputInDirREs = map[int]res{
0: {infoLogRE},
1: {warningLogRE},
2: {errorLogRE},
3: {fatalLogRE},
}

defaultNotExpectedInDirREs = map[int]res{
0: {},
Expand Down Expand Up @@ -135,6 +141,18 @@ func TestDestinationsWithDifferentFlags(t *testing.T) {
expectedInDir: defaultExpectedInDirREs,
notExpectedInDir: defaultNotExpectedInDirREs,
},
"with log dir only and one_output": {
// Everything, including the trace on fatal, goes to the log files in the log dir

logdir: true,
flags: []string{"-logtostderr=false", "-alsologtostderr=false", "-stderrthreshold=1000", "-one_output=true"},

expectedLogDir: true,

notExpectedOnStderr: allLogREs,
expectedInDir: expectedOneOutputInDirREs,
notExpectedInDir: defaultNotExpectedInDirREs,
},
"with log dir and logtostderr": {
// Everything, including the trace on fatal, goes to stderr. The -log_dir is
// ignored, nothing goes to the log files in the log dir.
Expand Down Expand Up @@ -182,6 +200,19 @@ func TestDestinationsWithDifferentFlags(t *testing.T) {
expectedInDir: defaultExpectedInDirREs,
notExpectedInDir: defaultNotExpectedInDirREs,
},
"with log dir, alsologtostderr and one_output": {
// Everything, including the trace on fatal, goes to the log file in the
// log dir AND to stderr.

logdir: true,
flags: []string{"-alsologtostderr=true", "-logtostderr=false", "-stderrthreshold=1000", "-one_output=true"},

expectedLogDir: true,

expectedOnStderr: allLogREs,
expectedInDir: expectedOneOutputInDirREs,
notExpectedInDir: defaultNotExpectedInDirREs,
},
}

binaryFileExtention := ""
Expand Down
33 changes: 21 additions & 12 deletions klog.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ func init() {
logging.skipHeaders = false
logging.addDirHeader = false
logging.skipLogHeaders = false
logging.oneOutput = false
go logging.flushDaemon()
}

Expand All @@ -432,6 +433,7 @@ func InitFlags(flagset *flag.FlagSet) {
flagset.Var(&logging.verbosity, "v", "number for the log level verbosity")
flagset.BoolVar(&logging.addDirHeader, "add_dir_header", logging.addDirHeader, "If true, adds the file directory to the header of the log messages")
flagset.BoolVar(&logging.skipHeaders, "skip_headers", logging.skipHeaders, "If true, avoid header prefixes in the log messages")
flagset.BoolVar(&logging.oneOutput, "one_output", logging.oneOutput, "If true, only write logs to their native severity level (vs also writing to each lower severity level")
flagset.BoolVar(&logging.skipLogHeaders, "skip_log_headers", logging.skipLogHeaders, "If true, avoid headers when opening log files")
flagset.Var(&logging.stderrThreshold, "stderrthreshold", "logs at or above this threshold go to stderr")
flagset.Var(&logging.vmodule, "vmodule", "comma-separated list of pattern=N settings for file-filtered logging")
Expand Down Expand Up @@ -505,6 +507,9 @@ type loggingT struct {

// If set, all output will be redirected unconditionally to the provided logr.Logger
logr logr.Logger

// If true, messages will not be propagated to lower severity log levels
oneOutput bool
}

// buffer holds a byte Buffer for reuse. The zero value is ready for use.
Expand Down Expand Up @@ -919,18 +924,22 @@ func (l *loggingT) output(s severity, log logr.Logger, buf *buffer, file string,
}
}

switch s {
case fatalLog:
l.file[fatalLog].Write(data)
fallthrough
case errorLog:
l.file[errorLog].Write(data)
fallthrough
case warningLog:
l.file[warningLog].Write(data)
fallthrough
case infoLog:
l.file[infoLog].Write(data)
if l.oneOutput {
l.file[s].Write(data)
} else {
switch s {
case fatalLog:
l.file[fatalLog].Write(data)
fallthrough
case errorLog:
l.file[errorLog].Write(data)
fallthrough
case warningLog:
l.file[warningLog].Write(data)
fallthrough
case infoLog:
l.file[infoLog].Write(data)
}
}
}
}
Expand Down
51 changes: 51 additions & 0 deletions klog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,33 @@ func TestError(t *testing.T) {
}
}

// Test that an Error log does not goes to Warning and Info.
// Even in the Info log, the source character will be E, so the data should
// all be identical.
func TestErrorWithOneOutput(t *testing.T) {
setFlags()
logging.oneOutput = true
buf := logging.newBuffers()
defer func() {
logging.swap(buf)
logging.oneOutput = false
}()
Error("test")
if !contains(errorLog, "E", t) {
t.Errorf("Error has wrong character: %q", contents(errorLog))
}
if !contains(errorLog, "test", t) {
t.Error("Error failed")
}
str := contents(errorLog)
if contains(warningLog, str, t) {
t.Error("Warning failed")
}
if contains(infoLog, str, t) {
t.Error("Info failed")
}
}

// Test that a Warning log goes to Info.
// Even in the Info log, the source character will be W, so the data should
// all be identical.
Expand All @@ -260,6 +287,30 @@ func TestWarning(t *testing.T) {
}
}

// Test that a Warning log does not goes to Info.
// Even in the Info log, the source character will be W, so the data should
// all be identical.
func TestWarningWithOneOutput(t *testing.T) {
setFlags()
logging.oneOutput = true
buf := logging.newBuffers()
defer func() {
logging.swap(buf)
logging.oneOutput = false
}()
Warning("test")
if !contains(warningLog, "W", t) {
t.Errorf("Warning has wrong character: %q", contents(warningLog))
}
if !contains(warningLog, "test", t) {
t.Error("Warning failed")
}
str := contents(warningLog)
if contains(infoLog, str, t) {
t.Error("Info failed")
}
}

// Test that a V log goes to Info.
func TestV(t *testing.T) {
setFlags()
Expand Down

0 comments on commit db8b2b2

Please sign in to comment.