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

OTLP endpoint #3561

Closed
wants to merge 1 commit into from
Closed

Conversation

marcingrzejszczak
Copy link

@marcingrzejszczak marcingrzejszczak commented Aug 29, 2023

One option - GRPC support

  • added generation of OTLP proto and grpc services (I think the latter is unnecessary)
  • added a converter from OTLP to Zipkin Spans
  • added an GRPC endpoint accepting OTLP proto format

TODO:

  • tests
  • docs
  • optimizations
  • classpath considerations

Another option - Proto over HTTP (https://github.com/openzipkin/zipkin/compare/master...marcingrzejszczak:zipkin:otlp_over_http?expand=1)

- added generation of OTLP proto and grpc services (I think the latter is unnecessary)
- added a converter from OTLP to Zipkin Spans
- added an GRPC endpoint accepting OTLP proto format

TODO:

- tests
- docs
- optimizations
- classpath considerationd
@marcingrzejszczak marcingrzejszczak changed the title Initial commit on the OTLP endpoint OTLP GRPC endpoint Aug 29, 2023
@marcingrzejszczak
Copy link
Author

@openzipkin/armeria @openzipkin/core any thoughts? Do you think that this makes any sense? Do you have any doubts about this?

@trajano
Copy link

trajano commented Oct 30, 2023

Any plans of integrating this soon so Zipkin server can accept OTLP traces?

@wu-sheng
Copy link
Member

A little update. We just added OTLP support on v3 branch. #3580

We are planning to cut an RC release before the end of Nov.

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Dec 3, 2023

TL;DR; We can't add maintenance of format drift and dependency management of an external data format. If we did, it would need to be proven out as a third party add on, and this project healthy, way before we do. At this point, none are true. No offense, but this needs to be moved to a separate repo, at least to begin with. The skywalking approach could also be moved to a separate repo. In any case, this won't land here in the near future.


Hi, marcin. long time no see. A long time ago, we spent a lot of effort to modularize non-core functionality, due to maintenance burden and deps. We evaluated a lot of options and went with spring for modularity and used that in a few areas, like zipkin-gcp. I think with the current state of maintenance, adding something with a lot of dependencies and an externally defined data format isn't a good idea now, even more so than before. We need to get more site-owners involved, and we have been neglecting their PRs, even to fix ES or armeria. We have to fix things like this before adding features.

Here is the rationale about modules, which was made after evaluating a lot of things several years ago

Here are several optional modules, which also can be added to the default server without causing maintenance here. Ack they haven't been maintained in a while, but that's more to the point on maintenance and also contributor status quo.

@codefromthecrypt
Copy link
Member

opened https://github.com/openzipkin-contrib/zipkin-otel to carry any of this forward, if people decide to commit to review and maintenance there.

@codefromthecrypt
Copy link
Member

I will create the scaffolding this morning like zipkin-gcp for https://github.com/openzipkin-contrib/zipkin-otel this would also allow the handler or some variant there as well. Folks can also choose whether or not to, for convenience, migrate in the propagation module for tracestate. One reason to do that could be easy versioning.

in general please follow pattern of zipkin-gcp in decisions for how to config, where to add modules, etc. It will make it easy for people to swap into zipkin-otel and also remove some bikeshedding from naming conventions

cc @marcingrzejszczak @shakuzen

@codefromthecrypt
Copy link
Member

https://groups.google.com/g/zipkin-dev/c/YwKHhoR-hZQ/m/Fs7UBWMmBAAJ is ready for you to take over and run with

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.

4 participants