opentelemetry-java icon indicating copy to clipboard operation
opentelemetry-java copied to clipboard

Refactor http, grpc senders and promote to public API

Open jack-berg opened this issue 2 months ago • 6 comments

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 Marshaler and related APIs, since they balloon the API surface area and would take lot of work to get ready for public. Introducing narrow focused GrpcMessageWriter, HttpRequestBodyWriter to serve the function currently performed by Marshaler.
  • Need to get rid of MarshalerServiceStub and related APIs from the gRPC senders stuff. It drags in a bunch of unnecessary cruft and io.grpc.stub dependency.
  • 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.
  • Compressor and 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 for UpstreamGrpcSender, behind an internal-only ExtendedGrpcSenderConfig

Leaving as a draft because:

  1. there are still some things to clean up
  2. now that i've laid out what's necessary, I want to confirm that we want to do this
  3. 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 avatar Oct 22 '25 20:10 jack-berg

@jack-berg, I didn't forgot this issue. I'm planning to give you a response in the week of Nov. 17.

brunobat avatar Nov 06 '25 17:11 brunobat

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

zeitlinger avatar Nov 24 '25 12:11 zeitlinger

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.

jack-berg avatar Nov 24 '25 22:11 jack-berg

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.

brunobat avatar Nov 25 '25 11:11 brunobat

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.

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.

brunobat avatar Nov 25 '25 11:11 brunobat

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.

jack-berg avatar Nov 25 '25 14:11 jack-berg