Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,12 @@ func registerFlags() {
"The port Agent will start the syslog server on for logs collection",
)

fs.Uint32(
MaxAccessLogFilesKey,
DefMaxAccessLogFiles,
"The maximum number of access log files to monitor",
)

registerCommonFlags(fs)
registerCommandFlags(fs)
registerAuxiliaryCommandFlags(fs)
Expand Down Expand Up @@ -1103,6 +1109,7 @@ func resolveDataPlaneConfig() *DataPlaneConfig {
RandomizationFactor: viperInstance.GetFloat64(NginxReloadBackoffRandomizationFactorKey),
Multiplier: viperInstance.GetFloat64(NginxReloadBackoffMultiplierKey),
},
MaxAccessLogFiles: viperInstance.GetUint32(MaxAccessLogFilesKey),
},
}
}
Expand Down
1 change: 1 addition & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1212,6 +1212,7 @@ func createConfig() *Config {
RandomizationFactor: 1.5,
Multiplier: 1.5,
},
MaxAccessLogFiles: 5,
},
},
Collector: &Collector{
Expand Down
1 change: 1 addition & 0 deletions internal/config/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const (
DefNginxReloadMonitoringPeriod = 10 * time.Second
DefTreatErrorsAsWarnings = false
DefNginxApiTlsCa = ""
DefMaxAccessLogFiles = 10

// Nginx Reload Backoff defaults
DefNginxReloadBackoffInitialInterval = 500 * time.Millisecond
Expand Down
1 change: 1 addition & 0 deletions internal/config/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,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"

Expand Down
1 change: 1 addition & 0 deletions internal/config/testdata/nginx-agent.conf
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ data_plane_config:
max_elapsed_time: 15s
randomization_factor: 1.5
multiplier: 1.5
max_access_log_files: 5
client:
http:
timeout: 15s
Expand Down
1 change: 1 addition & 0 deletions internal/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,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 uint32 `yaml:"max_access_log_files" mapstructure:"max_access_log_files"`
}

Client struct {
Expand Down
23 changes: 17 additions & 6 deletions internal/datasource/config/nginx_config_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
configPath string,
) (*model.NginxConfigContext, error) {
napEnabled := false
maxAccessLogReached := false

nginxConfigContext := &model.NginxConfigContext{
InstanceID: instance.GetInstanceMeta().GetInstanceId(),
Expand Down Expand Up @@ -225,10 +226,16 @@ func (ncp *NginxConfigParser) createNginxConfigContext(
case "log_format":
formatMap = ncp.formatMap(directive)
case "access_log":
if !ncp.ignoreLog(directive.Args[0]) {
if !ncp.ignoreLog(directive.Args[0]) && !maxAccessLogReached {
accessLog := ncp.accessLog(directive.Args[0], ncp.accessLogDirectiveFormat(directive),
formatMap)
nginxConfigContext.AccessLogs = ncp.addAccessLog(accessLog, nginxConfigContext.AccessLogs)
nginxConfigContext.AccessLogs, maxAccessLogReached = ncp.addAccessLog(
accessLog,
nginxConfigContext.AccessLogs)
if maxAccessLogReached {
slog.Warn("Maximum access log files have been reached, " +
"no further access logs will be monitored")
}
}
case "error_log":
if !ncp.ignoreLog(directive.Args[0]) {
Expand Down Expand Up @@ -345,25 +352,29 @@ func (ncp *NginxConfigParser) parseIncludeDirective(

func (ncp *NginxConfigParser) addAccessLog(accessLog *model.AccessLog,
accessLogs []*model.AccessLog,
) []*model.AccessLog {
) ([]*model.AccessLog, bool) {
for i, log := range accessLogs {
if accessLog.Name == log.Name {
if accessLog.Format != log.Format {
slog.Warn("Found multiple log_format directives for the same access log. Multiple log formats "+
"are not supported in the same access log, metrics from this access log "+
"will not be collected", "access_log", accessLog.Name)

return append(accessLogs[:i], accessLogs[i+1:]...)
return append(accessLogs[:i], accessLogs[i+1:]...), false
}
slog.Debug("Found duplicate access log, skipping", "access_log", accessLog.Name)

return accessLogs
return accessLogs, false
}
}

if len(accessLogs) >= int(ncp.agentConfig.DataPlaneConfig.Nginx.MaxAccessLogFiles) {
return accessLogs, true
}

slog.Debug("Found valid access log", "access_log", accessLog.Name)

return append(accessLogs, accessLog)
return append(accessLogs, accessLog), false
}

func (ncp *NginxConfigParser) crossplaneConfigTraverse(
Expand Down
72 changes: 64 additions & 8 deletions internal/datasource/config/nginx_config_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -750,11 +750,13 @@ func TestNginxConfigParser_checkLog(t *testing.T) {
logBuf := &bytes.Buffer{}
stub.StubLoggerWith(logBuf)
tests := []struct {
name string
expectedLog string
accessLog *model.AccessLog
currentAccessLogs []*model.AccessLog
expectedAccessLogs []*model.AccessLog
name string
expectedLog string
accessLog *model.AccessLog
currentAccessLogs []*model.AccessLog
expectedAccessLogs []*model.AccessLog
maxAccessLogFiles uint32
maxAccessLogReached bool
}{
{
name: "Test 1: valid access log",
Expand Down Expand Up @@ -790,7 +792,9 @@ func TestNginxConfigParser_checkLog(t *testing.T) {
Readable: true,
},
},
expectedLog: "Found valid access log",
expectedLog: "Found valid access log",
maxAccessLogFiles: 3,
maxAccessLogReached: false,
},
{
name: "Test 2: Duplicate access log, with same format",
Expand Down Expand Up @@ -819,7 +823,9 @@ func TestNginxConfigParser_checkLog(t *testing.T) {
Readable: true,
},
},
expectedLog: "Found duplicate access log, skipping",
expectedLog: "Found duplicate access log, skipping",
maxAccessLogFiles: 3,
maxAccessLogReached: false,
},

{
Expand All @@ -844,14 +850,64 @@ func TestNginxConfigParser_checkLog(t *testing.T) {
expectedLog: "Found multiple log_format directives for the same access log. " +
"Multiple log formats are not supported in the same access log, metrics from this access log " +
"will not be collected",
maxAccessLogFiles: 3,
maxAccessLogReached: false,
},

{
name: "Test 4: valid access log, maximum access logs reached",
accessLog: &model.AccessLog{
Name: "/var/log/nginx/access3.log",
Format: "$remote_addr - $remote_user [$time_local] \"$request\" \"$http_user_agent\" " +
"\"$http_x_forwarded_for\"$status $body_bytes_sent \"$http_referer\"",
Permissions: "",
Readable: true,
},
currentAccessLogs: []*model.AccessLog{
{
Name: "/var/log/nginx/access.log",
Format: "$remote_addr - $remote_user [$time_local] \"$request\" \"$http_user_agent\" " +
"\"$http_x_forwarded_for\"$status $body_bytes_sent \"$http_referer\"",
Permissions: "",
Readable: true,
},
{
Name: "/var/log/nginx/access2.log",
Format: "$remote_addr - $remote_user [$time_local] \"$request\" \"$http_user_agent\" " +
"\"$http_x_forwarded_for\"$status $body_bytes_sent \"$http_referer\"",
Permissions: "",
Readable: true,
},
},
expectedAccessLogs: []*model.AccessLog{
{
Name: "/var/log/nginx/access.log",
Format: "$remote_addr - $remote_user [$time_local] \"$request\" \"$http_user_agent\" " +
"\"$http_x_forwarded_for\"$status $body_bytes_sent \"$http_referer\"",
Permissions: "",
Readable: true,
},
{
Name: "/var/log/nginx/access2.log",
Format: "$remote_addr - $remote_user [$time_local] \"$request\" \"$http_user_agent\" " +
"\"$http_x_forwarded_for\"$status $body_bytes_sent \"$http_referer\"",
Permissions: "",
Readable: true,
},
},
expectedLog: "",
maxAccessLogFiles: 2,
maxAccessLogReached: true,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
ncp := NewNginxConfigParser(types.AgentConfig())
logs := ncp.addAccessLog(test.accessLog, test.currentAccessLogs)
ncp.agentConfig.DataPlaneConfig.Nginx.MaxAccessLogFiles = test.maxAccessLogFiles
logs, maxReached := ncp.addAccessLog(test.accessLog, test.currentAccessLogs)
assert.Equal(t, test.expectedAccessLogs, logs)
assert.Equal(t, test.maxAccessLogReached, maxReached)

helpers.ValidateLog(t, test.expectedLog, logBuf)

Expand Down
1 change: 1 addition & 0 deletions test/types/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ func AgentConfig() *config.Config {
TreatWarningsAsErrors: true,
ReloadMonitoringPeriod: reloadMonitoringPeriod,
ExcludeLogs: []string{},
MaxAccessLogFiles: config.DefMaxAccessLogFiles,
},
},
Watchers: &config.Watchers{
Expand Down
Loading