refactor max access file definition

Author: spencerugbo

api/grpc/mpi/v1/command_grpc.pb.go

@@ -158,7 +158,6 @@ func ResolveConfig() (*Config, error) {
 		Labels:             resolveLabels(),
 		LibDir:             viperInstance.GetString(LibDirPathKey),
 		SyslogServer:       resolveSyslogServer(),
-		MaxAccessLogFiles:  viperInstance.GetInt(MaxAccessLogFilesKey),
 	}
 
 	defaultCollector(collector, config)
@@ -1098,6 +1097,7 @@ func resolveDataPlaneConfig() *DataPlaneConfig {
 				RandomizationFactor: viperInstance.GetFloat64(NginxReloadBackoffRandomizationFactorKey),
 				Multiplier:          viperInstance.GetFloat64(NginxReloadBackoffMultiplierKey),
 			},
+			MaxAccessLogFiles: viperInstance.GetInt(MaxAccessLogFilesKey),
 		},
 	}
 }

api/grpc/mpi/v1/files_grpc.pb.go

@@ -25,7 +25,6 @@ const (
 	InstanceHealthWatcherMonitoringFrequencyKey = "watchers_instance_health_watcher_monitoring_frequency"
 	FileWatcherKey                              = "watchers_file_watcher"
 	LibDirPathKey                               = "lib_dir"
-	MaxAccessLogFilesKey                        = "max_access_log_files"
 )
 
 var (
@@ -137,6 +136,7 @@ var (
 	NginxReloadBackoffMultiplierKey          = pre(NginxReloadBackoffKey) + "multiplier"
 	NginxExcludeLogsKey                      = pre(DataPlaneConfigRootKey, "nginx") + "exclude_logs"
 	NginxApiTlsCa                            = pre(DataPlaneConfigRootKey, "nginx") + "api_tls_ca"
+	MaxAccessLogFilesKey                     = pre(DataPlaneConfigRootKey, "nginx") + "max_access_log_files"
 
 	SyslogServerPort = pre("syslog_server") + "port"
 

internal/config/config.go

@@ -36,22 +36,21 @@ func parseServerType(str string) (ServerType, bool) {
 
 type (
 	Config struct {
-		Command            *Command         `yaml:"command"              mapstructure:"command"`
-		AuxiliaryCommand   *Command         `yaml:"auxiliary_command"    mapstructure:"auxiliary_command"`
-		Log                *Log             `yaml:"log"                  mapstructure:"log"`
-		DataPlaneConfig    *DataPlaneConfig `yaml:"data_plane_config"    mapstructure:"data_plane_config"`
-		Client             *Client          `yaml:"client"               mapstructure:"client"`
-		Collector          *Collector       `yaml:"collector"            mapstructure:"collector"`
-		Watchers           *Watchers        `yaml:"watchers"             mapstructure:"watchers"`
-		SyslogServer       *SyslogServer    `yaml:"syslog_server"        mapstructure:"syslog_server"`
-		Labels             map[string]any   `yaml:"labels"               mapstructure:"labels"`
+		Command            *Command         `yaml:"command"             mapstructure:"command"`
+		AuxiliaryCommand   *Command         `yaml:"auxiliary_command"   mapstructure:"auxiliary_command"`
+		Log                *Log             `yaml:"log"                 mapstructure:"log"`
+		DataPlaneConfig    *DataPlaneConfig `yaml:"data_plane_config"   mapstructure:"data_plane_config"`
+		Client             *Client          `yaml:"client"              mapstructure:"client"`
+		Collector          *Collector       `yaml:"collector"           mapstructure:"collector"`
+		Watchers           *Watchers        `yaml:"watchers"            mapstructure:"watchers"`
+		SyslogServer       *SyslogServer    `yaml:"syslog_server"       mapstructure:"syslog_server"`
+		Labels             map[string]any   `yaml:"labels"              mapstructure:"labels"`
 		Version            string           `yaml:"-"`
 		Path               string           `yaml:"-"`
 		UUID               string           `yaml:"-"`
 		LibDir             string           `yaml:"-"`
-		AllowedDirectories []string         `yaml:"allowed_directories"  mapstructure:"allowed_directories"`
-		Features           []string         `yaml:"features"             mapstructure:"features"`
-		MaxAccessLogFiles  int              `yaml:"max_access_log_files" mapstructure:"max_access_log_files"`
+		AllowedDirectories []string         `yaml:"allowed_directories" mapstructure:"allowed_directories"`
+		Features           []string         `yaml:"features"            mapstructure:"features"`
 	}
 
 	Log struct {
@@ -72,6 +71,7 @@ type (
 		ExcludeLogs            []string      `yaml:"exclude_logs"             mapstructure:"exclude_logs"`
 		ReloadMonitoringPeriod time.Duration `yaml:"reload_monitoring_period" mapstructure:"reload_monitoring_period"`
 		TreatWarningsAsErrors  bool          `yaml:"treat_warnings_as_errors" mapstructure:"treat_warnings_as_errors"`
+		MaxAccessLogFiles      int           `yaml:"max_access_log_files"     mapstructure:"max_access_log_files"`
 	}
 
 	Client struct {
@@ -478,10 +478,6 @@ func (c *Config) IsCommandServerProxyConfigured() bool {
 	return c.Command.Server.Proxy.URL != ""
 }
 
-func (c *Config) IsMaxAccessLogFilesConfigured() bool {
-	return c.MaxAccessLogFiles != 0
-}
-
 // isAllowedDir checks if the given path is in the list of allowed directories.
 // It recursively checks the parent directories of the path, until it finds a match or reaches the root directory.
 func isAllowedDir(path string, allowedDirs []string) bool {

internal/config/flags.go

@@ -228,13 +228,7 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
 					if !ncp.ignoreLog(directive.Args[0]) {
 						accessLog := ncp.accessLog(directive.Args[0], ncp.accessLogDirectiveFormat(directive),
 							formatMap)
-						if ncp.agentConfig.IsMaxAccessLogFilesConfigured() {
-							nginxConfigContext.AccessLogs = ncp.addAccessLog(accessLog,
-								nginxConfigContext.AccessLogs, ncp.agentConfig.MaxAccessLogFiles)
-						} else {
-							nginxConfigContext.AccessLogs = ncp.addAccessLog(accessLog,
-								nginxConfigContext.AccessLogs, config.DefMaxAccessLogFiles)
-						}
+						nginxConfigContext.AccessLogs = ncp.addAccessLog(accessLog, nginxConfigContext.AccessLogs)
 					}
 				case "error_log":
 					if !ncp.ignoreLog(directive.Args[0]) {
@@ -350,7 +344,7 @@ func (ncp *NginxConfigParser) parseIncludeDirective(
 }
 
 func (ncp *NginxConfigParser) addAccessLog(accessLog *model.AccessLog,
-	accessLogs []*model.AccessLog, maxAccessLogFiles int,
+	accessLogs []*model.AccessLog,
 ) []*model.AccessLog {
 	for i, log := range accessLogs {
 		if accessLog.Name == log.Name {
@@ -367,8 +361,10 @@ func (ncp *NginxConfigParser) addAccessLog(accessLog *model.AccessLog,
 		}
 	}
 
-	if len(accessLogs) >= maxAccessLogFiles {
-		slog.Warn("Maximum access log files have been reached, additional logs will be skipped")
+	if len(accessLogs) >= ncp.agentConfig.DataPlaneConfig.Nginx.MaxAccessLogFiles {
+		slog.Warn("Maximum access log files have been reached, unable to monitor access log",
+			"access_log", accessLog.Name)
+
 		return accessLogs
 	}
 

internal/config/types.go

@@ -891,15 +891,16 @@ func TestNginxConfigParser_checkLog(t *testing.T) {
 					Readable:    true,
 				},
 			},
-			expectedLog:       "Maximum access log files have been reached, additional logs will be skipped",
+			expectedLog:       "Maximum access log files have been reached",
 			maxAccessLogFiles: 2,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			ncp := NewNginxConfigParser(types.AgentConfig())
-			logs := ncp.addAccessLog(test.accessLog, test.currentAccessLogs, test.maxAccessLogFiles)
+			ncp.agentConfig.DataPlaneConfig.Nginx.MaxAccessLogFiles = test.maxAccessLogFiles
+			logs := ncp.addAccessLog(test.accessLog, test.currentAccessLogs)
 			assert.Equal(t, test.expectedAccessLogs, logs)
 
 			helpers.ValidateLog(t, test.expectedLog, logBuf)

internal/datasource/config/nginx_config_parser.go

@@ -169,6 +169,7 @@ func AgentConfig() *config.Config {
 				TreatWarningsAsErrors:  true,
 				ReloadMonitoringPeriod: reloadMonitoringPeriod,
 				ExcludeLogs:            []string{},
+				MaxAccessLogFiles:      config.DefMaxAccessLogFiles,
 			},
 		},
 		Watchers: &config.Watchers{