fix(file source): avoid warning for empty files too small to fingerprint - #25897
fix(file source): avoid warning for empty files too small to fingerprint#25897an-cor wants to merge 2 commits into
Conversation
Signed-off-by: Angel Coronel <angelcoronel15@gmail.com>
|
All contributors have signed the CLA ✍️ ✅ |
This comment has been minimized.
This comment has been minimized.
![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: e8f82d1f3a
ℹ️ 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".
| match error.kind() { | ||
| ErrorKind::UnexpectedEof => { | ||
| if !known_small_files.contains_key(path) { | ||
| if file_size != Some(0) && !known_small_files.contains_key(path) { |
There was a problem hiding this comment.
When the file source uses checksum fingerprinting with remove_after_secs, this branch now suppresses both the warning and the insertion into known_small_files for zero-byte files. The cleanup path deletes only entries from known_small_files after the grace period (lib/file-source/src/file_server.rs cleanup loop), so an empty matched file is no longer removed even though it is already at EOF and previously was tracked for removal. Suppress emit_file_checksum_failed separately while still recording the path for remove_after_secs cleanup.
Useful? React with 👍 / 👎.
|
I have read the CLA Document and I hereby sign the CLA |
an-cor
changed the title
Signed-off-by: Angel Coronel <angelcoronel15@gmail.com>
|
Thanks for the review. I updated the implementation so empty files are still inserted into |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a67d8d131
ℹ️ 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".
| if file_size != Some(0) { | ||
| emitter.emit_file_checksum_failed(path); |
There was a problem hiding this comment.
When a matched file is first scanned while empty, this branch suppresses the warning but still inserts its path into known_small_files. If the file later gains non-empty content without enough newlines to fingerprint, subsequent discovery scans skip this entire block because the path is already present, so the warning that this change intends to preserve for non-empty undersized files is never emitted. Track whether a warning was emitted separately from the removal-tracking map, or allow the first non-empty failure to emit it.
Useful? React with 👍 / 👎.
| let metadata = match fs::metadata(path).await { | ||
| Ok(metadata) => { | ||
| if !metadata.is_dir() { | ||
| file_size = Some(metadata.len()); |
There was a problem hiding this comment.
For a valid gzip file whose decompressed stream is empty, metadata.len() is nonzero because it measures the compressed container, while fingerprint reads the decompressed stream and returns UnexpectedEof. The new check therefore still emits emit_file_checksum_failed for this supported form of empty input, contrary to the changelog's empty-file behavior. Determine emptiness from the stream being fingerprinted rather than the on-disk compressed size.
Useful? React with 👍 / 👎.
1 participant
Summary
This PR fixes #1065 by avoiding the
Currently ignoring file too small to fingerprintwarning for empty files.The existing behavior emits the same warning for empty files and for non-empty files that are too small to fingerprint. Empty files can be common in production logging environments, especially with short-lived jobs or pods, so warning on them can create noisy logs without giving users actionable information.
This change keeps the existing warning behavior for non-empty files that are too small to fingerprint, but skips the warning for empty files.
Vector configuration
No full Vector configuration was used for this change. The behavior is covered at the file-source-common fingerprinter level with a targeted unit test.
How did you test this PR?
Tested locally on macOS with targeted file-source-common tests.
Commands run:
Results:
no_error_on_empty_filepassedfingerprintertest filter passed: 8 passed, 0 failedcargo fmt --checkpassedgit diff --checkpassedChange Type
Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References
Notes
This PR includes a changelog fragment: