Refactor http, grpc senders and promote to public API
Resolves #6718.
Sketching out all the changes I think would be required to promote the HttpSender, GrpcSender interfaces to the public API. There's a lot. Summary:
- We need to hide
Marshalerand related APIs, since they balloon the API surface area and would take lot of work to get ready for public. Introducing narrow focusedGrpcMessageWriter,HttpRequestBodyWriterto serve the function currently performed byMarshaler. - Need to get rid of
MarshalerServiceStuband related APIs from the gRPC senders stuff. It drags in a bunch of unnecessary cruft andio.grpc.stubdependency. - Need to hide the
HttpSenderConfig#getExportAsJson()option. Senders don't need to be burdened with understanding whether the request is binary or JSON. They just need to be told the content type and a way to write the request body. -
Compressorand related APIs need to get promoted as well. - Need to refine
GrpcSenderConfig,HttpSenderConfig- Need to be interfaces instead of autovalue classes
- Need javadoc
- Make endpoint URI instead of string
- Provide grpc senders the fully qualified service name and method name
- Hide the
Object GrpcSenderConfig#getManagedChannel(), which is required for backwards compatibility and forUpstreamGrpcSender, behind an internal-onlyExtendedGrpcSenderConfig
Leaving as a draft because:
- there are still some things to clean up
- now that i've laid out what's necessary, I want to confirm that we want to do this
- if we do indeed want this, I want to discuss the best way to land it. 1500 lines changes across 94 files is obviously a lot to review. I can break it into smaller parts, but we'd want to get those parts merged in one minor release to minimize churn.
@jack-berg, I didn't forgot this issue. I'm planning to give you a response in the week of Nov. 17.
The end user changes are fair enough and after updating the Quarkus code
Why is this relavant for Quarkus? I'm asking because the spring starter doesn't care - and I'm trying to understand what the difference is
I've taken a look at the declarative config being proposed.
There's actually nothing in here related to declarative config. The getters in HttpSenderConfig, GrpcSenderConfig reflect the standard configuration options we expose via programmatic config for OTLP exporters (e.g. OtlpHttpSpanExporterBuilder, OtlpGrpcSpanExporterBuilder).
As the set of configuration options we expose via the Otlp{Signal}{Protocol}ExporterBuilders evolves, HttpSenderConfig and GrpcSenderConfig will evolve along side.
I would say that if a sender ignores any of those options, its technically not compliant. Non-compliant doesn't mean its not useful, but it does mean that the sender isn't able to behave like the user expects based on how they configured the builder.
The end user changes are fair enough and after updating the Quarkus code
Why is this relavant for Quarkus? I'm asking because the spring starter doesn't care - and I'm trying to understand what the difference is
Because Quarkus has it's own senders based on a Vert.x client. It does not use OkHttp. See https://github.com/open-telemetry/opentelemetry-java/issues/6718 and its comments for more details.
I've taken a look at the declarative config being proposed.
There's actually nothing in here related to declarative config. The getters in
HttpSenderConfig,GrpcSenderConfigreflect the standard configuration options we expose via programmatic config for OTLP exporters (e.g. OtlpHttpSpanExporterBuilder, OtlpGrpcSpanExporterBuilder).As the set of configuration options we expose via the
Otlp{Signal}{Protocol}ExporterBuilders evolves,HttpSenderConfigandGrpcSenderConfigwill evolve along side.I would say that if a sender ignores any of those options, its technically not compliant. Non-compliant doesn't mean its not useful, but it does mean that the sender isn't able to behave like the user expects based on how they configured the builder.
Thanks for the explanation @jack-berg. If those properties are not used, the compliance is left to the implementator. As in the other properties, we always try to match the property behavior, even if properties are handled by Quarkus and some behavior is not implemented directly on the SDK. We have other examples with samplers and several customizations we provide for the SDK. The Idea is to melt the OTel experience into Quarkus and have minimal friction for people with knowledge of both frameworks.
Ok so it seems like something close to this will work for quarkus. There are still some small tweaks (example), but its mostly correct.
The next step will be to coordinate with @jkwatson and @open-telemetry/java-approvers to choose a strategy and schedule to get this merged. I opened this as one big PR to make the whole picture clear, but I'm happy to break it up in smaller more reviewable pieces as long as we can commit to work quickly to get all those pieces merged for a single release. This will be important to minimize churn.