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

Add support for OTLP format #209

Closed
jonatan-ivanov opened this issue Mar 17, 2023 · 12 comments
Closed

Add support for OTLP format #209

jonatan-ivanov opened this issue Mar 17, 2023 · 12 comments

Comments

@jonatan-ivanov
Copy link

jonatan-ivanov commented Mar 17, 2023

I'm not sure if this is the right repo for this issue (please transfer to a better place if any).

Feature

Add support for publishing spans using OTLP (OpenTelemetry Line Protocol) format.

Rationale

There are users who like to use Brave but they want to use a backend that supports OTLP format (not necessarily Zipkin format). Right now (as far as I know) they have two options:

  1. Use the OTel collector and let it convert the data from Zipkin's format to OTLP (this needs extra infrastructure).
  2. Migrate to the OTel SDK (or use Micrometer Tracing/Sleuth-OTel but they also use the OTel SDK).

Since the OTLP specification is stable now, it opens the door for other libraries to publish data in OTLP format.

Example Scenario

This would enable users to use Brave with a backend that supports OTLP format (but does not necessarily support Zipkin format) and would not force them to migrate to OTel or add extra component in their infrastructure (OTel Collector).

Prior Art

I'm not really aware of any that would be similar.
The OTel Collector can be slightly related since it can receive data in the Zipkin format and convert it. Or Micrometer's OtlpMeterRegistry that can emit metrics data in OTLP format. Neither of them are really connected.

@shakuzen shakuzen changed the title Add support for OTLP Add support for OTLP format Mar 17, 2023
@marcingrzejszczak
Copy link

Come to think of it this is a zipkin-reporter and the OTLP protocol has nothing to do with Zipkin. If we were to add support for OTLP in this repo then I suggest that we also add it in Zipkin server. If we don't then a new project that connects Brave with OTLP e.g. otlp-reporter-brave should be created and versioned separately - WDYT ?

@basvanbeek
Copy link
Member

Yes if this is really desired than a new reporter project would make more sense, but would need a sponsor to create and maintain it.

By the way, OTel collector really is meant for these kind of conversions as well as being able to upstream data to central collection in case of multi cluster environments. It is fairly normal for OTel ecosystems to have OTel collectors installed. Zipkin import is pretty much always enabled in vendor distributions of OTel.

@marcingrzejszczak
Copy link

By the way, OTel collector really is meant for these kind of conversions as well as being able to upstream data to central collection in case of multi cluster environments. It is fairly normal for OTel ecosystems to have OTel collectors installed. Zipkin import is pretty much always enabled in vendor distributions of OTel.

I agree, the problem is that people have problems with using Brave and not being able to export in OTLP format. There are certain vendors that only started to accept OTLP format of traces 🤷

@jonatan-ivanov
Copy link
Author

By the way, OTel collector really is meant for these kind of conversions as well as being able to upstream data to central collection in case of multi cluster environments. It is fairly normal for OTel ecosystems to have OTel collectors installed. Zipkin import is pretty much always enabled in vendor distributions of OTel.

As far as I know, the OTel community has issues with maintaining Zipkin-related components (no code owner). Also, a lot of users do not want an extra component in their infrastructure only to convert data from one format to the other. That's why the OTel SDK has a Zipkin exporter and it does not force users to use the collector. This choice is not available with Brave right now, either users don't use Brave or they use the collector.

@jcchavezs
Copy link

jcchavezs commented Aug 26, 2023 via email

@jonatan-ivanov
Copy link
Author

I'm talking about Zipkin-related components in OTel, not Zipkin itself. A few months ago, in a comment on an issue in opentelemetry-collector-contrib, I was told that

we don't have Zipkin code owners in this repository

and

We do not have code owners for the Zipkin exporter/receiver/translator, and none of the current maintainers of this repository are Zipkin experts, from what I know.

I thought this is still the case.

@marcingrzejszczak
Copy link

I've created a draft PR here and a project here.

As for the PR I have still doubts if this code should live in zipkin-reporter-java project since it's not related to sending spans to Zipkin. It would be nice to add the feature to Zipkin though (but that's a separate topic). On the other hand it's reusing the Reporter mechanism so maybe we could consider creating an otlp-brave module inside the zipkin-reporter-java? So summing up pros and cons of this solution.

Pros

  • Project is already setup and has been released frequently, we could create a separate module and release the new feature quickly
  • It's still bound to the zipkin reporter abstractions (e.g. Reporter) even though it's not sending OTLP spans to Zipkin (yet)

Cons

  • Zipkin doesn't support OTLP spans so it's not a Zipkin reporter per se
  • Theoretically we could remove the Reporter abstraction usage and incorporate the logic into the handler or just not use the Reporter abstraction for that. Then there is 0 link between this new code and zipkin-reporter project thus it shouldn't live here

As for the project I moved the code out to separate modules that can be versioned and released separately. Summing up pros and cons of this solution.

Pros

  • Not related to Zipkin Reporter so it lives in a separate repo
  • Can be versioned and releaed separately
  • We can start with 0.0.x versions to show that this is incubating

Cons

  • Delay due to creating a new project
  • We're requiring the users to add yet another project with yet another version to their projects

What are @openzipkin/core opinions on this? How should we proceed?

@basvanbeek
Copy link
Member

Since Zipkin-Reporter-Java already supports a multitude of exporters using different protocols I, after some thoughts, see no issue with supporting OLTP within this repository.

@marcingrzejszczak
Copy link

Great, I like this idea :)

BTW this is a draft of an initial idea of making Zipkin accept OTLP spans (openzipkin/zipkin#3561)

@codefromthecrypt
Copy link
Member

TL;DR; adding external formats or other responsibility to the core repo was decided against years ago when we had a lot more active people. This position is the same, and there are alternatives available. Let's kick this to a repo like zipkin-gcp

both this and the http receiver should begin and probably end as a module, like zipkin-gcp. Doing things in monorepo adds responsibility to those unaffected who have been neglected for years I'd add. Back 3 or 4 years ago we decided to contain responsibility and use the module approach to prevent overloading or tagging on drift management to a skeleton crew. Moreover, it centralizes the concern so that drift is handled both on frontend and backend in the same place by the folks who know what they are doing. TL;DR; happy to help open a contrib repo and you can copy/paste find/replace some things from zipkin-gcp to get started.

@codefromthecrypt
Copy link
Member

ps to the point of zipkin already doing things, we only support zipkin protocols. we actually don't support anything defined externally in another org, except in different repos. It is probably easy to conflate transports like Kafka, used and defined by zipkin sites and in the zipkin org, from formats not defined or influenceable from here.

@codefromthecrypt
Copy link
Member

opened https://github.com/openzipkin-contrib/zipkin-otel to carry any of this forward. I can't personally assist, but perhaps other committers in support of this can review and/or grant karma to let folks make something like zipkin-gcp except for otel stuff!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants