Skip to content

Propagate Tracer when building TChannel #1042

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
natemort merged 1 commit into from
Dec 18, 2025
+87 −37
Merged

Propagate Tracer when building TChannel #1042

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ dependencies {
testCompile group: 'com.googlecode.junit-toolbox', name: 'junit-toolbox', version: '2.4'
testCompile group: 'ch.qos.logback', name: 'logback-classic', version: '1.2.3'
testCompile group: 'io.grpc', name: 'grpc-testing', version: '1.54.2'
testImplementation 'io.opentracing:opentracing-mock:0.33.0'
testImplementation 'io.opentracing:opentracing-mock:0.32.0'
Copy link
Contributor

@abhishekj720 abhishekj720 Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why opentracing-mock is pinned to 0.32.0 (what conflict it avoids)?

Copy link
Member Author

@natemort natemort Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TChannel version we're using at runtime uses the opentracing 0.32.0 core libraries. This test dependency ends up overwriting that, making our tests run against opentracing 0.33.0 core.

There was a breaking change in opentracing, so the tests all fail after the Tracer is added because the method they call no longer exists.

This change corrects the test version to match what we actually run against. It's a test dependency of the java client, so it doesn't impact users at all.

testImplementation group: 'org.mockito', name: 'mockito-core', version: '4.5.1'
testImplementation "org.mockito:mockito-inline:4.5.1" // for mocking final classes
}
Expand Down
13 changes: 10 additions & 3 deletions src/main/java/com/uber/cadence/serviceclient/ClientOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import io.grpc.ManagedChannel;
import io.opentracing.Tracer;
import io.opentracing.noop.NoopTracerFactory;
import io.opentracing.util.GlobalTracer;
import java.util.Map;

public class ClientOptions {
Expand Down Expand Up @@ -130,7 +131,13 @@ private ClientOptions(Builder builder) {
}
this.authProvider = builder.authProvider;
this.isolationGroup = builder.isolationGroup;
this.tracer = builder.tracer;
if (builder.tracer != null) {
this.tracer = builder.tracer;
} else {
// Default value is GlobalTracer. If the user overrides it with null, fall back to NoopTracer.
// We need some tracer instance
this.tracer = NoopTracerFactory.create();
}
}

public static ClientOptions defaultInstance() {
Expand Down Expand Up @@ -233,8 +240,8 @@ public static class Builder {
private IAuthorizationProvider authProvider;
private FeatureFlags featureFlags;
private String isolationGroup;
// by default NoopTracer
private Tracer tracer = NoopTracerFactory.create();
// by default GlobalTracer
private Tracer tracer = GlobalTracer.get();

private Builder() {}

Expand Down
3 changes: 2 additions & 1 deletion src/main/java/com/uber/cadence/serviceclient/WorkflowServiceTChannel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ public class WorkflowServiceTChannel implements IWorkflowService {
public WorkflowServiceTChannel(ClientOptions options) {
this.options = options;
this.thriftHeaders = getThriftHeaders(options);
this.tChannel = new TChannel.Builder(options.getClientAppName()).build();
this.tChannel =
new TChannel.Builder(options.getClientAppName()).setTracer(options.getTracer()).build();
this.tracingPropagator = new TracingPropagator(options.getTracer());
this.tracer = options.getTracer();

Expand Down
31 changes: 27 additions & 4 deletions src/test/java/com/uber/cadence/internal/sync/WorkflowClientInternalTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,18 @@ public void testEnqueueStart_includesTracing() {
stub.enqueueStart("input");

StartWorkflowExecutionAsyncRequest request = requestFuture.getNow(null).getStartRequest();
assertEquals(1, fakeService.getTracer().finishedSpans().size());
MockSpan mockSpan = fakeService.getTracer().finishedSpans().get(0);
MockSpan mockSpan =
fakeService
.getTracer()
.finishedSpans()
.stream()
.filter(span -> "cadence-StartWorkflowExecutionAsync".equals(span.operationName()))
.findFirst()
.orElseThrow(
() ->
new AssertionError(
"No span found for StartWorkflowExecutionAsync:"
+ fakeService.getTracer().finishedSpans()));
assertEquals(
mockSpan.context().toTraceId(),
Charsets.UTF_8
Expand Down Expand Up @@ -269,8 +279,21 @@ public void testEnqueueSignalWithStart_includesTracing() {

SignalWithStartWorkflowExecutionRequest request =
requestFuture.getNow(null).getSignalWithStartRequest().getRequest();
assertEquals(1, fakeService.getTracer().finishedSpans().size());
MockSpan mockSpan = fakeService.getTracer().finishedSpans().get(0);
assertEquals(2, fakeService.getTracer().finishedSpans().size());
MockSpan mockSpan =
fakeService
.getTracer()
.finishedSpans()
.stream()
.filter(
span ->
"cadence-SignalWithStartWorkflowExecutionAsync".equals(span.operationName()))
.findFirst()
.orElseThrow(
() ->
new AssertionError(
"No span found for SignalWithStartWorkflowExecutionAsync:"
+ fakeService.getTracer().finishedSpans()));
assertEquals(
mockSpan.context().toTraceId(),
Charsets.UTF_8.decode(request.getHeader().getFields().get("traceid")).toString());
Expand Down
75 changes: 47 additions & 28 deletions src/test/java/com/uber/cadence/internal/tracing/StartWorkflowTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,8 @@ public void testSignalWithStartWorkflowGRPC() {
public void testStartWorkflowTchannelNoPropagation() {
Assume.assumeTrue(useDockerService);
MockTracer mockTracer = new MockTracer();
IWorkflowService service = new WorkflowServiceTChannel(ClientOptions.newBuilder().build());
IWorkflowService service =
new WorkflowServiceTChannel(ClientOptions.newBuilder().setTracer(null).build());
testStartWorkflowHelper(service, mockTracer, false);
}

Expand All @@ -277,15 +278,17 @@ public void testStartWorkflowGRPCNoPropagation() {
MockTracer mockTracer = new MockTracer();
IWorkflowService service =
new Thrift2ProtoAdapter(
IGrpcServiceStubs.newInstance(ClientOptions.newBuilder().setPort(7833).build()));
IGrpcServiceStubs.newInstance(
ClientOptions.newBuilder().setPort(7833).setTracer(null).build()));
testStartWorkflowHelper(service, mockTracer, false);
}

@Test
public void testSignalStartWorkflowTchannelNoPropagation() {
Assume.assumeTrue(useDockerService);
MockTracer mockTracer = new MockTracer();
IWorkflowService service = new WorkflowServiceTChannel(ClientOptions.newBuilder().build());
IWorkflowService service =
new WorkflowServiceTChannel(ClientOptions.newBuilder().setTracer(null).build());
testSignalWithStartWorkflowHelper(service, mockTracer, false);
}

Expand All @@ -295,7 +298,8 @@ public void testSignalStartWorkflowGRPCNoPropagation() {
MockTracer mockTracer = new MockTracer();
IWorkflowService service =
new Thrift2ProtoAdapter(
IGrpcServiceStubs.newInstance(ClientOptions.newBuilder().setPort(7833).build()));
IGrpcServiceStubs.newInstance(
ClientOptions.newBuilder().setPort(7833).setTracer(null).build()));
testSignalWithStartWorkflowHelper(service, mockTracer, false);
}

Expand Down Expand Up @@ -330,7 +334,7 @@ private void testStartWorkflowHelper(
int res = wf.AddOneThenDouble(3);
assertEquals(8, res);
} catch (Exception e) {
fail("workflow failure: " + e);
throw new AssertionError("workflow failure", e);
} finally {
rootSpan.finish();
List<MockSpan> spans = mockTracer.finishedSpans();
Expand Down Expand Up @@ -358,19 +362,11 @@ private void testStartWorkflowHelper(
} else {
// assert start workflow
MockSpan spanStartWorkflow =
spans
.stream()
.filter(span -> span.operationName().contains("StartWorkflow"))
.findFirst()
.orElse(null);
if (spanStartWorkflow == null) {
fail("StartWorkflow span not found");
}
getFirstSpanByOperationName(spans, "cadence-StartWorkflowExecution");

// assert workflow spans
MockSpan spanExecuteWF = getLinkedSpans(spans, spanStartWorkflow.context()).get(0);
assertEquals(spanExecuteWF.operationName(), "cadence-ExecuteWorkflow");
assertSpanReferences(spanExecuteWF, "follows_from", spanStartWorkflow);
MockSpan spanExecuteWF = getFirstSpanByOperationName(spans, "cadence-ExecuteWorkflow");
assertChildOf(spans, spanExecuteWF, spanStartWorkflow);

MockSpan spanExecuteActivity = getLinkedSpans(spans, spanExecuteWF.context()).get(0);
assertEquals(spanExecuteActivity.operationName(), "cadence-ExecuteActivity");
Expand Down Expand Up @@ -432,7 +428,7 @@ private void testSignalWithStartWorkflowHelper(
int res = wf.getResult(Integer.class);
assertEquals(8, res);
} catch (Exception e) {
fail("workflow failure: " + e);
throw new AssertionError("Workflow failure", e);
} finally {
rootSpan.finish();
List<MockSpan> spans = mockTracer.finishedSpans();
Expand Down Expand Up @@ -460,21 +456,14 @@ private void testSignalWithStartWorkflowHelper(
} else {
// assert start workflow
MockSpan spanStartWorkflow =
spans
.stream()
.filter(span -> span.operationName().contains("StartWorkflow"))
.findFirst()
.orElse(null);
if (spanStartWorkflow == null) {
fail("StartWorkflow span not found");
}
getFirstSpanByOperationName(spans, "cadence-SignalWithStartWorkflowExecution");

// assert workflow spans
List<MockSpan> workflowSpans =
getSpansByTraceID(spans, spanStartWorkflow.context().toTraceId());
MockSpan spanExecuteWF = getLinkedSpans(spans, spanStartWorkflow.context()).get(0);
assertEquals(spanExecuteWF.operationName(), "cadence-ExecuteWorkflow");
assertSpanReferences(spanExecuteWF, "follows_from", spanStartWorkflow);
MockSpan spanExecuteWF =
getFirstSpanByOperationName(workflowSpans, "cadence-ExecuteWorkflow");
assertChildOf(spans, spanExecuteWF, spanStartWorkflow);

MockSpan spanExecuteActivity = getLinkedSpans(spans, spanExecuteWF.context()).get(0);
assertEquals(spanExecuteActivity.operationName(), "cadence-ExecuteActivity");
Expand All @@ -493,6 +482,15 @@ private void testSignalWithStartWorkflowHelper(
}
}

private MockSpan getFirstSpanByOperationName(List<MockSpan> spans, String operation) {
return spans
.stream()
.filter(span -> span.operationName().equals(operation))
.findFirst()
.orElseThrow(
() -> new IllegalStateException("Failed to find span with operation: " + operation));
}

private List<MockSpan> getSpansByTraceID(List<MockSpan> spans, String traceID) {
return spans
.stream()
Expand All @@ -517,6 +515,27 @@ private List<MockSpan> getLinkedSpans(List<MockSpan> spans, SpanContext spanCont
.collect(Collectors.toList());
}

void assertChildOf(List<MockSpan> spans, MockSpan span, MockSpan parentSpan) {
Map<String, MockSpan> byId = new HashMap<>();
for (MockSpan s : spans) {
byId.put(s.context().traceId() + "-" + s.context().toSpanId(), s);
}
Queue<MockSpan> toVisit = new LinkedList<>();
toVisit.add(span);
while (!toVisit.isEmpty()) {
MockSpan current = toVisit.poll();
MockSpan parent = byId.get(current.context().traceId() + "-" + current.parentId());
if (parent != null) {
if (parent.equals(parentSpan)) {
return;
} else {
toVisit.add(parent);
}
}
}
fail(String.format("span %s is not a child of parent span %s", span, parentSpan));
}

void assertSpanReferences(MockSpan span, String referenceType, MockSpan parentSpan) {
assertTrue(
String.format(
Expand Down
Loading