Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix relative paths in prospectors definitions #5443

Merged
merged 2 commits into from
Nov 6, 2017

Conversation

tsg
Copy link
Contributor

@tsg tsg commented Oct 25, 2017

If a relative path was used in a prospector definition, it could happen
that when loading the state from the registry, the path didn't match the
pattern, which made it ignore the saved state. Fixes #5442.

Additionally, this PR cleans up some code related to the recursive_glob
setting. The docs used to claim that the feature is disabled by default, but
that wasn't the case and it was impossible to disable it. This change leaves
it enabled by default, but makes it possible to disable it.

@tsg tsg added bug Filebeat Filebeat needs_backport PR is waiting to be backported to other branches. review labels Oct 25, 2017
@tsg tsg force-pushed the fix_globs_with_relative_paths branch from ccbe636 to 374b14b Compare October 25, 2017 22:39
@@ -189,3 +192,17 @@ func (c *config) resolvePaths() error {
c.Paths = paths
return nil
}

// makeGlobsAbsolute calls `filepath.Abs` on all the globs from config
func (c *config) makeGlobsAbsolute() error {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

normalizeGlobPatterns.

return nil, fmt.Errorf("Failed to resolve recursive globs in config: %+v", err)
}
if err := p.config.makeGlobsAbsolute(); err != nil {
return nil, fmt.Errorf("Failed to make globs absolute: %+v", err)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

msg: 'Failed to normalize glob patterns: ' ?

@tsg tsg force-pushed the fix_globs_with_relative_paths branch from 374b14b to 18a4929 Compare November 2, 2017 10:18
@tsg tsg added v6.1.0 and removed needs_backport PR is waiting to be backported to other branches. labels Nov 2, 2017
@tsg tsg force-pushed the fix_globs_with_relative_paths branch from 18a4929 to 0a7efae Compare November 2, 2017 10:21
tsg added 2 commits November 3, 2017 14:58
If a relative path was used in a prospector definition, it could happen
that when loading the state from the registry, the path didn't match the
pattern, which made it ignore the saved state. Fixes elastic#5442.

Additionally, this PR cleans up some code related to the `recursive_glob`
setting. The docs used to claim that the feature is disabled by default, but
that wasn't the case and it was impossible to disable it. This change leaves
it enabled by default, but makes it possible to disable it.
@tsg tsg force-pushed the fix_globs_with_relative_paths branch from 0a7efae to 3c36792 Compare November 3, 2017 13:58
Copy link
Contributor

@adriansr adriansr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@urso
Copy link

urso commented Nov 6, 2017

Jenkins, test it.

@urso urso merged commit a9c1933 into elastic:master Nov 6, 2017
@@ -115,7 +117,7 @@ func NewProspector(
// It goes through all states coming from the registry. Only the states which match the glob patterns of
// the prospector will be loaded and updated. All other states will not be touched.
func (p *Prospector) loadStates(states []file.State) error {
logp.Debug("prospector", "exclude_files: %s", p.config.ExcludeFiles)
logp.Debug("prospector", "exclude_files: %s. Number of stats: %d", p.config.ExcludeFiles, len(states))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/stats/states/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants