Skip to content

Commit 7443c37

Browse files
committed
fix: set OperationType on traced callables context
1 parent af531a4 commit 7443c37

8 files changed

Lines changed: 26 additions & 18 deletions

File tree

gax-java/gax/src/main/java/com/google/api/gax/tracing/MetricsTracerFactory.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131

3232
import com.google.api.core.BetaApi;
3333
import com.google.api.core.InternalApi;
34+
import com.google.common.base.Preconditions;
3435
import com.google.common.collect.ImmutableMap;
3536
import java.util.Map;
3637

@@ -66,6 +67,7 @@ public MetricsTracerFactory(MetricsRecorder metricsRecorder, Map<String, String>
6667

6768
@Override
6869
public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) {
70+
Preconditions.checkNotNull(operationType, "operationType cannot be null.");
6971
MetricsTracer metricsTracer =
7072
new MetricsTracer(
7173
MethodName.of(spanName.getClientName(), spanName.getMethodName()), metricsRecorder);

gax-java/gax/src/main/java/com/google/api/gax/tracing/OpencensusTracerFactory.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ public OpencensusTracerFactory(Map<String, String> spanAttributes) {
9696
/** {@inheritDoc } */
9797
@Override
9898
public ApiTracer newTracer(ApiTracer parent, SpanName spanName, OperationType operationType) {
99+
Preconditions.checkNotNull(operationType, "operationType cannot be null.");
99100
// Default to the current in context span. This is used for outermost tracers that inherit
100101
// the caller's parent span.
101102
Span parentSpan = internalTracer.getCurrentSpan();

gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedBatchingCallable.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ public TracedBatchingCallable(
7373
ApiTracerContext apiTracerContext,
7474
BatchingDescriptor<RequestT, ResponseT> batchingDescriptor) {
7575
this.tracerFactory = tracerFactory;
76-
this.apiTracerContext = apiTracerContext;
76+
this.apiTracerContext =
77+
apiTracerContext.toBuilder().setOperationType(OperationType.Batching).build();
7778
this.batchingDescriptor = batchingDescriptor;
7879
this.innerCallable = innerCallable;
7980
this.spanName = SpanName.of(apiTracerContext);

gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedBidiCallable.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ public TracedBidiCallable(
7474
@Nonnull ApiTracerContext apiTracerContext) {
7575
this.tracerFactory = Preconditions.checkNotNull(tracerFactory, "tracerFactory can't be null");
7676
this.apiTracerContext =
77-
Preconditions.checkNotNull(apiTracerContext, "apiTracerContext can't be null");
77+
Preconditions.checkNotNull(apiTracerContext, "apiTracerContext can't be null").toBuilder()
78+
.setOperationType(OperationType.BidiStreaming)
79+
.build();
7880
this.innerCallable = Preconditions.checkNotNull(innerCallable, "innerCallable can't be null");
7981
this.spanName = SpanName.of(this.apiTracerContext);
8082
}

gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedClientStreamingCallable.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ public TracedClientStreamingCallable(
7474
@Nonnull ApiTracerContext apiTracerContext) {
7575
this.tracerFactory = Preconditions.checkNotNull(tracerFactory, "tracerFactory can't be null");
7676
this.apiTracerContext =
77-
Preconditions.checkNotNull(apiTracerContext, "apiTracerContext can't be null");
77+
Preconditions.checkNotNull(apiTracerContext, "apiTracerContext can't be null").toBuilder()
78+
.setOperationType(OperationType.ClientStreaming)
79+
.build();
7880
this.innerCallable = Preconditions.checkNotNull(innerCallable, "innerCallable can't be null");
7981
this.spanName = SpanName.of(this.apiTracerContext);
8082
}

gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedServerStreamingCallable.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,9 @@ public TracedServerStreamingCallable(
7070
@Nonnull ApiTracerContext apiTracerContext) {
7171
this.tracerFactory = Preconditions.checkNotNull(tracerFactory, "tracerFactory can't be null");
7272
this.apiTracerContext =
73-
Preconditions.checkNotNull(apiTracerContext, "apiTracerContext can't be null");
73+
Preconditions.checkNotNull(apiTracerContext, "apiTracerContext can't be null").toBuilder()
74+
.setOperationType(OperationType.ClientStreaming)
75+
.build();
7476
this.innerCallable = Preconditions.checkNotNull(innerCallable, "innerCallable can't be null");
7577
this.spanName = SpanName.of(this.apiTracerContext);
7678
}

gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedUnaryCallable.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ public TracedUnaryCallable(
6868
ApiTracerContext apiTracerContext) {
6969
this.innerCallable = innerCallable;
7070
this.tracerFactory = tracerFactory;
71-
this.apiTracerContext = apiTracerContext;
71+
this.apiTracerContext =
72+
apiTracerContext.toBuilder().setOperationType(OperationType.Unary).build();
7273
this.spanName = SpanName.of(apiTracerContext);
7374
}
7475

gax-java/gax/src/test/java/com/google/api/gax/tracing/TracedServerStreamingCallableTest.java

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040

4141
import com.google.api.gax.rpc.ApiCallContext;
4242
import com.google.api.gax.rpc.CancelledException;
43-
import com.google.api.gax.rpc.LibraryMetadata;
4443
import com.google.api.gax.rpc.ResponseObserver;
4544
import com.google.api.gax.rpc.ServerStreamingCallable;
4645
import com.google.api.gax.rpc.StatusCode.Code;
@@ -49,7 +48,6 @@
4948
import com.google.api.gax.rpc.testing.MockStreamingApi.MockResponseObserver;
5049
import com.google.api.gax.rpc.testing.MockStreamingApi.MockServerStreamingCall;
5150
import com.google.api.gax.rpc.testing.MockStreamingApi.MockServerStreamingCallable;
52-
import com.google.api.gax.tracing.ApiTracerContext.Transport;
5351
import com.google.api.gax.tracing.ApiTracerFactory.OperationType;
5452
import org.junit.jupiter.api.extension.ExtendWith;
5553
import org.junit.jupiter.params.ParameterizedTest;
@@ -60,16 +58,11 @@
6058
@ExtendWith(MockitoExtension.class)
6159
class TracedServerStreamingCallableTest {
6260
private static final SpanName SPAN_NAME = SpanName.of("FakeClient", "FakeRpc");
63-
private static final ApiTracerContext TRACER_CONTEXT =
64-
ApiTracerContext.newBuilder()
65-
.setFullMethodName("FakeClient/FakeRpc")
66-
.setTransport(Transport.GRPC)
67-
.setLibraryMetadata(LibraryMetadata.empty())
68-
.setOperationType(OperationType.ServerStreaming)
69-
.build();
7061

7162
@Mock private ApiTracerFactory tracerFactory;
7263
@Mock private ApiTracer tracer;
64+
@Mock private ApiTracerContext tracerContext;
65+
@Mock private ApiTracerContext.Builder tracerContextBuilder;
7366

7467
private MockServerStreamingCallable<String, String> innerCallable;
7568
private TracedServerStreamingCallable<String, String> tracedCallable;
@@ -81,10 +74,14 @@ void init(boolean useContext) {
8174
innerCallable = new MockServerStreamingCallable<>();
8275
// Wire the mock tracer factory
8376
if (useContext) {
77+
when(tracerContext.toBuilder()).thenReturn(tracerContextBuilder);
78+
when(tracerContextBuilder.setOperationType(any())).thenReturn(tracerContextBuilder);
79+
when(tracerContextBuilder.build()).thenReturn(tracerContext);
8480
when(tracerFactory.newTracer(any(ApiTracer.class), any(ApiTracerContext.class)))
8581
.thenReturn(tracer);
86-
tracedCallable =
87-
new TracedServerStreamingCallable<>(innerCallable, tracerFactory, TRACER_CONTEXT);
82+
when(tracerContext.fullMethodName()).thenReturn("FakeClient/FakeRpc");
83+
t statutracedCallable =
84+
new TracedServerStreamingCallable<>(innerCallable, tracerFactory, tracerContext);
8885
} else {
8986
when(tracerFactory.newTracer(
9087
any(ApiTracer.class), any(SpanName.class), eq(OperationType.ServerStreaming)))
@@ -102,7 +99,7 @@ void testTracerCreated(boolean useContext) {
10299
init(useContext);
103100
tracedCallable.call("test", responseObserver, callContext);
104101
if (useContext) {
105-
verify(tracerFactory, times(1)).newTracer(parentTracer, TRACER_CONTEXT);
102+
verify(tracerFactory, times(1)).newTracer(parentTracer, tracerContext);
106103
} else {
107104
verify(tracerFactory, times(1))
108105
.newTracer(parentTracer, SPAN_NAME, OperationType.ServerStreaming);
@@ -189,7 +186,7 @@ void testSyncError(boolean useContext) {
189186
// Recreate the tracedCallable using the new inner callable
190187
if (useContext) {
191188
tracedCallable =
192-
new TracedServerStreamingCallable<>(innerCallable, tracerFactory, TRACER_CONTEXT);
189+
new TracedServerStreamingCallable<>(innerCallable, tracerFactory, tracerContext);
193190
} else {
194191
tracedCallable = new TracedServerStreamingCallable<>(innerCallable, tracerFactory, SPAN_NAME);
195192
}

0 commit comments

Comments
 (0)