Skip to content
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

Add Elastic Api Version header #195

Merged
merged 13 commits into from Sep 29, 2023

Conversation

kaisecheng
Copy link
Contributor

This commit adds "Elastic-Api-Version" : "2023-10-31" to request header when the Elasticsearch is serverless

Copy link
Contributor

@jsvd jsvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inspecting the request done by the manticore adapter in the ES ruby client, we can see the header is not being sent to the server.
a simple test is to bump the date to 2024 and confirm that we can still query a serverless deployment.
My suspicion is around header handling between the headers passed during client creation vs headers passed at request time.

@kaisecheng
Copy link
Contributor Author

Good catch! The client got headers but completely ignored and overwritten by initial headers. It seems a bug only in manticore implementation as faraday and curb do a merge with the input headers.

@kaisecheng
Copy link
Contributor Author

link the upstream issue elastic/elastic-transport-ruby#66

@kaisecheng
Copy link
Contributor Author

upstream has merged the fix. We are now waiting for the release of new version

Copy link
Contributor

@jsvd jsvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor code style fixes. Tested and works great. Would love to not have to have this add_headers constantly, but requires deeper refactoring.
LGTM

lib/logstash/inputs/elasticsearch.rb Outdated Show resolved Hide resolved
lib/logstash/inputs/elasticsearch.rb Outdated Show resolved Hide resolved
kaisecheng and others added 3 commits September 28, 2023 22:19
Co-authored-by: João Duarte <jsvd@users.noreply.github.com>
Co-authored-by: João Duarte <jsvd@users.noreply.github.com>
@kaisecheng
Copy link
Contributor Author

@jsvd The last commit recreated the client with header. es-input should be straightforward to replace the client

Copy link
Contributor

@jsvd jsvd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kaisecheng
Copy link
Contributor Author

link the issue for cleaning up resources #196

@kaisecheng kaisecheng merged commit f711348 into logstash-plugins:main Sep 29, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants