fix: PR comments

fix: PR comments

fix: PR comments

fix: PR comments

fix: PR comments

fix: PR comments

fix: PR comments

Author: JustinChengLZ

cmd/base/options/generic.go

@@ -17,13 +17,9 @@ limitations under the License.
 package options
 
 import (
-	"flag"
-	"os"
-
 	"k8s.io/apimachinery/pkg/util/errors"
 	cliflag "k8s.io/component-base/cli/flag"
 	componentbaseconfig "k8s.io/component-base/config"
-	"k8s.io/klog/v2"
 
 	"github.com/kubewharf/katalyst-core/pkg/config/generic"
 	"github.com/kubewharf/katalyst-core/pkg/util/process"
@@ -72,12 +68,6 @@ func NewGenericOptions() *GenericOptions {
 func (o *GenericOptions) AddFlags(fss *cliflag.NamedFlagSets) {
 	fs := fss.FlagSet("generic")
 
-	local := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
-	klog.InitFlags(local)
-	local.VisitAll(func(fl *flag.Flag) {
-		fs.AddGoFlag(fl)
-	})
-
 	fs.BoolVar(&o.DryRun, "dry-run", o.DryRun, "A bool to enable and disable dry-run.")
 	fs.BoolVar(&o.EnableHealthzCheck, "enable-healthz-check", o.EnableHealthzCheck, "A bool to enable and disable healthz check.")
 

cmd/base/options/log.go

@@ -17,7 +17,11 @@ limitations under the License.
 package options
 
 import (
+	"flag"
+	"os"
+
 	"github.com/spf13/pflag"
+	"k8s.io/klog/v2"
 
 	"github.com/kubewharf/katalyst-core/pkg/config/generic"
 	"github.com/kubewharf/katalyst-core/pkg/util/general"
@@ -28,7 +32,7 @@ type LogsOptions struct {
 	LogFileMaxSizeInMB  uint64
 	SupportAsyncLogging bool
 	LogDir              string
-	LogBufferSizeMB     int
+	LogBufferSize       int
 	LogFileMaxAge       int
 	LogFileMaxBackups   int
 }
@@ -37,19 +41,28 @@ func NewLogsOptions() *LogsOptions {
 	return &LogsOptions{
 		LogPackageLevel:    general.LoggingPKGFull,
 		LogFileMaxSizeInMB: 1800,
-		LogBufferSizeMB:    10000,
+		LogBufferSize:      10000,
 		LogFileMaxAge:      7,
 		LogFileMaxBackups:  10,
 	}
 }
 
 // AddFlags adds flags  to the specified FlagSet.
 func (o *LogsOptions) AddFlags(fs *pflag.FlagSet) {
+	fs.BoolVar(&o.SupportAsyncLogging, "support-async-logging", o.SupportAsyncLogging, "whether to support async logging")
+	// If async logging is enabled, we do not call klog.InitFlags to avoid conflicting with klog flags.
+	if !o.SupportAsyncLogging {
+		local := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
+		klog.InitFlags(local)
+		local.VisitAll(func(fl *flag.Flag) {
+			fs.AddGoFlag(fl)
+		})
+	} else {
+		fs.StringVar(&o.LogDir, "log_dir", o.LogDir, "directory to store logs")
+	}
 	fs.Var(&o.LogPackageLevel, "logs-package-level", "the default package level for logging")
 	fs.Uint64Var(&o.LogFileMaxSizeInMB, "log-file-max-size", o.LogFileMaxSizeInMB, "Max size of klog file in MB.")
-	fs.BoolVar(&o.SupportAsyncLogging, "support-async-logging", o.SupportAsyncLogging, "whether to support async logging")
-	fs.StringVar(&o.LogDir, "log-dir", o.LogDir, "directory of log file")
-	fs.IntVar(&o.LogBufferSizeMB, "log-buffer-size", o.LogBufferSizeMB, "size of the ring buffer to store async logs")
+	fs.IntVar(&o.LogBufferSize, "log-buffer-size", o.LogBufferSize, "size of the ring buffer to store async logs")
 	fs.IntVar(&o.LogFileMaxAge, "log-file-max-age", o.LogFileMaxAge, "max age of klog log file in days")
 	fs.IntVar(&o.LogFileMaxBackups, "log-file-max-backups", o.LogFileMaxBackups, "max number of klog log file backups")
 }
@@ -59,8 +72,8 @@ func (o *LogsOptions) ApplyTo(c *generic.LogConfiguration) error {
 	general.SetLogFileMaxSize(o.LogFileMaxSizeInMB)
 	c.SupportAsyncLogging = o.SupportAsyncLogging
 	c.LogDir = o.LogDir
+	c.LogBufferSize = o.LogBufferSize
 	c.LogFileMaxSize = int(o.LogFileMaxSizeInMB)
-	c.LogBufferSizeMB = o.LogBufferSizeMB
 	c.LogFileMaxAge = o.LogFileMaxAge
 	c.LogFileMaxBackups = o.LogFileMaxBackups
 	return nil

cmd/katalyst-agent/app/agent.go

@@ -64,7 +64,7 @@ func Run(
 	}
 
 	if conf.SupportAsyncLogging {
-		asyncLogger := logging.NewAsyncLogger(genericCtx, conf.LogDir, conf.LogFileMaxSize, conf.LogFileMaxAge, conf.LogFileMaxBackups, conf.LogBufferSizeMB)
+		asyncLogger := logging.NewAsyncLogger(genericCtx, conf.LogDir, conf.LogFileMaxSize, conf.LogFileMaxAge, conf.LogFileMaxBackups, conf.LogBufferSize)
 		defer asyncLogger.Shutdown()
 	}
 

pkg/config/generic/log.go

@@ -20,7 +20,7 @@ type LogConfiguration struct {
 	SupportAsyncLogging bool
 	LogDir              string
 	LogFileMaxSize      int
-	LogBufferSizeMB     int
+	LogBufferSize       int
 	LogFileMaxAge       int
 	LogFileMaxBackups   int
 }

pkg/util/logging/logger.go

@@ -17,7 +17,10 @@ limitations under the License.
 package logging
 
 import (
+	"fmt"
+	"os"
 	"path"
+	"path/filepath"
 	"time"
 
 	"github.com/rs/zerolog/diode"
@@ -44,11 +47,11 @@ const (
 	metricsNameNumDroppedFatalLogs   = "number_of_dropped_fatal_logs"
 )
 
-const (
-	defaultInfoLogFileName    = "agent.info.log"
-	defaultWarningLogFileName = "agent.warning.log"
-	defaultErrorLogFileName   = "agent.error.log"
-	defaultFatalLogFileName   = "agent.fatal.log"
+var (
+	defaultInfoLogFileName    = fmt.Sprintf("%s.%s.log", filepath.Base(os.Args[0]), InfoSeverity)
+	defaultWarningLogFileName = fmt.Sprintf("%s.%s.log", filepath.Base(os.Args[0]), WarningSeverity)
+	defaultErrorLogFileName   = fmt.Sprintf("%s.%s.log", filepath.Base(os.Args[0]), ErrorSeverity)
+	defaultFatalLogFileName   = fmt.Sprintf("%s.%s.log", filepath.Base(os.Args[0]), FatalSeverity)
 )
 
 type logInfo struct {
@@ -70,7 +73,7 @@ type AsyncLogger struct {
 // NewAsyncLogger creates an async logger that produces an async writer for each of the severity levels.
 // The async writer spins up a goroutine that periodically flushes the buffered logs to disk.
 func NewAsyncLogger(
-	agentCtx *agent.GenericContext, logDir string, maxSizeMB, maxAge, maxBackups, bufferSizeMB int,
+	agentCtx *agent.GenericContext, logDir string, maxSizeMB, maxAge, maxBackups, bufferSize int,
 ) *AsyncLogger {
 	wrappedEmitter := agentCtx.EmitterPool.GetDefaultMetricsEmitter()
 
@@ -85,7 +88,7 @@ func NewAsyncLogger(
 		}
 
 		// diodeWriter is a writer that stores logs in a ring buffer and asynchronously flushes them
-		diodeWriter := diode.NewWriter(lumberjackLogger, bufferSizeMB, 10*time.Millisecond, func(missed int) {
+		diodeWriter := diode.NewWriter(lumberjackLogger, bufferSize, 10*time.Millisecond, func(missed int) {
 			_ = wrappedEmitter.StoreInt64(logInfo.metricsName, int64(missed), metrics.MetricTypeNameRaw)
 		})
 		// Overrides the default synchronous writer with the diode writer