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

@@ -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

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"

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

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)
 	})

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

controllers/logging/log.go

@@ -42,41 +42,30 @@ 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
 	}
 	// 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
 }