From d9ad77aa83e824978912064dcd968001d4a3c854 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 17 Nov 2025 12:45:52 +0100 Subject: [PATCH] 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. --- .../devel/sig-instrumentation/logging.md | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/contributors/devel/sig-instrumentation/logging.md b/contributors/devel/sig-instrumentation/logging.md index ba6866bc31f..b229423e0da 100644 --- a/contributors/devel/sig-instrumentation/logging.md +++ b/contributors/devel/sig-instrumentation/logging.md @@ -60,8 +60,29 @@ Good practices when writing log messages are: ### What method to use? -* `klog.ErrorS` - Errors should be used to indicate unexpected behaviours in code, like unexpected errors returned by subroutine function calls. -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. +* `klog.ErrorS`, [`logr.Logger.Error`](https://pkg.go.dev/github.com/go-logr/logr#Logger.Error): + Use error logs to inform admins that they might have to do something to fix a problem. + 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`. + If the dependency is allowed, use `runtime.HandleErrorWithContext` or `runtime.HandleErrorWithLogger` instead. + + Don't emit an error log before returning an error because + it is usually uncertain if and how the caller is going to handle the returned error. + It might handle it gracefully, in which case alerting an administrator with an error log would be wrong. + Instead, use `fmt.Errorf` with `%w` to return a more informative error with a trail for where the error came from. + Avoid "failed to" as prefix when wrapping errors, it quickly becomes repetitive when added multiple times. + + Sometimes it may be useful to log an error for debugging purposes before returning it. Use an info log with at least + `V(4)` and `err` as key for the key/value pair. + +* [`runtime.HandleErrorWithContext`](https://pkg.go.dev/k8s.io/apimachinery@v0.34.2/pkg/util/runtime#HandleErrorWithContext), + [`runtime.HandleErrorWithLogger`](https://pkg.go.dev/k8s.io/apimachinery@v0.34.2/pkg/util/runtime#HandleErrorWithLogger): + Whenever possible, use this instead of normal error logging at the top of a call chain, i.e. the place where code + cannot return an error up to its caller, if there is no other way of handling the error (like logging it + and then exiting the process). + + The `runtime` package supports additional error handling mechanisms that downstream consumers of Kubernetes may add + (see [ErrorHandlers](https://pkg.go.dev/k8s.io/apimachinery@v0.34.2/pkg/util/runtime#pkg-variables)). + Some [downstream consumers](https://grep.app/search?q=runtime.ErrorHandlers) even suppress logging of errors completely. * `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). 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: