improve feature flag logging (#1986)

For cscli: it should provide a terse output, not nag users with configuration details. Although it's usually important that cscli and crowdsec have the same enabled features, having it list them every time the command is invoked can be too much.

For crowdsec: when features are set from the environment, it's too early to log where we should. So we can use log.Debug at activation time, and list them again once logging is configured.

 - wrap some functions in csconfig for convenience and DRY
 - for each enabled feature, log.Debug
 - log all enabled features once as Info (crowdsec) or Debug (cscli)
 - file does not exist -> log.Trace

cmd/crowdsec-cli/main.go

@@ -66,9 +66,14 @@ func initConfig() {
 		csConfig = csconfig.NewDefaultConfig()
 	}
 
-	featurePath := filepath.Join(csConfig.ConfigPaths.ConfigDir, "feature.yaml")
-	if err = fflag.Crowdsec.SetFromYamlFile(featurePath, log.StandardLogger()); err != nil {
-		log.Fatalf("File %s: %s", featurePath, err)
+	if err := csconfig.LoadFeatureFlagsFile(csConfig, log.StandardLogger()); err != nil {
+		log.Fatal(err)
+	}
+
+	// recap of the enabled feature flags, because logging
+	// was not enabled when we set them from envvars
+	if fflist := csconfig.ListFeatureFlags(); fflist != "" {
+		log.Debugf("Enabled feature flags: %s", fflist)
 	}
 
 	if csConfig.Cscli == nil {
@@ -141,10 +146,8 @@ func main() {
 		log.Fatalf("failed to register features: %s", err)
 	}
 
-	// some features can require configuration or command-line options,
-	// so we need to parse them asap. we'll load from feature.yaml later.
-	if err := fflag.Crowdsec.SetFromEnv(log.StandardLogger()); err != nil {
-		log.Fatalf("failed to set features from environment: %s", err)
+	if err := csconfig.LoadFeatureFlagsEnv(log.StandardLogger()); err != nil {
+		log.Fatalf("failed to set feature flags from env: %s", err)
 	}
 
 	var rootCmd = &cobra.Command{

cmd/crowdsec/main.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	_ "net/http/pprof"
 	"os"
-	"path/filepath"
 	"runtime"
 	"strings"
 	"time"
@@ -261,34 +260,15 @@ func LoadConfig(cConfig *csconfig.Config) error {
 		return err
 	}
 
-	err := LoadFeatureFlags(cConfig, log.StandardLogger())
-	if err != nil {
+	if err := csconfig.LoadFeatureFlagsFile(cConfig, log.StandardLogger()); err != nil {
 		return err
 	}
 
-	return nil
-}
-
-
-// LoadFeatureFlags parses {ConfigDir}/feature.yaml to enable/disable features.
-//
-// Since CROWDSEC_FEATURE_ envvars are parsed before config.yaml,
-// when the logger is not yet initialized, we also log here a recap
-// of what has been enabled.
-func LoadFeatureFlags(cConfig *csconfig.Config, logger *log.Logger) error {
-	featurePath := filepath.Join(cConfig.ConfigPaths.ConfigDir, "feature.yaml")
-
-	if err := fflag.Crowdsec.SetFromYamlFile(featurePath, logger); err != nil {
-		return fmt.Errorf("file %s: %s", featurePath, err)
-	}
-
-	enabledFeatures := fflag.Crowdsec.GetEnabledFeatures()
-
-	msg := "<none>"
-	if len(enabledFeatures) > 0 {
-		msg = strings.Join(enabledFeatures, ", ")
+	// recap of the enabled feature flags, because logging
+	// was not enabled when we set them from envvars
+	if fflist := csconfig.ListFeatureFlags(); fflist != "" {
+		log.Infof("Enabled feature flags: %s", fflist)
 	}
-	logger.Infof("Enabled features: %s", msg)
 
 	return nil
 }
@@ -324,8 +304,8 @@ func main() {
 
 	// some features can require configuration or command-line options,
 	// so wwe need to parse them asap. we'll load from feature.yaml later.
-	if err := fflag.Crowdsec.SetFromEnv(log.StandardLogger()); err != nil {
-		log.Fatalf("failed set features from environment: %s", err)
+	if err := csconfig.LoadFeatureFlagsEnv(log.StandardLogger()); err != nil {
+		log.Fatalf("failed to set feature flags from environment: %s", err)
 	}
 
 	crowdsecT0 = time.Now()

pkg/csconfig/fflag.go

@@ -0,0 +1,44 @@
+package csconfig
+
+import (
+	"fmt"
+	"path/filepath"
+	"strings"
+
+	log "github.com/sirupsen/logrus"
+
+	"github.com/crowdsecurity/crowdsec/pkg/fflag"
+)
+
+
+// LoadFeatureFlagsEnv parses the environment variables to enable feature flags.
+func LoadFeatureFlagsEnv(logger *log.Logger) error {
+	if err := fflag.Crowdsec.SetFromEnv(logger); err != nil {
+		return err
+	}
+	return nil
+}
+
+
+// LoadFeatureFlags parses {ConfigDir}/feature.yaml to enable feature flags.
+func LoadFeatureFlagsFile(cConfig *Config, logger *log.Logger) error {
+	featurePath := filepath.Join(cConfig.ConfigPaths.ConfigDir, "feature.yaml")
+
+	if err := fflag.Crowdsec.SetFromYamlFile(featurePath, logger); err != nil {
+		return fmt.Errorf("file %s: %s", featurePath, err)
+	}
+	return nil
+}
+
+
+// ListFeatureFlags returns a list of the enabled feature flags.
+func ListFeatureFlags() string {
+	enabledFeatures := fflag.Crowdsec.GetEnabledFeatures()
+
+	msg := "<none>"
+	if len(enabledFeatures) > 0 {
+		msg = strings.Join(enabledFeatures, ", ")
+	}
+
+	return msg
+}

pkg/fflag/features.go

@@ -181,7 +181,7 @@ func (fr *FeatureRegister) SetFromEnv(logger *logrus.Logger) error {
 			return err
 		}
 
-		logger.Infof("Feature flag: %s=%t (from envvar). %s", featureName, enable, feat.Description)
+		logger.Debugf("Feature flag: %s=%t (from envvar). %s", featureName, enable, feat.Description)
 	}
 
 	return nil
@@ -224,7 +224,7 @@ func (fr *FeatureRegister) SetFromYaml(r io.Reader, logger *logrus.Logger) error
 			return err
 		}
 
-		logger.Infof("Feature flag: %s=true (from config file). %s", k, feat.Description)
+		logger.Debugf("Feature flag: %s=true (from config file). %s", k, feat.Description)
 	}
 
 	return nil
@@ -234,7 +234,7 @@ func (fr *FeatureRegister) SetFromYamlFile(path string, logger *logrus.Logger) e
 	f, err := os.Open(path)
 	if err != nil {
 		if os.IsNotExist(err) {
-			logger.Debugf("Feature flags config file '%s' does not exist", path)
+			logger.Tracef("Feature flags config file '%s' does not exist", path)
 
 			return nil
 		}

pkg/fflag/features_test.go

@@ -286,7 +286,7 @@ func TestSetFromEnv(t *testing.T) {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			logger, hook := logtest.NewNullLogger()
-			logger.SetLevel(logrus.InfoLevel)
+			logger.SetLevel(logrus.DebugLevel)
 			t.Setenv(tc.envvar, tc.value)
 			err := fr.SetFromEnv(logger)
 			cstest.RequireErrorMessage(t, err, tc.expectedErr)
@@ -346,7 +346,7 @@ func TestSetFromYaml(t *testing.T) {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			logger, hook := logtest.NewNullLogger()
-			logger.SetLevel(logrus.InfoLevel)
+			logger.SetLevel(logrus.DebugLevel)
 			err := fr.SetFromYaml(strings.NewReader(tc.yml), logger)
 			cstest.RequireErrorMessage(t, err, tc.expectedErr)
 			for _, expectedMessage := range tc.expectedLog {
@@ -369,7 +369,7 @@ func TestSetFromYamlFile(t *testing.T) {
 
 	fr := setUp(t)
 	logger, hook := logtest.NewNullLogger()
-	logger.SetLevel(logrus.InfoLevel)
+	logger.SetLevel(logrus.DebugLevel)
 
 	err = fr.SetFromYamlFile(tmpfile.Name(), logger)
 	require.NoError(t, err)