From 282e1da934f8fdc55c71b17051189e86f76919a7 Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Tue, 26 Nov 2024 22:17:23 +0000 Subject: [PATCH 1/5] H4HIP: Enhanced logging library for Helm CLI Signed-off-by: Evans Mungai --- hips/hip-9999.md | 82 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 hips/hip-9999.md diff --git a/hips/hip-9999.md b/hips/hip-9999.md new file mode 100644 index 00000000..4f1fab3b --- /dev/null +++ b/hips/hip-9999.md @@ -0,0 +1,82 @@ +--- +hip: 9999 +title: "Enhanced logging library for Helm CLI" +authors: [ "Evans Mungai " ] +created: "2024-11-22" +type: "feature" +status: "draft" +--- + +## 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. + +## Motivation + +The current logging mechanism in Helm CLI lacks granularity and structure. Users and developers face difficulties when: + +- 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. + +The proposed solution addresses these gaps by integrating a logging library that supports log levels, verbosity, and structured output. + +## Rationale + +Two libraries were evaluated: + +1. **Go’s `slog`:** + - Provides log levels (`info`, `debug`, `error`, `warning`). + - Offers structured logging. + - Lightweight and straightforward to integrate. + - Is part of the standard library + +2. **`klog`:** + - Provides log levels and also verbosity levels, enabling more granular control. Verbosity level controls the amount of detail in logs, enabling users to filter messages based on their importance or relevance. + - 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. + +## Specification + +- Helm SDK should not be instrumented with error logs. Instead, errors ought to be returned. Any logging of such errors should be left to clients to choose whether to print or not. +- Verbosity levels to enable filtering of logs based on verbosity levels. Lower verbosity levels would be used for major operations in the flow control. __TODO: Should I specify actual numbers or would that be too prescriptive?__ +- When invoking plugins and post-renderers, `HELM_LOG_LEVEL` and `HELM_VERBOSITY_LEVEL` environment variables would be set, allowing them to output the appropriate amount of logs to stderr +- Structured logging with two options Raw text for humans to view, and JSON formatting that machines can consume. +- Ability to allow users to configure log levels, verbosity and formatting through CLI flags or environment variables. + +### Example: + + +## Backwards Compatibility + +The current logging system, Golang's `log` package will be replaced by `klog`. + +## Security Implications + + +## How to Teach This + +1. **Documentation Updates:** + - Provide examples of using the new logging features in the Helm documentation. + - Include a section on how to configure verbosity, log levels and formats. + - Document conventions of how to instrument logs. Specifically, what information should be in what verbosity level. + +2. **CLI help text:** + - Use CLI help messages (`helm --help`) to inform users about the new logging options. + +## Reference Implementation + + +## Rejected Ideas + +1. **Sticking with the current logging system:** + - Rejected due to lack of flexibility and limited usefulness for debugging. + +## Open Issues + + +## References + +- [Go’s slog Documentation](https://pkg.go.dev/log/slog) +- [klog Documentation](https://pkg.go.dev/k8s.io/klog/v2) From abc7f7f01720dff390d48ea2be5300764730f054 Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Wed, 27 Nov 2024 21:13:55 +0000 Subject: [PATCH 2/5] Add comment Signed-off-by: Evans Mungai --- hips/hip-9999.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hips/hip-9999.md b/hips/hip-9999.md index 4f1fab3b..57355ae9 100644 --- a/hips/hip-9999.md +++ b/hips/hip-9999.md @@ -24,7 +24,7 @@ The proposed solution addresses these gaps by integrating a logging library that Two libraries were evaluated: -1. **Go’s `slog`:** +1. **`slog`:** - Provides log levels (`info`, `debug`, `error`, `warning`). - Offers structured logging. - Lightweight and straightforward to integrate. @@ -47,6 +47,7 @@ While both libraries meet the basic requirements, `klog` offers richer features ### Example: +TODO: To be added once specification is agreed ## Backwards Compatibility From a39b11b29deb347cb9743c94cc336aaa6f5aabe2 Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Tue, 14 Jan 2025 19:46:58 +0000 Subject: [PATCH 3/5] chore: choose logr library instead of others Signed-off-by: Evans Mungai --- hips/hip-9999.md | 47 +++++++++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/hips/hip-9999.md b/hips/hip-9999.md index 57355ae9..ab38a69b 100644 --- a/hips/hip-9999.md +++ b/hips/hip-9999.md @@ -9,41 +9,51 @@ status: "draft" ## 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. +This proposal introduces an improved logging library for Helm CLI and SDK to enhance logging capabilities for troubleshooting, development, and debugging. By using newer logging features such as log formatting and different log levels, developers and CLI users gain flexibility when deciding how much logging information they want to consume, and in what format. + +The HIP also introduces the ability for SDK users to use their own logging library with the SDK. Users will be able to provide a logger implementing the `logr.Logger` interface, allowing them to consume instrumented logs in the SDK. ## Motivation -The current logging mechanism in Helm CLI lacks granularity and structure. Users and developers face difficulties when: +The current logging mechanism in Helm CLI and SDK lacks granularity and structure. Users and developers face difficulties when: - 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. +- Instrumenting Helm code with clear and consistent logging conventions for contributors, as well as consumers of CLI and SDK logs. +- Inflexibility using a different logging library to what the SDK uses. There is no way, in code, to have client loggers consume logs produced by the SDK -The proposed solution addresses these gaps by integrating a logging library that supports log levels, verbosity, and structured output. +The proposed solution addresses these gaps by integrating a logging library that supports log levels and structured output, as well as a widely supported logger implementation interface for SDK users to consume. ## Rationale -Two libraries were evaluated: +The following libraries were evaluated: 1. **`slog`:** - - Provides log levels (`info`, `debug`, `error`, `warning`). + - Provides log levels (`info`, `debug`, `error`, `warning`). More levels can be added - Offers structured logging. - Lightweight and straightforward to integrate. - Is part of the standard library 2. **`klog`:** - Provides log levels and also verbosity levels, enabling more granular control. Verbosity level controls the amount of detail in logs, enabling users to filter messages based on their importance or relevance. - - Well-aligned with Kubernetes development standards. - - Allows getting instrumented Kubernetes client library. + - Allows getting instrumented Kubernetes client library + - Its an external library, hence introducing a dependency + +3. **`logr`** + - Library defining interfaces for logger implementations and libraries expecting loggers that implement these APIs. `klog`, `slog`, `log` and many others are compatible with `logr`. + - Provides log levels also verbosity levels + - Its an external library, hence introducing a dependency -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. +While these libraries meet all the requirements we want, `logr` is the preferred choice here. It provides all functionality that the other choices have but stand out since it allows using different logger implementations. This will be benefitial to SDK users who have their existing logger implementations present. They would either implement `logr.LogSink` interface or add a dependency of an existing adapter logger implementation. ## Specification -- Helm SDK should not be instrumented with error logs. Instead, errors ought to be returned. Any logging of such errors should be left to clients to choose whether to print or not. -- Verbosity levels to enable filtering of logs based on verbosity levels. Lower verbosity levels would be used for major operations in the flow control. __TODO: Should I specify actual numbers or would that be too prescriptive?__ -- When invoking plugins and post-renderers, `HELM_LOG_LEVEL` and `HELM_VERBOSITY_LEVEL` environment variables would be set, allowing them to output the appropriate amount of logs to stderr -- Structured logging with two options Raw text for humans to view, and JSON formatting that machines can consume. -- Ability to allow users to configure log levels, verbosity and formatting through CLI flags or environment variables. +- Helm will instrument `debug`, `info`, `warning` and `error` log levels. These log levels will enable filtering of logs. +- Logs will be written to `stderr` by the CLI client. `stdout` will be left for output from operations. SDK users would choose where to write logs to through configuring the logger they pass to the SDK. +- Ability to allow users to configure log levels and formatting through CLI flags or environment variables. +- Helm SDK should not be instrumented with error logs. Instead, errors ought to be returned. Any logging of such errors should be left to clients to choose whether to log or not. +- When invoking plugins and post-renderers, `HELM_LOG_LEVEL` environment variable would be set, allowing them to output the appropriate amount of logs to stderr. +- There will be structured logging with two options. Raw text for humans to view, and JSON formatting that machines can consume. +- Ability for SDK users to override the default logger implementation. The SDK will expect at logger that implements `logr.Logger` interface. ### Example: @@ -51,7 +61,7 @@ TODO: To be added once specification is agreed ## Backwards Compatibility -The current logging system, Golang's `log` package will be replaced by `klog`. +The current logging system, Golang's `log` package will be replaced by a `slog` and the `logr` interface adapter. ## Security Implications @@ -60,8 +70,9 @@ The current logging system, Golang's `log` package will be replaced by `klog`. 1. **Documentation Updates:** - Provide examples of using the new logging features in the Helm documentation. - - Include a section on how to configure verbosity, log levels and formats. - - Document conventions of how to instrument logs. Specifically, what information should be in what verbosity level. + - Include a section on how to configure log levels and formats. + - Document conventions of how to instrument logs. Specifically, what information should be in what log level. Also document that the SDK does not log errors. + - Example for SDK users of how to override the default logger with their own implementation. For loggers that do not implement `logr.Logger`, document an example of how this can be done. 2. **CLI help text:** - Use CLI help messages (`helm --help`) to inform users about the new logging options. @@ -80,4 +91,4 @@ The current logging system, Golang's `log` package will be replaced by `klog`. ## References - [Go’s slog Documentation](https://pkg.go.dev/log/slog) -- [klog Documentation](https://pkg.go.dev/k8s.io/klog/v2) +- [logr Documentation](https://pkg.go.dev/github.com/go-logr/logr) From 82f2a55591a0143159aa56083d6c77d2c0ee1f94 Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Tue, 14 Jan 2025 19:57:19 +0000 Subject: [PATCH 4/5] chore: indicate slog and logr both have logger API interfaces Signed-off-by: Evans Mungai --- hips/hip-9999.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hips/hip-9999.md b/hips/hip-9999.md index ab38a69b..d0bedb53 100644 --- a/hips/hip-9999.md +++ b/hips/hip-9999.md @@ -32,6 +32,7 @@ The following libraries were evaluated: - Offers structured logging. - Lightweight and straightforward to integrate. - Is part of the standard library + - Has `slog.Handler` interface that other logger implementations can implement to be `slog` compatible 2. **`klog`:** - Provides log levels and also verbosity levels, enabling more granular control. Verbosity level controls the amount of detail in logs, enabling users to filter messages based on their importance or relevance. @@ -42,6 +43,7 @@ The following libraries were evaluated: - Library defining interfaces for logger implementations and libraries expecting loggers that implement these APIs. `klog`, `slog`, `log` and many others are compatible with `logr`. - Provides log levels also verbosity levels - Its an external library, hence introducing a dependency + - Has `logr.Handler` interface that other logger implementations can implement to be `logr` compatible While these libraries meet all the requirements we want, `logr` is the preferred choice here. It provides all functionality that the other choices have but stand out since it allows using different logger implementations. This will be benefitial to SDK users who have their existing logger implementations present. They would either implement `logr.LogSink` interface or add a dependency of an existing adapter logger implementation. From d19e11ad26310662b38ba9f895d1e769bcc0477a Mon Sep 17 00:00:00 2001 From: Evans Mungai Date: Mon, 27 Jan 2025 12:53:14 +0000 Subject: [PATCH 5/5] Prefer slog logging library Signed-off-by: Evans Mungai --- hips/hip-9999.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/hips/hip-9999.md b/hips/hip-9999.md index d0bedb53..af7c646d 100644 --- a/hips/hip-9999.md +++ b/hips/hip-9999.md @@ -11,7 +11,7 @@ status: "draft" This proposal introduces an improved logging library for Helm CLI and SDK to enhance logging capabilities for troubleshooting, development, and debugging. By using newer logging features such as log formatting and different log levels, developers and CLI users gain flexibility when deciding how much logging information they want to consume, and in what format. -The HIP also introduces the ability for SDK users to use their own logging library with the SDK. Users will be able to provide a logger implementing the `logr.Logger` interface, allowing them to consume instrumented logs in the SDK. +The HIP also introduces the ability for SDK users to use their own logging library with the SDK. Users will be able to provide a logger implementing `slog.Handler` interface, allowing them to consume instrumented logs in the SDK. ## Motivation @@ -31,7 +31,7 @@ The following libraries were evaluated: - Provides log levels (`info`, `debug`, `error`, `warning`). More levels can be added - Offers structured logging. - Lightweight and straightforward to integrate. - - Is part of the standard library + - Is part of Go standard library - Has `slog.Handler` interface that other logger implementations can implement to be `slog` compatible 2. **`klog`:** @@ -45,17 +45,17 @@ The following libraries were evaluated: - Its an external library, hence introducing a dependency - Has `logr.Handler` interface that other logger implementations can implement to be `logr` compatible -While these libraries meet all the requirements we want, `logr` is the preferred choice here. It provides all functionality that the other choices have but stand out since it allows using different logger implementations. This will be benefitial to SDK users who have their existing logger implementations present. They would either implement `logr.LogSink` interface or add a dependency of an existing adapter logger implementation. +While these libraries meet all the requirements we want, `slog` is the preferred choice here. It provides all functionality that the other choices have but stands out since it has a wider adoption given that its part of Go standard library. This will be benefitial to SDK users who have their existing logger implementations present. They would either implement `slog.Handler` interface or add a dependency of an existing adapter logger implementation. ## Specification - Helm will instrument `debug`, `info`, `warning` and `error` log levels. These log levels will enable filtering of logs. -- Logs will be written to `stderr` by the CLI client. `stdout` will be left for output from operations. SDK users would choose where to write logs to through configuring the logger they pass to the SDK. +- Logs will be written to `stderr` by the Helm CLI client. `stdout` will be left for output from operations. SDK users would choose where to write logs to through configuring the logger they pass to the SDK. - Ability to allow users to configure log levels and formatting through CLI flags or environment variables. - Helm SDK should not be instrumented with error logs. Instead, errors ought to be returned. Any logging of such errors should be left to clients to choose whether to log or not. -- When invoking plugins and post-renderers, `HELM_LOG_LEVEL` environment variable would be set, allowing them to output the appropriate amount of logs to stderr. +- When invoking plugins and post-renderers, `HELM_LOG_LEVEL` and `HELM_LOG_FORMAT` environment variables would be set, allowing them to output the appropriate amount of logs and format to stderr. - There will be structured logging with two options. Raw text for humans to view, and JSON formatting that machines can consume. -- Ability for SDK users to override the default logger implementation. The SDK will expect at logger that implements `logr.Logger` interface. +- Ability for SDK users to override the default logger implementation. The SDK will expect at logger that implements `slog.Handler` interface. ### Example: @@ -63,7 +63,7 @@ TODO: To be added once specification is agreed ## Backwards Compatibility -The current logging system, Golang's `log` package will be replaced by a `slog` and the `logr` interface adapter. +The current logging system, Golang's `log` package will be replaced by `slog`. ## Security Implications @@ -74,10 +74,11 @@ The current logging system, Golang's `log` package will be replaced by a `slog` - Provide examples of using the new logging features in the Helm documentation. - Include a section on how to configure log levels and formats. - Document conventions of how to instrument logs. Specifically, what information should be in what log level. Also document that the SDK does not log errors. - - Example for SDK users of how to override the default logger with their own implementation. For loggers that do not implement `logr.Logger`, document an example of how this can be done. + - Example for SDK users of how to override the default logger with their own implementation. For loggers that do not implement `slog.Handler`, document an example of how this can be done. + - Document how plugins and post-renderers can inherit log levels and formatting from helm CLI client. 2. **CLI help text:** - - Use CLI help messages (`helm --help`) to inform users about the new logging options. + - Use Helm CLI help messages (`helm --help`) to inform users about the new logging options. ## Reference Implementation @@ -93,4 +94,3 @@ The current logging system, Golang's `log` package will be replaced by a `slog` ## References - [Go’s slog Documentation](https://pkg.go.dev/log/slog) -- [logr Documentation](https://pkg.go.dev/github.com/go-logr/logr)