Log Directories Created After Startup not Tracked #385
Conversation
stsantilena
added
bug
stsantilena
changed the title
common/fs/src/cache/dir_path.rs
Outdated
| ) -> Result<DirPathBuf, DirPathBufError> { | ||
| match path { | ||
| Some(p) => { | ||
| if p.is_dir() { |
There was a problem hiding this comment.
does it work for:
- it is a file
- permission error
There was a problem hiding this comment.
The configuration is meant to be set as directory so a single file wouldn't be expected. I followed same pattern as existing directory check. As for permissions, in theory, I think could fail is for some reason it was running in non-root mode and didn't have directory access.
common/fs/src/cache/dir_path.rs
Outdated
| warn!("{:?} is not a directory; moving to parent directory", p); | ||
| let mut postfix_pathbuf = PathBuf::new(); | ||
| let mut pop_pathbuf = PathBuf::new(); | ||
| if let Some(path) = postfix { |
There was a problem hiding this comment.
shadowing fn arg on purpose?
There was a problem hiding this comment.
The postfix function input is only non-None in the event that a given directory doesn't exist, in which case it saves that piece of the path so it can properly construct the missing directory path.
There was a problem hiding this comment.
i was thinking about var name
There was a problem hiding this comment.
Ah, I see. Updated var name path -> pop
common/config/src/lib.rs
Outdated
| @@ -344,6 +344,7 @@ impl TryFrom<RawConfig> for Config { | |||
| // Filter off paths that are not directories and warn about them | |||
| .filter_map(|d| { | |||
| d.clone() | |||
| //TODO: Need to fix this iteration as warning now happens in dir_path.rs | |||
There was a problem hiding this comment.
@c-nixon I don't think there needs to any change here, perhaps the warning might be different message? Also, thinking the iteration structure should stay for Windows.
There was a problem hiding this comment.
if we allow to specify non-existing dirs - why do we filter them here out?
There was a problem hiding this comment.
This iteration just filters out anything that throws an error. Originally, if a directory didn't exist and error was thrown. Now, there is a more complex process happening down in find_valid_path that allows directories that don't yet exist; still propagating errors up but for different reasons. So really just the comment is no longer valid.
| Ok(DirPathBuf { inner: path }) | ||
| Ok(DirPathBuf { | ||
| inner: path, | ||
| postfix: None, |
There was a problem hiding this comment.
Had to add this line to fix the Windows build. This shouldn't affect existing Windows behavior.
| @@ -1573,7 +1573,6 @@ async fn test_line_redact() { | |||
| test_line_rules(None, None, redact, to_write, expected).await; | |||
| } | |||
|
|
|||
| #[ignore] | |||
| #[tokio::test] | |||
| async fn test_directory_created_after_initialization() { | |||
There was a problem hiding this comment.
Turned on bug specific integration test from previous PR.
common/fs/src/cache/dir_path.rs
Outdated
| let parent = match level_up(&p) { | ||
| Some(p) => { | ||
| if p == root_pathbuf { | ||
| panic!("configured directory recursed to root!"); |
There was a problem hiding this comment.
Set to panic if function recurses back to the root directory. This will watch the entire root file system which is most likely not desirable.
There was a problem hiding this comment.
Changed this so that instead of throwing a panic, it will warn and watch root level for missing dir.
| } else { | ||
| warn!("{:?} is not a directory; moving to parent directory", p); | ||
| let root_pathbuf = Path::new("/"); | ||
| let mut postfix_pathbuf = PathBuf::new(); |
There was a problem hiding this comment.
I feel like there might be some way to use the std::path Component types to implement this, but that might just complicate the logic
There was a problem hiding this comment.
There's probably a few different ways to go about it. The tricky part is flipping the path each integration to make sure it comes out correct, and plays nicely with the path rules.
c-nixon
force-pushed
the
LOG-9806-log-dir-not-tracked-cont
branch
from
909d8e8 to
fb32617
Compare
Enables the Agent to track directories that are listed in the configuration but are initially missing. Once the directory is create its added to the file system cache and tracked.