Refactor http, grpc senders and promote to public API #7782

jack-berg · 2025-10-22T20:33:51Z

Resolves #6718.

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 MessageWriter 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

brunobat · 2025-11-06T17:30:06Z

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

brunobat

@jack-berg, thanks very much for this.
The end user changes are fair enough and after updating the Quarkus code, it worked out of the box and the IT tests with a real OTel Collector pass.
I'm adding a couple of comments.

exporters/common/src/main/java/io/opentelemetry/exporter/grpc/GrpcResponse.java

exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java

exporters/common/src/main/java/io/opentelemetry/exporter/http/HttpSenderConfig.java

brunobat

I've taken a look at the declarative config being proposed. We currently don't use it but I have a few comments.

exporters/common/src/main/java/io/opentelemetry/exporter/grpc/GrpcSenderConfig.java

exporters/common/src/main/java/io/opentelemetry/exporter/http/HttpSenderConfig.java

exporters/common/src/main/java/io/opentelemetry/exporter/grpc/GrpcSenderConfig.java

zeitlinger · 2025-11-24T12:42:29Z

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

jack-berg · 2025-11-24T22:27:39Z

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.

brunobat · 2025-11-25T11:14:50Z

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 #6718 and its comments for more details.

brunobat · 2025-11-25T11:23:55Z

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.

jack-berg · 2025-11-25T14:47:13Z

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.

trask

(got through the easy 75/94 files in this first round)

...ler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/OkHttpGrpcService.java

.../okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpHttpSender.java

.../okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSender.java

...o/opentelemetry/exporter/sender/grpc/managedchannel/internal/UpstreamGrpcSenderProvider.java

...n/java/io/opentelemetry/exporter/sender/grpc/managedchannel/internal/UpstreamGrpcSender.java

exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporterBuilder.java

exporters/common/src/main/java/io/opentelemetry/exporter/grpc/GrpcMessageWriter.java

exporters/common/src/main/java/io/opentelemetry/exporter/grpc/GrpcSenderConfig.java

brunobat · 2026-01-08T14:15:54Z

Hi @jack-berg Any news about the split of this work into several PRs?

trask · 2026-01-08T19:02:08Z

I agreed to review this one big PR. Once @jack-berg addresses the first round of comments I'll make the next pass.

jack-berg · 2026-01-08T20:00:08Z

Thanks for the poke @trask - will clean this up, incorporate your first round of fedback, and mark it ready for review.

…y-java into public-sender-api

codecov · 2026-01-08T22:54:08Z

Codecov Report

❌ Patch coverage is 85.71429% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (a15659d) to head (4a4757e).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...orter/sender/okhttp/internal/OkHttpGrpcSender.java	60.86%	8 Missing and 1 partial ⚠️
...ension/trace/jaeger/sampler/OkHttpGrpcService.java	54.54%	3 Missing and 2 partials ⚠️
...pc/managedchannel/internal/UpstreamGrpcSender.java	91.11%	2 Missing and 2 partials ⚠️
...edchannel/internal/UpstreamGrpcSenderProvider.java	71.42%	2 Missing and 2 partials ⚠️
...y/exporter/internal/grpc/MarshalerInputStream.java	57.14%	3 Missing ⚠️
...telemetry/exporter/internal/marshal/Marshaler.java	50.00%	1 Missing and 2 partials ⚠️
...orter/sender/okhttp/internal/OkHttpHttpSender.java	85.00%	3 Missing ⚠️
...io/opentelemetry/exporter/grpc/GrpcStatusCode.java	92.30%	1 Missing and 1 partial ⚠️
...a/io/opentelemetry/exporter/grpc/GrpcResponse.java	0.00%	1 Missing ⚠️
...a/io/opentelemetry/exporter/http/HttpResponse.java	0.00%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #7782   +/-   ##
=========================================
  Coverage     90.12%   90.12%           
- Complexity     7468     7481   +13     
=========================================
  Files           834      834           
  Lines         22589    22601   +12     
  Branches       2239     2250   +11     
=========================================
+ Hits          20359    20370   +11     
+ Misses         1529     1527    -2     
- Partials        701      704    +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jack-berg · 2026-01-08T23:06:54Z

Ready for review. Updated the description to match the current state of things.

trask

(just the next round)

trask · 2026-01-08T23:16:29Z

...ters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/ImmutableGrpcResponse.java

+public abstract class ImmutableGrpcResponse implements GrpcResponse {
+
+  public static ImmutableGrpcResponse create(
+      GrpcStatusCode grpcStatusCode, @Nullable String grpcStatusDescription, byte[] responseBody) {


(just to match the accessors)

Suggested change

GrpcStatusCode grpcStatusCode, @Nullable String grpcStatusDescription, byte[] responseBody) {

GrpcStatusCode statusCode, @Nullable String statusDescription, byte[] responseMessage) {

trask · 2026-01-08T23:22:29Z

exporters/common/src/main/java/io/opentelemetry/exporter/marshal/MessageWriter.java

+  /** Write the message to the {@link OutputStream}. */
+  void writeMessage(OutputStream output) throws IOException;
+
+  /** Return the message length in bytes. */


something about returning -1 in some cases?

trask · 2026-01-08T23:23:45Z

...n/java/io/opentelemetry/exporter/sender/grpc/managedchannel/internal/UpstreamGrpcSender.java

+    this.methodDescriptor =
+        MethodDescriptor.<MessageWriter, byte[]>newBuilder()
+            .setType(MethodDescriptor.MethodType.UNARY)
+            .setFullMethodName(serviceAndMethodName)


wdyt of renaming serviceAndMethodName everywhere to fullMethodName? (fwiw grpc defines grpc.method attribute as the full method name: https://github.com/grpc/proposal/blob/master/A66-otel-stats.md#attributes)

trask · 2026-01-08T23:48:57Z

.../io/opentelemetry/sdk/extension/trace/jaeger/sampler/MarshallerRemoteSamplerServiceGrpc.java

            @Override
            public InputStream stream(SamplingStrategyParametersMarshaler value) {
-              return new MarshalerInputStream(value);
+              return new MarshalerInputStream(value.toMessageWriter(false));


maybe worth toJsonMessageWriter to avoid the boolean flag? realize it's internal so not important (not even sure if it's a good idea)

trask · 2026-01-08T23:52:36Z

...src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSenderProvider.java

+    return new OkHttpGrpcSender(
+        grpcSenderConfig
+            .getEndpoint()
+            .resolve("/" + grpcSenderConfig.getServiceAndMethodName())


I don't think the / should be needed

Suggested change

.resolve("/" + grpcSenderConfig.getServiceAndMethodName())

.resolve(grpcSenderConfig.getServiceAndMethodName())

The build was falling without it! Couldn't recreate it locally. Need to dig into it more.

Refactor http, grpc senders and promote to public API

b4ef764

brunobat reviewed Nov 20, 2025

View reviewed changes

exporters/common/src/main/java/io/opentelemetry/exporter/grpc/GrpcResponse.java Outdated Show resolved Hide resolved

exporters/common/src/main/java/io/opentelemetry/exporter/internal/http/HttpExporter.java Show resolved Hide resolved

brunobat mentioned this pull request Nov 20, 2025

Use HttpSender and GrpcSender from upstream OpenTelemetry API quarkusio/quarkus#51143

Draft

tylerbenson mentioned this pull request Nov 20, 2025

Make the disk buffered exporting more efficient open-telemetry/opentelemetry-java-contrib#2190

Closed

trask reviewed Nov 20, 2025

View reviewed changes

exporters/common/src/main/java/io/opentelemetry/exporter/http/HttpSenderConfig.java Show resolved Hide resolved

brunobat reviewed Nov 24, 2025

View reviewed changes

trask reviewed Dec 18, 2025

View reviewed changes

jack-berg added 4 commits January 8, 2026 14:37

PR feedback

fd565bb

Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…

ee21659

…y-java into public-sender-api

Common MessageWriter abstraction

5336b6b

Read response before accessing trailers

b8cf047

Fix build

4a4757e

jack-berg marked this pull request as ready for review January 8, 2026 23:05

jack-berg requested a review from a team as a code owner January 8, 2026 23:05

trask reviewed Jan 8, 2026

View reviewed changes

	GrpcStatusCode grpcStatusCode, @Nullable String grpcStatusDescription, byte[] responseBody) {
	GrpcStatusCode statusCode, @Nullable String statusDescription, byte[] responseMessage) {

	.resolve("/" + grpcSenderConfig.getServiceAndMethodName())
	.resolve(grpcSenderConfig.getServiceAndMethodName())

Refactor http, grpc senders and promote to public API #7782

Are you sure you want to change the base?

Refactor http, grpc senders and promote to public API #7782

Uh oh!

Conversation

jack-berg commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brunobat commented Nov 6, 2025

Uh oh!

brunobat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brunobat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zeitlinger commented Nov 24, 2025

Uh oh!

jack-berg commented Nov 24, 2025

Uh oh!

brunobat commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brunobat commented Nov 25, 2025

Uh oh!

jack-berg commented Nov 25, 2025

Uh oh!

trask left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brunobat commented Jan 8, 2026

Uh oh!

trask commented Jan 8, 2026

Uh oh!

jack-berg commented Jan 8, 2026

Uh oh!

codecov bot commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jack-berg commented Jan 8, 2026

Uh oh!

trask left a comment

Choose a reason for hiding this comment

Uh oh!

trask Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

trask Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

trask Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

trask Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

trask Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

jack-berg commented Oct 22, 2025 •

edited

Loading

brunobat commented Nov 25, 2025 •

edited

Loading

trask left a comment •

edited

Loading

codecov bot commented Jan 8, 2026 •

edited

Loading