Skip to content

Commit 5a11942

Browse files
committed
Propagate Tracer when building TChannel
While workflows still capture the tracing information in the headers, we should pass the Tracer to TChannel so that we correctly trace all RPC requests sent to the Cadence server. Ensure that Tracer can't be set to null and correct the testing library version to avoid classpath conflicts.
1 parent 951d18d commit 5a11942

File tree

5 files changed

+50
-15
lines changed

5 files changed

+50
-15
lines changed

build.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ dependencies {
100100
testCompile group: 'com.googlecode.junit-toolbox', name: 'junit-toolbox', version: '2.4'
101101
testCompile group: 'ch.qos.logback', name: 'logback-classic', version: '1.2.3'
102102
testCompile group: 'io.grpc', name: 'grpc-testing', version: '1.54.2'
103-
testImplementation 'io.opentracing:opentracing-mock:0.33.0'
103+
testImplementation 'io.opentracing:opentracing-mock:0.32.0'
104104
testImplementation group: 'org.mockito', name: 'mockito-core', version: '4.5.1'
105105
testImplementation "org.mockito:mockito-inline:4.5.1" // for mocking final classes
106106
}

src/main/java/com/uber/cadence/serviceclient/ClientOptions.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import io.grpc.ManagedChannel;
2727
import io.opentracing.Tracer;
2828
import io.opentracing.noop.NoopTracerFactory;
29+
import io.opentracing.util.GlobalTracer;
2930
import java.util.Map;
3031

3132
public class ClientOptions {
@@ -130,7 +131,13 @@ private ClientOptions(Builder builder) {
130131
}
131132
this.authProvider = builder.authProvider;
132133
this.isolationGroup = builder.isolationGroup;
133-
this.tracer = builder.tracer;
134+
if (builder.tracer != null) {
135+
this.tracer = builder.tracer;
136+
} else {
137+
// Default value is GlobalTracer. If the user overrides it with null, fall back to NoopTracer.
138+
// We need some tracer instance
139+
this.tracer = NoopTracerFactory.create();
140+
}
134141
}
135142

136143
public static ClientOptions defaultInstance() {
@@ -233,8 +240,8 @@ public static class Builder {
233240
private IAuthorizationProvider authProvider;
234241
private FeatureFlags featureFlags;
235242
private String isolationGroup;
236-
// by default NoopTracer
237-
private Tracer tracer = NoopTracerFactory.create();
243+
// by default GlobalTracer
244+
private Tracer tracer = GlobalTracer.get();
238245

239246
private Builder() {}
240247

src/main/java/com/uber/cadence/serviceclient/WorkflowServiceTChannel.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ public class WorkflowServiceTChannel implements IWorkflowService {
8686
public WorkflowServiceTChannel(ClientOptions options) {
8787
this.options = options;
8888
this.thriftHeaders = getThriftHeaders(options);
89-
this.tChannel = new TChannel.Builder(options.getClientAppName()).build();
89+
this.tChannel =
90+
new TChannel.Builder(options.getClientAppName()).setTracer(options.getTracer()).build();
9091
this.tracingPropagator = new TracingPropagator(options.getTracer());
9192
this.tracer = options.getTracer();
9293

src/test/java/com/uber/cadence/internal/sync/WorkflowClientInternalTest.java

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,18 @@ public void testEnqueueStart_includesTracing() {
116116
stub.enqueueStart("input");
117117

118118
StartWorkflowExecutionAsyncRequest request = requestFuture.getNow(null).getStartRequest();
119-
assertEquals(1, fakeService.getTracer().finishedSpans().size());
120-
MockSpan mockSpan = fakeService.getTracer().finishedSpans().get(0);
119+
MockSpan mockSpan =
120+
fakeService
121+
.getTracer()
122+
.finishedSpans()
123+
.stream()
124+
.filter(span -> "cadence-StartWorkflowExecutionAsync".equals(span.operationName()))
125+
.findFirst()
126+
.orElseThrow(
127+
() ->
128+
new AssertionError(
129+
"No span found for StartWorkflowExecutionAsync:"
130+
+ fakeService.getTracer().finishedSpans()));
121131
assertEquals(
122132
mockSpan.context().toTraceId(),
123133
Charsets.UTF_8
@@ -269,8 +279,21 @@ public void testEnqueueSignalWithStart_includesTracing() {
269279

270280
SignalWithStartWorkflowExecutionRequest request =
271281
requestFuture.getNow(null).getSignalWithStartRequest().getRequest();
272-
assertEquals(1, fakeService.getTracer().finishedSpans().size());
273-
MockSpan mockSpan = fakeService.getTracer().finishedSpans().get(0);
282+
assertEquals(2, fakeService.getTracer().finishedSpans().size());
283+
MockSpan mockSpan =
284+
fakeService
285+
.getTracer()
286+
.finishedSpans()
287+
.stream()
288+
.filter(
289+
span ->
290+
"cadence-SignalWithStartWorkflowExecutionAsync".equals(span.operationName()))
291+
.findFirst()
292+
.orElseThrow(
293+
() ->
294+
new AssertionError(
295+
"No span found for SignalWithStartWorkflowExecutionAsync:"
296+
+ fakeService.getTracer().finishedSpans()));
274297
assertEquals(
275298
mockSpan.context().toTraceId(),
276299
Charsets.UTF_8.decode(request.getHeader().getFields().get("traceid")).toString());

src/test/java/com/uber/cadence/internal/tracing/StartWorkflowTest.java

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,8 @@ public void testSignalWithStartWorkflowGRPC() {
267267
public void testStartWorkflowTchannelNoPropagation() {
268268
Assume.assumeTrue(useDockerService);
269269
MockTracer mockTracer = new MockTracer();
270-
IWorkflowService service = new WorkflowServiceTChannel(ClientOptions.newBuilder().build());
270+
IWorkflowService service =
271+
new WorkflowServiceTChannel(ClientOptions.newBuilder().setTracer(null).build());
271272
testStartWorkflowHelper(service, mockTracer, false);
272273
}
273274

@@ -277,15 +278,17 @@ public void testStartWorkflowGRPCNoPropagation() {
277278
MockTracer mockTracer = new MockTracer();
278279
IWorkflowService service =
279280
new Thrift2ProtoAdapter(
280-
IGrpcServiceStubs.newInstance(ClientOptions.newBuilder().setPort(7833).build()));
281+
IGrpcServiceStubs.newInstance(
282+
ClientOptions.newBuilder().setPort(7833).setTracer(null).build()));
281283
testStartWorkflowHelper(service, mockTracer, false);
282284
}
283285

284286
@Test
285287
public void testSignalStartWorkflowTchannelNoPropagation() {
286288
Assume.assumeTrue(useDockerService);
287289
MockTracer mockTracer = new MockTracer();
288-
IWorkflowService service = new WorkflowServiceTChannel(ClientOptions.newBuilder().build());
290+
IWorkflowService service =
291+
new WorkflowServiceTChannel(ClientOptions.newBuilder().setTracer(null).build());
289292
testSignalWithStartWorkflowHelper(service, mockTracer, false);
290293
}
291294

@@ -295,7 +298,8 @@ public void testSignalStartWorkflowGRPCNoPropagation() {
295298
MockTracer mockTracer = new MockTracer();
296299
IWorkflowService service =
297300
new Thrift2ProtoAdapter(
298-
IGrpcServiceStubs.newInstance(ClientOptions.newBuilder().setPort(7833).build()));
301+
IGrpcServiceStubs.newInstance(
302+
ClientOptions.newBuilder().setPort(7833).setTracer(null).build()));
299303
testSignalWithStartWorkflowHelper(service, mockTracer, false);
300304
}
301305

@@ -360,7 +364,7 @@ private void testStartWorkflowHelper(
360364
MockSpan spanStartWorkflow =
361365
spans
362366
.stream()
363-
.filter(span -> span.operationName().contains("StartWorkflow"))
367+
.filter(span -> span.operationName().contains("cadence-StartWorkflow"))
364368
.findFirst()
365369
.orElse(null);
366370
if (spanStartWorkflow == null) {
@@ -462,7 +466,7 @@ private void testSignalWithStartWorkflowHelper(
462466
MockSpan spanStartWorkflow =
463467
spans
464468
.stream()
465-
.filter(span -> span.operationName().contains("StartWorkflow"))
469+
.filter(span -> span.operationName().contains("cadence-StartWorkflow"))
466470
.findFirst()
467471
.orElse(null);
468472
if (spanStartWorkflow == null) {

0 commit comments

Comments
 (0)