out_kafka: add support for dynamic/static headers#1341
Conversation
|
Hi @DIFRIN, |
59f5378 to
3dec0b6
Compare
|
Hi @lecaros DCO ok. Can you please review my merge request at fluent-bit repo |
|
@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 |
e0be478 to
f41317f
Compare
|
Warning Review limit reached
More reviews will be available in 15 minutes and 54 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. 📝 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 |
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.
|
@DIFRIN also note the lint error for use of |
f41317f to
1bfcc57
Compare
|
@DIFRIN looks like you missed the lint issues: |
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>
e016d17 to
fe3dc5a
Compare
Signed-off-by: Eric D. Schabell <eric@schabell.org>
|
@DIFRIN resolved the conflicts. |
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