docs: filters: rewrite-tag: fix params, add yaml example#2389
docs: filters: rewrite-tag: fix params, add yaml example#2389eschabell wants to merge 1 commit intofluent:masterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDocumentation for the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pipeline/filters/rewrite-tag.md (2)
180-241: Excellent addition of YAML multiple-rules example.This new section directly addresses the PR objectives and issue
#2388by providing a clear YAML configuration example for multiple rewrite_tag rules. The structure is well-organized with:
- Clear explanation that rules are evaluated in order until one matches
- Both YAML and .conf format examples in tabs
- Consistent use of the 4-component rule format
The YAML syntax is correct, and the example demonstrates practical usage with multiple rules.
Optional: Consider using contraction for a conversational tone.
Line 182 could use "they're" instead of "they are" per the FluentBit style guide suggestion, though this is purely stylistic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pipeline/filters/rewrite-tag.md` around lines 180 - 241, Change the sentence "When multiple rules are defined, they are evaluated in order until one matches." to use a contraction for a more conversational tone: replace "they are" with "they're" in the rewrite-tag.md snippet so it reads "When multiple rules are defined, they're evaluated in order until one matches." This is the only change needed in the top explanatory line of the multi-rule example.
26-31: Configuration table correctly reflects YAML parameter conventions.The parameter table has been appropriately updated to use snake_case naming, which aligns with YAML configuration format conventions. The descriptions are clear and technically accurate.
Minor: Consider addressing style guide feedback.
The pipeline validation suggests reviewing the use of "below" on line 28 per the FluentBit Style Guide. This is a directional term that style guides often recommend replacing with more specific references.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pipeline/filters/rewrite-tag.md` around lines 26 - 31, Update the phrasing that uses the directional term "below" in the emitter buffer description to a more specific, style-guide-compliant phrase; for example, replace "The pipeline will remain paused until the output drains the buffer below the `10M` limit." with "The pipeline will remain paused until the output drains the buffer to a value less than the `10M` limit." Ensure this change is applied to the description for emitter_mem_buf_limit in the table and keep the parameter name `emitter_mem_buf_limit` and example value `10M` intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pipeline/filters/rewrite-tag.md`:
- Around line 180-241: Change the sentence "When multiple rules are defined,
they are evaluated in order until one matches." to use a contraction for a more
conversational tone: replace "they are" with "they're" in the rewrite-tag.md
snippet so it reads "When multiple rules are defined, they're evaluated in order
until one matches." This is the only change needed in the top explanatory line
of the multi-rule example.
- Around line 26-31: Update the phrasing that uses the directional term "below"
in the emitter buffer description to a more specific, style-guide-compliant
phrase; for example, replace "The pipeline will remain paused until the output
drains the buffer below the `10M` limit." with "The pipeline will remain paused
until the output drains the buffer to a value less than the `10M` limit." Ensure
this change is applied to the description for emitter_mem_buf_limit in the table
and keep the parameter name `emitter_mem_buf_limit` and example value `10M`
intact.
207db50 to
1d93613
Compare
|
@patrick-stephens and @cosmo0920 this is ready for review if you have some time please? |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pipeline/filters/rewrite-tag.md`:
- Around line 187-210: The YAML example uses multiple duplicate "rule:" keys
under the rewrite_tag filter which is invalid YAML; update the filters block for
the "rewrite_tag" entry so there is a single "rule:" key whose value is a YAML
sequence (array) of quoted rule strings, each following the format KEY REGEX
NEW_TAG KEEP (with an optional 5th AND_COMBINE field), preserving the original
rule order and quoting each line (e.g., "$log ^(1)$ newtag_or false", "$log
^(.*and)$ newtag_and_1 false", etc.) so Fluent Bit evaluates them in sequence.
1d93613 to
e41521e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pipeline/filters/rewrite-tag.md`:
- Around line 201-206: Update the multi-rule example so the expected output
matches first-match evaluation: change the displayed output tag for input
"10and" from newtag_and_2 to newtag_and_1 (because "$log ^(.*and)$ newtag_and_1
false" is evaluated before "$log ^(1.*)$ newtag_and_2 false"); ensure the same
correction is applied to the second occurrence of this example (lines referenced
in the comment).
- Sort configuration parameters table alphabetically
- Fix parameter name case to lowercase to match convention
- Add fluent-bit.yaml tab to the multiple rules configuration example
- Remove AND_COMBINE documentation: the fifth rule component is not
implemented in the source (process_config() only reads four components;
rewrite_rule struct has no and_combine field)
- Fix blank line before ## Configuration example with multiple rules header
- Fix MD031: add blank lines around code fences
- Fix MD040: add missing language specifier on output code fence
Fixes fluent#2388
Signed-off-by: Eric D. Schabell <eric@schabell.org>
e41521e to
f9db25f
Compare
Fixes #2388
Summary by CodeRabbit