Add LogLevel & LogFormat flags (#6520)

Author: AlexFenlon

charts/nginx-ingress/templates/_helpers.tpl

@@ -244,7 +244,8 @@ Build the args for the service binary.
 - -health-status={{ .Values.controller.healthStatus }}
 - -health-status-uri={{ .Values.controller.healthStatusURI }}
 - -nginx-debug={{ .Values.controller.nginxDebug }}
-- -v={{ .Values.controller.logLevel }}
+- -log-level={{ .Values.controller.logLevel }}
+- -log-format={{ .Values.controller.logFormat }}
 - -nginx-status={{ .Values.controller.nginxStatus.enable }}
 {{- if .Values.controller.nginxStatus.enable }}
 - -nginx-status-port={{ .Values.controller.nginxStatus.port }}

charts/nginx-ingress/values.schema.json

@@ -486,17 +486,32 @@
           ]
         },
         "logLevel": {
-          "type": "integer",
-          "default": 1,
+          "type": "string",
+          "default": "info",
           "title": "The logLevel of the Ingress Controller",
           "enum": [
-            0,
-            1,
-            2,
-            3
+            "trace",
+            "debug",
+            "info",
+            "warning",
+            "error",
+            "fatal"
+          ],
+          "examples": [
+            "info"
+          ]
+        },
+        "logFormat": {
+          "type": "string",
+          "default": "glog",
+          "title": "The logFormat of the Ingress Controller",
+          "enum": [
+            "glog",
+            "json",
+            "text"
           ],
           "examples": [
-            1
+            "json"
           ]
         },
         "customPorts": {
@@ -1674,7 +1689,7 @@
           "hostNetwork": false,
           "nginxDebug": false,
           "shareProcessNamespace": false,
-          "logLevel": 1,
+          "logLevel": "info",
           "customPorts": [],
           "image": {
             "repository": "nginx/nginx-ingress",
@@ -2214,7 +2229,7 @@
         },
         "hostNetwork": false,
         "nginxDebug": false,
-        "logLevel": 1,
+        "logLevel": "info",
         "customPorts": [],
         "image": {
           "repository": "nginx/nginx-ingress",

charts/nginx-ingress/values.yaml

@@ -123,8 +123,11 @@ controller:
   ## Share process namespace between containers in the Ingress Controller pod.
   shareProcessNamespace: false
 
-  ## The log level of the Ingress Controller.
-  logLevel: 1
+  ## The log level of the Ingress Controller. Options include: trace, debug, info, warning, error, fatal
+  logLevel: info
+
+  ## Sets the log format of Ingress Controller. Options include: glog, json, text
+  logFormat: glog
 
   ## A list of custom ports to expose on the NGINX Ingress Controller pod. Follows the conventional Kubernetes yaml syntax for container ports.
   customPorts: []

cmd/nginx-ingress/flags.go

@@ -19,6 +19,8 @@ const (
 	dynamicWeightChangesParam     = "weight-changes-dynamic-reload"
 	appProtectLogLevelDefault     = "fatal"
 	appProtectEnforcerAddrDefault = "127.0.0.1:50000"
+	logLevelDefault               = "info"
+	logFormatDefault              = "glog"
 )
 
 var (
@@ -208,6 +210,11 @@ var (
 
 	enableTelemetryReporting = flag.Bool("enable-telemetry-reporting", true, "Enable gathering and reporting of product related telemetry.")
 
+	logFormat = flag.String("log-format", logFormatDefault, "Set log format to either glog, text, or json.")
+
+	logLevel = flag.String("log-level", logLevelDefault,
+		`Sets log level for Ingress Controller. Allowed values: fatal, error, warning, info, debug, trace.`)
+
 	enableDynamicWeightChangesReload = flag.Bool(dynamicWeightChangesParam, false, "Enable changing weights of split clients without reloading NGINX. Requires -nginx-plus")
 
 	startupCheckFn func() error
@@ -223,6 +230,16 @@ func parseFlags() {
 }
 
 func initValidate() {
+	logFormatValidationError := validateLogFormat(*logFormat)
+	if logFormatValidationError != nil {
+		glog.Warningf("Invalid log format: %s. Valid options are: glog, text, json. Falling back to default: %s", *logFormat, logFormatDefault)
+	}
+
+	logLevelValidationError := validateLogLevel(*logLevel)
+	if logLevelValidationError != nil {
+		glog.Warningf("Invalid log level: %s. Valid options are: trace, debug, info, warning, error, fatal. Falling back to default: %s", *logLevel, logLevelDefault)
+	}
+
 	if *enableLatencyMetrics && !*enablePrometheusMetrics {
 		glog.Warning("enable-latency-metrics flag requires enable-prometheus-metrics, latency metrics will not be collected")
 		*enableLatencyMetrics = false
@@ -347,8 +364,8 @@ func mustValidateFlags() {
 	}
 
 	if *appProtectLogLevel != appProtectLogLevelDefault && *appProtect && *nginxPlus {
-		logLevelValidationError := validateAppProtectLogLevel(*appProtectLogLevel)
-		if logLevelValidationError != nil {
+		appProtectlogLevelValidationError := validateLogLevel(*appProtectLogLevel)
+		if appProtectlogLevelValidationError != nil {
 			glog.Fatalf("Invalid value for app-protect-log-level: %v", *appProtectLogLevel)
 		}
 	}
@@ -443,8 +460,8 @@ func validatePort(port int) error {
 	return nil
 }
 
-// validateAppProtectLogLevel makes sure a given logLevel is one of the allowed values
-func validateAppProtectLogLevel(logLevel string) error {
+// validateLogLevel makes sure a given logLevel is one of the allowed values
+func validateLogLevel(logLevel string) error {
 	switch strings.ToLower(logLevel) {
 	case
 		"fatal",
@@ -455,7 +472,16 @@ func validateAppProtectLogLevel(logLevel string) error {
 		"trace":
 		return nil
 	}
-	return fmt.Errorf("invalid App Protect log level: %v", logLevel)
+	return fmt.Errorf("invalid log level: %v", logLevel)
+}
+
+// validateLogFormat makes sure a given logFormat is one of the allowed values
+func validateLogFormat(logFormat string) error {
+	switch strings.ToLower(logFormat) {
+	case "glog", "json", "text":
+		return nil
+	}
+	return fmt.Errorf("invalid log format: %v", logFormat)
 }
 
 // parseNginxStatusAllowCIDRs converts a comma separated CIDR/IP address string into an array of CIDR/IP addresses.

cmd/nginx-ingress/flags_test.go

@@ -125,17 +125,17 @@ func TestValidateLocation(t *testing.T) {
 	}
 }
 
-func TestValidateAppProtectLogLevel(t *testing.T) {
+func TestValidateLogLevel(t *testing.T) {
 	badLogLevels := []string{
 		"",
 		"critical",
 		"none",
 		"info;",
 	}
 	for _, badLogLevel := range badLogLevels {
-		err := validateAppProtectLogLevel(badLogLevel)
+		err := validateLogLevel(badLogLevel)
 		if err == nil {
-			t.Errorf("validateAppProtectLogLevel(%v) returned no error when it should have returned an error", badLogLevel)
+			t.Errorf("validateLogLevel(%v) returned no error when it should have returned an error", badLogLevel)
 		}
 	}
 
@@ -148,9 +148,9 @@ func TestValidateAppProtectLogLevel(t *testing.T) {
 		"trace",
 	}
 	for _, goodLogLevel := range goodLogLevels {
-		err := validateAppProtectLogLevel(goodLogLevel)
+		err := validateLogLevel(goodLogLevel)
 		if err != nil {
-			t.Errorf("validateAppProtectLogLevel(%v) returned an error when it should have returned no error: %v", goodLogLevel, err)
+			t.Errorf("validateLogLevel(%v) returned an error when it should have returned no error: %v", goodLogLevel, err)
 		}
 	}
 }
@@ -172,3 +172,30 @@ func TestValidateNamespaces(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateLogFormat(t *testing.T) {
+	badLogFormats := []string{
+		"",
+		"jason",
+		"txt",
+		"gloog",
+	}
+	for _, badLogFormat := range badLogFormats {
+		err := validateLogFormat(badLogFormat)
+		if err == nil {
+			t.Errorf("validateLogFormat(%v) returned no error when it should have returned an error", badLogFormat)
+		}
+	}
+
+	goodLogFormats := []string{
+		"json",
+		"text",
+		"glog",
+	}
+	for _, goodLogFormat := range goodLogFormats {
+		err := validateLogFormat(goodLogFormat)
+		if err != nil {
+			t.Errorf("validateLogFormat(%v) returned an error when it should have returned no error: %v", goodLogFormat, err)
+		}
+	}
+}

cmd/nginx-ingress/main.go

@@ -3,6 +3,8 @@ package main
 import (
 	"context"
 	"fmt"
+	"io"
+	"log/slog"
 	"net"
 	"net/http"
 	"os"
@@ -41,12 +43,23 @@ import (
 
 	kitlog "github.com/go-kit/log"
 	"github.com/go-kit/log/level"
+
+	nic_logger "github.com/nginxinc/kubernetes-ingress/internal/logger"
+	nic_glog "github.com/nginxinc/kubernetes-ingress/internal/logger/glog"
 )
 
 // Injected during build
 var (
 	version           string
 	telemetryEndpoint string
+	logLevels         = map[string]slog.Level{
+		"trace":   nic_glog.LevelTrace,
+		"debug":   nic_glog.LevelDebug,
+		"info":    nic_glog.LevelInfo,
+		"warning": nic_glog.LevelWarning,
+		"error":   nic_glog.LevelError,
+		"fatal":   nic_glog.LevelFatal,
+	}
 )
 
 const (
@@ -63,6 +76,9 @@ func main() {
 	commitHash, commitTime, dirtyBuild := getBuildInfo()
 	fmt.Printf("NGINX Ingress Controller Version=%v Commit=%v Date=%v DirtyState=%v Arch=%v/%v Go=%v\n", version, commitHash, commitTime, dirtyBuild, runtime.GOOS, runtime.GOARCH, runtime.Version())
 	parseFlags()
+	ctx := initLogger(*logFormat, logLevels[*logLevel], os.Stdout)
+	_ = nic_logger.LoggerFromContext(ctx)
+
 	initValidate()
 	parsedFlags := os.Args[1:]
 
@@ -877,3 +893,25 @@ func updateSelfWithVersionInfo(kubeClient *kubernetes.Clientset, version, appPro
 		glog.Errorf("Failed to update pod labels after %d attempts", maxRetries)
 	}
 }
+
+func initLogger(logFormat string, level slog.Level, out io.Writer) context.Context {
+	programLevel := new(slog.LevelVar) // Info by default
+	var h slog.Handler
+	switch {
+	case logFormat == "glog":
+		h = nic_glog.New(out, &nic_glog.Options{Level: programLevel})
+	case logFormat == "json":
+		h = slog.NewJSONHandler(out, &slog.HandlerOptions{Level: programLevel})
+	case logFormat == "text":
+		h = slog.NewTextHandler(out, &slog.HandlerOptions{Level: programLevel})
+	default:
+		h = nic_glog.New(out, &nic_glog.Options{Level: programLevel})
+	}
+	l := slog.New(h)
+	slog.SetDefault(l)
+	c := context.Background()
+
+	programLevel.Set(level)
+
+	return nic_logger.ContextWithLogger(c, l)
+}

cmd/nginx-ingress/main_test.go

@@ -0,0 +1,48 @@
+package main
+
+import (
+	"bytes"
+	"regexp"
+	"testing"
+
+	nic_logger "github.com/nginxinc/kubernetes-ingress/internal/logger"
+	nic_glog "github.com/nginxinc/kubernetes-ingress/internal/logger/glog"
+)
+
+func TestLogFormats(t *testing.T) {
+	testCases := []struct {
+		name   string
+		format string
+		wantre string
+	}{
+		{
+			name:   "glog format message",
+			format: "glog",
+			wantre: `^I\d{8}\s\d+:\d+:\d+.\d{6}\s+\d+\s\w+\.go:\d+\]\s.*\s$`,
+		},
+		{
+			name:   "json format message",
+			format: "json",
+			wantre: `^{"time":"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d+.*","level":"INFO","msg":".*}`,
+		},
+		{
+			name:   "text format message",
+			format: "text",
+			wantre: `^time=\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}.\d+.*level=\w+\smsg=\w+`,
+		},
+	}
+	t.Parallel()
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var buf bytes.Buffer
+			ctx := initLogger(tc.format, nic_glog.LevelInfo, &buf)
+			l := nic_logger.LoggerFromContext(ctx)
+			l.Log(ctx, nic_glog.LevelInfo, "test")
+			got := buf.String()
+			re := regexp.MustCompile(tc.wantre)
+			if !re.MatchString(got) {
+				t.Errorf("\ngot:\n%q\nwant:\n%q", got, tc.wantre)
+			}
+		})
+	}
+}

docs/content/configuration/global-configuration/command-line-arguments.md

@@ -324,6 +324,26 @@ Path to the TransportServer NGINX configuration template for a TransportServer r
 
 Log level for V logs.
 
+<a name="cmdoption-log-level"></a>
+
+---
+
+### -log-level `<string>`
+
+Log level for Ingress Controller logs. Allowed values: fatal, error, warn, info, debug, trace.
+
+- Default is `info`.
+
+<a name="cmdoption-log-format"></a>
+
+---
+
+### -log-format `<string>`
+
+Log format for Ingress Controller logs. Allowed values: glog, json, text.
+
+- Default is `glog`.
+
 <a name="cmdoption-version"></a>
 
 ---

docs/content/installation/installing-nic/installation-with-helm.md

@@ -309,7 +309,8 @@ The following tables lists the configurable parameters of the NGINX Ingress Cont
 | **controller.hostNetwork** | Enables the Ingress Controller pods to use the host's network namespace. | false |
 | **controller.dnsPolicy** | DNS policy for the Ingress Controller pods. | ClusterFirst |
 | **controller.nginxDebug** | Enables debugging for NGINX. Uses the `nginx-debug` binary. Requires `error-log-level: debug` in the ConfigMap via `controller.config.entries`. | false |
-| **controller.logLevel** | The log level of the Ingress Controller. | 1 |
+| **controller.logLevel** | The log level of the Ingress Controller. | info |
+| **controller.logFormat** | The log format of the Ingress Controller. | glog |
 | **controller.image.digest** | The image digest of the Ingress Controller. | None |
 | **controller.image.repository** | The image repository of the Ingress Controller. | nginx/nginx-ingress |
 | **controller.image.tag** | The tag of the Ingress Controller image. | {{< nic-version >}} |