From 1fe3678c6b065bd38f22d27f9437de0e1ac29085 Mon Sep 17 00:00:00 2001 From: Timofey Ilinykh Date: Tue, 23 Mar 2021 01:06:19 +0100 Subject: [PATCH] Improve initial customer logging experience - Wire logging settings via helm chart values - Set `simple` log format as default preference - Use `NO_COLOR` env variable for color settings as per https://no-color.org/ - Disable `NO_COLOR` by default, but enforce for helm deployments - Use `RFC3339` as datetime format for all loggers --- Makefile | 7 ++++++- chart/k8gb/templates/operator.yaml | 6 ++++++ chart/k8gb/values.yaml | 3 +++ controllers/depresolver/depresolver_config.go | 6 +++--- controllers/depresolver/depresolver_test.go | 16 ++++++++-------- controllers/logging/log.go | 17 +++-------------- 6 files changed, 29 insertions(+), 26 deletions(-) diff --git a/Makefile b/Makefile index 0981fadb24..f53b303b91 100644 --- a/Makefile +++ b/Makefile @@ -27,6 +27,7 @@ HELM_ARGS ?= K8GB_COREDNS_IP ?= kubectl get svc k8gb-coredns -n k8gb -o custom-columns='IP:spec.clusterIP' --no-headers CLUSTER_GSLB2_HELM_ARGS ?= --set k8gb.clusterGeoTag='us' --set k8gb.extGslbClustersGeoTags='eu' --set k8gb.hostAlias.hostnames='{gslb-ns-cloud-example-com-eu.example.com}' LOG_FORMAT ?= simple +LOG_LEVEL ?= debug CONTROLLER_GEN_VERSION ?= v0.4.1 GOLIC_VERSION ?= v0.4.7 @@ -125,7 +126,9 @@ deploy-to-AbsaOSS-k3d-action: deploy-gslb-operator: ## Deploy k8gb operator kubectl apply -f deploy/namespace.yaml cd chart/k8gb && helm dependency update - helm -n k8gb upgrade -i k8gb chart/k8gb -f $(VALUES_YAML) $(HELM_ARGS) + helm -n k8gb upgrade -i k8gb chart/k8gb -f $(VALUES_YAML) $(HELM_ARGS) \ + --set k8gb.log.format=$(LOG_FORMAT) + --set k8gb.log.level=$(LOG_LEVEL) .PHONY: deploy-gslb-cr deploy-gslb-cr: ## Apply Gslb Custom Resources @@ -305,6 +308,8 @@ define deploy-local-cluster --set k8gb.hostAlias.enabled=true \ --set k8gb.hostAlias.ip="`$(call get-host-alias-ip,k3d-$1,k3d-$2)`" \ --set k8gb.imageTag=$3 $4 + --set k8gb.log.format=$(LOG_FORMAT) + --set k8gb.log.level=$(LOG_LEVEL) @echo "\n$(YELLOW)Deploy Ingress $(NC)" helm repo add --force-update stable https://charts.helm.sh/stable diff --git a/chart/k8gb/templates/operator.yaml b/chart/k8gb/templates/operator.yaml index 069f60914f..30fcd82ed8 100644 --- a/chart/k8gb/templates/operator.yaml +++ b/chart/k8gb/templates/operator.yaml @@ -115,3 +115,9 @@ spec: - name: COREDNS_EXPOSED value: "true" {{ end }} + - name: LOG_FORMAT + value: {{ quote .Values.k8gb.log.format }} + - name: LOG_LEVEL + value: {{ quote .Values.k8gb.log.level }} + - name: NO_COLOR + value: "true" diff --git a/chart/k8gb/values.yaml b/chart/k8gb/values.yaml index bb1b4dc4b5..45950c7194 100644 --- a/chart/k8gb/values.yaml +++ b/chart/k8gb/values.yaml @@ -22,6 +22,9 @@ k8gb: - "gslb-ns-cloud-example-com-us.example.com" reconcileRequeueSeconds: 30 exposeCoreDNS: false # Create Service type LoadBalancer to expose CoreDNS + log: + format: simple # log format (simple,json) + level: info # log level (panic,fatal,error,warn,info,debug,trace) externaldns: image: k8s.gcr.io/external-dns/external-dns:v0.7.5 diff --git a/controllers/depresolver/depresolver_config.go b/controllers/depresolver/depresolver_config.go index c126fe7200..688f4fad99 100644 --- a/controllers/depresolver/depresolver_config.go +++ b/controllers/depresolver/depresolver_config.go @@ -49,7 +49,7 @@ const ( CoreDNSExposedKey = "COREDNS_EXPOSED" LogLevelKey = "LOG_LEVEL" LogFormatKey = "LOG_FORMAT" - LogNoColorKey = "LOG_NO_COLOR" + LogNoColorKey = "NO_COLOR" ) // ResolveOperatorConfig executes once. It reads operator's configuration @@ -77,8 +77,8 @@ func (dr *DependencyResolver) ResolveOperatorConfig() (*Config, error) { dr.config.Override.FakeDNSEnabled = env.GetEnvAsBoolOrFallback(OverrideWithFakeDNSKey, false) dr.config.Override.FakeInfobloxEnabled = env.GetEnvAsBoolOrFallback(OverrideFakeInfobloxKey, false) dr.config.Log.Level, _ = zerolog.ParseLevel(strings.ToLower(env.GetEnvAsStringOrFallback(LogLevelKey, zerolog.InfoLevel.String()))) - dr.config.Log.Format = parseLogOutputFormat(strings.ToLower(env.GetEnvAsStringOrFallback(LogFormatKey, JSONFormat.String()))) - dr.config.Log.NoColor = env.GetEnvAsBoolOrFallback(LogNoColorKey, true) + dr.config.Log.Format = parseLogOutputFormat(strings.ToLower(env.GetEnvAsStringOrFallback(LogFormatKey, SimpleFormat.String()))) + dr.config.Log.NoColor = env.GetEnvAsBoolOrFallback(LogNoColorKey, false) dr.errorConfig = dr.validateConfig(dr.config) dr.config.EdgeDNSType = getEdgeDNSType(dr.config) }) diff --git a/controllers/depresolver/depresolver_test.go b/controllers/depresolver/depresolver_test.go index a95310cce6..7e4089b2ef 100644 --- a/controllers/depresolver/depresolver_test.go +++ b/controllers/depresolver/depresolver_test.go @@ -62,7 +62,7 @@ var predefinedConfig = Config{ false, }, Log: Log{ - Format: JSONFormat, + Format: SimpleFormat, }, } @@ -171,8 +171,8 @@ func TestResolveConfigWithoutEnvVarsSet(t *testing.T) { defaultConfig.EdgeDNSType = DNSTypeNoEdgeDNS defaultConfig.ExtClustersGeoTags = []string{} defaultConfig.Log.Level = zerolog.InfoLevel - defaultConfig.Log.Format = JSONFormat - defaultConfig.Log.NoColor = true + defaultConfig.Log.Format = SimpleFormat + defaultConfig.Log.NoColor = false resolver := NewDependencyResolver() // act config, err := resolver.ResolveOperatorConfig() @@ -1104,7 +1104,7 @@ func TestResolveLoggerUseDefaultValue(t *testing.T) { defer cleanup() expected := predefinedConfig expected.Log.Level = zerolog.InfoLevel - expected.Log.NoColor = true + expected.Log.NoColor = false // act // assert arrangeVariablesAndAssert(t, expected, assert.NoError, LogLevelKey, LogFormatKey, LogNoColorKey) @@ -1185,7 +1185,7 @@ func TestResolveLoggerLevelWithInvalidValue(t *testing.T) { // assert assert.Error(t, err) assert.Equal(t, zerolog.NoLevel, config.Log.Level) - assert.Equal(t, JSONFormat, config.Log.Format) + assert.Equal(t, SimpleFormat, config.Log.Format) } func TestResolveLoggerNoColorInvalidValue(t *testing.T) { @@ -1198,7 +1198,7 @@ func TestResolveLoggerNoColorInvalidValue(t *testing.T) { config, err := resolver.ResolveOperatorConfig() // assert assert.NoError(t, err) - assert.Equal(t, true, config.Log.NoColor) + assert.Equal(t, false, config.Log.NoColor) } func TestResolveLoggerOutputWithInvalidValue(t *testing.T) { @@ -1225,7 +1225,7 @@ func TestResolveLoggerWithEmptyValues(t *testing.T) { config, err := resolver.ResolveOperatorConfig() // assert assert.NoError(t, err) - assert.Equal(t, JSONFormat, config.Log.Format) + assert.Equal(t, SimpleFormat, config.Log.Format) assert.Equal(t, zerolog.InfoLevel, config.Log.Level) } @@ -1241,7 +1241,7 @@ func TestResolveLoggerEmptyValues(t *testing.T) { // assert assert.NoError(t, err) assert.Equal(t, zerolog.InfoLevel, config.Log.Level) - assert.Equal(t, JSONFormat, config.Log.Format) + assert.Equal(t, SimpleFormat, config.Log.Format) } // arrangeVariablesAndAssert sets string environment variables and asserts `expected` argument with diff --git a/controllers/logging/log.go b/controllers/logging/log.go index 69c7acd95f..4d023bece7 100644 --- a/controllers/logging/log.go +++ b/controllers/logging/log.go @@ -42,9 +42,8 @@ func newLogger(config *depresolver.Config) *loggerFactory { // In such case returns default logger func (l *loggerFactory) get() zerolog.Logger { var logger zerolog.Logger - var dt = time.RFC822Z if l.log.Format == depresolver.NoFormat { - l.log.Format = depresolver.JSONFormat + l.log.Format = depresolver.SimpleFormat } if l.log.Level == zerolog.NoLevel { l.log.Level = zerolog.InfoLevel @@ -52,31 +51,21 @@ func (l *loggerFactory) get() zerolog.Logger { // We can retrieve stack in case of pkg/errors zerolog.ErrorStackMarshaler = pkgerrors.MarshalStack zerolog.SetGlobalLevel(l.log.Level) - if l.log.Level <= zerolog.DebugLevel { - dt = "15:04:05" - } switch l.log.Format { case depresolver.JSONFormat: - // JSONFormat time format as seconds timestamp - zerolog.TimeFieldFormat = zerolog.TimeFormatUnix - // shortening field names timestamp=>@t, level=>@l , message=>@m, caller=>@c - zerolog.TimestampFieldName = "@t" - zerolog.LevelFieldName = "@l" - zerolog.MessageFieldName = "@m" - zerolog.CallerFieldName = "@c" logger = zerolog.New(os.Stdout). With(). Caller(). Timestamp(). Logger() case depresolver.SimpleFormat: - logger = zerolog.New(zerolog.ConsoleWriter{Out: os.Stderr, TimeFormat: dt, NoColor: l.log.NoColor}). + logger = zerolog.New(zerolog.ConsoleWriter{Out: os.Stderr, TimeFormat: time.RFC3339, NoColor: l.log.NoColor}). With(). Caller(). Timestamp(). Logger() } logger.Info().Msg("Logger configured") - logger.Debug().Msgf("Logger settings: [%s, %s]", l.log.Format, l.log.Level) + logger.Debug().Msgf("Logger settings: [format=%s, level=%s]", l.log.Format, l.log.Level) return logger }