in_tail: support recursive directory monitoring#11452
in_tail: support recursive directory monitoring#11452courageJ wants to merge 1 commit intofluent:masterfrom
Conversation
This commit introduces recursive directory monitoring support for the Tail input plugin. Key changes: - Added 'path_recursion' configuration option (default: false). - Implemented recursive directory scanning for POSIX (opendir/readdir). - Implemented recursive directory scanning for Windows (FindFirstFile/FindNextFile). - Added protection against symlinked directory recursion loops. - Added unit tests to verify recursion behavior. Signed-off-by: Gemini CLI <gemini-cli@google.com>
📝 WalkthroughWalkthroughThis PR introduces recursive directory scanning support to the tail plugin through a new Changes
Sequence DiagramsequenceDiagram
participant Cfg as Configuration
participant Scan as tail_scan_path
participant Dir as tail_scan_directory
participant File as tail_process_file
participant List as File List
Cfg->>Scan: path + recursive=true
alt Path is Directory
Scan->>Dir: invoke recursive scan
Dir->>Dir: iterate directory entries
alt Entry is Regular File
Dir->>File: validate file
File->>File: check blacklist, ignore_older
File->>List: append file
else Entry is Symlink to File
Dir->>File: validate symlink target
File->>List: append file
else Entry is Directory
Dir->>Dir: recurse into subdirectory
else Entry is Symlink to Directory
Dir->>Dir: skip (avoid loops)
end
else Path is File
Scan->>File: validate file
File->>List: append file
else Path has Wildcards
Scan->>List: glob matching
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41d6ae271a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| mkdir(dir, 0777); | ||
| mkdir(target_dir, 0777); | ||
| symlink(target_dir, sym_dir); /* Create symlink to directory */ |
There was a problem hiding this comment.
The symlink test currently creates link_dir with symlink(target_dir, sym_dir), but target_dir is a relative path ("recursion_test_sym_target") while sym_dir is inside recursion_test_sym_dir; this makes the link resolve to recursion_test_sym_dir/recursion_test_sym_target (nonexistent) instead of the intended sibling directory. As a result, the test can pass without exercising the symlinked-directory traversal behavior it is meant to validate, so regressions in symlink recursion handling may go undetected.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/in_tail/tail_scan_win32.c (1)
304-335:⚠️ Potential issue | 🟡 MinorAdd
tail_signal_managercall after registering files in Win32 implementation.The glob version (
tail_scan_glob.cline 396–398) callstail_signal_manager(ctx)when files are registered intail_scan_path(). The Win32 version (tail_scan_win32.c) lacks this call entirely, meaning newly discovered files via recursive directory scans won't signal the processing thread until the next refresh cycle.Add
tail_signal_manager(ctx)intail_scan_path()aftertail_scan_directory()returns (when count > 0), and also aftertail_register_file()in the non-directory case.
🤖 Fix all issues with AI agents
In `@tests/runtime/in_tail.c`:
- Line 2845: The test creates a relative symlink that points to a non-existent
path, so the "skip symlinked directory" behavior isn't exercised; update the
symlink creation in tests/runtime/in_tail.c by changing the symlink target used
in the symlink(...) call (currently using "recursion_test_sym_target" for the
link at "recursion_test_sym_dir/link_dir") to a path that actually resolves from
the link's parent directory (e.g. "../recursion_test_sym_target") so the kernel
resolves the link to recursion_test_sym_dir/recursion_test_sym_target and the
code path in your directory-recursion logic that skips symlinked directories is
properly tested.
🧹 Nitpick comments (3)
plugins/in_tail/tail_scan_win32.c (1)
250-302:tail_scan_directorylacks a recursion depth limit.Deeply nested directory trees (or edge cases with mount points) could cause stack overflow. The POSIX counterpart in
tail_scan_glob.chas the same issue. Consider adding a max depth parameter to guard against pathological directory structures.plugins/in_tail/tail_scan_glob.c (1)
253-320: Recursive traversal has no depth limit — same as the Win32 counterpart.A pathological or very deep directory tree could exhaust the stack. Consider adding a
max_depthparameter (e.g., 64) and decrementing on each recursive call. This applies to bothtail_scan_glob.candtail_scan_win32.c.Sketch of depth-limited recursion
-static int tail_scan_directory(const char *dir, struct flb_tail_config *ctx, time_t now) +static int tail_scan_directory(const char *dir, struct flb_tail_config *ctx, + time_t now, int depth) { + if (depth <= 0) { + flb_plg_warn(ctx->ins, "max recursion depth reached at: %s", dir); + return 0; + } ... else if (S_ISDIR(st.st_mode)) { - ret = tail_scan_directory(path, ctx, now); + ret = tail_scan_directory(path, ctx, now, depth - 1); ... }tests/runtime/in_tail.c (1)
2733-2820: Verbose inline comments could be cleaned up, but test logic is sound.Lines 2798–2807 contain extensive inline reasoning about expected behavior. While helpful during development, these comments are noisy for long-term maintenance. Consider trimming to a one-liner explaining the expected behavior.
|
|
||
| mkdir(dir, 0777); | ||
| mkdir(target_dir, 0777); | ||
| symlink(target_dir, sym_dir); /* Create symlink to directory */ |
There was a problem hiding this comment.
Broken symlink — test doesn't validate the intended behavior.
symlink("recursion_test_sym_target", "recursion_test_sym_dir/link_dir") creates a relative symlink. When resolved, the kernel computes the target relative to the link's parent directory: recursion_test_sym_dir/recursion_test_sym_target, which doesn't exist. The test still passes (expects 1 record) because stat() fails on the dangling link, not because the code correctly identifies and skips a symlinked directory.
Use ../recursion_test_sym_target so the symlink actually resolves and the "skip symlinked directory" logic is exercised:
Proposed fix
- symlink(target_dir, sym_dir); /* Create symlink to directory */
+ symlink("../recursion_test_sym_target", sym_dir); /* Create symlink to directory */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| symlink(target_dir, sym_dir); /* Create symlink to directory */ | |
| symlink("../recursion_test_sym_target", sym_dir); /* Create symlink to directory */ |
🤖 Prompt for AI Agents
In `@tests/runtime/in_tail.c` at line 2845, The test creates a relative symlink
that points to a non-existent path, so the "skip symlinked directory" behavior
isn't exercised; update the symlink creation in tests/runtime/in_tail.c by
changing the symlink target used in the symlink(...) call (currently using
"recursion_test_sym_target" for the link at "recursion_test_sym_dir/link_dir")
to a path that actually resolves from the link's parent directory (e.g.
"../recursion_test_sym_target") so the kernel resolves the link to
recursion_test_sym_dir/recursion_test_sym_target and the code path in your
directory-recursion logic that skips symlinked directories is properly tested.
courageJ
changed the title
1 participant
This PR introduces recursive directory monitoring support for the Tail input plugin.
Key changes:
Summary by CodeRabbit
New Features
Tests