From f3d08c90c74106c0d3ac5cd6a8e7e8fcff6516d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20Stefaniak?= Date: Fri, 24 May 2024 16:27:33 +0200 Subject: [PATCH] feat: structured logging (#43) --- DEVELOPMENT.md | 13 ++- cmd/main.go | 10 +- config/default/kustomization.yaml | 111 +------------------ config/default/manager_config_dev_patch.yaml | 15 +++ config/default/manager_config_patch.yaml | 2 - internal/config/config.go | 4 + internal/config/config_test.go | 3 + internal/config/env_provider.go | 8 ++ internal/config/env_provider_test.go | 8 ++ internal/config/logger.go | 64 +++++++++++ 10 files changed, 119 insertions(+), 119 deletions(-) create mode 100644 config/default/manager_config_dev_patch.yaml create mode 100644 internal/config/logger.go diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index fd7c5e1..3e2c572 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -39,7 +39,18 @@ kind create cluster --config kind-poc-cluster.yaml kind load docker-image --name poc kube-startup-cpu-boost:dev ``` -4. Deploy controller on a cluster +4. Enable development logging and other options if needed + + In `config/default/kustomization.yaml` uncomment the dev patch: + + ```yaml + # Uncomment below for development + - manager_config_dev_patch.yaml + ``` + + Adapt the `config/default/manager_config_dev_patch.yaml` if needed. + +5. Deploy controller on a cluster ```sh make deploy IMG=docker.io/library/kube-startup-cpu-boost:dev diff --git a/cmd/main.go b/cmd/main.go index 5ae8c14..f73401c 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -20,8 +20,7 @@ import ( // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. - "go.uber.org/zap" - "go.uber.org/zap/zapcore" + _ "k8s.io/client-go/plugin/pkg/client/auth" "k8s.io/apimachinery/pkg/runtime" @@ -29,7 +28,6 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" - ctrlZap "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/webhook" autoscalingv1alpha1 "github.com/google/kube-startup-cpu-boost/api/v1alpha1" @@ -61,11 +59,7 @@ func main() { setupLog.Error(err, "unable to load configuration") os.Exit(1) } - ctrlZapOpts := ctrlZap.Options{ - Development: true, - Level: zap.NewAtomicLevelAt(zapcore.Level(cfg.ZapLogLevel)), - } - ctrl.SetLogger(ctrlZap.New(ctrlZap.UseFlagOptions(&ctrlZapOpts))) + ctrl.SetLogger(config.Logger(cfg.ZapDevelopment, cfg.ZapLogLevel)) tlsOpts := []func(*tls.Config){} webhookServer := webhook.NewServer(webhook.Options{ diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 8bbacbf..63019eb 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -19,11 +19,8 @@ resources: - ../rbac - ../manager - ../internalcert -# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in -# crd/kustomization.yaml - ../webhook -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. -#- ../certmanager + # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. #- ../prometheus @@ -39,107 +36,5 @@ patchesStrategicMerge: # crd/kustomization.yaml - manager_webhook_patch.yaml -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. -# Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. -# 'CERTMANAGER' needs to be enabled to use ca injection -#- webhookcainjection_patch.yaml - -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix. -# Uncomment the following replacements to add the cert-manager CA injection annotations -#replacements: -# - source: # Add cert-manager annotation to ValidatingWebhookConfiguration, MutatingWebhookConfiguration and CRDs -# kind: Certificate -# group: cert-manager.io -# version: v1 -# name: serving-cert # this name should match the one in certificate.yaml -# fieldPath: .metadata.namespace # namespace of the certificate CR -# targets: -# - select: -# kind: ValidatingWebhookConfiguration -# fieldPaths: -# - .metadata.annotations.[cert-manager.io/inject-ca-from] -# options: -# delimiter: '/' -# index: 0 -# create: true -# - select: -# kind: MutatingWebhookConfiguration -# fieldPaths: -# - .metadata.annotations.[cert-manager.io/inject-ca-from] -# options: -# delimiter: '/' -# index: 0 -# create: true -# - select: -# kind: CustomResourceDefinition -# fieldPaths: -# - .metadata.annotations.[cert-manager.io/inject-ca-from] -# options: -# delimiter: '/' -# index: 0 -# create: true -# - source: -# kind: Certificate -# group: cert-manager.io -# version: v1 -# name: serving-cert # this name should match the one in certificate.yaml -# fieldPath: .metadata.name -# targets: -# - select: -# kind: ValidatingWebhookConfiguration -# fieldPaths: -# - .metadata.annotations.[cert-manager.io/inject-ca-from] -# options: -# delimiter: '/' -# index: 1 -# create: true -# - select: -# kind: MutatingWebhookConfiguration -# fieldPaths: -# - .metadata.annotations.[cert-manager.io/inject-ca-from] -# options: -# delimiter: '/' -# index: 1 -# create: true -# - select: -# kind: CustomResourceDefinition -# fieldPaths: -# - .metadata.annotations.[cert-manager.io/inject-ca-from] -# options: -# delimiter: '/' -# index: 1 -# create: true -# - source: # Add cert-manager annotation to the webhook Service -# kind: Service -# version: v1 -# name: webhook-service -# fieldPath: .metadata.name # namespace of the service -# targets: -# - select: -# kind: Certificate -# group: cert-manager.io -# version: v1 -# fieldPaths: -# - .spec.dnsNames.0 -# - .spec.dnsNames.1 -# options: -# delimiter: '.' -# index: 0 -# create: true -# - source: -# kind: Service -# version: v1 -# name: webhook-service -# fieldPath: .metadata.namespace # namespace of the service -# targets: -# - select: -# kind: Certificate -# group: cert-manager.io -# version: v1 -# fieldPaths: -# - .spec.dnsNames.0 -# - .spec.dnsNames.1 -# options: -# delimiter: '.' -# index: 1 -# create: true +# Uncomment below for development +#- manager_config_dev_patch.yaml diff --git a/config/default/manager_config_dev_patch.yaml b/config/default/manager_config_dev_patch.yaml new file mode 100644 index 0000000..1438795 --- /dev/null +++ b/config/default/manager_config_dev_patch.yaml @@ -0,0 +1,15 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: controller-manager + namespace: system +spec: + template: + spec: + containers: + - name: manager + env: + - name: ZAP_DEVELOPMENT + value: "true" + - name: ZAP_LOG_LEVEL + value: "-5" diff --git a/config/default/manager_config_patch.yaml b/config/default/manager_config_patch.yaml index 4adab2e..34ffc17 100644 --- a/config/default/manager_config_patch.yaml +++ b/config/default/manager_config_patch.yaml @@ -13,8 +13,6 @@ spec: valueFrom: fieldRef: fieldPath: metadata.namespace - - name: ZAP_LOG_LEVEL - value: "-5" - name: "LEADER_ELECTION" value: "true" - name: METRICS_PROBE_BIND_ADDR diff --git a/internal/config/config.go b/internal/config/config.go index 067e3ed..4d9452d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -23,6 +23,7 @@ const ( HealthProbeBindAddrDefault = ":8081" SecureMetricsDefault = false ZapLogLevelDefault = 0 // zapcore.InfoLevel + ZapDevelopmentDefault = false ) // ConfigProvider provides the Kube Startup CPU Boost configuration @@ -48,6 +49,8 @@ type Config struct { SecureMetrics bool // ZapLogLevel determines the log level for the ZAP logger ZapLogLevel int + // ZapDevelopment determines if the ZAP logger is in development mode + ZapDevelopment bool } // LoadDefaults loads the default configuration values @@ -59,4 +62,5 @@ func (c *Config) LoadDefaults() { c.HealthProbeBindAddr = HealthProbeBindAddrDefault c.SecureMetrics = SecureMetricsDefault c.ZapLogLevel = ZapLogLevelDefault + c.ZapDevelopment = ZapDevelopmentDefault } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 8c63b1c..5b05d6c 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -50,5 +50,8 @@ var _ = Describe("Config", func() { It("has valid ZAP log level", func() { Expect(cfg.ZapLogLevel).To(Equal(config.ZapLogLevelDefault)) }) + It("has valid ZAP development ", func() { + Expect(cfg.ZapDevelopment).To(Equal(config.ZapDevelopmentDefault)) + }) }) }) diff --git a/internal/config/env_provider.go b/internal/config/env_provider.go index 334c046..b721a5d 100644 --- a/internal/config/env_provider.go +++ b/internal/config/env_provider.go @@ -28,6 +28,7 @@ const ( HealthProbeBindAddrEnvVar = "HEALTH_PROBE_BIND_ADDR" SecureMetricsEnvVar = "SECURE_METRICS" ZapLogLevelEnvVar = "ZAP_LOG_LEVEL" + ZapDevelopmentEnvVar = "ZAP_DEVELOPMENT" ) type LookupEnvFunc func(key string) (string, bool) @@ -91,6 +92,13 @@ func (p *EnvConfigProvider) LoadConfig() (*Config, error) { errs = append(errs, fmt.Errorf("%s value is not an int: %s", ZapLogLevelEnvVar, err)) } } + if v, ok := p.lookupFunc(ZapDevelopmentEnvVar); ok { + boolVal, err := strconv.ParseBool(v) + config.ZapDevelopment = boolVal + if err != nil { + errs = append(errs, fmt.Errorf("%s value is not a bool: %s", ZapDevelopmentEnvVar, err)) + } + } var err error if len(errs) > 0 { err = errors.Join(errs...) diff --git a/internal/config/env_provider_test.go b/internal/config/env_provider_test.go index 0c6c684..6de20d7 100644 --- a/internal/config/env_provider_test.go +++ b/internal/config/env_provider_test.go @@ -115,5 +115,13 @@ var _ = Describe("EnvProvider", func() { Expect(cfg.ZapLogLevel).To(Equal(logLevel)) }) }) + When("zap development variable is set", func() { + BeforeEach(func() { + lookupFuncMap[config.ZapDevelopmentEnvVar] = "true" + }) + It("has valid zap development", func() { + Expect(cfg.ZapDevelopment).To(BeTrue()) + }) + }) }) }) diff --git a/internal/config/logger.go b/internal/config/logger.go new file mode 100644 index 0000000..9418900 --- /dev/null +++ b/internal/config/logger.go @@ -0,0 +1,64 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "github.com/go-logr/logr" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + ctrlZap "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +func Logger(development bool, level int) logr.Logger { + ctrlZapOpts := ctrlZap.Options{ + Development: true, + Level: zap.NewAtomicLevelAt(zapcore.Level(level)), + } + if !development { + ctrlZapOpts.Development = false + ctrlZapOpts.EncoderConfigOptions = []ctrlZap.EncoderConfigOption{ + logEncoderOptionsProd(), + } + } + return ctrlZap.New(ctrlZap.UseFlagOptions(&ctrlZapOpts)) +} + +// logEncoderOptionsProd +func logEncoderOptionsProd() ctrlZap.EncoderConfigOption { + return func(ec *zapcore.EncoderConfig) { + ec.LevelKey = "severity" + ec.MessageKey = "message" + ec.TimeKey = "time" + ec.EncodeTime = zapcore.RFC3339TimeEncoder + ec.EncodeLevel = func(l zapcore.Level, enc zapcore.PrimitiveArrayEncoder) { + switch l { + case zapcore.InfoLevel: + enc.AppendString("info") + case zapcore.WarnLevel: + enc.AppendString("warning") + case zapcore.ErrorLevel: + enc.AppendString("error") + case zapcore.DPanicLevel: + enc.AppendString("critical") + case zapcore.PanicLevel: + enc.AppendString("alert") + case zapcore.FatalLevel: + enc.AppendString("emergency") + default: + enc.AppendString("debug") + } + } + } +}