fluent-bit icon indicating copy to clipboard operation
fluent-bit copied to clipboard
verified publisher icon fluent

out_logrotate: add new out plugin

Open SagiROosto opened this issue 1 month ago • 8 comments

This PR present a new out plugin out_logrotate. This Will answer #7572 question and solve #7541 suggestion. This feature is highly required in bare-metal or physical machines where you have finite and limited storage.
The plugin is basically a copy of out_file with a rotation feature. The reason to do that in additional plugin and not on the out_file itself is to avoid accidentally turning this feature on and having redundant info/allocs/processing where it is not needed (so normal users can still use the regular file plugin without issue).

(this replaces closed #10824 PR)

Enter [N/A] in the box, if an item is not applicable to your change.

Testing Before we can approve your change; please submit the following in a comment:

  • [x] Example configuration file for the change
  • [ ] Debug log output from testing the change
  • [ ] Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • [ ] Run local packaging test showing all targets (including any new ones) build.
  • [x] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • [x] Documentation required for this feature

Backporting

  • [ ] Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Summary by CodeRabbit

  • New Features

    • Added a logrotate output plugin to write logs with automatic rotation.
    • Supports JSON, CSV, LTSV, PLAIN, MSGPACK and TEMPLATE formats.
    • Configurable max file size, retention count, optional gzip compression, template/delimiter options, and automatic directory creation.
    • Enabled in all-features builds and included in Windows defaults; example configuration added.

  • Tests

    • Added comprehensive runtime tests covering rotation, gzip, retention, formats, path/mkdir, delimiters and multithreaded scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

SagiROosto avatar Nov 05 '25 11:11 SagiROosto

[!NOTE]

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds a new out_logrotate output plugin: build options and defaults, CMake registration and target, full C implementation and header, an example configuration, and extensive runtime tests for size-based rotation, optional gzip compression, retention, multiple formats, and mkdir handling.

Changes

Cohort / File(s) Summary
Build enablement & defaults
CMakeLists.txt, cmake/plugins_options.cmake, cmake/windows-setup.cmake, plugins/CMakeLists.txt		 Introduce FLB_OUT_LOGROTATE build option (default ON in options), enable under FLB_ALL and Windows defaults, and register out_logrotate in the plugin list.
Plugin target CMake
plugins/out_logrotate/CMakeLists.txt		 Add plugin target registration, declare logrotate.c as source, register with FLB_PLUGIN, and add MSVC-specific linkage.
Plugin implementation
plugins/out_logrotate/logrotate.c		 New output plugin implementation: per-instance config/state, mutex-protected per-file tracking, size-triggered rotation with timestamped renames, optional streaming gzip, max-files pruning, multiple format writers (JSON/CSV/LTSV/PLAIN/MSGPACK/TEMPLATE), metrics, and lifecycle callbacks (init/flush/exit) plus config_map.
Plugin header
plugins/out_logrotate/logrotate.h		 New header with include guard and enum of supported formats: JSON, CSV, LTSV, PLAIN, MSGPACK, TEMPLATE.
Example configuration
conf/fluent-bit-logrotate.conf		 Add example SERVICE/INPUT/OUTPUT demonstrating Path/File/Format/Max_Size/Max_Files/Gzip/Mkdir and related options.
Runtime tests
tests/runtime/CMakeLists.txt, tests/runtime/out_logrotate.c		 Add runtime test target and comprehensive tests covering rotation, gzip, retention, formats, path/mkdir, delimiters, CSV headers, and multithreaded behavior.

Sequence Diagram(s)

sequenceDiagram
    rect rgb(248,250,255)
    participant Input as Input Plugin
    participant LR as out_logrotate
    participant FS as File System
    participant GZ as Gzip (optional)
    end

    Input->>LR: submit event chunk (flush)
    LR->>LR: lock per-file state (mutex)
    LR->>FS: ensure directory & open current file
    LR->>LR: check current size vs Max_Size
    alt size >= Max_Size
        LR->>FS: rename current -> rotated.<ts>
        alt Gzip enabled
            LR->>GZ: stream-compress rotated file
            GZ-->>FS: write rotated file.gz
            FS->>FS: remove uncompressed rotated file
        end
        LR->>FS: prune rotated files (Max_Files)
        LR->>FS: create new current file
    end
    LR->>LR: format records (JSON/CSV/LTSV/PLAIN/MSGPACK/TEMPLATE)
    LR->>FS: append formatted records to current file
    LR-->>Input: acknowledge flush

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Pay special attention to:
    • plugins/out_logrotate/logrotate.c — concurrency, rotation atomicity, gzip streaming, error/resource management, platform path handling.
    • tests/runtime/out_logrotate.c — timing assumptions, filesystem interactions, multithreaded determinism and teardown.
    • CMake changes (cmake/plugins_options.cmake, CMakeLists.txt, plugins/CMakeLists.txt) — option propagation and Windows/MSVC linkage.

Poem

🐇 I hop through folders, nudging bytes to sleep,

I roll old logs with timestamps soft and neat,
I gzip them snug and prune with tidy art,
New files bloom bright while older ones depart,
A rabbit's hum keeps diskspace light and sweet.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 32.56% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'out_logrotate: add new out plugin' accurately captures the main change - a new output plugin is being added with clear identification of the plugin name.
Linked Issues check ✅ Passed The pull request comprehensively implements all coding requirements from issue #10824: size-based rotation, gzip compression, max_files, mkdir, multiple formats, plugin registration, build configuration, example config, and extensive test coverage.
Out of Scope Changes check ✅ Passed All changes are scoped to the new out_logrotate plugin. File modifications are limited to: adding plugin source code, build configurations, test coverage, and example configuration - all directly related to the stated objective.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 776dd3b9a3e05878bff2d26c8bf41a10ca4c725b and 89c58ca7412c4020fee6bea822233ece7f97eb55.

📒 Files selected for processing (10)
  • CMakeLists.txt (1 hunks)
  • cmake/plugins_options.cmake (1 hunks)
  • cmake/windows-setup.cmake (1 hunks)
  • conf/fluent-bit-logrotate.conf (1 hunks)
  • plugins/CMakeLists.txt (1 hunks)
  • plugins/out_logrotate/CMakeLists.txt (1 hunks)
  • plugins/out_logrotate/logrotate.c (1 hunks)
  • plugins/out_logrotate/logrotate.h (1 hunks)
  • tests/runtime/CMakeLists.txt (1 hunks)
  • tests/runtime/out_logrotate.c (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • cmake/plugins_options.cmake
  • cmake/windows-setup.cmake
  • conf/fluent-bit-logrotate.conf
  • plugins/CMakeLists.txt
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-31T12:46:11.940Z 
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.

Applied to files:

  • CMakeLists.txt
  • tests/runtime/CMakeLists.txt
📚 Learning: 2025-08-31T12:46:11.940Z 
Learnt from: ThomasDevoogdt
Repo: fluent/fluent-bit PR: 9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit, the correct CMake flag for using system librdkafka is `FLB_PREFER_SYSTEM_LIB_KAFKA=ON`.

Applied to files:

  • CMakeLists.txt
📚 Learning: 2025-09-08T11:21:33.975Z 
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 10851
File: include/fluent-bit/flb_simd.h:60-66
Timestamp: 2025-09-08T11:21:33.975Z
Learning: Fluent Bit currently only supports MSVC compiler on Windows, so additional compiler compatibility guards may be unnecessary for Windows-specific code paths.

Applied to files:

  • tests/runtime/out_logrotate.c
📚 Learning: 2025-10-23T07:43:16.216Z 
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11059
File: plugins/in_tail/tail_file.c:1618-1640
Timestamp: 2025-10-23T07:43:16.216Z
Learning: In plugins/in_tail/tail_file.c, when truncate_long_lines is enabled and the buffer is full, the early truncation path uses `lines > 0` as the validation pattern to confirm whether process_content successfully processed content. This is intentional to track occurrences of line processing rather than byte consumption, and consuming bytes based on `processed_bytes > 0` would be overkill for this validation purpose.

Applied to files:

  • plugins/out_logrotate/logrotate.c
🧬 Code graph analysis (1)
tests/runtime/out_logrotate.c (2)
include/fluent-bit/flb_mem.h (1)
  • flb_free (126-128)
src/flb_lib.c (3)
  • flb_input (266-276)
  • flb_start (983-994)
  • flb_lib_push (843-870)
🪛 ast-grep (0.40.0)
tests/runtime/out_logrotate.c

[warning] 585-585: A check is done with stat and then the file is used. There is no guarantee that the status of the file has not changed since the call to stat which may allow attackers to bypass permission checks. Context: fopen Note: [CWE-367]: Time-of-check Time-of-use (TOCTOU) Race Condition [REFERENCES] - https://wiki.sei.cmu.edu/confluence/display/c/FIO45-C.+Avoid+TOCTOU+race+conditions+while+accessing+files

(file-stat-before-action-c)

🔇 Additional comments (9)
CMakeLists.txt (1)

310-310: LGTM!

The plugin is correctly registered in the FLB_ALL block alongside other output plugins, following the established pattern.

tests/runtime/CMakeLists.txt (1)

237-237: LGTM!

Runtime test is correctly registered in the non-Windows block, consistent with out_file test placement.

plugins/out_logrotate/CMakeLists.txt (1)

1-8: LGTM!

Standard plugin CMake configuration. The Shlwapi linkage on MSVC is correct for Windows path handling functions used in the plugin.

plugins/out_logrotate/logrotate.h (1)

1-32: LGTM!

Clean header with proper include guards and format enumeration for the plugin's public API.

plugins/out_logrotate/logrotate.c (4)

118-209: LGTM!

Initialization properly validates max_files >= 1 at lines 148-153, preventing out-of-bounds access in cleanup. Format parsing and configuration map handling are correct.

629-801: LGTM!

Streaming gzip compression correctly handles the edge case where file size is an exact multiple of GZIP_CHUNK_SIZE. The final flush at lines 740-765 ensures Z_FINISH is always sent, addressing the critical issue from past reviews.

804-1125: LGTM!

File rotation and cleanup logic correctly validates rotation filename patterns (lines 898-947), ensuring only legitimately rotated files are counted and cleaned up. The timestamp-based sorting and max_files enforcement are properly implemented.

1127-1485: LGTM!

The flush implementation uses correct hand-over-hand locking (lines 1180-1210): acquires list_lock to find/create entry, acquires entry->lock, then releases list_lock. All code paths correctly manage lock ownership, and file size is properly updated before releasing locks.

tests/runtime/out_logrotate.c (1)

1-1474: Comprehensive test coverage for the logrotate plugin

Excellent test suite covering:

  • Multiple output formats (CSV, LTSV, Plain, MsgPack, Template)
  • Size-based rotation and gzip compression (including edge case for exact-chunk-size files)
  • Max files cleanup and validation
  • Path handling, mkdir, and delimiters
  • Multithreaded concurrent writes

The tests properly validate file counts (using TEST_ASSERT(file_count >= 0) before bounds checks) and handle Windows/POSIX differences with conditional compilation.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 05 '25 11:11 coderabbitai[bot]

Once the coderabbit comments are addressed we should trigger a full package build but @SagiROosto can you confirm it builds for all targets?

patrick-stephens avatar Nov 05 '25 12:11 patrick-stephens

@patrick-stephens UT failed for typo issue I Fixed it along with coderabbit suggestions. This should work on all targets that out_file supports. Tho I'm not sure regarding windows because I use ZLIB for gzip compression

SagiROosto avatar Nov 05 '25 15:11 SagiROosto

You'll have to fix the commits that don't match the guidelines btw, i.e. you need a plugin/module prefix like out_logrotate

patrick-stephens avatar Nov 05 '25 15:11 patrick-stephens

@patrick-stephens I made a couple of changes hopefully it will pass CI this time. can you run it?

SagiROosto avatar Nov 18 '25 10:11 SagiROosto

crossing fingers 🤞🤞🤞

SagiROosto avatar Nov 26 '25 09:11 SagiROosto

I fixed the last coderabbit comment. A small fix so tests should still pass. Now ready to review ☺️

SagiROosto avatar Dec 10 '25 15:12 SagiROosto

windows failure seems like unrelated flaky issue. Can you re-run it?

(also happened in https://github.com/fluent/fluent-bit/pull/11006 -> https://github.com/fluent/fluent-bit/actions/runs/19861920849/job/56914033108)

SagiROosto avatar Dec 11 '25 12:12 SagiROosto