Refactor JaegerRemoteSampler to leverage senders by jack-berg · Pull Request #8046 · open-telemetry/opentelemetry-java

jack-berg · 2026-02-04T22:57:45Z

Contributes to #8001 by refactoring JaegerRemoteSampler to be built on top of the newly public sender APIs.

This simplifies the code, and allows JaegerRemoteSampler to be able to automatically take advantage of breaking out opentelemetry-exporter-sender-okhttp into separate versions for okhttp 4.x and 5.x.

jack-berg · 2026-02-04T22:58:26Z

exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java

  public String toString() {
    return toString(true);
  }
-


Just moving this code to a more neutral SenderUtil class since its now being used outside GrpcExporterBuilder.

jack-berg · 2026-02-04T22:58:48Z

...rc/testGrpcSenderProvider/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterTest.java

-import org.junit.jupiter.api.extension.RegisterExtension;
-import org.junitpioneer.jupiter.SetSystemProperty;
-
-class GrpcExporterTest {


Moved to SenderUtilTest

jack-berg · 2026-02-04T22:59:48Z

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

-          return new byte[] {}; // TODO: drain input to byte array
+        public byte[] parse(InputStream inputStream) {
+          try {
+            int bufLen = 4 * 0x400; // 4KB


Lifted from the now-deleted MarshallerRemoteSamplerServiceGrpc.

jack-berg · 2026-02-04T23:00:27Z

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

                    }));
  }

+  private static byte[] getResponseMessageBytes(byte[] bodyBytes) throws IOException {


Lifted from the now-deleted OkHttpGrpcService

codecov · 2026-02-04T23:07:51Z

Codecov Report

❌ Patch coverage is 84.16667% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.26%. Comparing base (fff95e0) to head (e876071).
⚠️ Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
...pc/managedchannel/internal/UpstreamGrpcSender.java	44.44%	9 Missing and 1 partial ⚠️
...orter/sender/okhttp/internal/OkHttpGrpcSender.java	57.14%	5 Missing and 1 partial ⚠️
...sion/trace/jaeger/sampler/JaegerRemoteSampler.java	90.62%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8046      +/-   ##
============================================
+ Coverage     90.20%   90.26%   +0.06%     
+ Complexity     7592     7565      -27     
============================================
  Files           841      839       -2     
  Lines         22911    22787     -124     
  Branches       2288     2271      -17     
============================================
- Hits          20666    20569      -97     
+ Misses         1529     1510      -19     
+ Partials        716      708       -8

☔ 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.

Copilot

Pull request overview

This pull request refactors JaegerRemoteSampler to use the public sender APIs (GrpcSender and GrpcSenderProvider) instead of maintaining its own internal HTTP client implementations. This change addresses issue #8001 regarding OkHttp compatibility, allowing the sampler to automatically benefit from separate versions of opentelemetry-exporter-sender-okhttp for OkHttp 4.x and 5.x.

Changes:

Replaced internal OkHttpGrpcService and UpstreamGrpcService classes with the public GrpcSender API
Converted JaegerRemoteSampler from synchronous to asynchronous callback-based communication
Extracted common sender resolution logic into a new SenderUtil class shared between GrpcExporterBuilder and HttpExporterBuilder

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`JaegerRemoteSampler.java`	Refactored to use `GrpcSender` with async callbacks instead of synchronous `GrpcService` execution
`JaegerRemoteSamplerBuilder.java`	Updated to resolve and create `GrpcSender` using `GrpcSenderProvider` instead of building custom HTTP clients
`OkHttpGrpcService.java`	Deleted - functionality moved to `OkHttpGrpcSender`
`UpstreamGrpcService.java`	Deleted - functionality exists in `UpstreamGrpcSender`
`GrpcService.java`	Deleted - replaced by public `GrpcSender` interface
`MarshallerRemoteSamplerServiceGrpc.java`	Deleted - no longer needed with sender abstraction
`SamplingStrategyResponseUnMarshaler.java`	Simplified to static `read()` method, removing stateful instance pattern
`OkHttpGrpcSender.java`	Added response message extraction logic from deleted `OkHttpGrpcService`
`UpstreamGrpcSender.java`	Added input stream parsing implementation for response messages
`SenderUtil.java`	New utility class consolidating sender provider resolution logic
`GrpcExporterBuilder.java`	Refactored to delegate sender resolution to `SenderUtil`
`HttpExporterBuilder.java`	Refactored to delegate sender resolution to `SenderUtil`
`ManagedChannelUtil.java`	Removed `shutdownChannel()` method (now handled by senders)
`ImmutableGrpcSenderConfig.java`	Made public to support external usage by `JaegerRemoteSamplerBuilder`
`build.gradle.kts` (jaeger-remote-sampler)	Removed direct OkHttp dependency, added test configuration for GrpcSenderProvider
`build.gradle.kts` (exporters/common)	Reorganized test suites to consolidate sender provider tests
Test files	Updated to reflect field name changes (`delegate` → `grpcSender`) and use `GrpcStatusCode` enum

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...r/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java

exporters/common/src/main/java/io/opentelemetry/exporter/internal/SenderUtil.java

...r/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java

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

…y-java into refactor-jaeger-remote-sampler

trask

nice

trask · 2026-02-15T20:04:25Z

exporters/common/src/main/java/io/opentelemetry/exporter/internal/SenderUtil.java

+    // TODO: remove support for reading OLD_SPI_PROPERTY after 1.61.0
+    if (configuredSender.isEmpty()) {
+      configuredSender = ConfigUtil.getString(OLD_GRPC_SPI_PROPERTY, "");
+      if (!configuredSender.isEmpty()) {


ah, an unrelated fix here 👍

trask · 2026-02-15T20:09:47Z

...r/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java


  private static final String WORKER_THREAD_NAME =
      JaegerRemoteSampler.class.getSimpleName() + "_WorkerThread";
+  private static final String type = "remoteSampling";


Suggested change

private static final String type = "remoteSampling";

private static final String TYPE = "remoteSampling";

trask · 2026-02-15T20:10:02Z

...r/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java

  }

  private void getAndUpdateSampler() {
+    SamplingStrategyParametersMarshaler marsher =


Suggested change

SamplingStrategyParametersMarshaler marsher =

SamplingStrategyParametersMarshaler marshaler =

trask · 2026-02-15T20:13:36Z

...r/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java

+    if (logger.isLoggable(Level.FINEST)) {
+      logger.log(Level.FINEST, "Failed to execute " + type + "s. Details follow: " + e);
+    }


(or did you want to not log the stack trace above at SEVERE?)

Suggested change

if (logger.isLoggable(Level.FINEST)) {

logger.log(Level.FINEST, "Failed to execute " + type + "s. Details follow: " + e);

}

trask · 2026-02-15T20:48:31Z

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

                          byte[] bodyBytes = null;
                          try {
-                            bodyBytes = body.bytes();
+                            bodyBytes = getResponseMessageBytes(body.bytes());


doc that GrpcResponse.getResponseMessage() should return protobuf bytes unwrapped from grpc framing?

trask · 2026-02-15T20:49:55Z

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

+    if (bodyBytes.length > 5) {
+      ByteArrayInputStream bodyStream = new ByteArrayInputStream(bodyBytes);
+      bodyStream.skip(5);
+      if (bodyBytes[0] == '1') {


(at least this is what AI tells me, and it seems believable, probably we need a compressed response test)

Suggested change

if (bodyBytes[0] == '1') {

if (bodyBytes[0] == 1) {

trask · 2026-02-15T20:50:09Z

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

+        Buffer buffer = new Buffer();
+        buffer.readFrom(bodyStream);
+        GzipSource gzipSource = new GzipSource(buffer);
+        bodyBytes = Okio.buffer(gzipSource).getBuffer().readByteArray();


Suggested change

bodyBytes = Okio.buffer(gzipSource).getBuffer().readByteArray();

bodyBytes = Okio.buffer(gzipSource).readByteArray();

Refactor JaegerRemoteSampler to leverage senders

a5f73ba

jack-berg requested a review from a team as a code owner February 4, 2026 22:57

jack-berg commented Feb 4, 2026

View reviewed changes

jack-berg requested a review from Copilot February 6, 2026 14:11

Copilot started reviewing on behalf of jack-berg February 6, 2026 14:12 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

jack-berg added 2 commits February 11, 2026 09:21

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

e360ddf

…y-java into refactor-jaeger-remote-sampler

PR feedback

e876071

jack-berg mentioned this pull request Feb 13, 2026

Split out okhttp4 sender #8086

Draft

trask reviewed Feb 15, 2026

View reviewed changes

	private static final String type = "remoteSampling";
	private static final String TYPE = "remoteSampling";

	SamplingStrategyParametersMarshaler marsher =
	SamplingStrategyParametersMarshaler marshaler =

	if (logger.isLoggable(Level.FINEST)) {
	logger.log(Level.FINEST, "Failed to execute " + type + "s. Details follow: " + e);
	}

	bodyBytes = Okio.buffer(gzipSource).getBuffer().readByteArray();
	bodyBytes = Okio.buffer(gzipSource).readByteArray();

Conversation

jack-berg commented Feb 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

trask left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Feb 4, 2026 •

edited

Loading