feat: update in logging to console and Unify case usage, seperators a…

…nd punctuation for logging (#322)

Signed-off-by: Mihir Mittal <105881639+mihirm21@users.noreply.github.com>

cmd/root.go

@@ -72,6 +72,6 @@ func initConfig() {
 
 	// If a config file is found, read it in.
 	if err := viper.ReadInConfig(); err == nil {
-		fmt.Fprintln(os.Stderr, "Using config file:", viper.ConfigFileUsed())
+		fmt.Fprintln(os.Stderr, "using config file:", viper.ConfigFileUsed())
 	}
 }

cmd/start.go

@@ -24,6 +24,7 @@ const (
 	bearerTokenFlagName    = "bearer-token"
 	corsFlagName           = "cors-origin"
 	syncProviderFlagName   = "sync-provider"
+	logFormatFlagName      = "log-format"
 )
 
 func init() {
@@ -55,6 +56,7 @@ func init() {
 	flags.StringP(
 		syncProviderFlagName, "y", "", "DEPRECATED: Set a sync provider e.g. filepath or remote",
 	)
+	flags.StringP(logFormatFlagName, "z", "console", "Set the logging format, e.g. console or json ")
 
 	_ = viper.BindPFlag(portFlagName, flags.Lookup(portFlagName))
 	_ = viper.BindPFlag(metricsPortFlagName, flags.Lookup(metricsPortFlagName))
@@ -67,6 +69,7 @@ func init() {
 	_ = viper.BindPFlag(bearerTokenFlagName, flags.Lookup(bearerTokenFlagName))
 	_ = viper.BindPFlag(corsFlagName, flags.Lookup(corsFlagName))
 	_ = viper.BindPFlag(syncProviderFlagName, flags.Lookup(syncProviderFlagName))
+	_ = viper.BindPFlag(logFormatFlagName, flags.Lookup(logFormatFlagName))
 }
 
 // startCmd represents the start command
@@ -83,7 +86,7 @@ var startCmd = &cobra.Command{
 		} else {
 			level = zapcore.InfoLevel
 		}
-		l, err := logger.NewZapLogger(level)
+		l, err := logger.NewZapLogger(level, viper.GetString(logFormatFlagName))
 		if err != nil {
 			log.Fatalf("can't initialize zap logger: %v", err)
 		}
@@ -99,7 +102,6 @@ var startCmd = &cobra.Command{
 			rtLogger.Warn("DEPRECATED: The --evaluator flag has been deprecated. " +
 				"Docs: https://github.com/open-feature/flagd/blob/main/docs/configuration/configuration.md")
 		}
-
 		// Build Runtime -----------------------------------------------------------
 		rt, err := runtime.FromConfig(logger, runtime.Config{
 			ServicePort:       viper.GetInt32(portFlagName),

cmd/version.go

@@ -27,6 +27,6 @@ var versionCmd = &cobra.Command{
 				}
 			}
 		}
-		fmt.Printf("flagd %s (%s) built at %s\n", Version, Commit, Date)
+		fmt.Printf("flagd: %s (%s), built at: %s\n", Version, Commit, Date)
 	},
 }

docs/configuration/flagd_start.md

@@ -13,6 +13,7 @@ flagd start [flags]
   -C, --cors-origin strings                 CORS allowed origins, * will allow all origins
   -e, --evaluator string                    DEPRECATED: Set an evaluator e.g. json, yaml/yml.Please note that yaml/yml and json evaluations work the same (yaml/yml files are converted to json internally) (default "json")
   -h, --help                                help for start
+  -z, --log-format string                   Set the logging format, e.g. console or json  (default "console")
   -m, --metrics-port int32                  Port to serve metrics on (default 8014)
   -p, --port int32                          Port to listen on (default 8013)
   -c, --server-cert-path string             Server side tls certificate path

pkg/eval/fractional_evaluation.go

@@ -16,7 +16,7 @@ type fractionalEvaluationDistribution struct {
 func (je *JSONEvaluator) fractionalEvaluation(values, data interface{}) interface{} {
 	valueToDistribute, feDistributions, err := parseFractionalEvaluationData(values, data)
 	if err != nil {
-		je.Logger.Error(fmt.Sprintf("parseFractionalEvaluationData: %v", err))
+		je.Logger.Error(fmt.Sprintf("parse fractional evaluation data: %v", err))
 		return nil
 	}
 
@@ -49,7 +49,7 @@ func parseFractionalEvaluationData(values, data interface{}) (string, []fraction
 
 	valueToDistribute, ok := v.(string)
 	if !ok {
-		return "", nil, fmt.Errorf("var %s isn't of type string", bucketBy)
+		return "", nil, fmt.Errorf("var: %s isn't of type string", bucketBy)
 	}
 
 	feDistributions, err := parseFractionalEvaluationDistributions(valuesArray)
@@ -92,7 +92,7 @@ func parseFractionalEvaluationDistributions(values []interface{}) ([]fractionalE
 	}
 
 	if sumOfPercentages != 100 {
-		return nil, fmt.Errorf("percentages must sum to 100, got %d", sumOfPercentages)
+		return nil, fmt.Errorf("percentages must sum to 100, got: %d", sumOfPercentages)
 	}
 
 	return feDistributions, nil

pkg/eval/json_evaluator.go

@@ -152,7 +152,7 @@ func (je *JSONEvaluator) ResolveAllValues(reqID string, context *structpb.Struct
 			)
 		}
 		if err != nil {
-			je.Logger.ErrorWithID(reqID, fmt.Sprintf("Bulk evaluation: key %s returned error %s", flagKey, err.Error()))
+			je.Logger.ErrorWithID(reqID, fmt.Sprintf("bulk evaluation: key: %s returned error: %s", flagKey, err.Error()))
 			continue
 		}
 		values = append(values, NewAnyValue(value, variant, reason, flagKey))
@@ -225,12 +225,12 @@ func (je *JSONEvaluator) evaluateVariant(
 	flag, ok := je.state.Flags[flagKey]
 	if !ok {
 		// flag not found
-		je.Logger.DebugWithID(reqID, fmt.Sprintf("requested flag could not be found %s", flagKey))
+		je.Logger.DebugWithID(reqID, fmt.Sprintf("requested flag could not be found: %s", flagKey))
 		return "", model.ErrorReason, errors.New(model.FlagNotFoundErrorCode)
 	}
 
 	if flag.State == Disabled {
-		je.Logger.DebugWithID(reqID, fmt.Sprintf("requested flag is disabled %s", flagKey))
+		je.Logger.DebugWithID(reqID, fmt.Sprintf("requested flag is disabled: %s", flagKey))
 		return "", model.ErrorReason, errors.New(model.FlagDisabledErrorCode)
 	}
 
@@ -240,21 +240,21 @@ func (je *JSONEvaluator) evaluateVariant(
 	if targeting != nil && string(targeting) != "{}" {
 		targetingBytes, err := targeting.MarshalJSON()
 		if err != nil {
-			je.Logger.ErrorWithID(reqID, fmt.Sprintf("Error parsing rules for flag %s, %s", flagKey, err))
+			je.Logger.ErrorWithID(reqID, fmt.Sprintf("Error parsing rules for flag: %s, %s", flagKey, err))
 			return "", model.ErrorReason, err
 		}
 
 		b, err := json.Marshal(context)
 		if err != nil {
-			je.Logger.ErrorWithID(reqID, fmt.Sprintf("error parsing context for flag %s, %s, %v", flagKey, err, context))
+			je.Logger.ErrorWithID(reqID, fmt.Sprintf("error parsing context for flag: %s, %s, %v", flagKey, err, context))
 
 			return "", model.ErrorReason, errors.New(model.ErrorReason)
 		}
 		var result bytes.Buffer
 		// evaluate json-logic rules to determine the variant
 		err = jsonlogic.Apply(bytes.NewReader(targetingBytes), bytes.NewReader(b), &result)
 		if err != nil {
-			je.Logger.ErrorWithID(reqID, fmt.Sprintf("Error applying rules %s", err))
+			je.Logger.ErrorWithID(reqID, fmt.Sprintf("error applying rules: %s", err))
 			return "", model.ErrorReason, err
 		}
 		// strip whitespace and quotes from the variant
@@ -265,7 +265,7 @@ func (je *JSONEvaluator) evaluateVariant(
 			return variant, model.TargetingMatchReason, nil
 		}
 
-		je.Logger.DebugWithID(reqID, fmt.Sprintf("returning default variant for flagKey %s, variant is not valid", flagKey))
+		je.Logger.DebugWithID(reqID, fmt.Sprintf("returning default variant for flagKey: %s, variant is not valid", flagKey))
 		reason = model.DefaultReason
 	} else {
 		reason = model.StaticReason
@@ -279,7 +279,7 @@ func validateDefaultVariants(flags Flags) error {
 	for name, flag := range flags.Flags {
 		if _, ok := flag.Variants[flag.DefaultVariant]; !ok {
 			return fmt.Errorf(
-				"default variant '%s' isn't a valid variant of flag '%s'", flag.DefaultVariant, name,
+				"default variant: '%s' isn't a valid variant of flag: '%s'", flag.DefaultVariant, name,
 			)
 		}
 	}

pkg/eval/json_evaluator_model.go

@@ -44,7 +44,7 @@ func (f Flags) Merge(logger *logger.Logger, source string, ff Flags) (Flags, map
 			if val.Source != source {
 				logger.Warn(
 					fmt.Sprintf(
-						"key value %s is duplicated across multiple sources this can lead to unexpected behavior: %s, %s",
+						"key value: %s is duplicated across multiple sources this can lead to unexpected behavior: %s, %s",
 						k,
 						val.Source,
 						source,

pkg/eval/json_evaluator_test.go

@@ -284,13 +284,13 @@ func TestGetState_Valid_ContainsFlag(t *testing.T) {
 	// get the state
 	state, err := evaluator.GetState()
 	if err != nil {
-		t.Fatalf("Expected no error")
+		t.Fatalf("expected no error")
 	}
 
 	// validate it contains the flag
 	wants := "validFlag"
 	if !strings.Contains(state, wants) {
-		t.Fatalf("Expected %s to contain %s", state, wants)
+		t.Fatalf("expected: %s to contain: %s", state, wants)
 	}
 }
 
@@ -300,7 +300,7 @@ func TestSetState_Invalid_Error(t *testing.T) {
 	// set state with an invalid flag definition
 	_, err := evaluator.SetState("", InvalidFlags)
 	if err == nil {
-		t.Fatalf("Expected error")
+		t.Fatalf("expected error")
 	}
 }
 
@@ -310,15 +310,15 @@ func TestSetState_Valid_NoError(t *testing.T) {
 	// set state with a valid flag definition
 	_, err := evaluator.SetState("", ValidFlags)
 	if err != nil {
-		t.Fatalf("Expected no error")
+		t.Fatalf("expected no error")
 	}
 }
 
 func TestResolveAllValues(t *testing.T) {
 	evaluator := eval.JSONEvaluator{Logger: logger.NewLogger(nil, false)}
 	_, err := evaluator.SetState("", Flags)
 	if err != nil {
-		t.Fatalf("Expected no error")
+		t.Fatalf("expected no error")
 	}
 	tests := []struct {
 		context map[string]interface{}
@@ -378,7 +378,7 @@ func TestResolveBooleanValue(t *testing.T) {
 	evaluator := eval.JSONEvaluator{Logger: logger.NewLogger(nil, false)}
 	_, err := evaluator.SetState("", Flags)
 	if err != nil {
-		t.Fatalf("Expected no error")
+		t.Fatalf("expected no error")
 	}
 
 	for _, test := range tests {
@@ -417,7 +417,7 @@ func BenchmarkResolveBooleanValue(b *testing.B) {
 	evaluator := eval.JSONEvaluator{Logger: logger.NewLogger(nil, false)}
 	_, err := evaluator.SetState("", Flags)
 	if err != nil {
-		b.Fatalf("Expected no error")
+		b.Fatalf("expected no error")
 	}
 	reqID := "test"
 	for _, test := range tests {
@@ -461,7 +461,7 @@ func TestResolveStringValue(t *testing.T) {
 	evaluator := eval.JSONEvaluator{Logger: logger.NewLogger(nil, false)}
 	_, err := evaluator.SetState("", Flags)
 	if err != nil {
-		t.Fatalf("Expected no error")
+		t.Fatalf("expected no error")
 	}
 
 	for _, test := range tests {
@@ -501,7 +501,7 @@ func BenchmarkResolveStringValue(b *testing.B) {
 	evaluator := eval.JSONEvaluator{Logger: logger.NewLogger(nil, false)}
 	_, err := evaluator.SetState("", Flags)
 	if err != nil {
-		b.Fatalf("Expected no error")
+		b.Fatalf("expected no error")
 	}
 	reqID := "test"
 	for _, test := range tests {
@@ -545,7 +545,7 @@ func TestResolveFloatValue(t *testing.T) {
 	evaluator := eval.JSONEvaluator{Logger: logger.NewLogger(nil, false)}
 	_, err := evaluator.SetState("", Flags)
 	if err != nil {
-		t.Fatalf("Expected no error")
+		t.Fatalf("expected no error")
 	}
 
 	for _, test := range tests {
@@ -585,15 +585,15 @@ func BenchmarkResolveFloatValue(b *testing.B) {
 	evaluator := eval.JSONEvaluator{Logger: logger.NewLogger(nil, false)}
 	_, err := evaluator.SetState("", Flags)
 	if err != nil {
-		b.Fatalf("Expected no error")
+		b.Fatalf("expected no error")
 	}
 	reqID := "test"
 	for _, test := range tests {
 		apStruct, err := structpb.NewStruct(test.context)
 		if err != nil {
 			b.Fatal(err)
 		}
-		b.Run(fmt.Sprintf("test %s", test.flagKey), func(b *testing.B) {
+		b.Run(fmt.Sprintf("test: %s", test.flagKey), func(b *testing.B) {
 			for i := 0; i < b.N; i++ {
 				val, _, reason, err := evaluator.ResolveFloatValue(reqID, test.flagKey, apStruct)
 
@@ -629,7 +629,7 @@ func TestResolveIntValue(t *testing.T) {
 	evaluator := eval.JSONEvaluator{Logger: logger.NewLogger(nil, false)}
 	_, err := evaluator.SetState("", Flags)
 	if err != nil {
-		t.Fatalf("Expected no error")
+		t.Fatalf("expected no error")
 	}
 
 	for _, test := range tests {
@@ -669,7 +669,7 @@ func BenchmarkResolveIntValue(b *testing.B) {
 	evaluator := eval.JSONEvaluator{Logger: logger.NewLogger(nil, false)}
 	_, err := evaluator.SetState("", Flags)
 	if err != nil {
-		b.Fatalf("Expected no error")
+		b.Fatalf("expected no error")
 	}
 	reqID := "test"
 	for _, test := range tests {
@@ -713,7 +713,7 @@ func TestResolveObjectValue(t *testing.T) {
 	evaluator := eval.JSONEvaluator{Logger: logger.NewLogger(nil, false)}
 	_, err := evaluator.SetState("", Flags)
 	if err != nil {
-		t.Fatalf("Expected no error")
+		t.Fatalf("expected no error")
 	}
 
 	for _, test := range tests {
@@ -756,7 +756,7 @@ func BenchmarkResolveObjectValue(b *testing.B) {
 	evaluator := eval.JSONEvaluator{Logger: logger.NewLogger(nil, false)}
 	_, err := evaluator.SetState("", Flags)
 	if err != nil {
-		b.Fatalf("Expected no error")
+		b.Fatalf("expected no error")
 	}
 	reqID := "test"
 	for _, test := range tests {

pkg/logger/logger.go

@@ -166,9 +166,9 @@ func (l *Logger) ClearFields(reqID string) {
 }
 
 // NewZapLogger creates a *zap.Logger using the base config
-func NewZapLogger(level zapcore.Level) (*zap.Logger, error) {
+func NewZapLogger(level zapcore.Level, logFormat string) (*zap.Logger, error) {
 	cfg := zap.Config{
-		Encoding:         "json",
+		Encoding:         logFormat,
 		Level:            zap.NewAtomicLevelAt(level),
 		OutputPaths:      []string{"stderr"},
 		ErrorOutputPaths: []string{"stderr"},

pkg/runtime/from_config.go

@@ -73,7 +73,7 @@ func (r *Runtime) setSyncImplFromConfig(logger *logger.Logger) error {
 				),
 				ProviderArgs: r.config.ProviderArgs,
 			})
-			rtLogger.Debug(fmt.Sprintf("Using filepath sync-provider for %q", uri))
+			rtLogger.Debug(fmt.Sprintf("using filepath sync-provider for: %q", uri))
 		case regCrd.Match(uriB):
 			r.SyncImpl = append(r.SyncImpl, &kubernetes.Sync{
 				Logger: logger.WithFields(
@@ -83,7 +83,7 @@ func (r *Runtime) setSyncImplFromConfig(logger *logger.Logger) error {
 				URI:          regCrd.ReplaceAllString(uri, ""),
 				ProviderArgs: r.config.ProviderArgs,
 			})
-			rtLogger.Debug(fmt.Sprintf("Using kubernetes sync-provider for %s", uri))
+			rtLogger.Debug(fmt.Sprintf("using kubernetes sync-provider for: %s", uri))
 		case regURL.Match(uriB):
 			r.SyncImpl = append(r.SyncImpl, &httpSync.Sync{
 				URI:         uri,
@@ -98,7 +98,7 @@ func (r *Runtime) setSyncImplFromConfig(logger *logger.Logger) error {
 				ProviderArgs: r.config.ProviderArgs,
 				Cron:         cron.New(),
 			})
-			rtLogger.Debug(fmt.Sprintf("Using remote sync-provider for %q", uri))
+			rtLogger.Debug(fmt.Sprintf("using remote sync-provider for: %q", uri))
 		default:
 			return fmt.Errorf("invalid sync uri argument: %s, must start with 'file:', 'http(s)://', or 'core.openfeature.dev'",
 				uri)

pkg/runtime/runtime.go

@@ -42,13 +42,13 @@ type Config struct {
 
 func (r *Runtime) Start() error {
 	if r.Service == nil {
-		return errors.New("no Service set")
+		return errors.New("no service set")
 	}
 	if len(r.SyncImpl) == 0 {
-		return errors.New("no SyncImplementation set")
+		return errors.New("no sync implementation set")
 	}
 	if r.Evaluator == nil {
-		return errors.New("no Evaluator set")
+		return errors.New("no evaluator set")
 	}
 
 	ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)

pkg/service/connect_metrics.go

@@ -91,7 +91,7 @@ type Middleware struct {
 
 func (c *middlewareConfig) defaults() {
 	if c.Recorder == nil {
-		panic("Recorder is required")
+		panic("recorder is required")
 	}
 }