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

Default client is missing for opentelemetry-exporter-otlp #3123

Closed
jonatan-ivanov opened this issue Apr 6, 2021 · 8 comments · Fixed by #3786
Closed

Default client is missing for opentelemetry-exporter-otlp #3123

jonatan-ivanov opened this issue Apr 6, 2021 · 8 comments · Fixed by #3786
Labels
Feature Request Suggest an idea for this project

Comments

@jonatan-ivanov
Copy link
Member

jonatan-ivanov commented Apr 6, 2021

Please see the description of spring-projects-experimental/spring-cloud-sleuth-otel#39
opentelemetry-exporter-otlp does not have with a default client so it is not usable out of the box. Given that it is not an API module, this results in a bad user experience: the users are unable to use it unless they add a client dependency.

The root cause is the compileOnly dependency.
I think it would be a much better user experience if opentelemetry-exporter-otlp could provide a default client but it would make it possible for the users to change it.

From the implementation point of view, this means changing one of the compileOnly dependencies to implementation and writing a few lines of documentation that show the user how to exclude dependencies using maven/gradle.

It seems that other exporter modules include a client. Can opentelemetry-exporter-otlp consistent to them?

@jonatan-ivanov jonatan-ivanov added the Bug Something isn't working label Apr 6, 2021
@jkwatson jkwatson added Feature Request Suggest an idea for this project and removed Bug Something isn't working labels Apr 6, 2021
@jkwatson
Copy link
Contributor

jkwatson commented Apr 6, 2021

Changing this to a Feature Request since this was as-designed and intended.

@shakuzen
Copy link

shakuzen commented Apr 7, 2021

Despite my opinion that it is a separate issue, at the request of @jkwatson, I will comment here. Separate from there not being a default client dependency, the dependency versions are not managed in the BOM. So this means that third party libraries and end users need to somehow know what version of the clients to use that is compatible with the version of OpenTelemetry they're using. Is that too the design? We have a user asking us to manage the dependency in Sleuth, but I don't see why Sleuth should be responsible for knowing the version of the client dependency that works with the version of OpenTelemetry.

@anuraaga
Copy link
Contributor

anuraaga commented Apr 7, 2021

gRPC is a pretty complicated library - it means there are many cases where we can't provide a good experience no matter what we do really. For example, if we publish grpc-netty-shaded in our POM, but the user uses gRPC in their own app with grpc-netty, it's quite likely for the version of grpc-netty-shaded to be out of sync with the other ones on the classpath which can lead to crashes at runtime if there is an incompatibility - I've seen this a couple of times in the past so I would say gRPC has a requirement to manage their versions with a BOM, similarly to what we do. Such crashes are generally more mysterious than the more explicit missing transport error.

One of the reasons other exporters do publish the dependency on the transport is because these transports aren't complicated libraries, nor do they have an option of shaded vs non-shaded, etc.

That's the background - IMO there's no good solution so we just have to pick one. Currently, we assume a persona that generally uses the SDK via another distribution, most commonly the java agent right now, but hopefully sleuth too going forward. I don't know if there's a difference between Sleuth and Spring Boot in this regard but I know the latter manages a large amount of dependencies in the app - does managing gRPC version seem to fit with this? It helps ensure compatibility with gRPC usage outside of the OTel exporter.

@anuraaga
Copy link
Contributor

anuraaga commented Apr 7, 2021

the dependency versions are not managed in the BOM

I don't think I've ever seen a "core library" (not spring-boot) that manages dependency versions in the BOM, it's just to align the published artifacts. We definitely can't manage gRPC in our BOM since we don't want to surprise users if they user gRPC themselves.

responsible for knowing the version of the client dependency that works with the version of OpenTelemetry.

The issue at hand really isn't about version but about the decision of whether to use grpc-netty or grpc-netty-shaded (or maybe even grpc-okhttp but that's very uncommon on server). We currently aren't comfortable with deciding on one of these in the SDK, but in the future maybe we can if there's a lot of demand for a default transport transitive dependency + we have more data on how the SDK is being used in the wild. For example, we don't accomplish anything if after picking one, most usage of the SDK involves excluding our choice.

Edit: Realize you're agreeing with the "issue at hand" bit :P

Version itself, it's the same issue as any library really, we hope that semver protects against dependency hell, and in general users should use the latest version of a library if they can. OTel is always compatible with the latest version of grpc-java (at least until they release a 2.x).

@jonatan-ivanov
Copy link
Member Author

Base on these:

gRPC is a pretty complicated library - it means there are many cases where we can't provide a good experience no matter what we do really.

One of the reasons other exporters do publish the dependency on the transport is because these transports aren't complicated libraries, nor do they have an option of shaded vs non-shaded, etc.

IMO there's no good solution so we just have to pick one.

Is dropping gRPC and using something that actually works instead an option?

I don't know if there's a difference between Sleuth and Spring Boot in this regard but I know the latter manages a large amount of dependencies in the app - does managing gRPC version seem to fit with this?

I think Spring Boot manages its own dependencies in the BOM (it has auto-configuration for those things) but I don't think it would manage dependencies for its dependencies like in this case managing dependencies of OTel.

@jonatan-ivanov
Copy link
Member Author

jonatan-ivanov commented Apr 7, 2021

we don't accomplish anything if after picking one, most usage of the SDK involves excluding our choice.

I think we would accomplish a lot by having a default client: having a library that works out of the box.
(I might made a mistake by saying GRPC client is missing in the title, fixing it)

@jonatan-ivanov jonatan-ivanov changed the title Default GRPC client is missing Default client is missing for opentelemetry-exporter-otlp Apr 8, 2021
@jonatan-ivanov
Copy link
Member Author

jonatan-ivanov commented Jun 8, 2021

Following-up on this: is shading a gRPC client in the otlp-exporter a considerable option?

The advantage would be having an exporter that works out-of-the-box without the need for the users to figure out what version of what client works for them. The disadvantage is having a bigger jar but a client will be needed anyways.

@kenfinnigan
Copy link
Member

For Quarkus, the OTLP exporter extension includes the necessary gRPC dependencies for the user, which are aligned with gRPC versions across Quarkus.

If a gRPC client dependency was added to the OTLP exporter, Quarkus would exclude it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Suggest an idea for this project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants