Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
3c0aea9
refactor to prepare for struct logs
AlexFenlon Sep 25, 2024
1a2dd82
refactor to solve comments
AlexFenlon Sep 25, 2024
711305c
Merge branch 'main' into chore/flagsgo-refactor-for-struct-logs
AlexFenlon Sep 25, 2024
3cdd96a
Merge remote-tracking branch 'refs/remotes/origin/main' into chore/fl…
AlexFenlon Sep 25, 2024
402891c
fix lint
AlexFenlon Sep 25, 2024
2d594ca
Merge remote-tracking branch 'origin/chore/flagsgo-refactor-for-struc…
AlexFenlon Sep 25, 2024
ea033cb
Merge remote-tracking branch 'origin/main' into feat/logformat-flag
AlexFenlon Sep 26, 2024
f05bdb9
Add Flags for loglevel and logformat.
AlexFenlon Sep 26, 2024
8c5a5f9
Add tests
AlexFenlon Sep 26, 2024
90feb34
add tests for log format
pdabelf5 Sep 26, 2024
7187be0
fix tests
AlexFenlon Sep 26, 2024
63eb0cc
add logformat comments and in schema
AlexFenlon Sep 26, 2024
0956ac4
fix spacing
AlexFenlon Sep 26, 2024
27c6075
remove debug
AlexFenlon Sep 26, 2024
a049cf2
move logformat to go near loglevel as they are related - address comment
AlexFenlon Sep 27, 2024
37ceb7f
Merge remote-tracking branch 'refs/remotes/origin/main' into feat/log…
AlexFenlon Sep 27, 2024
23b4e49
update logFormat test case names
AlexFenlon Sep 27, 2024
b97840b
add docs
AlexFenlon Sep 27, 2024
b2a3a79
change fatal messages to warning, update values comment, add glog as …
AlexFenlon Sep 27, 2024
99219c6
Merge branch 'main' into feat/logformat-flag
AlexFenlon Sep 27, 2024
f689e48
Merge branch 'main' into feat/logformat-flag
jjngx Sep 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion charts/nginx-ingress/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
33 changes: 24 additions & 9 deletions charts/nginx-ingress/values.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -1674,7 +1689,7 @@
"hostNetwork": false,
"nginxDebug": false,
"shareProcessNamespace": false,
"logLevel": 1,
"logLevel": "info",
"customPorts": [],
"image": {
"repository": "nginx/nginx-ingress",
Expand Down Expand Up @@ -2214,7 +2229,7 @@
},
"hostNetwork": false,
"nginxDebug": false,
"logLevel": 1,
"logLevel": "info",
"customPorts": [],
"image": {
"repository": "nginx/nginx-ingress",
Expand Down
7 changes: 5 additions & 2 deletions charts/nginx-ingress/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: []
Expand Down
36 changes: 31 additions & 5 deletions cmd/nginx-ingress/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ const (
dynamicWeightChangesParam = "weight-changes-dynamic-reload"
appProtectLogLevelDefault = "fatal"
appProtectEnforcerAddrDefault = "127.0.0.1:50000"
logLevelDefault = "info"
logFormatDefault = "glog"
)

var (
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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",
Expand All @@ -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.
Expand Down
37 changes: 32 additions & 5 deletions cmd/nginx-ingress/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand All @@ -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)
}
}
}
Expand All @@ -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)
}
}
}
38 changes: 38 additions & 0 deletions cmd/nginx-ingress/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package main
import (
"context"
"fmt"
"io"
"log/slog"
"net"
"net/http"
"os"
Expand Down Expand Up @@ -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 (
Expand All @@ -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:]

Expand Down Expand Up @@ -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)
}
48 changes: 48 additions & 0 deletions cmd/nginx-ingress/main_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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>

---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 >}} |
Expand Down
Loading