add flag to enable logging output in json to consul-k8s

CHANGELOG.md

@@ -1,6 +1,7 @@
 ## UNRELEASED
 
 IMPROVEMENTS:
+* Add flags `-log-level`, `-log-json` to all subcommands to control log level and json formatting. [[GH-523](https://github.com/hashicorp/consul-k8s/pull/523)]
 * Connect: skip service registration when a service with the same name but in a different Kubernetes namespace is found
   and Consul namespaces are not enabled. [[GH-527](https://github.com/hashicorp/consul-k8s/pull/527)]
 * Delete secrets created by webhook-cert-manager when the deployment is deleted. [[GH-530](https://github.com/hashicorp/consul-k8s/pull/530)]

connect-inject/consul_sidecar.go

@@ -24,6 +24,8 @@ func (h *Handler) consulSidecar(pod corev1.Pod) (corev1.Container, error) {
 		fmt.Sprintf("-merged-metrics-port=%s", metricsPorts.mergedPort),
 		fmt.Sprintf("-service-metrics-port=%s", metricsPorts.servicePort),
 		fmt.Sprintf("-service-metrics-path=%s", metricsPorts.servicePath),
+		fmt.Sprintf("-log-level=%s", h.LogLevel),
+		fmt.Sprintf("-log-json=%t", h.LogJSON),
 	}
 
 	return corev1.Container{

connect-inject/handler.go

@@ -141,6 +141,9 @@ type Handler struct {
 
 	// Log
 	Log logr.Logger
+	// Log settings for consul-sidecar
+	LogLevel string
+	LogJSON  bool
 
 	decoder *admission.Decoder
 }

subcommand/common/common.go

@@ -9,8 +9,11 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/go-logr/logr"
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/go-hclog"
+	"go.uber.org/zap/zapcore"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 )
 
 const (
@@ -24,18 +27,36 @@ const (
 	ACLTokenSecretKey = "token"
 )
 
-// Logger returns an hclog instance or an error if level is invalid.
-func Logger(level string) (hclog.Logger, error) {
+// Logger returns an hclog instance with log level set and JSON logging enabled/disabled, or an error if level is invalid.
+func Logger(level string, jsonLogging bool) (hclog.Logger, error) {
 	parsedLevel := hclog.LevelFromString(level)
 	if parsedLevel == hclog.NoLevel {
 		return nil, fmt.Errorf("unknown log level: %s", level)
 	}
 	return hclog.New(&hclog.LoggerOptions{
-		Level:  parsedLevel,
-		Output: os.Stderr,
+		JSONFormat: jsonLogging,
+		Level:      parsedLevel,
+		Output:     os.Stderr,
 	}), nil
 }
 
+// ZapLogger returns a logr.Logger instance with log level set and JSON logging enabled/disabled, or an error if the level is invalid.
+func ZapLogger(level string, jsonLogging bool) (logr.Logger, error) {
+	var zapLevel zapcore.Level
+	// It is possible that a user passes in "trace" from global.logLevel, until we standardize on one logging framework
+	// we will assume they meant debug here and not fail.
+	if level == "trace" || level == "TRACE" {
+		level = "debug"
+	}
+	if err := zapLevel.UnmarshalText([]byte(level)); err != nil {
+		return nil, fmt.Errorf("unknown log level %q: %s", level, err.Error())
+	}
+	if jsonLogging {
+		return zap.New(zap.UseDevMode(false), zap.Level(zapLevel), zap.JSONEncoder()), nil
+	}
+	return zap.New(zap.UseDevMode(false), zap.Level(zapLevel), zap.ConsoleEncoder()), nil
+}
+
 // ValidateUnprivilegedPort converts flags representing ports into integer and validates
 // that it's in the unprivileged port range.
 func ValidateUnprivilegedPort(flagName, flagValue string) error {

subcommand/common/common_test.go

@@ -16,12 +16,23 @@ import (
 )
 
 func TestLogger_InvalidLogLevel(t *testing.T) {
-	_, err := Logger("invalid")
+	_, err := Logger("invalid", false)
 	require.EqualError(t, err, "unknown log level: invalid")
 }
 
+func TestZapLogger_InvalidLogLevel(t *testing.T) {
+	_, err := ZapLogger("invalid", false)
+	require.EqualError(t, err, "unknown log level \"invalid\": unrecognized level: \"invalid\"")
+}
+
+// ZapLogger should convert "trace" log level to "debug"
+func TestZapLogger_TraceLogLevel(t *testing.T) {
+	_, err := ZapLogger("trace", false)
+	require.NoError(t, err)
+}
+
 func TestLogger(t *testing.T) {
-	lgr, err := Logger("debug")
+	lgr, err := Logger("debug", false)
 	require.NoError(t, err)
 	require.NotNil(t, lgr)
 	require.True(t, lgr.IsDebug())

subcommand/connect-init/command.go

@@ -39,6 +39,7 @@ type Command struct {
 	flagServiceAccountName     string // Service account name.
 	flagServiceName            string // Service name.
 	flagLogLevel               string
+	flagLogJSON                bool
 
 	bearerTokenFile                    string // Location of the bearer token. Default is /var/run/secrets/kubernetes.io/serviceaccount/token.
 	tokenSinkFile                      string // Location to write the output token. Default is defaultTokenSinkFile.
@@ -65,6 +66,8 @@ func (c *Command) init() {
 	c.flagSet.StringVar(&c.flagLogLevel, "log-level", "info",
 		"Log verbosity level. Supported values (in order of detail) are \"trace\", "+
 			"\"debug\", \"info\", \"warn\", and \"error\".")
+	c.flagSet.BoolVar(&c.flagLogJSON, "log-json", false,
+		"Enable or disable JSON output format for logging.")
 
 	if c.bearerTokenFile == "" {
 		c.bearerTokenFile = defaultBearerTokenFile
@@ -109,7 +112,7 @@ func (c *Command) Run(args []string) int {
 	// Set up logging.
 	if c.logger == nil {
 		var err error
-		c.logger, err = common.Logger(c.flagLogLevel)
+		c.logger, err = common.Logger(c.flagLogLevel, c.flagLogJSON)
 		if err != nil {
 			c.UI.Error(err.Error())
 			return 1

subcommand/consul-sidecar/command.go

@@ -34,6 +34,7 @@ type Command struct {
 	flagSyncPeriod                time.Duration
 	flagSet                       *flag.FlagSet
 	flagLogLevel                  string
+	flagLogJSON                   bool
 
 	// Flags to configure metrics merging
 	flagEnableMetricsMerging bool
@@ -67,6 +68,8 @@ func (c *Command) init() {
 	c.flagSet.StringVar(&c.flagLogLevel, "log-level", "info",
 		"Log verbosity level. Supported values (in order of detail) are \"trace\", "+
 			"\"debug\", \"info\", \"warn\", and \"error\". Defaults to info.")
+	c.flagSet.BoolVar(&c.flagLogJSON, "log-json", false,
+		"Enable or disable JSON output format for logging.")
 
 	c.flagSet.BoolVar(&c.flagEnableMetricsMerging, "enable-metrics-merging", false, "Enables consul sidecar to run a merged metrics endpoint. Defaults to false.")
 	// -merged-metrics-port, -service-metrics-port, and -service-metrics-path
@@ -109,7 +112,7 @@ func (c *Command) Run(args []string) int {
 		return 1
 	}
 
-	logger, err := common.Logger(c.flagLogLevel)
+	logger, err := common.Logger(c.flagLogLevel, c.flagLogJSON)
 	if err != nil {
 		c.UI.Error(err.Error())
 		return 1

subcommand/controller/command.go

@@ -8,6 +8,7 @@ import (
 	"github.com/hashicorp/consul-k8s/api/common"
 	"github.com/hashicorp/consul-k8s/api/v1alpha1"
 	"github.com/hashicorp/consul-k8s/controller"
+	cmdCommon "github.com/hashicorp/consul-k8s/subcommand/common"
 	"github.com/hashicorp/consul-k8s/subcommand/flags"
 	"github.com/mitchellh/cli"
 	"go.uber.org/zap/zapcore"
@@ -16,7 +17,6 @@ import (
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 )
 
@@ -32,6 +32,7 @@ type Command struct {
 	flagEnableWebhooks       bool
 	flagDatacenter           string
 	flagLogLevel             string
+	flagLogJSON              bool
 
 	// Flags to support Consul Enterprise namespaces.
 	flagEnableNamespaces           bool
@@ -81,6 +82,8 @@ func (c *Command) init() {
 	c.flagSet.StringVar(&c.flagLogLevel, "log-level", zapcore.InfoLevel.String(),
 		fmt.Sprintf("Log verbosity level. Supported values (in order of detail) are "+
 			"%q, %q, %q, and %q.", zapcore.DebugLevel.String(), zapcore.InfoLevel.String(), zapcore.WarnLevel.String(), zapcore.ErrorLevel.String()))
+	c.flagSet.BoolVar(&c.flagLogJSON, "log-json", false,
+		"Enable or disable JSON output format for logging.")
 
 	c.httpFlags = &flags.HTTPFlags{}
 	flags.Merge(c.flagSet, c.httpFlags.Flags())
@@ -106,23 +109,20 @@ func (c *Command) Run(args []string) int {
 		return 1
 	}
 
-	var zapLevel zapcore.Level
-	if err := zapLevel.UnmarshalText([]byte(c.flagLogLevel)); err != nil {
-		c.UI.Error(fmt.Sprintf("Error parsing -log-level %q: %s", c.flagLogLevel, err.Error()))
+	zapLogger, err := cmdCommon.ZapLogger(c.flagLogLevel, c.flagLogJSON)
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Error setting up logging: %s", err.Error()))
 		return 1
 	}
-	// We set UseDevMode to true because we don't want our logs json
-	// formatted.
-	logger := zap.New(zap.UseDevMode(true), zap.Level(zapLevel))
-	ctrl.SetLogger(logger)
-	klog.SetLogger(logger)
+	ctrl.SetLogger(zapLogger)
+	klog.SetLogger(zapLogger)
 
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		Scheme:           scheme,
 		Port:             9443,
 		LeaderElection:   c.flagEnableLeaderElection,
 		LeaderElectionID: "consul.hashicorp.com",
-		Logger:           logger,
+		Logger:           zapLogger,
 	})
 	if err != nil {
 		setupLog.Error(err, "unable to start manager")

subcommand/controller/command_test.go

@@ -28,7 +28,7 @@ func TestRun_FlagValidation(t *testing.T) {
 		},
 		{
 			flags:  []string{"-webhook-tls-cert-dir", "/foo", "-datacenter", "foo", "-log-level", "invalid"},
-			expErr: `Error parsing -log-level "invalid": unrecognized level: "invalid"`,
+			expErr: `unknown log level "invalid": unrecognized level: "invalid"`,
 		},
 	}
 

subcommand/create-federation-secret/command.go

@@ -59,6 +59,7 @@ type Command struct {
 	flagResourcePrefix         string
 	flagK8sNamespace           string
 	flagLogLevel               string
+	flagLogJSON                bool
 	flagMeshGatewayServiceName string
 
 	k8sClient    kubernetes.Interface
@@ -89,6 +90,8 @@ func (c *Command) init() {
 	c.flags.StringVar(&c.flagLogLevel, "log-level", "info",
 		"Log verbosity level. Supported values (in order of detail) are \"trace\", "+
 			"\"debug\", \"info\", \"warn\", and \"error\".")
+	c.flags.BoolVar(&c.flagLogJSON, "log-json", false,
+		"Enable or disable JSON output format for logging.")
 
 	c.http = &flags.HTTPFlags{}
 	c.k8s = &flags.K8SFlags{}
@@ -108,7 +111,7 @@ func (c *Command) Run(args []string) int {
 		return 1
 	}
 
-	logger, err := common.Logger(c.flagLogLevel)
+	logger, err := common.Logger(c.flagLogLevel, c.flagLogJSON)
 	if err != nil {
 		c.UI.Error(err.Error())
 		return 1

subcommand/delete-completed-job/command.go

@@ -18,8 +18,6 @@ import (
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
 )
 
-const logLevel = "info"
-
 // Command is the command for deleting completed jobs.
 type Command struct {
 	UI cli.Ui
@@ -28,6 +26,8 @@ type Command struct {
 	k8s           *flags.K8SFlags
 	flagNamespace string
 	flagTimeout   string
+	flagLogLevel  string
+	flagLogJSON   bool
 
 	once      sync.Once
 	help      string
@@ -45,6 +45,11 @@ func (c *Command) init() {
 		"Name of Kubernetes namespace where the job is deployed")
 	c.flags.StringVar(&c.flagTimeout, "timeout", "30m",
 		"How long we'll wait for the job to complete before timing out, e.g. 1ms, 2s, 3m")
+	c.flags.StringVar(&c.flagLogLevel, "log-level", "info",
+		"Log verbosity level. Supported values (in order of detail) are \"trace\", "+
+			"\"debug\", \"info\", \"warn\", and \"error\".")
+	c.flags.BoolVar(&c.flagLogJSON, "log-json", false,
+		"Enable or disable JSON output format for logging.")
 	flags.Merge(c.flags, c.k8s.Flags())
 	c.help = flags.Usage(help, c.flags)
 
@@ -96,7 +101,7 @@ func (c *Command) Run(args []string) int {
 		}
 	}
 
-	logger, err := common.Logger(logLevel)
+	logger, err := common.Logger(c.flagLogLevel, c.flagLogJSON)
 	if err != nil {
 		c.UI.Error(err.Error())
 		return 1

subcommand/get-consul-client-ca/command.go

@@ -34,6 +34,7 @@ type Command struct {
 	flagTLSServerName   string
 	flagPollingInterval time.Duration
 	flagLogLevel        string
+	flagLogJSON         bool
 
 	once sync.Once
 	help string
@@ -58,6 +59,8 @@ func (c *Command) init() {
 	c.flags.StringVar(&c.flagLogLevel, "log-level", "info",
 		"Log verbosity level. Supported values (in order of detail) are \"trace\", "+
 			"\"debug\", \"info\", \"warn\", and \"error\".")
+	c.flags.BoolVar(&c.flagLogJSON, "log-json", false,
+		"Enable or disable JSON output format for logging.")
 
 	c.help = flags.Usage(help, c.flags)
 }
@@ -82,7 +85,7 @@ func (c *Command) Run(args []string) int {
 		return 1
 	}
 
-	logger, err := common.Logger(c.flagLogLevel)
+	logger, err := common.Logger(c.flagLogLevel, c.flagLogJSON)
 	if err != nil {
 		c.UI.Error(err.Error())
 		return 1

subcommand/inject-connect/command.go

@@ -29,7 +29,6 @@ import (
 	"k8s.io/client-go/rest"
 	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 )
 
@@ -48,6 +47,7 @@ type Command struct {
 	flagConsulCACert         string // [Deprecated] Path to CA Certificate to use when communicating with Consul clients
 	flagEnvoyExtraArgs       string // Extra envoy args when starting envoy
 	flagLogLevel             string
+	flagLogJSON              bool
 
 	flagAllowK8sNamespacesList []string // K8s namespaces to explicitly inject
 	flagDenyK8sNamespacesList  []string // K8s namespaces to deny injection (has precedence)
@@ -165,6 +165,8 @@ func (c *Command) init() {
 	c.flagSet.StringVar(&c.flagLogLevel, "log-level", zapcore.InfoLevel.String(),
 		fmt.Sprintf("Log verbosity level. Supported values (in order of detail) are "+
 			"%q, %q, %q, and %q.", zapcore.DebugLevel.String(), zapcore.InfoLevel.String(), zapcore.WarnLevel.String(), zapcore.ErrorLevel.String()))
+	c.flagSet.BoolVar(&c.flagLogJSON, "log-json", false,
+		"Enable or disable JSON output format for logging.")
 
 	// Proxy sidecar resource setting flags.
 	c.flagSet.StringVar(&c.flagDefaultSidecarProxyCPURequest, "default-sidecar-proxy-cpu-request", "", "Default sidecar proxy CPU request.")
@@ -353,16 +355,14 @@ func (c *Command) Run(args []string) int {
 	allowK8sNamespaces := flags.ToSet(c.flagAllowK8sNamespacesList)
 	denyK8sNamespaces := flags.ToSet(c.flagDenyK8sNamespacesList)
 
-	var zapLevel zapcore.Level
-	if err := zapLevel.UnmarshalText([]byte(c.flagLogLevel)); err != nil {
-		c.UI.Error(fmt.Sprintf("Error parsing -log-level %q: %s", c.flagLogLevel, err.Error()))
+	zapLogger, err := common.ZapLogger(c.flagLogLevel, c.flagLogJSON)
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Error setting up logging: %s", err.Error()))
 		return 1
 	}
-
-	// We set UseDevMode to true because we don't want our logs json formatted.
-	zapLogger := zap.New(zap.UseDevMode(true), zap.Level(zapLevel))
 	ctrl.SetLogger(zapLogger)
 	klog.SetLogger(zapLogger)
+
 	listenSplits := strings.SplitN(c.flagListen, ":", 2)
 	if len(listenSplits) < 2 {
 		c.UI.Error(fmt.Sprintf("missing port in address: %s", c.flagListen))
@@ -453,6 +453,8 @@ func (c *Command) Run(args []string) int {
 			TProxyOverwriteProbes:      c.flagTransparentProxyDefaultOverwriteProbes,
 			EnableOpenShift:            c.flagEnableOpenShift,
 			Log:                        ctrl.Log.WithName("handler").WithName("connect"),
+			LogLevel:                   c.flagLogLevel,
+			LogJSON:                    c.flagLogJSON,
 		}})
 
 	if err := mgr.Start(ctx); err != nil {

subcommand/inject-connect/command_test.go

@@ -30,7 +30,7 @@ func TestRun_FlagValidation(t *testing.T) {
 		{
 			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
 				"-log-level", "invalid"},
-			expErr: "Error parsing -log-level \"invalid\": unrecognized level: \"invalid\"",
+			expErr: "unknown log level \"invalid\": unrecognized level: \"invalid\"",
 		},
 		{
 			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",