feat: Harvest should use slogs text format by default (#3179)

NetApp · Sep 27, 2024 · acc870e · acc870e
1 parent c00a27a
commit acc870e
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 167 deletions.
diff --git a/cmd/harvest/harvest.go b/cmd/harvest/harvest.go
@@ -46,7 +46,10 @@ import (
 	"time"
 )
 
-const maxCol = 40
+const (
+	maxCol           = 40
+	defaultLogFormat = "plain"
+)
 
 type options struct {
 	command    string
@@ -57,7 +60,8 @@ type options struct {
 	debug      bool
 	foreground bool
 	loglevel   int
-	logToFile  bool // only used when running in foreground
+	logToFile  bool   // only used when running in the foreground
+	logFormat  string // one of plain or json
 	config     string
 	confPath   string
 	profiling  bool
@@ -358,6 +362,7 @@ func startPoller(pollerName string, promPort int, opts *options) {
 	if promPort != 0 {
 		argv = append(argv, "--promPort", strconv.Itoa(promPort))
 	}
+
 	if opts.debug {
 		argv = append(argv, "--debug")
 	}
@@ -370,6 +375,10 @@ func startPoller(pollerName string, promPort int, opts *options) {
 		argv = append(argv, "--confpath", opts.confPath)
 	}
 
+	if opts.logFormat != defaultLogFormat {
+		argv = append(argv, "--logformat", opts.logFormat)
+	}
+
 	if opts.profiling {
 		if opts.foreground {
 			// Always pick the same port when profiling in foreground
@@ -537,82 +546,23 @@ Feedback
   Open issues at https://github.com/NetApp/harvest
 `)
 
-	startCmd.PersistentFlags().BoolVarP(
-		&opts.debug,
-		"debug",
-		"d",
-		false,
-		"enable debug logging (same as -loglevel 1). If both debug and loglevel are specified, loglevel wins",
-	)
-	startCmd.PersistentFlags().BoolVarP(
-		&opts.verbose,
-		"verbose",
-		"v",
-		false,
-		"verbose logging (loglevel=1)",
-	)
-	startCmd.PersistentFlags().BoolVarP(
-		&opts.trace,
-		"trace",
-		"t",
-		false,
-		"trace logging (loglevel=0)",
-	)
-	startCmd.PersistentFlags().BoolVarP(
-		&opts.foreground,
-		"foreground",
-		"f",
-		false,
-		"start single poller in foreground",
-	)
-	startCmd.PersistentFlags().BoolVar(
-		&opts.daemon,
-		"daemon",
-		true,
-		"start poller in background",
-	)
-	startCmd.PersistentFlags().IntVarP(
-		&opts.loglevel,
-		"loglevel",
-		"l",
-		2,
-		"logging level (0=trace, 1=debug, 2=info, 3=warning, 4=error, 5=critical)",
-	)
-	startCmd.PersistentFlags().BoolVar(
-		&opts.logToFile,
-		"logtofile",
-		false,
-		"When running in the foreground, log to file instead of stdout",
-	)
-	startCmd.PersistentFlags().BoolVar(
-		&opts.profiling,
-		"profiling",
-		false,
-		"if profiling port > 0, enables profiling via localhost:PORT/debug/pprof/",
-	)
-	startCmd.PersistentFlags().IntVar(
-		&opts.promPort,
-		"promPort",
-		0,
-		"prometheus port to use for HTTP endpoint",
-	)
-	startCmd.PersistentFlags().StringSliceVarP(
-		&opts.collectors,
-		"collectors",
-		"c",
-		[]string{},
-		"only start these collectors (overrides harvest.yml)",
-	)
-	startCmd.PersistentFlags().StringSliceVarP(
-		&opts.objects,
-		"objects",
-		"o",
-		[]string{},
-		"only start these objects (overrides collector config)",
-	)
-	_ = startCmd.PersistentFlags().MarkHidden("logtofile")
-	_ = startCmd.PersistentFlags().MarkHidden("verbose")
-	_ = startCmd.PersistentFlags().MarkHidden("trace")
+	start := startCmd.PersistentFlags()
+	start.BoolVarP(&opts.debug, "debug", "d", false, "enable debug logging (same as -loglevel 1). If both debug and loglevel are specified, loglevel wins")
+	start.BoolVarP(&opts.verbose, "verbose", "v", false, "verbose logging (loglevel=1)")
+	start.BoolVarP(&opts.trace, "trace", "t", false, "trace logging (loglevel=0)")
+	start.BoolVarP(&opts.foreground, "foreground", "f", false, "start single poller in foreground")
+	start.BoolVar(&opts.daemon, "daemon", true, "start poller in background")
+	start.IntVarP(&opts.loglevel, "loglevel", "l", 2, "logging level (0=trace, 1=debug, 2=info, 3=warning, 4=error, 5=critical)")
+	start.BoolVar(&opts.logToFile, "logtofile", false, "when running in the foreground, log to file instead of stdout")
+	start.StringVar(&opts.logFormat, "logformat", defaultLogFormat, "log format (plain or json)")
+	start.BoolVar(&opts.profiling, "profiling", false, "if profiling port > 0, enables profiling via localhost:PORT/debug/pprof/")
+	start.IntVar(&opts.promPort, "promPort", 0, "prometheus port to use for HTTP endpoint")
+	start.StringSliceVarP(&opts.collectors, "collectors", "c", []string{}, "only start these collectors (overrides harvest.yml)")
+	start.StringSliceVarP(&opts.objects, "objects", "o", []string{}, "only start these objects (overrides collector config)")
+
+	_ = start.MarkHidden("logtofile")
+	_ = start.MarkHidden("verbose")
+	_ = start.MarkHidden("trace")
 }
 
 // The management commands: start|status|stop|restart|kill

diff --git a/cmd/poller/options/options.go b/cmd/poller/options/options.go
@@ -27,6 +27,7 @@ type Options struct {
 	Config     string   // filepath of Harvest config (defaults to "harvest.yml") can be relative or absolute path
 	HomePath   string   // path to harvest home (usually "/opt/harvest")
 	LogPath    string   // log files location (usually "/var/log/harvest")
+	LogFormat  string   // log format to use (plain or json)
 	LogLevel   int      // logging level, 0 for trace, 5 for fatal
 	LogToFile  bool     // when running in the foreground, log to file instead of stdout
 	Version    string   // harvest version

diff --git a/cmd/poller/poller.go b/cmd/poller/poller.go
@@ -203,6 +203,7 @@ func (p *Poller) Init() error {
 		PrefixKey:             "Poller",
 		PrefixValue:           p.name,
 		LogLevel:              logLevel,
+		LogFormat:             p.options.LogFormat,
 		FileLoggingEnabled:    fileLoggingEnabled,
 		Directory:             p.options.LogPath,
 		Filename:              logFileName,
@@ -1503,10 +1504,11 @@ func init() {
 
 	var flags = pollerCmd.Flags()
 	flags.StringVarP(&opts.Poller, "poller", "p", "", "Poller name as defined in config")
-	flags.BoolVarP(&opts.Debug, "debug", "d", false, "Enable debug logging (same as -loglevel 1). If both debug and loglevel are specified, loglevel wins")
+	flags.BoolVarP(&opts.Debug, "debug", "d", false, "Enable debug logging (same as --loglevel 1). If both debug and loglevel are specified, loglevel wins")
 	flags.BoolVar(&opts.Daemon, "daemon", false, "Start as daemon")
 	flags.IntVarP(&opts.LogLevel, "loglevel", "l", 2, "Logging level (0=trace, 1=debug, 2=info, 3=warning, 4=error, 5=critical)")
-	flags.BoolVar(&opts.LogToFile, "logtofile", false, "When running in the foreground, log to file instead of stdout")
+	flags.StringVar(&opts.LogFormat, "logformat", "plain", "Log format (plain or json)")
+	flags.BoolVar(&opts.LogToFile, "logtofile", false, "Log to the poller_ name prefixed file in the "+logging.GetLogPath()+" directory, instead of stdout")
 	flags.IntVar(&opts.Profiling, "profiling", 0, "If profiling port > 0, enables profiling via localhost:PORT/debug/pprof/")
 	flags.IntVar(&opts.PromPort, "promPort", 0, "Prometheus Port")
 	flags.StringVar(&opts.Config, "config", conf.HarvestYML, "Harvest config file path")

diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go
@@ -15,21 +15,19 @@ import (
 const (
 	defaultLogFileName           = "harvest.log"
 	defaultConsoleLoggingEnabled = true
-	defaultFileLoggingEnabled    = false // false to avoid opening many file descriptors for same log file
+	defaultFileLoggingEnabled    = false // false to avoid opening many file descriptors for the same log file
 	DefaultLogMaxMegaBytes       = 10    // 10 MB
 	DefaultLogMaxBackups         = 5
 	DefaultLogMaxAge             = 7
 )
 
 // LogConfig defines the configuration for logging
 type LogConfig struct {
-	// Enable console logging
 	ConsoleLoggingEnabled bool
-	// Log Level
-	LogLevel slog.Level
-	// Prefix
-	PrefixKey   string
-	PrefixValue string
+	LogLevel              slog.Level
+	LogFormat             string // one of "plain" or "json"
+	PrefixKey             string
+	PrefixValue           string
 	// FileLoggingEnabled makes the framework log to a file
 	FileLoggingEnabled bool
 	// Directory to log to when filelogging is enabled
@@ -54,13 +52,12 @@ var (
 func Get() *slog.Logger {
 	once.Do(func() {
 		if logger == nil {
-			defaultPrefixKey := "harvest"
-			defaultPrefixValue := "harvest"
 			logConfig := LogConfig{
 				ConsoleLoggingEnabled: defaultConsoleLoggingEnabled,
-				PrefixKey:             defaultPrefixKey,
-				PrefixValue:           defaultPrefixValue,
+				PrefixKey:             "harvest",
+				PrefixValue:           "harvest",
 				LogLevel:              slog.LevelInfo,
+				LogFormat:             "plain",
 				FileLoggingEnabled:    defaultFileLoggingEnabled,
 				Directory:             GetLogPath(),
 				Filename:              defaultLogFileName,
@@ -100,24 +97,24 @@ func Configure(config LogConfig) *slog.Logger {
 	}
 
 	var (
-		handlers []slog.Handler
-		aLogger  *slog.Logger
+		handler slog.Handler
+		aLogger *slog.Logger
+		writer  io.Writer
 	)
 
-	if config.ConsoleLoggingEnabled {
-		handlers = append(handlers, slog.NewTextHandler(os.Stderr, handlerOptions))
-	}
-
+	writer = os.Stderr
 	if config.FileLoggingEnabled {
-		handlers = append(handlers, slog.NewJSONHandler(newRollingFile(config), handlerOptions))
+		writer = newRollingFile(config)
 	}
 
-	if len(handlers) == 1 {
-		aLogger = slog.New(handlers[0])
+	if config.LogFormat == "plain" {
+		handler = slog.NewTextHandler(writer, handlerOptions)
 	} else {
-		aLogger = slog.New(MultiHandler(handlers...))
+		handler = slog.NewJSONHandler(writer, handlerOptions)
 	}
 
+	aLogger = slog.New(handler)
+
 	if config.PrefixKey != "" {
 		aLogger = aLogger.With(slog.String(config.PrefixKey, config.PrefixValue))
 	}

diff --git a/pkg/logging/multi.go b/pkg/logging/multi.go