config: Add support for --no-banner command line argument #11307
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit 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. WalkthroughAdds a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
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.
ℹ️ 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".
src/fluent-bit.c
Outdated
| "b:c:dDf:C:i:m:M:o:R:r:F:p:e:" | ||
| "t:T:l:vw:qVhJL:HP:s:SWYZ", | ||
| "t:T:l:v:Bw:qVhJL:HP:s:SWYZ", |
There was a problem hiding this comment.
The updated short options string now has v: because the new B flag was inserted as v:B. In getopt_long, a colon after a flag means it requires an argument, so -v will start consuming the next token (or error if it is last), breaking the existing verbosity flag. This will affect any invocation that uses -v without an argument (the documented/expected usage). The short option string should list B without introducing v: (e.g., ...l:vBw:...).
Useful? React with 👍 / 👎.
|
Startup log with and without |
![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
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
include/fluent-bit/flb_config.hsrc/flb_config.csrc/fluent-bit.c
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-21T06:23:29.770Z
Learnt from: cosmo0920
Repo: fluent/fluent-bit PR: 11171
File: include/fluent-bit/flb_lib.h:52-53
Timestamp: 2025-11-21T06:23:29.770Z
Learning: In Fluent Bit core (fluent/fluent-bit repository), function descriptions/documentation are not required for newly added functions in header files.
Applied to files:
src/fluent-bit.cinclude/fluent-bit/flb_config.h
📚 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:
include/fluent-bit/flb_config.h
🔇 Additional comments (7)
src/flb_config.c (1)
277-277: LGTM! Initialization is correct.The banner field is properly initialized to
FLB_TRUE, making the banner display the default behavior. The placement afterverboseand beforegraceis logical and consistent with the struct definition order.include/fluent-bit/flb_config.h (1)
81-83: LGTM! Field declaration is well-placed and documented.The new
bannerfield is appropriately positioned in the struct afterverboseand the comment clearly indicates the default behavior. Theinttype is consistent with other boolean-like configuration flags in the codebase.src/fluent-bit.c (5)
173-173: LGTM! Help text is clear and descriptive.The help text clearly describes the flag's purpose. The capitalization is consistent with other help text entries in the file.
1072-1072: LGTM! Long option definition is correct.The
--no-banneroption is properly defined withno_argumentand correctly maps to the short option 'B'.
1130-1130: LGTM! Option character correctly added to getopt string.The 'B' character is properly added without a colon, correctly indicating that
--no-bannertakes no argument.
1289-1291: LGTM! Flag handler is correctly implemented.The case handler properly sets
config->bannertoFLB_FALSEwhen the-Bflag is encountered, following the same pattern as other boolean flags in the codebase.
1289-1291: Verify: Is command-line-only control sufficient?The
bannerflag is not exposed in theservice_configsarray insrc/flb_config.c, meaning it cannot be controlled via configuration files—only through the command-line flag.Given the PR's use case (service sidecars with frequent rotation), command-line control is likely sufficient. However, please confirm this is intentional and whether config file support might be needed for other deployment scenarios.
| /* Show banner only if the configuration explicitly says so */ | ||
| if (config->verbose != FLB_LOG_OFF && config->banner == FLB_TRUE) { | ||
| flb_version_banner(); | ||
| } |
There was a problem hiding this comment.
Update misleading comment.
The logic is correct, but the comment is misleading. It states "only if the configuration explicitly says so", but the banner is actually shown by default unless explicitly disabled via -B or quiet mode (-q).
The comment should reflect that the banner is the default behavior, not an opt-in feature.
🔎 Suggested comment correction
- /* Show banner only if the configuration explicitly says so */
+ /* Show banner unless disabled via --no-banner or quiet mode */
if (config->verbose != FLB_LOG_OFF && config->banner == FLB_TRUE) {
flb_version_banner();
}📝 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.
| /* Show banner only if the configuration explicitly says so */ | |
| if (config->verbose != FLB_LOG_OFF && config->banner == FLB_TRUE) { | |
| flb_version_banner(); | |
| } | |
| /* Show banner unless disabled via --no-banner or quiet mode */ | |
| if (config->verbose != FLB_LOG_OFF && config->banner == FLB_TRUE) { | |
| flb_version_banner(); | |
| } |
🤖 Prompt for AI Agents
In src/fluent-bit.c around lines 1349 to 1352, the existing comment "Show banner
only if the configuration explicitly says so" is misleading because the banner
is shown by default unless disabled with -B or quiet mode; update the comment to
state that the banner is displayed by default and will be suppressed only when
config->banner is false or verbose is set to FLB_LOG_OFF (e.g., via -B or -q),
so the comment accurately reflects default behavior and suppression conditions.
Implement --no-banner command line argument in order to disable version banner printing altogether. Signed-off-by: José Quadrado <[email protected]>
1 participant
Implement --no-banner command line argument in order to disable version banner printing altogether.
This PR adds a new config flag that disables the version banner but preserves the normal logging mechanism.
Using FluentBit in the context of a service sidecar with frequent service rotation, the version banner adds a non-negligent amount of log data to be consumed. Additionally, the MOTD is not indexed correctly and pollutes the log stream of FluentBit itself.
Having this flag will enable users to preserve needed logging but avoid the overhead of the version banner.
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:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
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
✏️ Tip: You can customize this high-level summary in your review settings.