openzipkin / zipkin

Zipkin is a distributed tracing system
https://zipkin.io/
Apache License 2.0
17.02k stars 3.09k forks source link

Can Zipkin support AWS Opensearch? #3753

Open kendarkfire opened 2 years ago

kendarkfire commented 2 years ago

Feature

Can Zipkin support AWS Opensearch?

Rationale

Elasticsearch has changed license and it is not open source project anymore, AWS Opensearch is open source project and it is almost the same as elasticsearch7.10, but it failed to use opensearch directly with Zipkin because Zipkin check the version and only accept ES 5-7.X, opensearch latest version is 1.2.

reta commented 2 years ago

The Elasticsearch clients up to 7.12.x release could connect to Opensearch clusters, you may need to set the persistent.compatibility.override_main_response_version setting [1]:

PUT _cluster/settings
{
  "persistent": {
    "compatibility": {
      "override_main_response_version": true
    }
  }
}

In this regards, Opensearch is inplace replacement for Elasticsearch. But Elasticsearch clients 7.13+ would not work and dedicated Opensearch client has to be used.

Update: seems like Zipkin does not use official Elasticsearch client, setting the persistent.compatibility.override_main_response_version should enough, in this case Opensearch would return Elasticsearch's version.

[1] https://opensearch.org/blog/technical-posts/2021/10/moving-from-opensource-elasticsearch-to-opensearch/

kendarkfire commented 2 years ago

This is great, I just tried it and it works fine.

Thanks!

MB-teamdevops commented 2 years ago

Hi @kendarkfire, I added the persistent.compatibility.override_main_response_version parameter cluster settings. Still I am unable to load zipkin index in opensearch.

Will you please help me to integrate zipkin with Opensearch.

AWS Opensearch - v1.2 zipkin - v2.23, v2.21

emanzat commented 2 years ago

The override main response setting compatibility.override_main_response_version is deprecated from OpenSearch version 1.x and removed from OpenSearch 2.0.0. This setting is no longer supported for compatibility with legacy clients.

reta commented 2 years ago

@emanzat the change is indeed deprecated, was removed and reverted later on [1], it should be there for 2.x but very likely not in 3.x.

[1] https://github.com/opensearch-project/OpenSearch/pull/3530

AndrewFarley commented 8 months ago

So what is the fix, if any, for opensearch 2.x? As this does not work on 2.5... {"Message":"Your request: '/_cluster/settings' payload is not allowed."}

Seems really silly, but can we just have a variable which allows us to bypass elasticsaerch version check upon startup? If OS is API compatible with ES, then Zipkin should "just work" but because of this version check it's failing to even try. Thoughts?

ES_SKIP_VERSION_CHECK == false by default, but can be set to true for people who want to use Zipkin with OS? Should be a quick fix eh?

reta commented 8 months ago

@AndrewFarley I think the best option to move on is to have dedicated support for OpenSearch, since divergence between two (OS and ES) would only grow with time, @codefromthecrypt what do you think?

AndrewFarley commented 8 months ago

@reta Native OS support would be great also. I don't know the complexity of that, which is why I felt like a skip-version-check might be a quicker and temporary fix until someone got around to adding OS support natively.

reta commented 8 months ago

@reta Native OS support would be great also.

I think I could pick it up, if @codefromthecrypt LGTMs this new storage integration

codefromthecrypt commented 8 months ago

I think maybe folks forgot we have an openzipkin/zipkin-aws image which is configured for their elasticsearch https://github.com/openzipkin/zipkin-aws/tree/master/module#elasticsearch-storage

I'll move this issue over there!

codefromthecrypt commented 8 months ago

sorry I'll move it back.. I mistook this for being about aws hosted opensearch.

codefromthecrypt commented 8 months ago

@reta we own the client here so can make whatever changes we need. I don't think we should add a native client because then we have double maintenance and probably there will be dependencies which makes it not viable for the slim build. We have a lot of armeria here to leverage and that extends to self-tracing, boringssl, etc all made for elasticsearch and probably not wise to jump into something else unless we must.

My suggestion is to make a separate module, sure, but keep the same code. Only new tests and builder-specific stuff should be in the opensearch module, as well we'll need a new test image with the same base layer as the others.

don't forget that if we "support" this, it means we need the spark job updated. That means either swapping its existing native client out with one that also works with opensearch, or using two clients and dealing with dep alignment.

Do you mind giving a try to make this a drop in first? I think that's the best way, and conserves our energy! If we fail, we fail..

codefromthecrypt commented 8 months ago

@AndrewFarley for the very specific issue here, yeah I agree that we should have a "force version" setting, because we still need to pick a version to emulate. This would be handy in case a contributor stops being active in maintenance (as happened with me in the past), a future version that's compatible with the others yet in a different scheme.. this doesn't stop us in our tracks. That said..

@reta I think that a version translator which isn't terribly high maintenance will help. We might be able to say "future versions of ES are assumed to work without maintenance", so use numeric expressions (e.g. 10 and 9 are assumed to work with 8 schema). This at least gives folks a chance for things to work imho

codefromthecrypt commented 8 months ago

ps @kendarkfire sorry for being not really aware of naming, but is opensearch "AWS opensearch" or is "AWS Opensearch" when you use it on hosted AWS? One thing that would be nice to get out of this issue is a full scope of things folks want. I'm guessing:

reta commented 8 months ago

@reta we own the client here so can make whatever changes we need. I don't think we should add a native client because then we have double maintenance and probably there will be dependencies which makes it not viable for the slim build.

@codefromthecrypt makes perfect sense, it think that should work, since we don't use any ES specific features that OS does not have

Do you mind giving a try to make this a drop in first? I think that's the best way, and conserves our energy! If we fail, we fail

Absolutely

codefromthecrypt commented 7 months ago

@reta a hint here is that we made LazyHttpClient to allow aws variant to customize things. It is perfectly fine to add things to the ElasticsearchStorage component that make this easier for you to achieve. swapping a function or two may let this all work.