Skip to content

Conversation

@mashhurs
Copy link
Contributor

@mashhurs mashhurs commented Mar 4, 2025

Adds elastic-transport client support where opens LS-core a way to start using newer ES ruby client.

FYI: failed Test plugin docs CI is not related to this change.


def get_transport_client_class
require "elasticsearch/transport/transport/http/manticore"
require_relative "elasticsearch/patches/_elasticsearch_transport_http_manticore"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

review note:

  • _elasticsearch_transport_http_manticore sets
    set user-agent and accept-encoding headers for the es-transport-client versions between 7.2 and 7.16. It seems to me we can remove this but I have kept so far as there are active 7.x users;
  • _elasticsearch_transport_connections_selector has a round-robin load balancer logic for the es-client versions <7.2.0

I have kept them for now but it looks to me we can remove.

@mashhurs mashhurs requested a review from jsvd March 10, 2025 17:19
@mashhurs mashhurs mentioned this pull request Mar 10, 2025
Copy link
Member

@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.

This is a good opportunity to be more explicit about the versions of es ruby this plugin supports, so instead of:

  s.add_runtime_dependency 'elasticsearch', '>= 7.17.9'

we could do

  s.add_runtime_dependency 'elasticsearch', '>= 7.17.9', '< 9'

as we haven't tested with 9 at all.

end
end

def get_transport_client_class
Copy link
Member

Choose a reason for hiding this comment

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

let's add a comment here explaining what's happening

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just added.

mashhurs and others added 2 commits March 11, 2025 11:26
…ments on the logics to support both elasticsearch and elastic transport gems.
Co-authored-by: João Duarte <[email protected]>
@mashhurs
Copy link
Contributor Author

This is a good opportunity to be more explicit about the versions of es ruby this plugin supports, so instead of:

  s.add_runtime_dependency 'elasticsearch', '>= 7.17.9'

we could do

  s.add_runtime_dependency 'elasticsearch', '>= 7.17.9', '< 9'

as we haven't tested with 9 at all.

Yes, this is good point!
Updated. Thank you.

@mashhurs mashhurs requested a review from jsvd March 11, 2025 19:23
Copy link
Member

@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

@mashhurs mashhurs merged commit b920701 into logstash-plugins:main Mar 17, 2025
2 of 3 checks passed
@mashhurs mashhurs deleted the support-elastic-transport-client branch March 17, 2025 03:57
mashhurs added a commit to mashhurs/logstash-input-elasticsearch that referenced this pull request Mar 17, 2025
…-transport-client

Adds elastic-transport client support.

(cherry picked from commit b920701)
mashhurs added a commit to mashhurs/logstash-input-elasticsearch that referenced this pull request Mar 17, 2025
mashhurs added a commit to mashhurs/logstash-input-elasticsearch that referenced this pull request Mar 17, 2025
mashhurs added a commit that referenced this pull request Mar 17, 2025
mashhurs added a commit that referenced this pull request Mar 17, 2025
Merge pull request #223 from mashhurs/support-elastic-transport-client
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

migrate logstash-input-elasticsearch to elasticsearch-ruby 8.x

2 participants