Skip to content

Commit 0a5b5a3

Browse files
authored
Add Exclude Files to File Watcher & Switch to Regex for Exclude Logs (#975)
1 parent 6fc1f16 commit 0a5b5a3

File tree

11 files changed

+133
-37
lines changed

11 files changed

+133
-37
lines changed

internal/config/config.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,8 @@ func ResolveConfig() (*Config, error) {
110110

111111
slog.Debug("Agent config", "config", config)
112112
slog.Info("Enabled features", "features", config.Features)
113+
slog.Info("Excluded files from being watched for file changes", "exclude_files",
114+
config.Watchers.FileWatcher.ExcludeFiles)
113115

114116
return config, nil
115117
}
@@ -154,7 +156,7 @@ func registerFlags() {
154156
fs.StringSlice(
155157
NginxExcludeLogsKey, []string{},
156158
"A comma-separated list of one or more NGINX log paths that you want to exclude from metrics "+
157-
"collection or error monitoring",
159+
"collection or error monitoring. This includes absolute paths or regex patterns",
158160
)
159161

160162
fs.StringSlice(AllowedDirectoriesKey,
@@ -179,6 +181,12 @@ func registerFlags() {
179181
"How often the NGINX Agent will check for file changes.",
180182
)
181183

184+
fs.StringSlice(
185+
NginxExcludeFilesKey, DefaultExcludedFiles(),
186+
"A comma-separated list of one or more file paths that you want to exclude from file monitoring. "+
187+
"This includes absolute paths or regex patterns",
188+
)
189+
182190
fs.StringSlice(
183191
FeaturesKey,
184192
DefaultFeatures(),
@@ -881,6 +889,7 @@ func resolveWatchers() *Watchers {
881889
},
882890
FileWatcher: FileWatcher{
883891
MonitoringFrequency: viperInstance.GetDuration(FileWatcherMonitoringFrequencyKey),
892+
ExcludeFiles: viperInstance.GetStringSlice(NginxExcludeFilesKey),
884893
},
885894
}
886895
}

internal/config/config_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,7 @@ func createConfig() *Config {
759759
},
760760
DataPlaneConfig: &DataPlaneConfig{
761761
Nginx: &NginxDataPlaneConfig{
762-
ExcludeLogs: []string{"/var/log/nginx/error.log", "/var/log/nginx/access.log"},
762+
ExcludeLogs: []string{"/var/log/nginx/error.log", "^/var/log/nginx/.*.log$"},
763763
ReloadMonitoringPeriod: 30 * time.Second,
764764
TreatWarningsAsErrors: true,
765765
},
@@ -920,6 +920,7 @@ func createConfig() *Config {
920920
},
921921
FileWatcher: FileWatcher{
922922
MonitoringFrequency: 10 * time.Second,
923+
ExcludeFiles: []string{"\\.*log$"},
923924
},
924925
},
925926
Labels: map[string]any{

internal/config/defaults.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,12 @@ func DefaultAllowedDirectories() []string {
9494
}
9595
}
9696

97+
func DefaultExcludedFiles() []string {
98+
return []string{
99+
"^.*(\\.log|.swx|~|.swp)$",
100+
}
101+
}
102+
97103
func DefaultLabels() map[string]string {
98104
return make(map[string]string)
99105
}

internal/config/flags.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const (
2222
FeaturesKey = "features"
2323
InstanceWatcherMonitoringFrequencyKey = "watchers_instance_watcher_monitoring_frequency"
2424
InstanceHealthWatcherMonitoringFrequencyKey = "watchers_instance_health_watcher_monitoring_frequency"
25-
FileWatcherMonitoringFrequencyKey = "watchers_file_watcher_monitoring_frequency"
25+
FileWatcherKey = "watchers_file_watcher"
2626
)
2727

2828
var (
@@ -96,6 +96,8 @@ var (
9696
NginxReloadMonitoringPeriodKey = pre(DataPlaneConfigRootKey, "nginx") + "reload_monitoring_period"
9797
NginxTreatWarningsAsErrorsKey = pre(DataPlaneConfigRootKey, "nginx") + "treat_warnings_as_errors"
9898
NginxExcludeLogsKey = pre(DataPlaneConfigRootKey, "nginx") + "exclude_logs"
99+
FileWatcherMonitoringFrequencyKey = pre(FileWatcherKey) + "monitoring_frequency"
100+
NginxExcludeFilesKey = pre(FileWatcherKey) + "exclude_files"
99101
)
100102

101103
func pre(prefixes ...string) string {

internal/config/testdata/nginx-agent.conf

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ watchers:
99
monitoring_frequency: 10s
1010
file_watcher:
1111
monitoring_frequency: 10s
12+
exclude_files:
13+
- \.*log$
1214

1315
labels:
1416
label1: label 1
@@ -21,7 +23,7 @@ data_plane_config:
2123
treat_warnings_as_errors: true
2224
exclude_logs:
2325
- /var/log/nginx/error.log
24-
- /var/log/nginx/access.log
26+
- ^/var/log/nginx/.*.log$
2527
client:
2628
http:
2729
timeout: 15s

internal/config/types.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,9 +285,9 @@ type (
285285
}
286286

287287
Watchers struct {
288+
FileWatcher FileWatcher `yaml:"-" mapstructure:"file_watcher"`
288289
InstanceWatcher InstanceWatcher `yaml:"-" mapstructure:"instance_watcher"`
289290
InstanceHealthWatcher InstanceHealthWatcher `yaml:"-" mapstructure:"instance_health_watcher"`
290-
FileWatcher FileWatcher `yaml:"-" mapstructure:"file_watcher"`
291291
}
292292

293293
InstanceWatcher struct {
@@ -299,6 +299,7 @@ type (
299299
}
300300

301301
FileWatcher struct {
302+
ExcludeFiles []string `yaml:"-" mapstructure:"exclude_files"`
302303
MonitoringFrequency time.Duration `yaml:"-" mapstructure:"monitoring_frequency"`
303304
}
304305
)

internal/watcher/file/file_watcher_service.go

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@ package file
88
import (
99
"context"
1010
"errors"
11+
"io/fs"
1112
"log/slog"
1213
"os"
1314
"path/filepath"
15+
"regexp"
1416
"strings"
1517
"sync"
1618
"sync/atomic"
@@ -110,20 +112,33 @@ func (fws *FileWatcherService) watchDirectories(ctx context.Context) {
110112

111113
slog.DebugContext(ctx, "Creating file watchers", "directory", dir)
112114

113-
err := filepath.Walk(dir, func(path string, info os.FileInfo, fileWalkErr error) error {
114-
if fileWalkErr != nil {
115-
return fileWalkErr
116-
}
117-
fws.addWatcher(ctx, path, info)
118-
119-
return nil
120-
})
115+
err := fws.walkDir(ctx, dir)
121116
if err != nil {
122117
slog.ErrorContext(ctx, "Failed to create file watchers", "directory", dir, "error", err)
123118
}
124119
}
125120
}
126121

122+
func (fws *FileWatcherService) walkDir(ctx context.Context, dir string) error {
123+
return filepath.WalkDir(dir, func(path string, d fs.DirEntry, fileWalkErr error) error {
124+
if fileWalkErr != nil {
125+
return fileWalkErr
126+
}
127+
128+
info, infoErr := d.Info()
129+
if infoErr != nil {
130+
slog.ErrorContext(ctx, "Error getting info for file", "error", infoErr)
131+
return infoErr
132+
}
133+
134+
if d.IsDir() {
135+
fws.addWatcher(ctx, path, info)
136+
}
137+
138+
return nil
139+
})
140+
}
141+
127142
func (fws *FileWatcherService) addWatcher(ctx context.Context, path string, info os.FileInfo) {
128143
if info.IsDir() && !fws.isWatching(path) {
129144
if err := fws.watcher.Add(path); err != nil {
@@ -164,7 +179,7 @@ func (fws *FileWatcherService) isWatching(name string) bool {
164179

165180
func (fws *FileWatcherService) handleEvent(ctx context.Context, event fsnotify.Event) {
166181
if fws.enabled.Load() {
167-
if isEventSkippable(event) {
182+
if fws.isEventSkippable(event) {
168183
slog.DebugContext(ctx, "Skipping FSNotify event", "event", event)
169184
return
170185
}
@@ -204,10 +219,29 @@ func (fws *FileWatcherService) checkForUpdates(ctx context.Context, ch chan<- Fi
204219
}
205220
}
206221

207-
func isEventSkippable(event fsnotify.Event) bool {
222+
func (fws *FileWatcherService) isEventSkippable(event fsnotify.Event) bool {
208223
return event == emptyEvent ||
209-
event.Name == "" ||
210-
strings.HasSuffix(event.Name, ".swp") ||
211-
strings.HasSuffix(event.Name, ".swx") ||
212-
strings.HasSuffix(event.Name, "~")
224+
event.Name == "" || isExcludedFile(event.Name, fws.agentConfig.Watchers.FileWatcher.ExcludeFiles)
225+
}
226+
227+
func isExcludedFile(path string, excludeFiles []string) bool {
228+
path = strings.ToLower(path)
229+
for _, pattern := range excludeFiles {
230+
_, compileErr := regexp.Compile(pattern)
231+
if compileErr != nil {
232+
slog.Error("Invalid path for excluding file", "file_path", pattern)
233+
continue
234+
}
235+
236+
ok, err := regexp.MatchString(pattern, path)
237+
if err != nil {
238+
slog.Error("Invalid path for excluding file", "file_path", pattern)
239+
continue
240+
} else if ok {
241+
slog.Debug("Excluding file from watcher as specified in config", "file_path", path)
242+
return true
243+
}
244+
}
245+
246+
return false
213247
}

internal/watcher/file/file_watcher_service_test.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,27 @@ func TestFileWatcherService_removeWatcher(t *testing.T) {
133133
}
134134

135135
func TestFileWatcherService_isEventSkippable(t *testing.T) {
136-
assert.False(t, isEventSkippable(fsnotify.Event{Name: "test.conf"}))
137-
assert.True(t, isEventSkippable(fsnotify.Event{Name: "test.swp"}))
138-
assert.True(t, isEventSkippable(fsnotify.Event{Name: "test.swx"}))
139-
assert.True(t, isEventSkippable(fsnotify.Event{Name: "test.conf~"}))
136+
config := types.AgentConfig()
137+
config.Watchers.FileWatcher.ExcludeFiles = []string{"^/var/log/nginx/.*.log$", "\\.*swp$", "\\.*swx$", ".*~$"}
138+
fws := NewFileWatcherService(config)
139+
140+
assert.False(t, fws.isEventSkippable(fsnotify.Event{Name: "test.conf"}))
141+
assert.True(t, fws.isEventSkippable(fsnotify.Event{Name: "test.swp"}))
142+
assert.True(t, fws.isEventSkippable(fsnotify.Event{Name: "test.swx"}))
143+
assert.True(t, fws.isEventSkippable(fsnotify.Event{Name: "test.conf~"}))
144+
assert.True(t, fws.isEventSkippable(fsnotify.Event{Name: "/var/log/nginx/access.log"}))
145+
}
146+
147+
func TestFileWatcherService_isExcludedFile(t *testing.T) {
148+
excludeFiles := []string{"/var/log/nginx/access.log", "^.*(\\.log|.swx|~|.swp)$"}
149+
150+
assert.True(t, isExcludedFile("/var/log/nginx/error.log", excludeFiles))
151+
assert.True(t, isExcludedFile("/var/log/nginx/error.swx", excludeFiles))
152+
assert.True(t, isExcludedFile("test.swp", excludeFiles))
153+
assert.True(t, isExcludedFile("/var/log/nginx/error~", excludeFiles))
154+
assert.True(t, isExcludedFile("/var/log/nginx/access.log", excludeFiles))
155+
assert.False(t, isExcludedFile("/etc/nginx/nginx.conf", excludeFiles))
156+
assert.False(t, isExcludedFile("/var/log/accesslog", excludeFiles))
140157
}
141158

142159
func TestFileWatcherService_Watch(t *testing.T) {

internal/watcher/instance/nginx_config_parser.go

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -198,14 +198,8 @@ func (ncp *NginxConfigParser) ignoreLog(logPath string) bool {
198198
return true
199199
}
200200

201-
for _, path := range ncp.agentConfig.DataPlaneConfig.Nginx.ExcludeLogs {
202-
ok, err := filepath.Match(path, logPath)
203-
if err != nil {
204-
slog.Error("Invalid path for excluding log", "log_path", path)
205-
} else if ok {
206-
slog.Info("Excluding log as specified in config", "log_path", logPath)
207-
return true
208-
}
201+
if ncp.isExcludeLog(logLower) {
202+
return true
209203
}
210204

211205
if !ncp.agentConfig.IsDirectoryAllowed(logLower) {
@@ -215,6 +209,28 @@ func (ncp *NginxConfigParser) ignoreLog(logPath string) bool {
215209
return false
216210
}
217211

212+
func (ncp *NginxConfigParser) isExcludeLog(path string) bool {
213+
for _, pattern := range ncp.agentConfig.DataPlaneConfig.Nginx.ExcludeLogs {
214+
_, compileErr := regexp.Compile(pattern)
215+
if compileErr != nil {
216+
slog.Error("Invalid path for excluding log", "log_path", pattern)
217+
continue
218+
}
219+
220+
ok, err := regexp.MatchString(pattern, path)
221+
if err != nil {
222+
slog.Error("Invalid path for excluding log", "file_path", pattern)
223+
continue
224+
} else if ok {
225+
slog.Info("Excluding log as specified in config", "log_path", path)
226+
227+
return true
228+
}
229+
}
230+
231+
return false
232+
}
233+
218234
func (ncp *NginxConfigParser) formatMap(directive *crossplane.Directive) map[string]string {
219235
formatMap := make(map[string]string)
220236

internal/watcher/instance/nginx_config_parser_test.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -909,23 +909,30 @@ func TestNginxConfigParser_ignoreLog(t *testing.T) {
909909
expectedLog: "",
910910
},
911911
{
912-
name: "Test 7: exclude logs set, log path should be excluded",
912+
name: "Test 7: exclude logs set, log path should be excluded - regex",
913913
logPath: "/tmp/var/log/nginx/alert.log",
914-
excludeLogs: []string{"/tmp/var/log/nginx/[^ace]*", "/tmp/var/log/nginx/a[^c]*"},
914+
excludeLogs: []string{"\\.log$"},
915915
expected: true,
916916
expectedLog: "",
917917
},
918918
{
919-
name: "Test 8: exclude logs set, log path is allowed",
919+
name: "Test 8: exclude logs set, log path should be excluded - full path",
920+
logPath: "/tmp/var/log/nginx/alert.log",
921+
excludeLogs: []string{"/tmp/var/log/nginx/alert.log"},
922+
expected: true,
923+
expectedLog: "",
924+
},
925+
{
926+
name: "Test 9: exclude logs set, log path is allowed",
920927
logPath: "/tmp/var/log/nginx/access.log",
921-
excludeLogs: []string{"/tmp/var/log/nginx/[^ace]*", "/tmp/var/log/nginx/a[^c]*"},
928+
excludeLogs: []string{"/tmp/var/log/nginx/alert.log", "\\.swp$"},
922929
expected: false,
923930
expectedLog: "",
924931
},
925932
{
926-
name: "Test 9: log path outside allowed dir",
933+
name: "Test 10: log path outside allowed dir",
927934
logPath: "/var/log/nginx/access.log",
928-
excludeLogs: []string{"/var/log/nginx/[^ace]*", "/var/log/nginx/a[^c]*"},
935+
excludeLogs: []string{"/tmp/var/log/nginx/alert.log", "\\.swp$"},
929936
expected: false,
930937
expectedLog: "Log being read is outside of allowed directories",
931938
},

0 commit comments

Comments
 (0)