Refactor JaegerRemoteSampler to leverage senders #8046
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jack-berg
commented
Feb 4, 2026
| public String toString() { | ||
| return toString(true); | ||
| } | ||
|
|
Member
Author
There was a problem hiding this comment.
Just moving this code to a more neutral SenderUtil class since its now being used outside GrpcExporterBuilder.
jack-berg
commented
Feb 4, 2026
| import org.junit.jupiter.api.extension.RegisterExtension; | ||
| import org.junitpioneer.jupiter.SetSystemProperty; | ||
|
|
||
| class GrpcExporterTest { |
Member
Author
There was a problem hiding this comment.
Moved to SenderUtilTest
jack-berg
commented
Feb 4, 2026
| return new byte[] {}; // TODO: drain input to byte array | ||
| public byte[] parse(InputStream inputStream) { | ||
| try { | ||
| int bufLen = 4 * 0x400; // 4KB |
Member
Author
There was a problem hiding this comment.
Lifted from the now-deleted MarshallerRemoteSamplerServiceGrpc.
jack-berg
commented
Feb 4, 2026
| })); | ||
| } | ||
|
|
||
| private static byte[] getResponseMessageBytes(byte[] bodyBytes) throws IOException { |
Member
Author
There was a problem hiding this comment.
Lifted from the now-deleted OkHttpGrpcService
Codecov Report❌ Patch coverage is 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. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
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
OkHttpGrpcServiceandUpstreamGrpcServiceclasses with the publicGrpcSenderAPI - Converted
JaegerRemoteSamplerfrom synchronous to asynchronous callback-based communication - Extracted common sender resolution logic into a new
SenderUtilclass shared betweenGrpcExporterBuilderandHttpExporterBuilder
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
Outdated
Show resolved
Hide resolved
...r/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java
Outdated
Show resolved
Hide resolved
exporters/common/src/main/java/io/opentelemetry/exporter/internal/SenderUtil.java
Outdated
Show resolved
Hide resolved
exporters/common/src/main/java/io/opentelemetry/exporter/internal/SenderUtil.java
Outdated
Show resolved
Hide resolved
...r/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java
Show resolved
Hide resolved
...r/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/JaegerRemoteSampler.java
Outdated
Show resolved
Hide resolved
.../okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSender.java
Show resolved
Hide resolved
…y-java into refactor-jaeger-remote-sampler
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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-okhttpinto separate versions for okhttp 4.x and 5.x.