diff --git a/internal/resource/nginx_log_tailer_operator.go b/internal/resource/nginx_log_tailer_operator.go index cf3a24f3d..3a62f12ee 100644 --- a/internal/resource/nginx_log_tailer_operator.go +++ b/internal/resource/nginx_log_tailer_operator.go @@ -24,12 +24,11 @@ type NginxLogTailerOperator struct { var _ logTailerOperator = (*NginxLogTailerOperator)(nil) var ( - reloadErrorList = []*re.Regexp{ - re.MustCompile(`.*\[emerg\].*`), - re.MustCompile(`.*\[alert\].*`), - re.MustCompile(`.*\[crit\].*`), - } - warningRegex = re.MustCompile(`.*\[warn\].*`) + // Line is over 120 characters long, regex needs to be on one line so needs to be ignored by linter + // nolint: lll + reloadErrorList = re.MustCompile(`\d{1,4}\/\d{1,2}\/\d{1,2} ([0-1][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9] ?(nginx\:|) (\[emerg\]|\[alert\]|\[crit\])`) + // nolint: lll + warningRegex = re.MustCompile(`\d{1,4}\/\d{1,2}\/\d{1,2} ([0-1][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9] ?(nginx\:|) (\[warn\])`) ignoreErrorList = re.MustCompile(`.*(usage report| license expired).*`) ) @@ -72,16 +71,12 @@ func (l *NginxLogTailerOperator) Tail(ctx context.Context, errorLog string, erro } func (l *NginxLogTailerOperator) doesLogLineContainError(line string) bool { - if l.agentConfig.DataPlaneConfig.Nginx.TreatWarningsAsErrors && warningRegex.MatchString(line) && - !ignoreErrorList.MatchString(line) { + if ignoreErrorList.MatchString(line) { + return false + } else if (l.agentConfig.DataPlaneConfig.Nginx.TreatWarningsAsErrors && warningRegex.MatchString(line)) || + reloadErrorList.MatchString(line) { return true } - for _, errorRegex := range reloadErrorList { - if errorRegex.MatchString(line) && !ignoreErrorList.MatchString(line) { - return true - } - } - return false } diff --git a/internal/resource/nginx_log_tailer_operator_test.go b/internal/resource/nginx_log_tailer_operator_test.go index 6126e6666..c7643d9d0 100644 --- a/internal/resource/nginx_log_tailer_operator_test.go +++ b/internal/resource/nginx_log_tailer_operator_test.go @@ -8,21 +8,26 @@ package resource import ( "bytes" "context" - "errors" "fmt" "sync" "testing" "time" + "github.com/nginx/agent/v3/test/types" + "github.com/stretchr/testify/assert" "github.com/nginx/agent/v3/test/helpers" - "github.com/nginx/agent/v3/test/types" "github.com/stretchr/testify/require" ) const ( - errorLogLine = "2023/03/14 14:16:23 [emerg] 3871#3871: bind() to 0.0.0.0:8081 failed (98: Address already in use)" + errorLogLine = "2023/03/14 14:16:23 [emerg] 3871#3871: bind() to 0.0.0.0:8081 failed (98: Address already in use)" + critLogLine = "2023/07/07 11:30:00 [crit] 123456#123456: *1 connect() to unix:/test/test/test/test.sock failed" + + " (2: No such file or directory) while connecting to upstream, client: 0.0.0.0, server: _, request:" + + " \"POST /test HTTP/2.0\", upstream: \"grpc://unix:/test/test/test/test.sock:\", host: \"0.0.0.0:0\"" + alertLogLine = "2023/06/20 11:01:56 [alert] 4138#4138: open() \"/var/log/nginx/error.log\" failed (13: Permission" + + " denied)" warningLogLine = "2023/03/14 14:16:23 nginx: [warn] 2048 worker_connections exceed open file resource limit: 1024" ) @@ -34,7 +39,6 @@ func TestLogOperator_Tail(t *testing.T) { tests := []struct { out *bytes.Buffer - err error expected error name string errorLogs string @@ -45,7 +49,6 @@ func TestLogOperator_Tail(t *testing.T) { out: bytes.NewBufferString(""), errorLogs: errorLogFile.Name(), errorLogContents: "", - err: nil, expected: nil, }, { @@ -53,16 +56,14 @@ func TestLogOperator_Tail(t *testing.T) { out: bytes.NewBufferString(""), errorLogs: errorLogFile.Name(), errorLogContents: errorLogLine, - err: nil, - expected: errors.Join(fmt.Errorf("%s", errorLogLine)), + expected: fmt.Errorf("%s", errorLogLine), }, { name: "Test 3: Warning in error logs", out: bytes.NewBufferString(""), errorLogs: errorLogFile.Name(), errorLogContents: warningLogLine, - err: nil, - expected: errors.Join(fmt.Errorf("%s", warningLogLine)), + expected: fmt.Errorf("%s", warningLogLine), }, { name: "Test 4: ignore error log: usage report ", @@ -70,7 +71,6 @@ func TestLogOperator_Tail(t *testing.T) { errorLogs: errorLogFile.Name(), errorLogContents: "2025/06/25 15:08:04 [error] 123456#123456: certificate verify error: " + "(10:certificate has expired) during usage report", - err: nil, expected: nil, }, { @@ -79,7 +79,6 @@ func TestLogOperator_Tail(t *testing.T) { errorLogs: errorLogFile.Name(), errorLogContents: "2025/06/25 15:07:24 [alert] 123456#123456: license expired; the grace period " + "will end in 71 days", - err: nil, expected: nil, }, { @@ -87,35 +86,113 @@ func TestLogOperator_Tail(t *testing.T) { out: bytes.NewBufferString(""), errorLogs: errorLogFile.Name(), errorLogContents: "2024/12/25 15:00:04 [error] 123456#123456: server returned 400 during usage report", - err: nil, expected: nil, }, } for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - operator := NewLogTailerOperator(types.AgentConfig()) + t.Run(test.name, func(tt *testing.T) { + agentConfig := types.AgentConfig() + agentConfig.DataPlaneConfig.Nginx.ReloadMonitoringPeriod = 5 * time.Second + operator := NewLogTailerOperator(agentConfig) logErrorChannel := make(chan error, len(test.errorLogs)) defer close(logErrorChannel) var wg sync.WaitGroup wg.Add(1) - operator.Tail(ctx, test.errorLogs, logErrorChannel) go func(testErr error) { defer wg.Done() + operator.Tail(ctx, test.errorLogs, logErrorChannel) err := <-logErrorChannel - assert.Equal(t, testErr, err) - }(test.err) + assert.Equal(tt, testErr, err) + }(test.expected) - time.Sleep(200 * time.Millisecond) + time.Sleep(100 * time.Millisecond) if test.errorLogContents != "" { _, err := errorLogFile.WriteString(test.errorLogContents) - require.NoError(t, err, "Error writing data to error log file") + require.NoError(tt, err, "Error writing data to error log file") } wg.Wait() }) } } + +func TestLogOperator_doesLogLineContainError(t *testing.T) { + tests := []struct { + name string + line string + treatWarnAsError bool + expected bool + }{ + { + name: "Test 1: no error in line", + line: "", + treatWarnAsError: false, + expected: false, + }, + { + name: "Test 2: emerg in line", + line: errorLogLine, + treatWarnAsError: false, + expected: true, + }, + { + name: "Test 3: crit in line", + line: critLogLine, + treatWarnAsError: false, + expected: true, + }, + { + name: "Test 4: alert in line", + line: alertLogLine, + treatWarnAsError: false, + expected: true, + }, + { + name: "Test 5: warn in line, treat warn as error off", + line: warningLogLine, + treatWarnAsError: false, + expected: false, + }, + { + name: "Test 6: warn in line, treat warn as error on", + line: warningLogLine, + treatWarnAsError: true, + expected: true, + }, + { + name: "Test 7: ignore error, usage report", + line: "2025/06/25 15:08:04 [error] 123456#123456: certificate verify error: " + + "(10:certificate has expired) during usage report", + treatWarnAsError: false, + expected: false, + }, + { + name: "Test 8: ignore error, license expired", + line: "2025/06/25 15:07:24 [alert] 123456#123456: license expired; the grace period " + + "will end in 71 days", + treatWarnAsError: false, + expected: false, + }, + { + name: "Test 9: check log is ignored, [emerg] in log but not at emerg level", + line: "2025/06/25 15:07:24 [info] 123456#123456: checking [emerg] is ignored", + treatWarnAsError: false, + expected: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(tt *testing.T) { + agentConfig := types.AgentConfig() + agentConfig.DataPlaneConfig.Nginx.TreatWarningsAsErrors = test.treatWarnAsError + operator := NewLogTailerOperator(agentConfig) + + result := operator.doesLogLineContainError(test.line) + assert.Equal(tt, test.expected, result) + }) + } +}