Add support for JSON logging

Introduce logging-format driver option for the controller and node pods to set the log format. Permitted formats: text (default), json. Migrate to Structured Logging. Deprecate logtostderr flag. Signed-off-by: Eddie Torres <[email protected]>
kubernetes-sigs · Jan 18, 2023 · ad25982 · ad25982
1 parent 78c845b
commit ad25982
Show file tree

Hide file tree

Showing 34 changed files with 214 additions and 154 deletions.
diff --git a/charts/aws-ebs-csi-driver/templates/controller.yaml b/charts/aws-ebs-csi-driver/templates/controller.yaml
@@ -87,7 +87,9 @@ spec:
             {{- if .Values.controller.sdkDebugLog }}
             - --aws-sdk-debug-log=true
             {{- end}}
-            - --logtostderr
+            {{- with .Values.controller.loggingFormat }}
+            - --logging-format={{ . }}
+            {{- end }}
             - --v={{ .Values.controller.logLevel }}
             {{- range .Values.controller.additionalArgs }}
             - {{ . }}

diff --git a/charts/aws-ebs-csi-driver/templates/node-windows.yaml b/charts/aws-ebs-csi-driver/templates/node-windows.yaml
@@ -54,7 +54,9 @@ spec:
             {{- with .Values.node.volumeAttachLimit }}
             - --volume-attach-limit={{ . }}
             {{- end }}
-            - --logtostderr
+            {{- with .Values.node.loggingFormat }}
+            - --logging-format={{ . }}
+            {{- end }}
             - --v={{ .Values.node.logLevel }}
           env:
             - name: CSI_ENDPOINT

diff --git a/charts/aws-ebs-csi-driver/templates/node.yaml b/charts/aws-ebs-csi-driver/templates/node.yaml
@@ -58,7 +58,9 @@ spec:
             {{- with .Values.node.volumeAttachLimit }}
             - --volume-attach-limit={{ . }}
             {{- end }}
-            - --logtostderr
+            {{- with .Values.node.loggingFormat }}
+            - --logging-format={{ . }}
+            {{- end }}
             - --v={{ .Values.node.logLevel }}
           env:
             - name: CSI_ENDPOINT

diff --git a/charts/aws-ebs-csi-driver/values.yaml b/charts/aws-ebs-csi-driver/values.yaml
@@ -112,6 +112,7 @@ fullnameOverride:
 controller:
   additionalArgs: []
   sdkDebugLog: false
+  loggingFormat: text
   affinity:
     nodeAffinity:
       preferredDuringSchedulingIgnoredDuringExecution:
@@ -238,6 +239,7 @@ node:
   env: []
   envFrom: []
   kubeletPath: /var/lib/kubelet
+  loggingFormat: text
   logLevel: 2
   priorityClassName:
   affinity:

diff --git a/cmd/main.go b/cmd/main.go
@@ -17,18 +17,26 @@ limitations under the License.
 package main
 
 import (
-	"flag"
 	"net/http"
 
+	flag "github.com/spf13/pflag"
+
 	"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud"
 	"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver"
+	logsapi "k8s.io/component-base/logs/api/v1"
+	json "k8s.io/component-base/logs/json"
 	"k8s.io/component-base/metrics/legacyregistry"
 
 	"k8s.io/klog/v2"
 )
 
 func main() {
 	fs := flag.NewFlagSet("aws-ebs-csi-driver", flag.ExitOnError)
+
+	if err := logsapi.RegisterLogFormat(logsapi.JSONLogFormat, json.Factory{}, logsapi.LoggingBetaOptions); err != nil {
+		klog.ErrorS(err, "failed to register JSON log format")
+	}
+
 	options := GetOptions(fs)
 
 	cloud.RegisterMetrics()
@@ -38,7 +46,8 @@ func main() {
 		go func() {
 			err := http.ListenAndServe(options.ServerOptions.HttpEndpoint, mux)
 			if err != nil {
-				klog.Fatalf("failed to listen & serve metrics from %q: %v", options.ServerOptions.HttpEndpoint, err)
+				klog.ErrorS(err, "failed to listen & serve metrics", "endpoint", options.ServerOptions.HttpEndpoint)
+				klog.FlushAndExit(klog.ExitFlushTimeout, 1)
 			}
 		}()
 	}
@@ -54,9 +63,11 @@ func main() {
 		driver.WithWarnOnInvalidTag(options.ControllerOptions.WarnOnInvalidTag),
 	)
 	if err != nil {
-		klog.Fatalln(err)
+		klog.ErrorS(err, "failed to create driver")
+		klog.FlushAndExit(klog.ExitFlushTimeout, 1)
 	}
 	if err := drv.Run(); err != nil {
-		klog.Fatalln(err)
+		klog.ErrorS(err, "failed to run driver")
+		klog.FlushAndExit(klog.ExitFlushTimeout, 1)
 	}
 }
diff --git a/cmd/options.go b/cmd/options.go
@@ -17,14 +17,17 @@ limitations under the License.
 package main
 
 import (
-	"flag"
 	"fmt"
 	"os"
 	"strings"
 
+	flag "github.com/spf13/pflag"
+
 	"github.com/kubernetes-sigs/aws-ebs-csi-driver/cmd/options"
 	"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver"
 
+	"k8s.io/component-base/featuregate"
+	logsapi "k8s.io/component-base/logs/api/v1"
 	"k8s.io/klog/v2"
 )
 
@@ -40,11 +43,14 @@ type Options struct {
 // used for testing
 var osExit = os.Exit
 
+var featureGate = featuregate.NewFeatureGate()
+
 // GetOptions parses the command line options and returns a struct that contains
 // the parsed options.
 func GetOptions(fs *flag.FlagSet) *Options {
 	var (
-		version = fs.Bool("version", false, "Print the version and exit.")
+		version  = fs.Bool("version", false, "Print the version and exit.")
+		toStderr = fs.Bool("logtostderr", false, "log to standard error instead of files. DEPRECATED: will be removed in a future release.")
 
 		args = os.Args[1:]
 		mode = driver.AllMode
@@ -55,7 +61,15 @@ func GetOptions(fs *flag.FlagSet) *Options {
 	)
 
 	serverOptions.AddFlags(fs)
-	klog.InitFlags(fs)
+
+	c := logsapi.NewLoggingConfiguration()
+
+	err := logsapi.AddFeatureGates(featureGate)
+	if err != nil {
+		klog.ErrorS(err, "failed to add feature gates")
+	}
+
+	logsapi.AddFlags(c, fs)
 
 	if len(os.Args) > 1 {
 		cmd := os.Args[1]
@@ -87,17 +101,25 @@ func GetOptions(fs *flag.FlagSet) *Options {
 		}
 	}
 
-	if err := fs.Parse(args); err != nil {
+	if err = fs.Parse(args); err != nil {
 		panic(err)
 	}
 
+	err = logsapi.ValidateAndApply(c, featureGate)
+	if err != nil {
+		klog.ErrorS(err, "failed to validate and apply logging configuration")
+	}
+
 	if *version {
 		info, err := driver.GetVersionJSON()
 		if err != nil {
-			klog.Fatalln(err)
+			klog.ErrorS(err, "failed to get version", "version", info)
+			klog.FlushAndExit(klog.ExitFlushTimeout, 1)
 		}
-		fmt.Println(info)
-		osExit(0)
+	}
+
+	if *toStderr {
+		klog.SetOutput(os.Stderr)
 	}
 
 	return &Options{

diff --git a/cmd/options/controller_options.go b/cmd/options/controller_options.go
@@ -17,7 +17,7 @@ limitations under the License.
 package options
 
 import (
-	"flag"
+	flag "github.com/spf13/pflag"
 
 	cliflag "k8s.io/component-base/cli/flag"
 )

diff --git a/cmd/options/controller_options_test.go b/cmd/options/controller_options_test.go
@@ -17,8 +17,9 @@ limitations under the License.
 package options
 
 import (
-	"flag"
 	"testing"
+
+	flag "github.com/spf13/pflag"
 )
 
 func TestControllerOptions(t *testing.T) {

diff --git a/cmd/options/node_options.go b/cmd/options/node_options.go
@@ -17,7 +17,7 @@ limitations under the License.
 package options
 
 import (
-	"flag"
+	flag "github.com/spf13/pflag"
 )
 
 // NodeOptions contains options and configuration settings for the node service.

diff --git a/cmd/options/node_options_test.go b/cmd/options/node_options_test.go
@@ -17,8 +17,9 @@ limitations under the License.
 package options
 
 import (
-	"flag"
 	"testing"
+
+	flag "github.com/spf13/pflag"
 )
 
 func TestNodeOptions(t *testing.T) {

diff --git a/cmd/options/server_options.go b/cmd/options/server_options.go
@@ -17,7 +17,7 @@ limitations under the License.
 package options
 
 import (
-	"flag"
+	flag "github.com/spf13/pflag"
 
 	"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver"
 )

diff --git a/cmd/options/server_options_test.go b/cmd/options/server_options_test.go
@@ -17,8 +17,9 @@ limitations under the License.
 package options
 
 import (
-	"flag"
 	"testing"
+
+	flag "github.com/spf13/pflag"
 )
 
 func TestServerOptions(t *testing.T) {

diff --git a/cmd/options_test.go b/cmd/options_test.go
@@ -17,12 +17,13 @@ limitations under the License.
 package main
 
 import (
-	"flag"
 	"os"
 	"reflect"
 	"strconv"
 	"testing"
 
+	flag "github.com/spf13/pflag"
+
 	"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver"
 )
 
@@ -56,20 +57,19 @@ func TestGetOptions(t *testing.T) {
 		}, additionalArgs...)
 
 		if withServerOptions {
-			args = append(args, "-"+endpointFlagName+"="+endpoint)
+			args = append(args, "--"+endpointFlagName+"="+endpoint)
 		}
 		if withControllerOptions {
-			args = append(args, "-"+extraTagsFlagName+"="+extraTagKey+"="+extraTagValue)
-			args = append(args, "-"+awsSdkDebugFlagName+"="+strconv.FormatBool(awsSdkDebugFlagValue))
+			args = append(args, "--"+extraTagsFlagName+"="+extraTagKey+"="+extraTagValue)
+			args = append(args, "--"+awsSdkDebugFlagName+"="+strconv.FormatBool(awsSdkDebugFlagValue))
 		}
 		if withNodeOptions {
-			args = append(args, "-"+VolumeAttachLimitFlagName+"="+strconv.FormatInt(VolumeAttachLimit, 10))
+			args = append(args, "--"+VolumeAttachLimitFlagName+"="+strconv.FormatInt(VolumeAttachLimit, 10))
 		}
 
 		oldArgs := os.Args
 		defer func() { os.Args = oldArgs }()
 		os.Args = args
-
 		options := GetOptions(flagSet)
 
 		if withServerOptions {
@@ -172,7 +172,7 @@ func TestGetOptions(t *testing.T) {
 				defer func() { os.Args = oldArgs }()
 				os.Args = []string{
 					"aws-ebs-csi-driver",
-					"-version",
+					"--version",
 				}
 
 				flagSet := flag.NewFlagSet("test-flagset", flag.ContinueOnError)

diff --git a/deploy/kubernetes/base/controller.yaml b/deploy/kubernetes/base/controller.yaml
@@ -66,7 +66,7 @@ spec:
           args:
             # - {all,controller,node} # specify the driver mode
             - --endpoint=$(CSI_ENDPOINT)
-            - --logtostderr
+            - --logging-format=text
             - --v=2
           env:
             - name: CSI_ENDPOINT

diff --git a/deploy/kubernetes/base/node.yaml b/deploy/kubernetes/base/node.yaml
@@ -49,7 +49,7 @@ spec:
           args:
             - node
             - --endpoint=$(CSI_ENDPOINT)
-            - --logtostderr
+            - --logging-format=text
             - --v=2
           env:
             - name: CSI_ENDPOINT

diff --git a/docs/options.md b/docs/options.md
@@ -8,3 +8,4 @@ There are a couple of driver options that can be passed as arguments when starti
 | extra-tags                  | key1=value1,key2=value2                           |                                                     | Tags attached to each dynamically provisioned resource|
 | k8s-tag-cluster-id          | aws-cluster-id-1                                  |                                                     | ID of the Kubernetes cluster used for tagging provisioned EBS volumes|
 | aws-sdk-debug-log           | true                                              | false                                               | If set to true, the driver will enable the aws sdk debug log level|
+| logging-format              | json                                              | text                                                | Sets the log format. Permitted formats: text, json|
diff --git a/go.mod b/go.mod
@@ -10,6 +10,7 @@ require (
 	github.com/kubernetes-csi/external-snapshotter/client/v4 v4.2.0
 	github.com/onsi/ginkgo/v2 v2.4.0
 	github.com/onsi/gomega v1.24.0
+	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.8.1
 	golang.org/x/sys v0.3.0
 	google.golang.org/grpc v1.51.0
@@ -40,6 +41,7 @@ require (
 	github.com/fsnotify/fsnotify v1.6.0 // indirect
 	github.com/go-logr/logr v1.2.3 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
+	github.com/go-logr/zapr v1.2.3 // indirect
 	github.com/go-openapi/jsonpointer v0.19.5 // indirect
 	github.com/go-openapi/jsonreference v0.20.0 // indirect
 	github.com/go-openapi/swag v0.22.3 // indirect
@@ -73,7 +75,6 @@ require (
 	github.com/prometheus/common v0.37.0 // indirect
 	github.com/prometheus/procfs v0.8.0 // indirect
 	github.com/spf13/cobra v1.6.1 // indirect
-	github.com/spf13/pflag v1.0.5 // indirect
 	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.35.0 // indirect
 	go.opentelemetry.io/otel v1.10.0 // indirect
 	go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.10.0 // indirect
@@ -83,6 +84,9 @@ require (
 	go.opentelemetry.io/otel/sdk v1.10.0 // indirect
 	go.opentelemetry.io/otel/trace v1.10.0 // indirect
 	go.opentelemetry.io/proto/otlp v0.19.0 // indirect
+	go.uber.org/atomic v1.7.0 // indirect
+	go.uber.org/multierr v1.6.0 // indirect
+	go.uber.org/zap v1.19.0 // indirect
 	golang.org/x/crypto v0.1.0 // indirect
 	golang.org/x/mod v0.7.0 // indirect
 	golang.org/x/net v0.4.0 // indirect

diff --git a/go.sum b/go.sum
@@ -51,6 +51,8 @@ github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkY
 github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
 github.com/aws/aws-sdk-go v1.44.160 h1:F41sWUel1CJ69ezoBGCg8sDyu9kyeKEpwmDrLXbCuyA=
 github.com/aws/aws-sdk-go v1.44.160/go.mod h1:aVsgQcEevwlmQ7qHE9I3h+dtQgpqhFB+i8Phjh7fkwI=
+github.com/benbjohnson/clock v1.1.0 h1:Q92kusRqC1XV2MjkWETPvjJVqKetz1OzxZB7mHJLju8=
+github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
 github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
@@ -130,6 +132,8 @@ github.com/go-logr/logr v1.2.3 h1:2DntVwHkVopvECVRSlL5PSo9eG+cAkDCuckLubN+rq0=
 github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
 github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
 github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE=
+github.com/go-logr/zapr v1.2.3 h1:a9vnzlIBPQBBkeaR9IuMUfmVOrQlkoC4YfPoFkX3T7A=
+github.com/go-logr/zapr v1.2.3/go.mod h1:eIauM6P8qSvTw5o2ez6UEAfGjQKrxQTl5EoK+Qa2oG4=
 github.com/go-openapi/jsonpointer v0.19.3/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34VGT37dQOVbmoatg=
 github.com/go-openapi/jsonpointer v0.19.5 h1:gZr+CIYByUqjcgeLXnQu2gHYQC9o73G2XUeOFYEICuY=
 github.com/go-openapi/jsonpointer v0.19.5/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34VGT37dQOVbmoatg=
@@ -418,7 +422,14 @@ go.opentelemetry.io/otel/trace v1.10.0/go.mod h1:Sij3YYczqAdz+EhmGhE6TpTxUO5/F/A
 go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI=
 go.opentelemetry.io/proto/otlp v0.19.0 h1:IVN6GR+mhC4s5yfcTbmzHYODqvWAp3ZedA2SJPI1Nnw=
 go.opentelemetry.io/proto/otlp v0.19.0/go.mod h1:H7XAot3MsfNsj7EXtrA2q5xSNQ10UqI405h3+duxN4U=
+go.uber.org/atomic v1.7.0 h1:ADUqmZGgLDDfbSL9ZmPxKTybcoEYHgpYfELNoN+7hsw=
+go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
+go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A=
 go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk=
+go.uber.org/multierr v1.6.0 h1:y6IPFStTAIT5Ytl7/XYmHvzXQ7S3g/IeZW9hyZ5thw4=
+go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU=
+go.uber.org/zap v1.19.0 h1:mZQZefskPPCMIBCSEH0v2/iUqqLrYtaeqwD6FUGUnFE=
+go.uber.org/zap v1.19.0/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI=
 golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
@@ -628,6 +639,7 @@ golang.org/x/tools v0.0.0-20190628153133-6cdbf07be9d0/go.mod h1:/rFqwRUd4F7ZHNgw
 golang.org/x/tools v0.0.0-20190816200558-6889da9d5479/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20190911174233-4f2ddba30aff/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20191012152004-8de300cfc20a/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
+golang.org/x/tools v0.0.0-20191108193012-7d206e10da11/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20191113191852-77e3bb0ad9e7/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20191115202509-3a792d9c32b2/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=