From 71be90d992da6e30e5a18ecf8b9e3acb6138047a Mon Sep 17 00:00:00 2001 From: Sebastian Spaink <3441183+sspaink@users.noreply.github.com> Date: Tue, 16 Feb 2021 15:53:50 -0600 Subject: [PATCH] Support exclamation mark to create non-matching list in tail plugin (#8613) * Replace exclamation mark with caret * Update README and use table driven tests * Use ReplaceAll instead * Use doublestar package instead to glob filepath * Add license * Fix order of dependencies * Doc improvement, maybe better then str replace? * Forgot to remove nil from test * Use regex instead of library * Revert unnecessary change * Go back to using library replace string twice to handle edge case --- docs/LICENSE_OF_DEPENDENCIES.md | 1 + go.mod | 1 + go.sum | 2 + internal/globpath/globpath.go | 43 ++++---------------- internal/globpath/globpath_test.go | 61 ++++++++++++++++------------ internal/globpath/testdata/log[!.log | 0 plugins/inputs/phpfpm/phpfpm.go | 2 +- plugins/inputs/phpfpm/phpfpm_test.go | 2 +- plugins/inputs/tail/README.md | 3 +- plugins/inputs/tail/tail.go | 3 +- 10 files changed, 52 insertions(+), 66 deletions(-) create mode 100644 internal/globpath/testdata/log[!.log diff --git a/docs/LICENSE_OF_DEPENDENCIES.md b/docs/LICENSE_OF_DEPENDENCIES.md index 543d59c3e..f68d85e7b 100644 --- a/docs/LICENSE_OF_DEPENDENCIES.md +++ b/docs/LICENSE_OF_DEPENDENCIES.md @@ -36,6 +36,7 @@ following works: - github.com/aws/smithy-go [Apache License 2.0](https://github.com/aws/smithy-go/blob/main/LICENSE) - github.com/benbjohnson/clock [MIT License](https://github.com/benbjohnson/clock/blob/master/LICENSE) - github.com/beorn7/perks [MIT License](https://github.com/beorn7/perks/blob/master/LICENSE) +- github.com/bmatcuk/doublestar [MIT License](https://github.com/bmatcuk/doublestar/blob/master/LICENSE) - github.com/caio/go-tdigest [MIT License](https://github.com/caio/go-tdigest/blob/master/LICENSE) - github.com/cenkalti/backoff [MIT License](https://github.com/cenkalti/backoff/blob/master/LICENSE) - github.com/cespare/xxhash [MIT License](https://github.com/cespare/xxhash/blob/master/LICENSE.txt) diff --git a/go.mod b/go.mod index 15a296424..f6fd3df03 100644 --- a/go.mod +++ b/go.mod @@ -34,6 +34,7 @@ require ( github.com/aws/smithy-go v1.0.0 github.com/benbjohnson/clock v1.0.3 github.com/bitly/go-hostpool v0.1.0 // indirect + github.com/bmatcuk/doublestar/v3 v3.0.0 github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 github.com/caio/go-tdigest v2.3.0+incompatible // indirect github.com/cenkalti/backoff v2.0.0+incompatible // indirect diff --git a/go.sum b/go.sum index 611ab4e49..5ff6de3a3 100644 --- a/go.sum +++ b/go.sum @@ -142,6 +142,8 @@ github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/bitly/go-hostpool v0.1.0 h1:XKmsF6k5el6xHG3WPJ8U0Ku/ye7njX7W81Ng7O2ioR0= github.com/bitly/go-hostpool v0.1.0/go.mod h1:4gOCgp6+NZnVqlKyZ/iBZFTAJKembaVENUpMkpg42fw= +github.com/bmatcuk/doublestar/v3 v3.0.0 h1:TQtVPlDnAYwcrVNB2JiGuMc++H5qzWZd9PhkNo5WyHI= +github.com/bmatcuk/doublestar/v3 v3.0.0/go.mod h1:6PcTVMw80pCY1RVuoqu3V++99uQB3vsSYKPTd8AWA0k= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 h1:DDGfHa7BWjL4YnC6+E63dPcxHo2sUxDIu8g3QgEJdRY= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869/go.mod h1:Ekp36dRnpXw/yCqJaO+ZrUyxD+3VXMFFr56k5XYrpB4= github.com/caio/go-tdigest v2.3.0+incompatible h1:zP6nR0nTSUzlSqqr7F/LhslPlSZX/fZeGmgmwj2cxxY= diff --git a/internal/globpath/globpath.go b/internal/globpath/globpath.go index d4e7ffd87..f69f5bfb6 100644 --- a/internal/globpath/globpath.go +++ b/internal/globpath/globpath.go @@ -5,8 +5,8 @@ import ( "path/filepath" "strings" + "github.com/bmatcuk/doublestar/v3" "github.com/gobwas/glob" - "github.com/karrick/godirwalk" ) type GlobPath struct { @@ -45,42 +45,13 @@ func Compile(path string) (*GlobPath, error) { // If it's a static path, returns path. // All returned path will have the host platform separator. func (g *GlobPath) Match() []string { - if !g.hasMeta { - return []string{g.path} - } - if !g.HasSuperMeta { - files, _ := filepath.Glob(g.path) - return files - } - roots, err := filepath.Glob(g.rootGlob) - if err != nil { - return []string{} - } - out := []string{} - walkfn := func(path string, _ *godirwalk.Dirent) error { - if g.g.Match(path) { - out = append(out, path) - } - return nil + // This string replacement is for backwards compatibility support + // The original implemention allowed **.txt but the double star package requires **/**.txt + g.path = strings.ReplaceAll(g.path, "**/**", "**") + g.path = strings.ReplaceAll(g.path, "**", "**/**") - } - for _, root := range roots { - fileinfo, err := os.Stat(root) - if err != nil { - continue - } - if !fileinfo.IsDir() { - if g.MatchString(root) { - out = append(out, root) - } - continue - } - godirwalk.Walk(root, &godirwalk.Options{ - Callback: walkfn, - Unsorted: true, - }) - } - return out + files, _ := doublestar.Glob(g.path) + return files } // MatchString tests the path string against the glob. The path should contain diff --git a/internal/globpath/globpath_test.go b/internal/globpath/globpath_test.go index 92af2d20b..4897ab2f8 100644 --- a/internal/globpath/globpath_test.go +++ b/internal/globpath/globpath_test.go @@ -19,32 +19,41 @@ var ( ) func TestCompileAndMatch(t *testing.T) { - // test super asterisk - g1, err := Compile(filepath.Join(testdataDir, "**")) - require.NoError(t, err) - // test single asterisk - g2, err := Compile(filepath.Join(testdataDir, "*.log")) - require.NoError(t, err) - // test no meta characters (file exists) - g3, err := Compile(filepath.Join(testdataDir, "log1.log")) - require.NoError(t, err) - // test file that doesn't exist - g4, err := Compile(filepath.Join(testdataDir, "i_dont_exist.log")) - require.NoError(t, err) - // test super asterisk that doesn't exist - g5, err := Compile(filepath.Join(testdataDir, "dir_doesnt_exist", "**")) - require.NoError(t, err) - matches := g1.Match() - require.Len(t, matches, 6) - matches = g2.Match() - require.Len(t, matches, 2) - matches = g3.Match() - require.Len(t, matches, 1) - matches = g4.Match() - require.Len(t, matches, 1) - matches = g5.Match() - require.Len(t, matches, 0) + type test struct { + path string + matches int + } + + tests := []test{ + //test super asterisk + {path: filepath.Join(testdataDir, "**"), matches: 7}, + // test single asterisk + {path: filepath.Join(testdataDir, "*.log"), matches: 3}, + // test no meta characters (file exists) + {path: filepath.Join(testdataDir, "log1.log"), matches: 1}, + // test file that doesn't exist + {path: filepath.Join(testdataDir, "i_dont_exist.log"), matches: 0}, + // test super asterisk that doesn't exist + {path: filepath.Join(testdataDir, "dir_doesnt_exist", "**"), matches: 0}, + // test exclamation mark creates non-matching list with a range + {path: filepath.Join(testdataDir, "log[!1-2]*"), matches: 1}, + // test caret creates non-matching list + {path: filepath.Join(testdataDir, "log[^1-2]*"), matches: 1}, + // test exclamation mark creates non-matching list without a range + {path: filepath.Join(testdataDir, "log[!2]*"), matches: 2}, + // test exclamation mark creates non-matching list without a range + {path: filepath.Join(testdataDir, "log\\[!*"), matches: 1}, + // test exclamation mark creates non-matching list without a range + {path: filepath.Join(testdataDir, "log\\[^*"), matches: 0}, + } + + for _, tc := range tests { + g, err := Compile(tc.path) + require.Nil(t, err) + matches := g.Match() + require.Len(t, matches, tc.matches) + } } func TestRootGlob(t *testing.T) { @@ -82,7 +91,7 @@ func TestMatch_ErrPermission(t *testing.T) { input string expected []string }{ - {"/root/foo", []string{"/root/foo"}}, + {"/root/foo", []string(nil)}, {"/root/f*", []string(nil)}, } diff --git a/internal/globpath/testdata/log[!.log b/internal/globpath/testdata/log[!.log new file mode 100644 index 000000000..e69de29bb diff --git a/plugins/inputs/phpfpm/phpfpm.go b/plugins/inputs/phpfpm/phpfpm.go index e0f21176a..87eb4f649 100644 --- a/plugins/inputs/phpfpm/phpfpm.go +++ b/plugins/inputs/phpfpm/phpfpm.go @@ -299,7 +299,7 @@ func globUnixSocket(url string) ([]string, error) { } paths := glob.Match() if len(paths) == 0 { - return nil, fmt.Errorf("socket doesn't exist %q: %v", pattern, err) + return nil, fmt.Errorf("socket doesn't exist %q", pattern) } addresses := make([]string, 0, len(paths)) diff --git a/plugins/inputs/phpfpm/phpfpm_test.go b/plugins/inputs/phpfpm/phpfpm_test.go index 6db740df4..645782289 100644 --- a/plugins/inputs/phpfpm/phpfpm_test.go +++ b/plugins/inputs/phpfpm/phpfpm_test.go @@ -327,7 +327,7 @@ func TestPhpFpmGeneratesMetrics_Throw_Error_When_Socket_Path_Is_Invalid(t *testi err = acc.GatherError(r.Gather) require.Error(t, err) - assert.Equal(t, `dial unix /tmp/invalid.sock: connect: no such file or directory`, err.Error()) + assert.Equal(t, `socket doesn't exist "/tmp/invalid.sock"`, err.Error()) } diff --git a/plugins/inputs/tail/README.md b/plugins/inputs/tail/README.md index 7f5315038..5664f8704 100644 --- a/plugins/inputs/tail/README.md +++ b/plugins/inputs/tail/README.md @@ -29,7 +29,8 @@ The plugin expects messages in one of the ## "/var/log/**.log" -> recursively find all .log files in /var/log ## "/var/log/*/*.log" -> find all .log files with a parent dir in /var/log ## "/var/log/apache.log" -> just tail the apache log file - ## + ## "/var/log/log[!1-2]* -> tail files without 1-2 + ## "/var/log/log[^1-2]* -> identical behavior as above ## See https://github.com/gobwas/glob for more examples ## files = ["/var/mymetrics.out"] diff --git a/plugins/inputs/tail/tail.go b/plugins/inputs/tail/tail.go index 557885e1b..54d42e44a 100644 --- a/plugins/inputs/tail/tail.go +++ b/plugins/inputs/tail/tail.go @@ -81,7 +81,8 @@ const sampleConfig = ` ## "/var/log/**.log" -> recursively find all .log files in /var/log ## "/var/log/*/*.log" -> find all .log files with a parent dir in /var/log ## "/var/log/apache.log" -> just tail the apache log file - ## + ## "/var/log/log[!1-2]* -> tail files without 1-2 + ## "/var/log/log[^1-2]* -> identical behavior as above ## See https://github.com/gobwas/glob for more examples ## files = ["/var/mymetrics.out"]