Skip to content

Commit 5b58828

Browse files
committed
logging: clarify error logging
The main purpose of error logging is informing admins about problems. Our logging libraries are not configured to log additional debug information for errors. Producing an error log before returning an error is usually wrong. runtime.HandleErrorWithContext and runtime.HandleErrorWithLogger were not mentioned even though they are meant to be used in certain places.
1 parent a26e098 commit 5b58828

File tree

1 file changed

+23
-2
lines changed

1 file changed

+23
-2
lines changed

contributors/devel/sig-instrumentation/logging.md

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,29 @@ Good practices when writing log messages are:
6060

6161
### What method to use?
6262

63-
* `klog.ErrorS` - Errors should be used to indicate unexpected behaviours in code, like unexpected errors returned by subroutine function calls.
64-
Logs generated by `ErrorS` command may be enhanced with additional debug information (by logging library). Calling `ErrorS` with `nil` as error may be acceptable if there is error condition that deserves a stack trace at this origin point.
63+
* `klog.ErrorS`, [`logr.Logger.Error`](https://pkg.go.dev/github.com/go-logr/logr#Logger.Error):
64+
Use error logs to inform admins that they might have to do something to fix a problem.
65+
If there is no `error` instance, pass nil and use the message string and key/value pairs to describe the problem instead of fabricating an error with `fmt.Errorf`.
66+
If the dependency is allowed, use `runtime.HandleErrorWithContext` or `runtime.HandleErrorWithLogger` instead.
67+
68+
Don't emit an error log before returning an error because
69+
it is usually uncertain if and how the caller is going to handle the returned error.
70+
It might handle it gracefully, in which case alerting an administrator with an error log would be wrong.
71+
Instead, use `fmt.Errorf` with `%w` to return a more informative error with a trail for where the error came from.
72+
Avoid "failed to" as prefix when wrapping errors, it quickly becomes repetitive when added multiple times.
73+
74+
Sometimes it may be useful to log an error for debugging purposes before returning it. Use an info log with at least
75+
`V(4)` and `err` as key for the key/value pair.
76+
77+
* [`runtime.HandleErrorWithContext`](https://pkg.go.dev/k8s.io/[email protected]/pkg/util/runtime#HandleErrorWithContext),
78+
[`runtime.HandleErrorWithLogger`](https://pkg.go.dev/k8s.io/[email protected]/pkg/util/runtime#HandleErrorWithLogger):
79+
Whenever possible, use this instead of normal error logging at the top of a call chain, i.e. the place where code
80+
cannot return an error up to its caller, if there is no other way of handling the error (like logging it
81+
and then exiting the process).
82+
83+
The `runtime` package supports additional error mechanisms that downstream consumers of Kubernetes may add
84+
(see [ErrorHandlers](https://pkg.go.dev/k8s.io/[email protected]/pkg/util/runtime#pkg-variables)).
85+
Some [downstream consumers](https://grep.app/search?q=runtime.ErrorHandlers) even suppress logging of errors completely.
6586

6687
* `klog.InfoS` - Structured logs to the INFO log. `InfoS` should be used for routine logging. It can also be used to log warnings for expected errors (errors that can happen during routine operations).
6788
Depending on log severity it's important to pick a proper verbosity level to ensure that consumer is neither under nor overwhelmed by log volume:

0 commit comments

Comments
 (0)