-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
fix[elasticsearch output]: add scheme to fix error in sniffing option #10513
Conversation
Add the scheme from the first elastic URL passed in and derive the scheme to ensure that sniffing is successful
947e55f
to
59c9842
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.
Hey @zpriddy, thanks for tackling this issue! I have a minor comment and a question regarding scheme collection. To me using the first URL is not the best approch as it might leave you with only one possible not out of your set if, by chance, the first node is "special". I have some suggestions in the code, please have a look...
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.
@zpriddy thanks for your quick response. The code looks good, I just put one more minor comment there, but I can also live with this being merged as-is.
@zpriddy please resolve the merge conflict. |
Co-authored-by: Sven Rebhan <36194019+srebhan@users.noreply.github.com>
Hi, Can you please run |
📦 Looks like new artifacts were built from this PR. Expand this list to get them here ! 🐯Artifact URLs |
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.
Perfect. Thanks @zpriddy for working on this PR!
resolves #10512
Set an elasticsearch configuration for
elastic.SetScheme()
matching the URL scheme from the first elastic address set in URLs. This enables the sniffing to search with the proper scheme forhttps://
otherwise it will default tohttp://
We have an upstream issue opened as it seems like this should be done automatically with the elasticsearch plugin.. However currently it requires it to be configured when calling the plugin.
olivere/elastic#1569