search

openzipkin / zipkin-dependencies

Spark job that aggregates zipkin spans for use in the UI
Apache License 2.0
176 stars 81 forks source link

Elasticsearch 7.x/OpenSearch 1.1 span processing fails #202

Closed Le1632 closed 8 months ago

Le1632 commented 2 years ago

Describe the Bug

When using Elasticsearch 7.x (or OpenSearch 1.1 pretending to be Elasticsearch 7.x) processing of spans fails in zipkin2.dependencies.elasticsearch.ElasticsearchDependenciesJob#run(). This is because the method first tries to get the span index as is was before ES 7+, e. .g, zipkin:span-2022-03-31/span without catching the resulting exception in zipkin2.dependencies.elasticsearch.ElasticsearchDependenciesJob#run(java.lang.String, java.lang.String, zipkin2.codec.SpanBytesDecoder).

Steps to Reproduce

  1. Set up Elasticsearch 7.x (or OpenSearch 1.1).
  2. Add span data to a Zipkin index.
  3. Try to process that index.

Expected Behaviour

I see two possible solutions:

  1. Decide in zipkin2.dependencies.elasticsearch.ElasticsearchDependenciesJob#run() whether ES 7+ or previous ES processing is required an proceed accordingly. Do not run both.
  2. Simply catch (and ignore or log) exceptions in zipkin2.dependencies.elasticsearch.ElasticsearchDependenciesJob#run(java.lang.String, java.lang.String, zipkin2.codec.SpanBytesDecoder)
QualoZe0t commented 2 years ago

Hi, I have also configured for Zipkin storage_component: elasticsearch provided by AWS where is installed version Elasticsearch 7.10 but I am getting also the same error message where after index_name is added ":". Can you please fix the code otherwise we are unable to use Zipkin-dependencies(2.6.4.) if we are using external AWS ES as a storage_type component.

jcchavezs commented 2 years ago

Are you up for a fix PR?

On Fri, 12 Aug 2022, 09:34 QualoZe0t, @.***> wrote:

Hi, I have also configured for Zipkin storage_component: elasticsearch provided by AWS where is installed version Elasticsearch 7.10 but I am getting also the same error message where after index_name is added ":". Can you please fix the code otherwise we are unable to use Zipkin-dependencies(2.6.4.) if we are using external AWS ES as a storage_type component.

— Reply to this email directly, view it on GitHub https://github.com/openzipkin/zipkin-dependencies/issues/202#issuecomment-1212819539, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYAVO4UQTCRV4UU2ITKTVYX5CBANCNFSM5SFQJ53A . You are receiving this because you are subscribed to this thread.Message ID: @.***>

QualoZe0t commented 2 years ago

Are you up for a fix PR? … On Fri, 12 Aug 2022, 09:34 QualoZe0t, @.> wrote: Hi, I have also configured for Zipkin storage_component: elasticsearch provided by AWS where is installed version Elasticsearch 7.10 but I am getting also the same error message where after index_name is added ":". Can you please fix the code otherwise we are unable to use Zipkin-dependencies(2.6.4.) if we are using external AWS ES as a storage_type component. — Reply to this email directly, view it on GitHub <#202 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYAVO4UQTCRV4UU2ITKTVYX5CBANCNFSM5SFQJ53A . You are receiving this because you are subscribed to this thread.Message ID: @.>

Hi, well it would be great to fix this bug as classic zipkin docker image properly identified aws elasticsearch version.

Le1632 commented 1 year ago

@jcchavezs I can provide the "fix" I did in my fork (https://github.com/openzipkin/zipkin-dependencies/commit/ab835795d3c71a62252505ce1d23d091c7d1e8d6) as PR. The fix lacks automatic (unit) testing though and AFAIR the code doesn't lend itself to unit testing.

shakuzen commented 1 year ago

@Le1632 That seems like it would help. Please feel free to open the PR.

Le1632 commented 1 year ago

@shakuzen I've created the PR.

codefromthecrypt commented 8 months ago

this should be fine now, but re-open if there are any issues.