Add possibility to configure log format #799

.golangci.yml

@@ -459,3 +459,8 @@ issues:
         - gocritic
       path: "pkg/(appsec|acquisition|dumps|alertcontext|leakybucket|exprhelpers)"
       text: "rangeValCopy: .*"
+
+    - linters:
+        - revive
+      path: "pkg/types/utils.go"
+      text: "argument-limit: .*"

cmd/crowdsec-cli/main.go

@@ -146,7 +146,10 @@ func (cli *cliRoot) initialize() error {
 		return fmt.Errorf("output format '%s' not supported: must be one of human, json, raw", csConfig.Cscli.Output)
 	}
 
-	log.SetFormatter(&log.TextFormatter{DisableTimestamp: true})
+	log.SetFormatter(&log.TextFormatter{
+		DisableTimestamp: true,
+		DisableLevelTruncation: true,
+	})
 
 	if csConfig.Cscli.Output == "json" {
 		log.SetFormatter(&log.JSONFormatter{})

cmd/crowdsec/fatalhook.go

@@ -2,23 +2,43 @@ package main
 
 import (
 	"io"
+	"os"
 
 	log "github.com/sirupsen/logrus"
 )
 
 // FatalHook is used to log fatal messages to stderr when the rest goes to a file
 type FatalHook struct {
 	Writer    io.Writer
+	Formatter log.Formatter
 	LogLevels []log.Level
 }
 
+func newFatalHook() *FatalHook {
+	return &FatalHook{
+		Writer: os.Stderr,
+		Formatter: &log.TextFormatter{
+			DisableTimestamp: true,
+			// XXX: logrus.TextFormatter has either key pairs with no colors,
+			// or "LEVEL [optional timestamp] message", with colors.
+			// We force colors to make sure we get the latter, even if
+			// the output is not a terminal.
+			// There are more flexible formatters that don't conflate the two concepts,
+			// or we can write our own.
+			ForceColors: true,
+			DisableLevelTruncation: true,
+		},
+		LogLevels: []log.Level{log.FatalLevel, log.PanicLevel},
+	}
+}
+
 func (hook *FatalHook) Fire(entry *log.Entry) error {
-	line, err := entry.String()
+	line, err := hook.Formatter.Format(entry)
 	if err != nil {
 		return err
 	}
 
-	_, err = hook.Writer.Write([]byte(line))
+	_, err = hook.Writer.Write(line)
 
 	return err
 }

cmd/crowdsec/main.go

@@ -249,16 +249,13 @@ func LoadConfig(configFile string, disableAgent bool, disableAPI bool, quiet boo
 	if err := types.SetDefaultLoggerConfig(cConfig.Common.LogMedia,
 		cConfig.Common.LogDir, *cConfig.Common.LogLevel,
 		cConfig.Common.LogMaxSize, cConfig.Common.LogMaxFiles,
-		cConfig.Common.LogMaxAge, cConfig.Common.CompressLogs,
+		cConfig.Common.LogMaxAge, cConfig.Common.LogFormat, cConfig.Common.CompressLogs,
 		cConfig.Common.ForceColorLogs); err != nil {
 		return nil, err
 	}
 
 	if cConfig.Common.LogMedia != "stdout" {
-		log.AddHook(&FatalHook{
-			Writer:    os.Stderr,
-			LogLevels: []log.Level{log.FatalLevel, log.PanicLevel},
-		})
+		log.AddHook(newFatalHook())
 	}
 
 	if err := csconfig.LoadFeatureFlagsFile(configFile, log.StandardLogger()); err != nil {

pkg/apiserver/apiserver_test.go

@@ -387,7 +387,7 @@ func TestLoggingDebugToFileConfig(t *testing.T) {
 	cfg.LogLevel = ptr.Of(log.DebugLevel)
 
 	// Configure logging
-	err := types.SetDefaultLoggerConfig(cfg.LogMedia, cfg.LogDir, *cfg.LogLevel, cfg.LogMaxSize, cfg.LogMaxFiles, cfg.LogMaxAge, cfg.CompressLogs, false)
+	err := types.SetDefaultLoggerConfig(cfg.LogMedia, cfg.LogDir, *cfg.LogLevel, cfg.LogMaxSize, cfg.LogMaxFiles, cfg.LogMaxAge, cfg.LogFormat, cfg.CompressLogs, false)
 	require.NoError(t, err)
 
 	api, err := NewServer(ctx, &cfg)
@@ -439,7 +439,7 @@ func TestLoggingErrorToFileConfig(t *testing.T) {
 	cfg.LogLevel = ptr.Of(log.ErrorLevel)
 
 	// Configure logging
-	err := types.SetDefaultLoggerConfig(cfg.LogMedia, cfg.LogDir, *cfg.LogLevel, cfg.LogMaxSize, cfg.LogMaxFiles, cfg.LogMaxAge, cfg.CompressLogs, false)
+	err := types.SetDefaultLoggerConfig(cfg.LogMedia, cfg.LogDir, *cfg.LogLevel, cfg.LogMaxSize, cfg.LogMaxFiles, cfg.LogMaxAge, cfg.LogFormat, cfg.CompressLogs, false)
 	require.NoError(t, err)
 
 	api, err := NewServer(ctx, &cfg)

pkg/csconfig/api.go

@@ -271,6 +271,7 @@ type LocalApiServerCfg struct {
 	LogMaxSize                    int                      `yaml:"-"`
 	LogMaxAge                     int                      `yaml:"-"`
 	LogMaxFiles                   int                      `yaml:"-"`
+	LogFormat                     string                   `yaml:"-"`
 	TrustedIPs                    []string                 `yaml:"trusted_ips,omitempty"`
 	PapiLogLevel                  *log.Level               `yaml:"papi_log_level"`
 	DisableRemoteLapiRegistration bool                     `yaml:"disable_remote_lapi_registration,omitempty"`
@@ -391,6 +392,7 @@ func (c *Config) LoadAPIServer(inCli bool) error {
 	c.API.Server.CompressLogs = c.Common.CompressLogs
 	c.API.Server.LogMaxSize = c.Common.LogMaxSize
 	c.API.Server.LogMaxAge = c.Common.LogMaxAge
+	c.API.Server.LogFormat = c.Common.LogFormat
 	c.API.Server.LogMaxFiles = c.Common.LogMaxFiles
 
 	if c.API.Server.UseForwardedForHeaders && c.API.Server.TrustedProxies == nil {

pkg/csconfig/common.go

@@ -17,6 +17,7 @@ type CommonCfg struct {
 	WorkingDir     string     `yaml:"working_dir,omitempty"` // TODO: This is just for backward compat. Remove this later
 	CompressLogs   *bool      `yaml:"compress_logs,omitempty"`
 	LogMaxSize     int        `yaml:"log_max_size,omitempty"`
+	LogFormat      string     `yaml:"log_format,omitempty"`
 	LogMaxAge      int        `yaml:"log_max_age,omitempty"`
 	LogMaxFiles    int        `yaml:"log_max_files,omitempty"`
 	ForceColorLogs bool       `yaml:"force_color_logs,omitempty"`

pkg/types/utils.go

@@ -16,8 +16,24 @@
 	logLevel     log.Level
 )
 
-func SetDefaultLoggerConfig(cfgMode string, cfgFolder string, cfgLevel log.Level, maxSize int, maxFiles int, maxAge int, compress *bool, forceColors bool) error {
-	/*Configure logs*/
+func SetDefaultLoggerConfig(cfgMode string, cfgFolder string, cfgLevel log.Level, maxSize int, maxFiles int, maxAge int, format string, compress *bool, forceColors bool) error {
+	if format == "" {
+		format = "text"
+	}
+
+	switch format {
+		case "text":
+			logFormatter = &log.TextFormatter{
+				TimestampFormat: time.RFC3339,
+				FullTimestamp: true,
+				ForceColors: forceColors,
+			}
+		case "json":
+			logFormatter = &log.JSONFormatter{TimestampFormat: time.RFC3339}
+		default:
+			return fmt.Errorf("unknown log_format '%s'", format)
+	}
+
 	if cfgMode == "file" {
 		_maxsize := 500
 		if maxSize != 0 {
@@ -47,9 +63,10 @@
 	} else if cfgMode != "stdout" {
 		return fmt.Errorf("log mode '%s' unknown", cfgMode)
 	}
+
 	logLevel = cfgLevel
 	log.SetLevel(logLevel)
-	logFormatter = &log.TextFormatter{TimestampFormat: time.RFC3339, FullTimestamp: true, ForceColors: forceColors}
+
 	log.SetFormatter(logFormatter)
 	return nil
 }

test/bats/01_crowdsec.bats

@@ -67,6 +67,40 @@ teardown() {
     refute_output
 }
 
+@test "crowdsec - log format" {
+    # fail early
+    config_disable_lapi
+    config_disable_agent
+
+    config_set '.common.log_media="stdout"'
+
+    config_set '.common.log_format=""'
+    rune -0 wait-for --err "you must run at least the API Server or crowdsec" "$CROWDSEC"
+    assert_stderr --partial 'level=fatal msg="you must run at least the API Server or crowdsec"'
+
+    config_set '.common.log_format="text"'
+    rune -0 wait-for --err "you must run at least the API Server or crowdsec" "$CROWDSEC"
+    assert_stderr --partial 'level=fatal msg="you must run at least the API Server or crowdsec"'
+
+    config_set '.common.log_format="json"'
+    rune -0 wait-for --err "you must run at least the API Server or crowdsec" "$CROWDSEC"
+    rune -0 jq -c 'select(.msg=="you must run at least the API Server or crowdsec") | .level' <(stderr | grep "^{")
+    assert_output '"fatal"'
+
+    # If log_media='file', a hook to stderr is added only for fatal messages,
+    # with a predefined formatter (level + msg, no timestamp, ignore log_format)
+
+    config_set '.common.log_media="file"'
+
+    config_set '.common.log_format="text"'
+    rune -0 wait-for --err "you must run at least the API Server or crowdsec" "$CROWDSEC"
+    assert_stderr --regexp 'FATAL.* you must run at least the API Server or crowdsec$'
+
+    config_set '.common.log_format="json"'
+    rune -0 wait-for --err "you must run at least the API Server or crowdsec" "$CROWDSEC"
+    assert_stderr --regexp 'FATAL.* you must run at least the API Server or crowdsec$'
+}
+
 @test "CS_LAPI_SECRET not strong enough" {
     CS_LAPI_SECRET=foo rune -1 wait-for "$CROWDSEC"
     assert_stderr --partial "api server init: unable to run local API: controller init: CS_LAPI_SECRET not strong enough"

test/bats/crowdsec-acquisition.bats

@@ -33,7 +33,7 @@ teardown() {
 	EOT
 
     rune -1 "$CROWDSEC" -t
-    assert_stderr --partial "crowdsec init: while loading acquisition config: while configuring datasource of type file from $ACQUIS_DIR/file.yaml (position 0): cannot parse FileAcquisition configuration: yaml: unmarshal errors:\n  line 6: cannot unmarshal !!seq into string"
+    assert_stderr --partial "crowdsec init: while loading acquisition config: while configuring datasource of type file from $ACQUIS_DIR/file.yaml (position 0): cannot parse FileAcquisition configuration: yaml: unmarshal errors:"
 }
 
 @test "datasource type detection" {