out_kafka: add support for dynamic/static headers#1341
out_kafka: add support for dynamic/static headers#1341DIFRIN wants to merge 1 commit intofluent:masterfrom
Conversation
|
Hi @DIFRIN, |
lecaros
added
waiting-for-user
DIFRIN
force-pushed
the
output-kafka-add_headers
branch
from
59f5378 to
3dec0b6
Compare
|
Hi @lecaros DCO ok. Can you please review my merge request at fluent-bit repo |
lecaros
added
conflict
|
@DIFRIN can you please resolve the conflicts here? I've nudged you on the code PR for the same conflict resolution task fluent/fluent-bit#8583. Both needed to merge. |
|
The conflicts were due to formatting changes for style, I went ahead and fixed the conflicts in the interest of keeping things moving. There's one addition to the table that should be doublechecked by a dev-type person. |
|
I think we're still waiting on the code merge |
eschabell
removed
waiting-for-user
DIFRIN
force-pushed
the
output-kafka-add_headers
branch
from
e0be478 to
f41317f
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds documentation for a new Kafka output Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pipeline/outputs/kafka.md (1)
57-57:⚠️ Potential issue | 🟡 MinorPre-existing typo: Incorrect IP address format.
The IP address
192.1681.3:9092is missing a period and should be192.168.1.3:9092. This same typo also appears at line 265. While these are example IPs, the inconsistency with the correct format used elsewhere in the document (lines 18, 39, 72, 109, 125, etc.) should be corrected.📝 Suggested fix
- host: 192.1681.3:9092 + host: 192.168.1.3:9092Also apply the same fix at line 265.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pipeline/outputs/kafka.md` at line 57, Update the malformed host entries where the IP is written as "192.1681.3:9092" to the correct dotted form "192.168.1.3:9092"; specifically change the occurrences matching the string "host: 192.1681.3:9092" (found around the currently flagged line and again at the other occurrence) so they match the correct format used elsewhere in the document.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@pipeline/outputs/kafka.md`:
- Line 57: Update the malformed host entries where the IP is written as
"192.1681.3:9092" to the correct dotted form "192.168.1.3:9092"; specifically
change the occurrences matching the string "host: 192.1681.3:9092" (found around
the currently flagged line and again at the other occurrence) so they match the
correct format used elsewhere in the document.
eschabell
added
waiting-on-review
|
@DIFRIN also note the lint error for use of |
DIFRIN
force-pushed
the
output-kafka-add_headers
branch
from
f41317f to
1bfcc57
Compare
|
@DIFRIN looks like you missed the lint issues: |
DIFRIN
force-pushed
the
output-kafka-add_headers
branch
from
1bfcc57 to
e016d17
Compare
|
@eschabell Thanks for pointing that out. The suggested lint fixes have now been applied |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/outputs/kafka.md`:
- Line 57: Replace the incorrect YAML key "host" with the documented "brokers"
in the Kafka examples (update occurrences of host: 192.168.1.3:9092 to use
brokers instead), and ensure the value format matches other examples (e.g.,
brokers: ["192.168.1.3:9092"] or brokers: 192.168.1.3:9092 depending on the
surrounding examples). Update both occurrences that currently use the "host" key
so they align with the rest of the docs.
- Around line 111-112: The YAML uses duplicate mapping keys "header"
(specifically the entries 'X-Environment production' and 'X-TraceId $trace_id'),
which is invalid; change this to a single "header" key with an array value (or a
single comma-separated header string if the Kafka plugin supports it) so both
'X-Environment production' and 'X-TraceId $trace_id' are preserved under the
same header key.
Document the new 'header' configuration option added to the Kafka output plugin, which allows attaching custom headers to produced messages. Add a row to the parameter table describing the 'header' key, and add a new 'Message headers' section under 'Get started' with examples for both static headers (literal values) and dynamic headers (values resolved from log record fields using the '$field_name' syntax). Signed-off-by: difrin <frindi.abderrahmane.ied@gmail.com>
DIFRIN
force-pushed
the
output-kafka-add_headers
branch
from
e016d17 to
fe3dc5a
Compare
5 participants
Add support for dynamic or static header for the kafka output plugin
Example:
'docker run --rm --network kafka_default -e value_test='${text}' -it fluent/fluent-bit-local:latest -i dummy -p dummy='{"text": "hello world"}' -p samples=1 -o kafka -p brokers=kafka:9092 -p topics=test_fluentbit -p header="key2 <text>" -vvv'
Gives
kafka-console-consumer --topic test_fluentbit --bootstrap-server kafka:9092 --property print.headers=true
key2:hello world {"@timestamp":1710505560.027627,"text":"hello world"}
Summary by CodeRabbit
New Features
Documentation