Skip to content
This repository has been archived by the owner on May 25, 2022. It is now read-only.

filelog: add file_name_resolved and file_path_resolved attributes #184

Closed
wants to merge 3 commits into from
Closed

filelog: add file_name_resolved and file_path_resolved attributes #184

wants to merge 3 commits into from

Conversation

sumo-drosiek
Copy link
Member

Add file_name_resolved and file_path_resolved attributes. They are going to keep information about absolute path and file name after symlinks resolution

fixes #181

@sumo-drosiek sumo-drosiek requested a review from a team June 15, 2021 07:40
@codecov
Copy link

codecov bot commented Jun 15, 2021

Codecov Report

Merging #184 (e2fcca1) into main (cc61797) will increase coverage by 0.0%.
The diff coverage is 86.2%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #184   +/-   ##
=====================================
  Coverage   75.5%   75.6%           
=====================================
  Files         95      95           
  Lines       4360    4386   +26     
=====================================
+ Hits        3296    3319   +23     
- Misses       740     741    +1     
- Partials     324     326    +2     
Impacted Files Coverage Δ
operator/builtin/input/file/reader.go 64.7% <69.2%> (-0.5%) ⬇️
operator/builtin/input/file/config.go 77.0% <100.0%> (+3.5%) ⬆️
operator/builtin/input/file/file.go 71.4% <100.0%> (ø)
pipeline/config.go 87.0% <0.0%> (-0.8%) ⬇️
agent/builder.go 91.6% <0.0%> (+0.4%) ⬆️
operator/builtin/input/tcp/tcp.go 74.5% <0.0%> (+1.6%) ⬆️
testutil/mocks.go 74.1% <0.0%> (+3.2%) ⬆️

return err
}

abs, err := filepath.Abs(resolved)
Copy link
Member

Choose a reason for hiding this comment

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

Do you expect a performance hit here?

This could be computed once per Reader, but that would mean it does not detect changes in symlinks. So perhaps the next best thing would be to check if it is needed at all. Since the default configuration does not need it, this would potentially avoid a performance hit for many users.

Dominik Rosiek added 2 commits June 16, 2021 08:27
@sumo-drosiek
Copy link
Member Author

sumo-drosiek commented Jun 16, 2021

Closing in favor of #189

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

Successfully merging this pull request may close these issues.

Add real filepath to the file input
2 participants