Add possibility to configure log format #799 (#2941)

* make it possible to enable json log * fix * fix typo * fix typo * fix typo * fix typo * fix typo * fix typo * Add error handling * Add log_format to default config * Fix syntax error in if statement * Fix typo * Fix typo * Fix some typos and change naming from native to text, makes more sense * Set same timestamp format for json logging * Fix formatting * Move in if statement under previous * Fix some formatting that got messed up * Default to text formatter, if log_format is not configured. * defining logFormatter outside if statement so that log.SetFormatter(logFormatter) is not undefined when function is called * Add variables that were undefined * Argument were missing when calling SetDefaultLoggerConfig function * Fix order of arguments passed * Fix order of arguments passed * Fix typo * Implicit log_format = "text" * functional test * ignore log_format in FatalHook * make it possible to enable json log * fix * fix typo * fix typo * fix typo * fix typo * fix typo * fix typo * Add error handling * Add log_format to default config * Fix syntax error in if statement * Fix typo * Fix typo * Fix some typos and change naming from native to text, makes more sense * Set same timestamp format for json logging * Fix formatting * Move in if statement under previous * Fix some formatting that got messed up * Default to text formatter, if log_format is not configured. * defining logFormatter outside if statement so that log.SetFormatter(logFormatter) is not undefined when function is called * Add variables that were undefined * Argument were missing when calling SetDefaultLoggerConfig function * Fix order of arguments passed * Fix order of arguments passed * Fix typo * Implicit log_format = "text" * functional test * ignore log_format in FatalHook * lint * fix func test * lint * remove < > characters from log --------- Co-authored-by: Victor Edvardsson <victor.edvardsson@loopia.se> Co-authored-by: marco <marco@crowdsec.net> Co-authored-by: Thibault "bui" Koechlin <thibault@crowdsec.net>
crowdsecurity · Dec 23, 2024 · 466f39b · 466f39b
1 parent 4748720
commit 466f39b
Show file tree

Hide file tree

Showing 11 changed files with 107 additions and 18 deletions.
diff --git a/.golangci.yml b/.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: .*"
diff --git a/cmd/crowdsec-cli/main.go b/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{})

diff --git a/cmd/crowdsec/fatalhook.go b/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
 }

diff --git a/cmd/crowdsec/main.go b/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 {

diff --git a/pkg/apiserver/apiserver_test.go b/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)

diff --git a/pkg/csconfig/api.go b/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"`
@@ -351,7 +352,7 @@ func (c *Config) LoadAPIServer(inCli bool) error {
 		log.Printf("push and pull to Central API disabled")
 	}
 
-	//Set default values for CAPI push/pull
+	// Set default values for CAPI push/pull
 	if c.API.Server.OnlineClient != nil {
 		if c.API.Server.OnlineClient.PullConfig.Community == nil {
 			c.API.Server.OnlineClient.PullConfig.Community = ptr.Of(true)
@@ -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 {

diff --git a/pkg/csconfig/common.go b/pkg/csconfig/common.go
@@ -12,18 +12,20 @@ type CommonCfg struct {
 	Daemonize      bool
 	PidDir         string     `yaml:"pid_dir,omitempty"` // TODO: This is just for backward compat. Remove this later
 	LogMedia       string     `yaml:"log_media"`
-	LogDir         string     `yaml:"log_dir,omitempty"` //if LogMedia = file
+	LogDir         string     `yaml:"log_dir,omitempty"` // if LogMedia = file
 	LogLevel       *log.Level `yaml:"log_level"`
 	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"`
 }
 
 func (c *Config) loadCommon() error {
 	var err error
+
 	if c.Common == nil {
 		c.Common = &CommonCfg{}
 	}
@@ -32,13 +34,15 @@ func (c *Config) loadCommon() error {
 		c.Common.LogMedia = "stdout"
 	}
 
-	var CommonCleanup = []*string{
+	CommonCleanup := []*string{
 		&c.Common.LogDir,
 	}
+
 	for _, k := range CommonCleanup {
 		if *k == "" {
 			continue
 		}
+
 		*k, err = filepath.Abs(*k)
 		if err != nil {
 			return fmt.Errorf("failed to get absolute path of '%s': %w", *k, err)

diff --git a/pkg/csconfig/fflag.go b/pkg/csconfig/fflag.go
@@ -38,7 +38,7 @@ func LoadFeatureFlagsFile(configPath string, logger *log.Logger) error {
 func ListFeatureFlags() string {
 	enabledFeatures := fflag.Crowdsec.GetEnabledFeatures()
 
-	msg := "<none>"
+	msg := "none"
 	if len(enabledFeatures) > 0 {
 		msg = strings.Join(enabledFeatures, ", ")
 	}

diff --git a/pkg/types/utils.go b/pkg/types/utils.go
@@ -16,21 +16,40 @@ var (
 	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 {
 			_maxsize = maxSize
 		}
+
 		_maxfiles := 3
 		if maxFiles != 0 {
 			_maxfiles = maxFiles
 		}
+
 		_maxage := 28
 		if maxAge != 0 {
 			_maxage = maxAge
 		}
+
 		_compress := true
 		if compress != nil {
 			_compress = *compress
@@ -47,10 +66,11 @@ func SetDefaultLoggerConfig(cfgMode string, cfgFolder string, cfgLevel log.Level
 	} 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
 }
 
@@ -63,7 +83,9 @@ func ConfigureLogger(clog *log.Logger) error {
 	if logFormatter != nil {
 		clog.SetFormatter(logFormatter)
 	}
+
 	clog.SetLevel(logLevel)
+
 	return nil
 }
 
@@ -76,6 +98,8 @@ func IsNetworkFS(path string) (bool, string, error) {
 	if err != nil {
 		return false, "", err
 	}
+
 	fsType = strings.ToLower(fsType)
+
 	return fsType == "nfs" || fsType == "cifs" || fsType == "smb" || fsType == "smb2", fsType, nil
 }
diff --git a/test/bats/01_crowdsec.bats b/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"

diff --git a/test/bats/crowdsec-acquisition.bats b/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" {