Skip to content

Commit

Permalink
Improve handling of different path variants on Windows (#3781)
Browse files Browse the repository at this point in the history
* Improve handling of different path variants on Windows

If forward slashes were used on Windows the glob was not matching on startup which lead to the issue that data was resent. This is solved in 5.3 by using path.Abs for all paths which also includes Clean. To make sure Path and Glob are always clean, Cleanup was added to the MatchFile part. This makes sure in case old data / incorrect data is loaded, the comparison will still work. More tests were added for windows to verify change.

Before this change, option 1 below did not work.

```
"F:/wwwLogs/flights-wsapi/WebServicesWebApi.log"
"F:\\wwwLogs\\flights-wsapi\\WebServicesWebApi.log"
'F:\wwwLogs\flights-wsapi\WebServicesWebApi.log'
```

Based on https://discuss.elastic.co/t/duplicate-events-with-filebeat-on-windows-on-service-restart/78743/10

* Move clean path outside for loop
  • Loading branch information
ruflin authored and tsg committed Mar 22, 2017
1 parent 40556dd commit 4527162
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ https://github.com/elastic/beats/compare/v5.1.1...master[Check the HEAD diff]
- Update regular expressions used for matching file names or lines (multiline, include/exclude functionality) to new matchers improving performance of simple string matches. {pull}3469[3469]
- The `symlinks` and `harverster_limit` settings are now GA, instead of experimental. {pull}3525[3525]
- close_timeout is also applied when the output is blocking. {pull}3511[3511]
- Improve handling of different path variants on Windows. {pull}3781[3781]

*Heartbeat*

Expand Down
13 changes: 6 additions & 7 deletions filebeat/prospector/prospector_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"fmt"
"os"
"path/filepath"
"runtime"
"strings"
"time"

"github.com/elastic/beats/filebeat/harvester"
Expand Down Expand Up @@ -180,13 +178,14 @@ func (p *ProspectorLog) getFiles() map[string]os.FileInfo {

// matchesFile returns true in case the given filePath is part of this prospector, means matches its glob patterns
func (p *ProspectorLog) matchesFile(filePath string) bool {

// Path is cleaned to ensure we always compare clean paths
filePath = filepath.Clean(filePath)

for _, glob := range p.config.Paths {

if runtime.GOOS == "windows" {
// Windows allows / slashes which makes glob patterns with / work
// But for match we need paths with \ as only file names are compared and no lookup happens
glob = strings.Replace(glob, "/", "\\", -1)
}
// Glob is cleaned to ensure we always compare clean paths
glob = filepath.Clean(glob)

// Evaluate if glob matches filePath
match, err := filepath.Match(glob, filePath)
Expand Down
32 changes: 28 additions & 4 deletions filebeat/prospector/prospector_log_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,38 @@ var matchTestsWindows = []struct {
result bool
}{
{
"C:\\\\hello\\test\\test.log", // Path are always in windows format
[]string{"C:\\\\hello/test/*.log"}, // Globs can also be with forward slashes
`C:\\hello\test\test.log`,
[]string{`C:\\hello/test/*.log`},
nil,
true,
},
{
"C:\\\\hello\\test\\test.log", // Path are always in windows format
[]string{"C:\\\\hello\\test/*.log"}, // Globs can also be mixed
`C:\\hello\test\test.log`,
[]string{`C:\\hello\test/*.log`},
nil,
true,
},
{
`C:\\hello\test\test.log`,
[]string{`C://hello/test/*.log`},
nil,
true,
},
{
`C:\\hello\test\test.log`,
[]string{`C://hello//test//*.log`},
nil,
true,
},
{
`C://hello/test/test.log`,
[]string{`C:\\hello\test\*.log`},
nil,
true,
},
{
`C://hello/test/test.log`,
[]string{`C:/hello/test/*.log`},
nil,
true,
},
Expand Down

0 comments on commit 4527162

Please sign in to comment.