dont watch log files

aphralG · aphralG · commit d909648f3d40 · 2025-02-06T17:07:34.000Z
diff --git a/api/grpc/mpi/v1/command.pb.go b/api/grpc/mpi/v1/command.pb.go
diff --git a/api/grpc/mpi/v1/common.pb.go b/api/grpc/mpi/v1/common.pb.go
diff --git a/api/grpc/mpi/v1/files.pb.go b/api/grpc/mpi/v1/files.pb.go
diff --git a/internal/config/config.go b/internal/config/config.go
@@ -110,6 +110,7 @@ func ResolveConfig() (*Config, error) {
 
 	slog.Debug("Agent config", "config", config)
 	slog.Info("Enabled features", "features", config.Features)
+	slog.Info("Excluded files from file monitoring", "exclude_files", config.Watchers.FileWatcher.ExcludeFiles)
 
 	return config, nil
 }
@@ -154,7 +155,7 @@ func registerFlags() {
 	fs.StringSlice(
 		NginxExcludeLogsKey, []string{},
 		"A comma-separated list of one or more NGINX log paths that you want to exclude from metrics "+
-			"collection or error monitoring",
+			"collection or error monitoring. This includes absolute paths or regex patterns",
 	)
 
 	fs.StringSlice(AllowedDirectoriesKey,
@@ -179,6 +180,12 @@ func registerFlags() {
 		"How often the NGINX Agent will check for file changes.",
 	)
 
+	fs.StringSlice(
+		NginxExcludeFilesKey, DefaultExcludedFiles(),
+		"A comma-separated list of one or more file paths that you want to exclude from file monitoring. "+
+			"This includes absolute paths or regex patterns",
+	)
+
 	fs.StringSlice(
 		FeaturesKey,
 		DefaultFeatures(),
@@ -881,6 +888,7 @@ func resolveWatchers() *Watchers {
 		},
 		FileWatcher: FileWatcher{
 			MonitoringFrequency: DefFileWatcherMonitoringFrequency,
+			ExcludeFiles:        viperInstance.GetStringSlice(NginxExcludeFilesKey),
 		},
 	}
 }
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
@@ -44,6 +44,11 @@ func TestResolveConfig(t *testing.T) {
 		"/etc/nginx", "/usr/local/etc/nginx", "/var/run/nginx",
 		"/usr/share/nginx/modules", "/var/log/nginx",
 	}
+
+	excludeFiles := []string{
+		"\\.*log$",
+	}
+
 	viperInstance = viper.NewWithOptions(viper.KeyDelimiter(KeyDelimiter))
 	err := loadPropertiesFromFile("./testdata/nginx-agent.conf")
 	require.NoError(t, err)
@@ -64,8 +69,9 @@ func TestResolveConfig(t *testing.T) {
 
 	assert.Equal(t, 30*time.Second, actual.DataPlaneConfig.Nginx.ReloadMonitoringPeriod)
 	assert.False(t, actual.DataPlaneConfig.Nginx.TreatWarningsAsErrors)
-	assert.Equal(t, []string{"/var/log/nginx/error.log", "/var/log/nginx/access.log"},
+	assert.Equal(t, []string{"/var/log/nginx/error.log", "^/var/log/nginx/.*.log$"},
 		actual.DataPlaneConfig.Nginx.ExcludeLogs)
+	assert.Equal(t, excludeFiles, actual.Watchers.FileWatcher.ExcludeFiles)
 
 	require.NotNil(t, actual.Collector)
 	assert.Equal(t, "/etc/nginx-agent/nginx-agent-otelcol.yaml", actual.Collector.ConfigPath)
diff --git a/internal/config/defaults.go b/internal/config/defaults.go
@@ -94,6 +94,12 @@ func DefaultAllowedDirectories() []string {
 	}
 }
 
+func DefaultExcludedFiles() []string {
+	return []string{
+		"^.*(\\.log|.swx|~|.swp)$",
+	}
+}
+
 func DefaultLabels() map[string]string {
 	return make(map[string]string)
 }
diff --git a/internal/config/flags.go b/internal/config/flags.go
@@ -22,7 +22,7 @@ const (
 	FeaturesKey                                 = "features"
 	InstanceWatcherMonitoringFrequencyKey       = "watchers_instance_watcher_monitoring_frequency"
 	InstanceHealthWatcherMonitoringFrequencyKey = "watchers_instance_health_watcher_monitoring_frequency"
-	FileWatcherMonitoringFrequencyKey           = "watchers_file_watcher_monitoring_frequency"
+	FileWatcherKey                              = "watchers_file_watcher"
 )
 
 var (
@@ -96,6 +96,8 @@ var (
 	NginxReloadMonitoringPeriodKey              = pre(DataPlaneConfigRootKey, "nginx") + "reload_monitoring_period"
 	NginxTreatWarningsAsErrorsKey               = pre(DataPlaneConfigRootKey, "nginx") + "treat_warnings_as_errors"
 	NginxExcludeLogsKey                         = pre(DataPlaneConfigRootKey, "nginx") + "exclude_logs"
+	FileWatcherMonitoringFrequencyKey           = pre(FileWatcherKey) + "monitoring_frequency"
+	NginxExcludeFilesKey                        = pre(FileWatcherKey) + "exclude_files"
 )
 
 func pre(prefixes ...string) string {
diff --git a/internal/config/testdata/nginx-agent.conf b/internal/config/testdata/nginx-agent.conf
@@ -9,14 +9,16 @@ watchers:
         monitoring_frequency: 5s
     file_watcher:
         monitoring_frequency: 5s
+        exclude_files: 
+            - \.*log$
 
 data_plane_config:
     nginx:
         reload_monitoring_period: 30s
         treat_warnings_as_errors: false
         exclude_logs: 
             - /var/log/nginx/error.log
-            - /var/log/nginx/access.log
+            - ^/var/log/nginx/.*.log$
 client:
     http: 
         timeout: 15s
diff --git a/internal/config/types.go b/internal/config/types.go
@@ -290,9 +290,9 @@ type (
 	}
 
 	Watchers struct {
+		FileWatcher           FileWatcher           `yaml:"-" mapstructure:"file_watcher"`
 		InstanceWatcher       InstanceWatcher       `yaml:"-" mapstructure:"instance_watcher"`
 		InstanceHealthWatcher InstanceHealthWatcher `yaml:"-" mapstructure:"instance_health_watcher"`
-		FileWatcher           FileWatcher           `yaml:"-" mapstructure:"file_watcher"`
 	}
 
 	InstanceWatcher struct {
@@ -304,6 +304,7 @@ type (
 	}
 
 	FileWatcher struct {
+		ExcludeFiles        []string      `yaml:"-" mapstructure:"exclude_files"`
 		MonitoringFrequency time.Duration `yaml:"-" mapstructure:"monitoring_frequency"`
 	}
 )
diff --git a/internal/watcher/file/file_watcher_service.go b/internal/watcher/file/file_watcher_service.go
@@ -8,9 +8,11 @@ package file
 import (
 	"context"
 	"errors"
+	"io/fs"
 	"log/slog"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -110,20 +112,33 @@ func (fws *FileWatcherService) watchDirectories(ctx context.Context) {
 
 		slog.DebugContext(ctx, "Creating file watchers", "directory", dir)
 
-		err := filepath.Walk(dir, func(path string, info os.FileInfo, fileWalkErr error) error {
-			if fileWalkErr != nil {
-				return fileWalkErr
-			}
-			fws.addWatcher(ctx, path, info)
-
-			return nil
-		})
+		err := fws.walkDir(ctx, dir)
 		if err != nil {
 			slog.ErrorContext(ctx, "Failed to create file watchers", "directory", dir, "error", err)
 		}
 	}
 }
 
+func (fws *FileWatcherService) walkDir(ctx context.Context, dir string) error {
+	return filepath.WalkDir(dir, func(path string, d fs.DirEntry, fileWalkErr error) error {
+		if fileWalkErr != nil {
+			return fileWalkErr
+		}
+
+		info, infoErr := d.Info()
+		if infoErr != nil {
+			slog.ErrorContext(ctx, "Error getting info for file", "error", infoErr)
+			return infoErr
+		}
+
+		if d.IsDir() {
+			fws.addWatcher(ctx, path, info)
+		}
+
+		return nil
+	})
+}
+
 func (fws *FileWatcherService) addWatcher(ctx context.Context, path string, info os.FileInfo) {
 	if info.IsDir() && !fws.isWatching(path) {
 		if err := fws.watcher.Add(path); err != nil {
@@ -164,7 +179,7 @@ func (fws *FileWatcherService) isWatching(name string) bool {
 
 func (fws *FileWatcherService) handleEvent(ctx context.Context, event fsnotify.Event) {
 	if fws.enabled.Load() {
-		if isEventSkippable(event) {
+		if fws.isEventSkippable(event) {
 			slog.DebugContext(ctx, "Skipping FSNotify event", "event", event)
 			return
 		}
@@ -204,10 +219,29 @@ func (fws *FileWatcherService) checkForUpdates(ctx context.Context, ch chan<- Fi
 	}
 }
 
-func isEventSkippable(event fsnotify.Event) bool {
+func (fws *FileWatcherService) isEventSkippable(event fsnotify.Event) bool {
 	return event == emptyEvent ||
-		event.Name == "" ||
-		strings.HasSuffix(event.Name, ".swp") ||
-		strings.HasSuffix(event.Name, ".swx") ||
-		strings.HasSuffix(event.Name, "~")
+		event.Name == "" || isExcludedFile(event.Name, fws.agentConfig.Watchers.FileWatcher.ExcludeFiles)
+}
+
+func isExcludedFile(path string, excludeFiles []string) bool {
+	path = strings.ToLower(path)
+	for _, pattern := range excludeFiles {
+		_, compileErr := regexp.Compile(pattern)
+		if compileErr != nil {
+			slog.Error("Invalid path for excluding file", "file_path", pattern)
+			continue
+		}
+
+		ok, err := regexp.MatchString(pattern, path)
+		if err != nil {
+			slog.Error("Invalid path for excluding file", "file_path", pattern)
+			continue
+		} else if ok {
+			slog.Debug("Excluding file from watcher as specified in config", "file_path", path)
+			return true
+		}
+	}
+
+	return false
 }
diff --git a/internal/watcher/file/file_watcher_service_test.go b/internal/watcher/file/file_watcher_service_test.go
@@ -133,10 +133,27 @@ func TestFileWatcherService_removeWatcher(t *testing.T) {
 }
 
 func TestFileWatcherService_isEventSkippable(t *testing.T) {
-	assert.False(t, isEventSkippable(fsnotify.Event{Name: "test.conf"}))
-	assert.True(t, isEventSkippable(fsnotify.Event{Name: "test.swp"}))
-	assert.True(t, isEventSkippable(fsnotify.Event{Name: "test.swx"}))
-	assert.True(t, isEventSkippable(fsnotify.Event{Name: "test.conf~"}))
+	config := types.AgentConfig()
+	config.Watchers.FileWatcher.ExcludeFiles = []string{"^/var/log/nginx/.*.log$", "\\.*swp$", "\\.*swx$", ".*~$"}
+	fws := NewFileWatcherService(config)
+
+	assert.False(t, fws.isEventSkippable(fsnotify.Event{Name: "test.conf"}))
+	assert.True(t, fws.isEventSkippable(fsnotify.Event{Name: "test.swp"}))
+	assert.True(t, fws.isEventSkippable(fsnotify.Event{Name: "test.swx"}))
+	assert.True(t, fws.isEventSkippable(fsnotify.Event{Name: "test.conf~"}))
+	assert.True(t, fws.isEventSkippable(fsnotify.Event{Name: "/var/log/nginx/access.log"}))
+}
+
+func TestFileWatcherService_isExcludedFile(t *testing.T) {
+	excludeFiles := []string{"/var/log/nginx/access.log", "^.*(\\.log|.swx|~|.swp)$"}
+
+	assert.True(t, isExcludedFile("/var/log/nginx/error.log", excludeFiles))
+	assert.True(t, isExcludedFile("/var/log/nginx/error.swx", excludeFiles))
+	assert.True(t, isExcludedFile("test.swp", excludeFiles))
+	assert.True(t, isExcludedFile("/var/log/nginx/error~", excludeFiles))
+	assert.True(t, isExcludedFile("/var/log/nginx/access.log", excludeFiles))
+	assert.False(t, isExcludedFile("/etc/nginx/nginx.conf", excludeFiles))
+	assert.False(t, isExcludedFile("/var/log/accesslog", excludeFiles))
 }
 
 func TestFileWatcherService_Watch(t *testing.T) {
diff --git a/internal/watcher/instance/nginx_config_parser.go b/internal/watcher/instance/nginx_config_parser.go
@@ -191,14 +191,8 @@ func (ncp *NginxConfigParser) ignoreLog(logPath string) bool {
 		return true
 	}
 
-	for _, path := range ncp.agentConfig.DataPlaneConfig.Nginx.ExcludeLogs {
-		ok, err := filepath.Match(path, logPath)
-		if err != nil {
-			slog.Error("Invalid path for excluding log", "log_path", path)
-		} else if ok {
-			slog.Info("Excluding log as specified in config", "log_path", logPath)
-			return true
-		}
+	if ncp.isExcludeLog(logLower) {
+		return true
 	}
 
 	if !ncp.agentConfig.IsDirectoryAllowed(logLower) {
@@ -208,6 +202,28 @@ func (ncp *NginxConfigParser) ignoreLog(logPath string) bool {
 	return false
 }
 
+func (ncp *NginxConfigParser) isExcludeLog(path string) bool {
+	for _, pattern := range ncp.agentConfig.DataPlaneConfig.Nginx.ExcludeLogs {
+		_, compileErr := regexp.Compile(pattern)
+		if compileErr != nil {
+			slog.Error("Invalid path for excluding log", "log_path", pattern)
+			continue
+		}
+
+		ok, err := regexp.MatchString(pattern, path)
+		if err != nil {
+			slog.Error("Invalid path for excluding log", "file_path", pattern)
+			continue
+		} else if ok {
+			slog.Info("Excluding log as specified in config", "log_path", path)
+
+			return true
+		}
+	}
+
+	return false
+}
+
 func (ncp *NginxConfigParser) formatMap(directive *crossplane.Directive) map[string]string {
 	formatMap := make(map[string]string)
 
diff --git a/internal/watcher/instance/nginx_config_parser_test.go b/internal/watcher/instance/nginx_config_parser_test.go
@@ -907,23 +907,30 @@ func TestNginxConfigParser_ignoreLog(t *testing.T) {
 			expectedLog: "",
 		},
 		{
-			name:        "Test 7: exclude logs set, log path should be excluded",
+			name:        "Test 7: exclude logs set, log path should be excluded - regex",
 			logPath:     "/tmp/var/log/nginx/alert.log",
-			excludeLogs: []string{"/tmp/var/log/nginx/[^ace]*", "/tmp/var/log/nginx/a[^c]*"},
+			excludeLogs: []string{"\\.log$"},
 			expected:    true,
 			expectedLog: "",
 		},
 		{
-			name:        "Test 8: exclude logs set, log path is allowed",
+			name:        "Test 8: exclude logs set, log path should be excluded - full path",
+			logPath:     "/tmp/var/log/nginx/alert.log",
+			excludeLogs: []string{"/tmp/var/log/nginx/alert.log"},
+			expected:    true,
+			expectedLog: "",
+		},
+		{
+			name:        "Test 9: exclude logs set, log path is allowed",
 			logPath:     "/tmp/var/log/nginx/access.log",
-			excludeLogs: []string{"/tmp/var/log/nginx/[^ace]*", "/tmp/var/log/nginx/a[^c]*"},
+			excludeLogs: []string{"/tmp/var/log/nginx/alert.log", "\\.swp$"},
 			expected:    false,
 			expectedLog: "",
 		},
 		{
-			name:        "Test 9: log path outside allowed dir",
+			name:        "Test 10: log path outside allowed dir",
 			logPath:     "/var/log/nginx/access.log",
-			excludeLogs: []string{"/var/log/nginx/[^ace]*", "/var/log/nginx/a[^c]*"},
+			excludeLogs: []string{"/tmp/var/log/nginx/alert.log", "\\.swp$"},
 			expected:    false,
 			expectedLog: "Log being read is outside of allowed directories",
 		},
diff --git a/test/types/config.go b/test/types/config.go
@@ -161,6 +161,7 @@ func AgentConfig() *config.Config {
 			},
 			FileWatcher: config.FileWatcher{
 				MonitoringFrequency: config.DefFileWatcherMonitoringFrequency,
+				ExcludeFiles:        config.DefaultExcludedFiles(),
 			},
 		},
 		Features: config.DefaultFeatures(),

Original file line number	Diff line number	Diff line change
`@@ -94,6 +94,12 @@ func DefaultAllowedDirectories() []string {`
`94`	`94`	`}`
`95`	`95`	`}`
`96`	`96`
	`97`	`+func DefaultExcludedFiles() []string {`
	`98`	`+ return []string{`
	`99`	`+ "^.*(\\.log\|.swx\|~\|.swp)$",`
	`100`	`+ }`
	`101`	`+}`
	`102`	`+`
`97`	`103`	`func DefaultLabels() map[string]string {`
`98`	`104`	`return make(map[string]string)`
`99`	`105`	`}`
Original file line number	Diff line number	Diff line change
`@@ -290,9 +290,9 @@ type (`
`290`	`290`	`}`
`291`	`291`
`292`	`292`	`Watchers struct {`
	`293`	+ FileWatcher FileWatcher `yaml:"-" mapstructure:"file_watcher"`
`293`	`294`	InstanceWatcher InstanceWatcher `yaml:"-" mapstructure:"instance_watcher"`
`294`	`295`	InstanceHealthWatcher InstanceHealthWatcher `yaml:"-" mapstructure:"instance_health_watcher"`
`295`		- FileWatcher FileWatcher `yaml:"-" mapstructure:"file_watcher"`
`296`	`296`	`}`
`297`	`297`
`298`	`298`	`InstanceWatcher struct {`
`@@ -304,6 +304,7 @@ type (`
`304`	`304`	`}`
`305`	`305`
`306`	`306`	`FileWatcher struct {`
	`307`	+ ExcludeFiles []string `yaml:"-" mapstructure:"exclude_files"`
`307`	`308`	MonitoringFrequency time.Duration `yaml:"-" mapstructure:"monitoring_frequency"`
`308`	`309`	`}`
`309`	`310`	`)`