H4HIP: Enhanced logging library for Helm CLI #372
Conversation
Signed-off-by: Evans Mungai <[email protected]>
Signed-off-by: Evans Mungai <[email protected]>
There was a problem hiding this comment.
I like the path this is going one. But, there are some changes I would like to see.
First, how should we handle structured logging? Right now Helm variable details into many logging strings. Should these be handled as structured logs so that they are easier to query in log searching tools?
Second, handling the logging in the SDK vs the client (and not mixing concerns).
|
|
||
| ## Abstract | ||
|
|
||
| This proposal introduces an improved logging library for Helm CLI to enhance logging capabilities for troubleshooting, development, and debugging. By using newer logging features such as log formatting, different log levels and verbosity levels, developers and CLI users gain flexibility when deciding how much logging information they want to consume, and in what format. |
There was a problem hiding this comment.
It might be useful to call out the SDK users and how they can integrate with Helm in a standard way.
| - Troubleshooting Helm CLI commands due to inability to increase the details that can get logged. More logs come in handy when submitting bug reports for example. | ||
| - Instrumenting code with clear and consistent logging conventions for contributors, as well as consumers of CLI logs. |
There was a problem hiding this comment.
Also, SDK users integrating with Helm have to use Helm's custom logging mechanism. This is an integration problem for users (e.g., Flux)
| - Lightweight and straightforward to integrate. | ||
| - Is part of the standard library | ||
|
|
||
| 2. **`klog`:** |
There was a problem hiding this comment.
In addition to klog there is logr, an interface that klog suffices. Can you add that in.
| - Well-aligned with Kubernetes development standards. | ||
| - Allows getting instrumented Kubernetes client library. | ||
|
|
||
| While both libraries meet the basic requirements, `klog` offers richer features for developers familiar with Kubernetes' logging patterns, making it the preferred choice for this proposal. |
There was a problem hiding this comment.
There are two parts to Helm. There is the SDK and then the client that uses the SDK.
The SDK shouldn't use an implementation of logging. It should accept a logger from any application that uses it and follows the right interface. So, the SDK should use either slog or logr. It may have a default one but that needs to be easy to override.
Note, SDK users may not be developers familiar with Kubernetes. There will be people familiar with and those not familiar with it.
Then there is the Helm client (a.k.a CLI). This should pick an implementation. It could be slog, klog, zap, or something else. Note, klog appears to support the slog interface (see this example).
We need to make decisions on both of these. If we go the klog route for the Helm Client we should document the log levels to use and what they mean. There is no common clarity on what levels to use for what. This is one of the weakness of this model. What level should you set for what purposes?
| Two libraries were evaluated: | ||
|
|
||
| 1. **`slog`:** | ||
| - Provides log levels (`info`, `debug`, `error`, `warning`). |
There was a problem hiding this comment.
Note, it's fairly easy to add levels if an app wants to use more. The Go docs provide an example.
No description provided.