-
Notifications
You must be signed in to change notification settings - Fork 543
es_out: support Upstream Servers with configuration overriding #1143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
es_out: support Upstream Servers with configuration overriding #1143
Conversation
cb36f75 to
7611f0e
Compare
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
|
This pull request is waiting for review of fluent/fluent-bit#7608 |
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days. |
7611f0e to
25c7642
Compare
25c7642 to
7138a28
Compare
7138a28 to
c1092dc
Compare
cnorris-cs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left suggestions, but I've added my technical writing-based approval based on that to keep this moving.
Primarily, I was looking to help the table be more consistent and readable, while implementing some style-based suggestions for consistency.
a78ebf2 to
c620db5
Compare
|
Hi @cnorris-cs, I applied some (!) parts of your suggestions as well as introduced additional changes for consistency with your suggestions. Please take a look at this pull request one more time when you have time. Some of the changes are implemented in dedicated commits (for ease of revert in case of concerns) - please let me know if I need to squash all commits into a single one. Thank you for your review. |
c620db5 to
23f04ef
Compare
23f04ef to
d8bad1b
Compare
d8bad1b to
d465436
Compare
312b9a1 to
02dbd5d
Compare
esmerel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of minor stylistic suggestions for consistency, but these docs changes look good to me otherwise.
02dbd5d to
da614db
Compare
dee1a67 to
6379bf2
Compare
6379bf2 to
dd6d5da
Compare
dd6d5da to
a458946
Compare
9d9fc20 to
e38893e
Compare
|
@esmerel can you look at the Vale check failures, I'm seeing this error before: Raw Output: |
esmerel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making suggestions to correct vale warnings.
c200512 to
ea63874
Compare
ea63874 to
d8aa628
Compare
📝 WalkthroughWalkthroughAdded Upstream support documentation for Elasticsearch: updated plugin list, introduced Upstream-aware parameter overrides, clarified AWS Sigv4/TLS interactions with Upstream NODE overrides, and added Classic/YAML configuration examples and usage snippets. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✏️ Tip: You can disable this entire section by setting 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 |
Signed-off-by: Marat Abrarov <[email protected]>
Signed-off-by: Marat Abrarov <[email protected]>
…d on review comments. Signed-off-by: Marat Abrarov <[email protected]>
d8aa628 to
9f9a5ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@pipeline/outputs/elasticsearch.md`:
- Around line 207-218: The YAML example uses an unquoted asterisk for the match
value ("match: *"), which YAML treats as an anchor reference; update the example
so the match key uses a quoted literal asterisk (e.g., change match: * to match:
"*" ) to ensure the YAML parses as intended and preserves the wildcard
semantics.
- Around line 226-249: The YAML has an invalid unquoted wildcard in the pipeline
output config: change the match field from an unquoted asterisk to a quoted
string (e.g., change match: * to match: "*" or match: "'*'") in the pipeline
outputs block so the YAML parser accepts the wildcard value; update the pipeline
outputs entry with the quoted wildcard for the match key.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
administration/configuring-fluent-bit/classic-mode/upstream-servers.mdpipeline/outputs/elasticsearch.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-12T14:30:18.461Z
Learnt from: kalavt
Repo: fluent/fluent-bit-docs PR: 2294
File: pipeline/inputs/kafka.md:147-168
Timestamp: 2025-12-12T14:30:18.461Z
Learning: In Fluent Bit v4.0.4+, when using AWS MSK IAM authentication (rdkafka.sasl.mechanism: aws_msk_iam), the rdkafka.security.protocol is automatically set to SASL_SSL and the AWS region is auto-detected from the broker hostname for standard MSK endpoints. The aws_msk_iam_cluster_arn parameter was removed - users only need to set rdkafka.sasl.mechanism: aws_msk_iam (and optionally aws_region for custom DNS/PrivateLink).
Applied to files:
pipeline/outputs/elasticsearch.md
🔇 Additional comments (7)
administration/configuring-fluent-bit/classic-mode/upstream-servers.md (1)
8-8: LGTM! Elasticsearch correctly added to Upstream-supported plugins.The addition properly documents Elasticsearch as an Upstream-capable plugin, consistent with the extensive Upstream configuration documentation added in the elasticsearch.md file.
pipeline/outputs/elasticsearch.md (6)
13-20: LGTM! Clear documentation of Upstream override capability.The introduction and table header effectively explain which configuration parameters can be overridden in Upstream NODE sections.
21-58: LGTM! Comprehensive override documentation for Elasticsearch parameters.The "Allows overrides" column correctly identifies which parameters can be overridden in Upstream NODE sections, with helpful explanations for Host and Port requirements.
66-68: LGTM! Important clarification about AWS Sigv4 and Upstream interaction.This section correctly distinguishes between TLS parameters for AWS authentication (not overridable in NODE) and TLS parameters for Elasticsearch connections (overridable in NODE), preventing potential configuration confusion.
163-163: Note: Vale style check flagged this heading.Per the PR objectives, Vale reported that this heading should use sentence-style capitalization. However, "Upstream" appears to be used as a proper noun (feature name) throughout the documentation, so the current capitalization may be intentional. Please verify whether this should be "Classic mode configuration file with upstream" or if "Upstream" should remain capitalized as a feature name.
165-201: LGTM! Well-documented Classic mode Upstream configuration.The Classic mode configuration examples and Upstream server definitions are syntactically correct and clearly demonstrate how to configure Elasticsearch with Upstream load balancing.
220-224: LGTM! Helpful explanation of YAML Upstream configuration options.The explanation clearly documents that Upstream configuration can use either Classic mode format or YAML format, and can optionally be embedded in the main configuration file when using YAML.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Signed-off-by: Marat Abrarov <[email protected]>
Signed-off-by: Marat Abrarov <[email protected]>
9f9a5ee to
92e2433
Compare
Implementation of Upstream feature for the Elasticsearch output plugin.
Refer to fluent/fluent-bit#7608 for the changes in Fluent Bit code.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.