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

Should we drop the native elasticsearch transport? #1511

Closed
codefromthecrypt opened this issue Feb 2, 2017 · 7 comments
Closed

Should we drop the native elasticsearch transport? #1511

codefromthecrypt opened this issue Feb 2, 2017 · 7 comments

Comments

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Feb 2, 2017

Currently, the zipkin-server bundles a dep on elasticsearch 2.x drivers (which themselves pull deps on guava). The 2.x drivers are incompatible with later versions of Elasticsearch, and cannot coexist since the 2.x and 5.x libraries use the same class names. We need to decide what do do about it.

The ES dependency is needed for the transport protocol. The transport protocol is only used by java clients or a "tribe node". Incidentally, not all java clients support the transport protocol, either. For example, elasticsearch's spark support uses http, and the elasticsearch rest client does as well. There are some advantages to the native protocol around multiplexing, but it adds a weight to the project I don't think we can afford.

We made "elasticsearch-http" to support amazon deployments which only support http. Incidentally we also use http in zipkin-dependencies, too, as that's what spark supports. Recently (1.20), we decoupled "elasticsearch-http" from the native elasticsearch driver. In doing so, we solved the case where custom servers were blocked from using ES 5.x libraries (#1431). We also dodged a guava dependency which held users back to version 18. However, we doubled our maintenance as changes to http or the transport codebases need to be synchronized. This also linearly impacts CI time.

At the moment, eventhough from a library component pov we are decoupled, the default server packages the transport protocol. This means we are still effectively limited by the library dependencies and also the double-maintenance until we change.

I recommend that we drop the transport codebase, with some warning, and coerce ES_HOSTS to use http ports instead. In this case, it would act just like the spark job which does the same thing.

@openzipkin/elasticsearch what do you all think?

@shakuzen
Copy link
Member

shakuzen commented Feb 2, 2017

Drop it like it's hot

@mansu
Copy link

mansu commented Feb 2, 2017

+1 for dropping the native driver.

@anuraaga
Copy link
Contributor

anuraaga commented Feb 2, 2017

SGTM!

@ImFlog
Copy link
Contributor

ImFlog commented Feb 2, 2017

+1

@dan-tr
Copy link

dan-tr commented Feb 2, 2017

We are deploying Zipkin Server with an Elasticsearch backend, and would actually prefer to use the HTTP rather than the transport protocol.

@codefromthecrypt
Copy link
Member Author

going to drop this shortly

codefromthecrypt pushed a commit that referenced this issue Mar 30, 2017
This drops the Elasticsearch Native transport, which allows us to focus
efforts on the http variant. As a side-effect, this will implicitly
convert configuration pointing to the native port 9300 to the http port
9200 after logging a warning. This is an attempt to not break existing
users who expose both ports.

Fixes #1511
codefromthecrypt pushed a commit that referenced this issue Mar 30, 2017
This drops the Elasticsearch Native transport, which allows us to focus
efforts on the http variant. It also stops us from interfering with
Spring Boot autoconfiguration of Elasticsearch. Finally, it speeds up
the build as related docker tests are no longer needed.

To help in transition, this implicitly converts server configuration
pointing to the native port 9300 towards the http port 9200 after
logging a warning. This is an attempt to not break existing users.

Fixes #1511
codefromthecrypt pushed a commit that referenced this issue Mar 30, 2017
This drops the Elasticsearch Native transport, which allows us to focus
efforts on the http variant. It also stops us from interfering with
Spring Boot autoconfiguration of Elasticsearch. Finally, it speeds up
the build as related docker tests are no longer needed.

To help in transition, this implicitly converts server configuration
pointing to the native port 9300 towards the http port 9200 after
logging a warning. This is an attempt to not break existing users.

Fixes #1511
@codefromthecrypt
Copy link
Member Author

#1547 drops this

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

No branches or pull requests

6 participants