open-telemetry
Make the exporter sender a public SPI
Right now we have the exporter sender SPI under private folders: https://github.com/open-telemetry/opentelemetry-java/blob/main/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/ And https://github.com/open-telemetry/opentelemetry-java/blob/main/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/
We should allow frameworks to create their own senders and use an official SPI for it.
Describe the solution you'd like Publicly support the SPI, classes and interfaces under the above packages.
Additional context There are many sender implementations already. One example is Quarkus, which has implemented a Vert.x client based sender. Wildfly will use it as well and other application servers will also have the same need.
I'm not comfortable making these SPIs stable for a couple reasons:
- The API for HttpSender, GrpcSender continue to churn. I'm not confident that we've reached the correct API contract and want to preserve the ability to making breaking changes. This is only possible while the SPIs are kept as an internal implementation detail.
- I don't want to support
OtlpHttp{Signal}Exporter,OtlpGrpc{Signal}Exporterbased on HttpSender, GrpcSenders which are not published by this project. We have limited resource capacity in this project and already maintain 2 HttpSender and 2 GrpcSender implementations. An ecosystem where is possible / popular to bring-your-own sender adds more support burden and complexity since its likely that users won't understand this subtlety and will open issues in this repo. - I don't understand when the built-in senders are insufficient given that there's a zero-dependency java HttpClient based implementation of HttpSender for Java 11+. Users would have to be stuck on java 8 AND be unable to use OkHttp or any of the grpc upstream transports.
Thanks fort the reply, Jack.
- Are there any concrete plans to change this API in any way or is this just an hypothesis?
- If HttpSender, GrpcSenders are not published by this project, you don't have to support them. Have you received any questions about the Vert.x based senders?
- They are insufficient because OkHttp depends on Kotlin, which is a hard stop for most integrators and the JDK HTTP Client is too simplistic. All frameworks have their own optimised clients of choice.
I don't understand when the built-in senders are insufficient given that there's a zero-dependency java HttpClient based implementation of HttpSender for Java 11+. Users would have to be stuck on java 8 AND be unable to use OkHttp or any of the grpc upstream transports.
As the person who wrote the Vert.x senders for Quarkus, I can chime in on this: As Bruno said we did not want OkHttp anywhere on our classpath because it has a dependency on Kotlin. The reason we chose Vert.x instead of some other stack, is because we want our observability stack to have as low overhead as possible on the application. In this specific case, overhead can creep in by loading tons of extra classes (which would otherwise never be loaded) and management of client specific thread pools.
Are there any concrete plans to change this API in any way or is this just an hypothesis?
Check out the history in the sender / providers:
- https://github.com/open-telemetry/opentelemetry-java/commits/main/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcSenderProvider.java
- https://github.com/open-telemetry/opentelemetry-java/commits/main/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcSender.java
- HttpSenderProvider
- https://github.com/open-telemetry/opentelemetry-java/commits/main/exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpSender.java
The churn in the APIs is very real, and could continue to change with additional enhancements for things like authentication providers, retry configurability, and whatever other features users request.
If HttpSender, GrpcSenders are not published by this project, you don't have to support them. Have you received any questions about the Vert.x based senders?
Users won't know the difference. Can't say whether or not the things we've supported are based on Vert.x senders.
They are insufficient because OkHttp depends on Kotlin, which is a hard stop for most integrators and the JDK HTTP Client is too simplistic. All frameworks have their own optimised clients of choice.
"too simplistic" - not a convincing argument for an implementation detail. You can always implement your own SpanExporter, MetricExporter, LogRecordExporter from the ground up. After all, OTLP is a generic protocol and there's no reason that all implementations in java have to be based on the same stack of tooling.
The reason we chose Vert.x instead of some other stack, is because we want our observability stack to have as low overhead as possible on the application. In this specific case, overhead can creep in by loading tons of extra classes (which would otherwise never be loaded) and management of client specific thread pools.
Why not use the built-in JDK HTTP sender? No extra classes required and it appears quarkus already requries java 11 (or 17).
The JDK HTTP client has many drawbacks for our case, here are a few:
- It's inefficient for high throughput loads unlike the reactive Vert.x HTTP client. Requires many additional threads on those cases.
- We would need to include the JDK HTTP client related classes in the builds, just for this, increasing the footprint. Classes that are not used are excluded by the Quarkus build.
- We would need to add additional integrations for that client like certificate, thread, fault tolerance management, and others. All this is already implemented and battle tested for the Vert.x HTTP client.
- We do use GRPC (still default on Quarkus) and HTTP protocols. Doing the 1st with that client is not possible.
No extra classes required and it appears quarkus already requries java 11 (or 17).
Sure, but those are never loaded. And the thread pool issue still remain
the thread pool issue still remain
OpenTelemetry exporters are batched and single-threaded, can you elaborate on what kind of thread pool issue you are seeing?
The JDK client manages its own Thread Pool and that inevitably leads to some overhead
The JDK client manages its own Thread Pool and that inevitably leads to some overhead
JdkHttpSender only sends synchronous requests (i.e. it never calls sendAsync(..)). I assume this means that the calling thread is used to perform the work and no thread pool is created / used. The executor used to execute async requests is configurable, and it looks like if its not configured Executors.newCachedThreadPool(ThreadFactory) is used. I believe this will only create threads if needed, which should be never since we don't use any async methods.
@trask @jack-berg I would appreciate some if this issue is revisited. This is causing us problems.
Just an example of what I'm talking about: During the work of the exporter metrics this class was created and the costructor made package friendly: https://github.com/open-telemetry/opentelemetry-java/blob/056837afdd5c0b94676701f6acb7ac6349a9140e/sdk/common/src/main/java/io/opentelemetry/sdk/internal/StandardComponentId.java#L54
This prevents anyone outside the SDK to create a sender. CC @JonasKunz