From e7eedae97cb8ea9050ec220a7cb33445c0b791f9 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Thu, 10 Apr 2025 16:01:12 +0200 Subject: [PATCH] Use the logger with proper handling of dynamic debug on 2 locations Signed-off-by: Benoit Tigeot --- .../logger.go => internal/logging/logging.go | 26 +++++++++++-------- pkg/action/action.go | 2 -- pkg/action/action_test.go | 20 +++----------- pkg/cmd/root.go | 3 ++- 4 files changed, 21 insertions(+), 30 deletions(-) rename pkg/cli/logger.go => internal/logging/logging.go (76%) diff --git a/pkg/cli/logger.go b/internal/logging/logging.go similarity index 76% rename from pkg/cli/logger.go rename to internal/logging/logging.go index 03a69be24..946a211ef 100644 --- a/pkg/cli/logger.go +++ b/internal/logging/logging.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cli +package logging import ( "context" @@ -22,16 +22,20 @@ import ( "os" ) +// DebugEnabledFunc is a function type that determines if debug logging is enabled +// We use a function because we want to check the setting at log time, not when the logger is created +type DebugEnabledFunc func() bool + // DebugCheckHandler checks settings.Debug at log time type DebugCheckHandler struct { - handler slog.Handler - settings *EnvSettings + handler slog.Handler + debugEnabled DebugEnabledFunc } // Enabled implements slog.Handler.Enabled func (h *DebugCheckHandler) Enabled(_ context.Context, level slog.Level) bool { if level == slog.LevelDebug { - return h.settings.Debug // Check settings.Debug at log time + return h.debugEnabled() } return true // Always log other levels } @@ -44,21 +48,21 @@ func (h *DebugCheckHandler) Handle(ctx context.Context, r slog.Record) error { // WithAttrs implements slog.Handler.WithAttrs func (h *DebugCheckHandler) WithAttrs(attrs []slog.Attr) slog.Handler { return &DebugCheckHandler{ - handler: h.handler.WithAttrs(attrs), - settings: h.settings, + handler: h.handler.WithAttrs(attrs), + debugEnabled: h.debugEnabled, } } // WithGroup implements slog.Handler.WithGroup func (h *DebugCheckHandler) WithGroup(name string) slog.Handler { return &DebugCheckHandler{ - handler: h.handler.WithGroup(name), - settings: h.settings, + handler: h.handler.WithGroup(name), + debugEnabled: h.debugEnabled, } } // NewLogger creates a new logger with dynamic debug checking -func NewLogger(settings *EnvSettings) *slog.Logger { +func NewLogger(debugEnabled DebugEnabledFunc) *slog.Logger { // Create base handler that removes timestamps baseHandler := slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ // Always use LevelDebug here to allow all messages through @@ -75,8 +79,8 @@ func NewLogger(settings *EnvSettings) *slog.Logger { // Wrap with our dynamic debug-checking handler dynamicHandler := &DebugCheckHandler{ - handler: baseHandler, - settings: settings, + handler: baseHandler, + debugEnabled: debugEnabled, } return slog.New(dynamicHandler) diff --git a/pkg/action/action.go b/pkg/action/action.go index 09c1887bb..47a7da99c 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -378,8 +378,6 @@ func (cfg *Configuration) Init(getter genericclioptions.RESTClientGetter, namesp clientFn: kc.Factory.KubernetesClientSet, } - // slog.SetDefault() - var store *storage.Storage switch helmDriver { case "secret", "secrets", "": diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index f544d3281..4a0691afb 100644 --- a/pkg/action/action_test.go +++ b/pkg/action/action_test.go @@ -20,12 +20,12 @@ import ( "fmt" "io" "log/slog" - "os" "testing" "github.com/stretchr/testify/assert" fakeclientset "k8s.io/client-go/kubernetes/fake" + "helm.sh/helm/v4/internal/logging" chart "helm.sh/helm/v4/pkg/chart/v2" chartutil "helm.sh/helm/v4/pkg/chart/v2/util" kubefake "helm.sh/helm/v4/pkg/kube/fake" @@ -41,21 +41,9 @@ var verbose = flag.Bool("test.log", false, "enable test logging (debug by defaul func actionConfigFixture(t *testing.T) *Configuration { t.Helper() - logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if *verbose { - // Create a handler that removes timestamps - handler := slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ - Level: slog.LevelDebug, - ReplaceAttr: func(_ []string, a slog.Attr) slog.Attr { - // Remove the time attribute - if a.Key == slog.TimeKey { - return slog.Attr{} - } - return a - }, - }) - logger = slog.New(handler) - } + logger := logging.NewLogger(func() bool { + return *verbose + }) slog.SetDefault(logger) registryClient, err := registry.NewClient() diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index e9305206a..ee22533f0 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -32,6 +32,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/clientcmd" + "helm.sh/helm/v4/internal/logging" "helm.sh/helm/v4/internal/tlsutil" "helm.sh/helm/v4/pkg/action" "helm.sh/helm/v4/pkg/cli" @@ -139,7 +140,7 @@ func newRootCmdWithConfig(actionConfig *action.Configuration, out io.Writer, arg settings.AddFlags(flags) addKlogFlags(flags) - logger := cli.NewLogger(settings) + logger := logging.NewLogger(func() bool { return settings.Debug }) slog.SetDefault(logger) // Setup shell completion for the namespace flag