Skip to content

Commit

Permalink
Stop using init() and generate registryFileRegExps once needed
Browse files Browse the repository at this point in the history
This commit moves the code that populates `registryFileRegExps` from a
`init()` function to a place before `matchRegistyFiles` is called.

Because generating diagnostics is very sporadic, there is not
meaningful performance change between having it generated once on
`init()` or generating it every time it's needed.
  • Loading branch information
belimawr committed Jan 7, 2025
1 parent 216a5d1 commit 7f3e5ae
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 10 deletions.
39 changes: 30 additions & 9 deletions filebeat/beater/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ import (
"github.com/elastic/elastic-agent-libs/paths"
)

func init() {
func getRegexpsForRegistryFiles() ([]*regexp.Regexp, error) {
// We use regexps here because globs do not support specifying a character
// range like we do in the checkpoint file.

registryFileRegExps := []*regexp.Regexp{}
preFilesList := [][]string{
[]string{"^registry$"},
[]string{"^registry", "filebeat$"},
Expand All @@ -50,8 +54,20 @@ func init() {
} else {
path = filepath.Join(lst...)
}
registryFileRegExps = append(registryFileRegExps, regexp.MustCompile(path))

// Compile the reg exp, if there is an error, stop and return.
// There should be no error here as this code is tested in all
// supported OSes, however to avoid a code path that leads to a
// panic, we cannot use `regexp.MustCompile` and handle the error
re, err := regexp.Compile(path)
if err != nil {
return nil, fmt.Errorf("cannot compile reg exp: %w", err)
}

registryFileRegExps = append(registryFileRegExps, re)
}

return registryFileRegExps, nil
}

func gzipRegistry() []byte {
Expand Down Expand Up @@ -142,14 +158,24 @@ func tarFolder(logger *logp.Logger, src, dst string) error {
baseDir := filepath.Base(src)

logger.Debugf("starting to walk '%s'", fullPath)

// This error should never happen at runtime, if something
// breaks it should break the tests and be fixed before a
// release. We handle the error here to avoid a code path
// that can end into a panic.
registryFileRegExps, err := getRegexpsForRegistryFiles()
if err != nil {
return err
}

return filepath.Walk(fullPath, func(path string, info fs.FileInfo, prevErr error) error {
// Stop if there is any errors
if prevErr != nil {
return prevErr
}

pathInTar := filepath.Join(baseDir, strings.TrimPrefix(path, src))
if !matchRegistyFiles(pathInTar) {
if !matchRegistyFiles(registryFileRegExps, pathInTar) {
return nil
}
header, err := tar.FileInfoHeader(info, info.Name())
Expand Down Expand Up @@ -181,12 +207,7 @@ func tarFolder(logger *logp.Logger, src, dst string) error {
})
}

// We use regexps here because globs do not support specifying a character
// range like we do in the checkpoint file. This slice is populated in the
// `init` function because Windows path separators need to be escaped.
var registryFileRegExps = []*regexp.Regexp{}

func matchRegistyFiles(path string) bool {
func matchRegistyFiles(registryFileRegExps []*regexp.Regexp, path string) bool {
for _, regExp := range registryFileRegExps {
if regExp.MatchString(path) {
return true
Expand Down
6 changes: 5 additions & 1 deletion filebeat/beater/diagnostics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,13 @@ func TestMatchRegistryFiles(t *testing.T) {
filepath.Join("registry", "42.json"),
filepath.Join("nop", "active.dat"),
}
registryFileRegExps, err := getRegexpsForRegistryFiles()
if err != nil {
t.Fatalf("cannot compile regexps for registry paths: %s", err)
}

testFn := func(t *testing.T, path string, match bool) {
result := matchRegistyFiles(path)
result := matchRegistyFiles(registryFileRegExps, path)
if result != match {
t.Errorf(
"mathRegisryFiles('%s') should return %t, got %t instead",
Expand Down

0 comments on commit 7f3e5ae

Please sign in to comment.