-
Notifications
You must be signed in to change notification settings - Fork 544
Fixed elasticsearch input plugin doc with missing config parameters and some cleanup. Fixed #2245 #2246
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
Conversation
WalkthroughDocumentation update for the Elasticsearch input plugin: reorganized configuration parameters, added Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (4)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pipeline/inputs/elasticsearch.md (2)
22-24: TLS/SSL section reference could include more specific guidance.The new TLS/SSL section (lines 22–24) is helpful but provides only a pointer to general Transport Security documentation. Consider adding a brief note about common TLS configuration patterns specific to Elasticsearch ingestion (e.g., certificate validation, client authentication) to reduce friction for users configuring secure Elasticsearch endpoints.
26-26: Clarify sniffing concept for new users.Line 26 mentions cluster "sniffing" for auto-discovery but assumes familiarity. Consider expanding briefly: what sniffing does, why users might need it (e.g., cluster topology changes), and any trade-offs or requirements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pipeline/inputs/elasticsearch.md(1 hunks)
🔇 Additional comments (3)
pipeline/inputs/elasticsearch.md (3)
12-12: Inconsistency: Parameter removal claims not reflected in final table.The AI summary indicates that
buffer_max_size,tag_key,meta_key, andversionwere removed from the parameters table, but these entries remain visible in lines 12, 16, 18, and 20. Additionally, examples on lines 87–88 and 103–104 referencebuffer_max_size, which would be unavailable if removal were intentional.Please clarify whether these parameters should remain documented or whether removal is incomplete.
Also applies to: 16-16, 18-18, 20-20
14-14: Verify the intended parameter name per PR objectives.The PR objective states "Add the missing http configuration parameter," but the added parameter is named
http2(line 14). Confirm whether:
http2is the correct parameter name intended to address the objective, or- An additional
httpparameter should also be documented.
14-17: Well-documented parameter additions.The new
http2,listen, andportparameters are clearly documented with sensible defaults, and the examples correctly demonstrate their usage. This directly addresses the stated objective of adding missing configuration guidance.Also applies to: 51-52, 65-66, 85-86, 101-102
Signed-off-by: Eric D. Schabell <eric@schabell.org>
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pipeline/inputs/elasticsearch.md(1 hunks)
🔇 Additional comments (2)
pipeline/inputs/elasticsearch.md (2)
22-31: Verify TLS property names against Transport Security documentation.The TLS/SSL section references
tls.verify,tls.ca_file,tls.crt_file, andtls.key_fileas common configuration options. Ensure these property names match those defined in the Transport Security documentation and that they are the correct properties supported by this input plugin.
1-144: Documentation update successfully addresses PR objectives.The changes effectively implement the stated PR objectives:
- ✓ Added missing HTTP configuration parameters (
listen,port,http2)- ✓ Reorganized documentation with new TLS/SSL and Sniffing sections
- ✓ Updated hostname parameter description and linked it to the new Sniffing section
- ✓ Updated configuration examples to use the new parameters
- ✓ Clarified parameter defaults and layout
The overall structure is well-organized and the new sections provide helpful context for users configuring Elasticsearch ingestion.
Signed-off-by: Eric D. Schabell <eric@schabell.org>
|
AI review passed, merging. |
Fixed elasticsearch input plugin doc with missing config parameters and some cleanup. Fixed #2245
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.