in_amqp: Add documentation for in_amqp plugin#2387
Open
matwey wants to merge 1 commit intofluent:masterfrom
Open
in_amqp: Add documentation for in_amqp plugin#2387matwey wants to merge 1 commit intofluent:masterfrom
matwey wants to merge 1 commit intofluent:masterfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new AMQP input documentation page and updates the table of contents; documents configuration options, message-to-record mapping (body → record content, properties → metadata, headers → nested metadata), parsing examples, reconnect/thread buffer settings, and a rabbitmq-c requirement. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
2 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
pipeline/inputs/amqp.md (1)
9-15: Missingthread.ring_buffer.*configuration parameters.The configuration table omits the ring buffer settings common to Fluent Bit input plugins:
thread.ring_buffer.capacity,thread.ring_buffer.window, andthread.ring_buffer.retry_limit. Based on learnings, in Fluent Bit input plugin documentation underpipeline/inputs, use thethread.prefix for ring buffer settings (e.g.,thread.ring_buffer.capacity,thread.ring_buffer.window,thread.ring_buffer.retry_limit).📝 Suggested table rows to add
| `reconnect.retry_interval` | Retry interval (in seconds) to connect to the broker | `60` | +| `thread.ring_buffer.capacity` | Set the capacity of the ring buffer | _default_ | +| `thread.ring_buffer.window` | Set the window size of the ring buffer | _default_ | +| `thread.ring_buffer.retry_limit` | Set the retry limit for the ring buffer | _default_ |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pipeline/inputs/amqp.md` around lines 9 - 15, Add the missing Fluent Bit ring buffer settings to the AMQP input configuration table: include rows for thread.ring_buffer.capacity, thread.ring_buffer.window, and thread.ring_buffer.retry_limit with appropriate descriptions and defaults (use typical Fluent Bit defaults or project-consistent values); update any documentation references for the amqp input parser/queue/reconnect entries if needed and ensure the keys use the thread. prefix exactly (thread.ring_buffer.capacity, thread.ring_buffer.window, thread.ring_buffer.retry_limit) so they match other pipeline/inputs docs and the plugin implementation.
🤖 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/inputs/amqp.md`:
- Around line 138-140: Summary: Clarify that message requeue on connection loss
depends on acknowledgment mode and timing. Update the bullet that currently
reads "Messages consumed but not yet processed will be requeued by the broker
(assuming proper acknowledgment settings)" to explicitly state this only applies
when the plugin uses manual acknowledgment (not auto-ack), and only if the
connection drops before the client sends basic.ack or basic.nack; reference the
reconnect behavior controlled by reconnect.retry_limits and
reconnect.retry_interval. Edit the text to mention "manual (explicit) ACK mode"
vs "auto-ack", note broker requeue happens on client disconnect prior to
ack/nack, and recommend users ensure the plugin is configured for manual ack if
they require this requeue guarantee.
- Around line 95-110: The YAML places the parsers list under pipeline:, which is
invalid for Fluent Bit; move the "parsers:" block out to top-level (sibling to
"pipeline:"), remove the nested "parsers" under pipeline, and ensure the parser
name ("json") referenced in pipeline.inputs (the amqp input) matches the
top-level parsers entry; verify the "pipeline:", "inputs:", and "outputs:"
structure remains unchanged.
- Around line 9-15: Move the parsers: block out from under pipeline: so parsers:
is a top-level YAML key (alongside pipeline:) and ensure the example uses
parser: json under the amqp input (references: parsers:, pipeline:, inputs:,
parser:). Also update the configuration table to include the two missing
threading parameters thread.ring_buffer.capacity (default 1024) and
thread.ring_buffer.window (default 5) with their descriptions so the AMQP input
docs match the other input plugins.
---
Nitpick comments:
In `@pipeline/inputs/amqp.md`:
- Around line 9-15: Add the missing Fluent Bit ring buffer settings to the AMQP
input configuration table: include rows for thread.ring_buffer.capacity,
thread.ring_buffer.window, and thread.ring_buffer.retry_limit with appropriate
descriptions and defaults (use typical Fluent Bit defaults or project-consistent
values); update any documentation references for the amqp input
parser/queue/reconnect entries if needed and ensure the keys use the thread.
prefix exactly (thread.ring_buffer.capacity, thread.ring_buffer.window,
thread.ring_buffer.retry_limit) so they match other pipeline/inputs docs and the
plugin implementation.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
pipeline/inputs/amqp.md (3)
95-110:⚠️ Potential issue | 🟠 MajorFix YAML example structure:
parsers:must be top-level.In Line 103,
parsers:is nested underpipeline:. Move it to top-level (sibling ofpipeline:), and keepparser: jsonon the input.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pipeline/inputs/amqp.md` around lines 95 - 110, The YAML example nests "parsers:" under "pipeline:" which is incorrect; move the "parsers:" block to be a top-level sibling of "pipeline:" (so it sits alongside "pipeline:" and "outputs:"), leaving the input's "parser: json" reference inside the "pipeline.inputs" entry unchanged; update the example so the keys "pipeline:", "parsers:", and "outputs:" are all top-level keys and "parser: json" remains under the amqp input.
9-15:⚠️ Potential issue | 🟠 MajorAdd missing threaded ring buffer parameters to the config table.
Please document
thread.ring_buffer.capacityandthread.ring_buffer.window(andthread.ring_buffer.retry_limitif supported by this input) so AMQP matches other input plugin docs.Based on learnings: In Fluent Bit input plugin documentation under pipeline/inputs, use the
thread.prefix for ring buffer settings (for examplethread.ring_buffer.capacity,thread.ring_buffer.window,thread.ring_buffer.retry_limit).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pipeline/inputs/amqp.md` around lines 9 - 15, The AMQP input docs are missing the threaded ring buffer settings; update the config table to include entries for thread.ring_buffer.capacity, thread.ring_buffer.window and (if supported) thread.ring_buffer.retry_limit with short descriptions and defaults consistent with other input docs (e.g., capacity = default buffer size, window = default window size, retry_limit = default retry count); reference the existing keys `uri`, `queue`, `parser`, `reconnect.retry_limits`, and `reconnect.retry_interval` so the new `thread.ring_buffer.*` rows follow the same format and naming convention used elsewhere in pipeline/inputs documentation.
138-140:⚠️ Potential issue | 🟡 MinorClarify requeue behavior for ACK mode and timing.
Line 140 currently implies a broad guarantee. Please state this only applies in manual ACK mode and only if disconnect happens before
ack/nackis sent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pipeline/inputs/amqp.md` around lines 138 - 140, The sentence about requeued messages is too broad; update the docs around the third bullet so it explicitly states that message requeueing by the broker applies only when the consumer is using manual ACK mode and the disconnect occurs before the consumer has sent an ack or nack for the message (i.e., unacknowledged messages at disconnect). Reference the existing wording about "Messages consumed but not yet processed will be requeued by the broker" and change it to mention "manual ACK mode" and "disconnect before ack/nack is sent" to make the scope precise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pipeline/inputs/amqp.md`:
- Around line 95-110: The YAML example nests "parsers:" under "pipeline:" which
is incorrect; move the "parsers:" block to be a top-level sibling of "pipeline:"
(so it sits alongside "pipeline:" and "outputs:"), leaving the input's "parser:
json" reference inside the "pipeline.inputs" entry unchanged; update the example
so the keys "pipeline:", "parsers:", and "outputs:" are all top-level keys and
"parser: json" remains under the amqp input.
- Around line 9-15: The AMQP input docs are missing the threaded ring buffer
settings; update the config table to include entries for
thread.ring_buffer.capacity, thread.ring_buffer.window and (if supported)
thread.ring_buffer.retry_limit with short descriptions and defaults consistent
with other input docs (e.g., capacity = default buffer size, window = default
window size, retry_limit = default retry count); reference the existing keys
`uri`, `queue`, `parser`, `reconnect.retry_limits`, and
`reconnect.retry_interval` so the new `thread.ring_buffer.*` rows follow the
same format and naming convention used elsewhere in pipeline/inputs
documentation.
- Around line 138-140: The sentence about requeued messages is too broad; update
the docs around the third bullet so it explicitly states that message requeueing
by the broker applies only when the consumer is using manual ACK mode and the
disconnect occurs before the consumer has sent an ack or nack for the message
(i.e., unacknowledged messages at disconnect). Reference the existing wording
about "Messages consumed but not yet processed will be requeued by the broker"
and change it to mention "manual ACK mode" and "disconnect before ack/nack is
sent" to make the scope precise.
Collaborator
|
@matwey before I review I would like to ask that you address all the comments from coderabbitai bot, please? |
eschabell
added
waiting-for-user
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pipeline/inputs/amqp.md (1)
16-18:⚠️ Potential issue | 🟡 MinorUse concrete defaults instead of
_default_placeholders.Please replace
_default_with actual values (or explicitly state “implementation-defined” if truly unspecified), so operators can configure reliably.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pipeline/inputs/amqp.md` around lines 16 - 18, Replace the `_default_` placeholders for thread.ring_buffer.capacity, thread.ring_buffer.window, and thread.ring_buffer.retry_limit with concrete defaults (or explicitly "implementation-defined" if the implementation truly decides them at runtime); update the table lines for the keys thread.ring_buffer.capacity, thread.ring_buffer.window, and thread.ring_buffer.retry_limit to show numeric defaults (e.g., capacity: 1024, window: 256, retry_limit: 3) or the phrase "implementation-defined", and include units or semantics (items, entries, attempts) so operators know what to configure; ensure the chosen values match the actual implementation/config constants and adjust any duplicate docs accordingly.
🤖 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/inputs/amqp.md`:
- Line 139: Edit the sentence that mentions reconnect.retry_limits and
reconnect.retry_interval so it reads "the plugin will automatically attempt to
reconnect…" (i.e., add the definite article "the" before "plugin") to improve
clarity; locate the sentence containing the property names
reconnect.retry_limits and reconnect.retry_interval and update it accordingly.
---
Duplicate comments:
In `@pipeline/inputs/amqp.md`:
- Around line 16-18: Replace the `_default_` placeholders for
thread.ring_buffer.capacity, thread.ring_buffer.window, and
thread.ring_buffer.retry_limit with concrete defaults (or explicitly
"implementation-defined" if the implementation truly decides them at runtime);
update the table lines for the keys thread.ring_buffer.capacity,
thread.ring_buffer.window, and thread.ring_buffer.retry_limit to show numeric
defaults (e.g., capacity: 1024, window: 256, retry_limit: 3) or the phrase
"implementation-defined", and include units or semantics (items, entries,
attempts) so operators know what to configure; ensure the chosen values match
the actual implementation/config constants and adjust any duplicate docs
accordingly.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Matwey V. Kornilov <matwey.kornilov@gmail.com>
Contributor
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/inputs/amqp.md`:
- Around line 16-18: Update the table to replace the `_default_` placeholders
for `thread.ring_buffer.capacity` and `thread.ring_buffer.window` with concrete
defaults `1024` and `5` respectively (matching other input plugins like
tail/opentelemetry/prometheus-textfile), and investigate the
`thread.ring_buffer.retry_limit` entry: check the AMQP input plugin docs/source
for that parameter and either add the correct documented default if it exists or
remove the `thread.ring_buffer.retry_limit` row from the AMQP docs if it is not
a valid/configurable option for the plugin.
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Author
|
@coderabbitai resume |
Contributor
✅ Actions performedReviews resumed. |
Author
Done |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reference: fluent/fluent-bit#11427
Summary by CodeRabbit