fixes for PR #280, refactoring, textlogger, unit test #287

We need the ability to make experimental changes without new definitions immediately becoming part of the stable v2 API.

The new test cases are applied to structured logging calls in klog and the stand-alone klogr logger with output through klog. The output is expected to be the same.

klog has a lot of functions, but not all of them are covered with unit tests. Only a few check stack unwinding. This new test is designed to: - check stack unwinding for all functions - check the difference between fmt.Print and fmt.Println (only the latter inserts spaces between strings) - check the output with and without a Logger This therefore covers bugs in kubernetes#280: - depth was not passed on to Logger - ErrorlnDepth and WarninglnDepth used fmt.Print instead of fmt.Println

kubernetes#280 turned println and printf into wrapper functions, but the underlying *Depth functions kept passing the same 0 depth to l.output and thus any logr.Logger. That off-by-one error broke JSON tests in Kubernetes, luckily well before any of the PR went into a release. It would be nice to have regression testing for this in klog, but we cannot depend on a Logger which does stack unwinding in the code, so the only thing that could be done is some kind of GitHub action which combines klog with some external logger. The depth=1 in println is also not covered by unit tests, but seems to be correct based on the JSON tests.

kubernetes#280 added several new functions and didn't quite get the print vs println difference right: the corresponding non-Depth functions use fmt.Println and the Depth versions should do the same.

The implementation of V is performance critical: it may get called frequently on hot code paths and will be responsible for all of the logging overhead when the log messages then do not need to be formatted.

This will be needed for Logger implementations that want to emulate the traditional verbosity check in klog. The original code in klog.go intentionally doesn't get modified, for two reasons: - avoid the risk of regressions - an attempt to do so showed that V became slower (extra calls, more parameters, etc.)

This makes it possible to reuse the klog output formatting in logr.Logger implementations.

This will enable writing Logger implementations which reproduce the traditional klog header.

klogr had a special value for a missing value in a key/value pair, so the intention probably was to use that as replacement, the same way as klog does it. But because of a bug in the code which removed duplicates, it treated a missing value like nil. Now the behavior is consistent with klog.

This is invalid: logger.WithValue("keyWithoutValue").WithValue("anotherKeyWithoutValue") Because it concatenated key/values without checking, klogr treated this like: logger.WithValue("keyWithoutValue", "anotherKeyWithoutValue") Now the parameters are checked and a "(MISSING)" is appended as in the other cases where a missing value is detected.

In contrast to klogr, the formatting and printing is handled entirely by this logger without depending on klog.go. The intention is to use this in two ways: - for testing of SetLogger - as simpler text logger in Kubernetes for contextual logging

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixes for PR #280, refactoring, textlogger, unit test #287

fixes for PR #280, refactoring, textlogger, unit test #287

Commits on Feb 14, 2022