refactor: remove SpanName constructor from TracedUnaryCallable

Author: blakeli0

sdk-platform-java/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcCallableFactory.java

@@ -199,7 +199,7 @@ OperationCallable<RequestT, ResponseT, MetadataT> createOperationCallable(
     // Create a sub-trace for the initial RPC that starts the operation.
     UnaryCallable<RequestT, OperationSnapshot> tracedInitialCallable =
         new TracedOperationInitialCallable<>(
-            initialCallable, clientContext.getTracerFactory(), initialSpanName);
+            initialCallable, clientContext.getTracerFactory(), tracerContext);
 
     LongRunningClient longRunningClient = new GrpcLongRunningClient(operationsStub);
     OperationCallable<RequestT, ResponseT, MetadataT> operationCallable =

sdk-platform-java/gax-java/gax/src/main/java/com/google/api/gax/tracing/TracedOperationInitialCallable.java

@@ -49,8 +49,8 @@ public class TracedOperationInitialCallable<RequestT>
   public TracedOperationInitialCallable(
       UnaryCallable<RequestT, OperationSnapshot> innerCallable,
       ApiTracerFactory tracedFactory,
-      SpanName spanName) {
-    super(innerCallable, tracedFactory, spanName);
+      ApiTracerContext apiTracerContext) {
+    super(innerCallable, tracedFactory, apiTracerContext, null);
   }
 
   @Override

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

@@ -51,19 +51,10 @@ public class TracedUnaryCallable<RequestT, ResponseT> extends UnaryCallable<Requ
   private final UnaryCallable<RequestT, ResponseT> innerCallable;
   private final ApiTracerFactory tracerFactory;
   private final SpanName spanName;
-  @Nullable private final ApiTracerContext apiTracerContext;
+  private final ApiTracerContext apiTracerContext;
   @Nullable private final ResourceNameExtractor<RequestT> resourceNameExtractor;
 
-  public TracedUnaryCallable(
-      UnaryCallable<RequestT, ResponseT> innerCallable,
-      ApiTracerFactory tracerFactory,
-      SpanName spanName) {
-    this.innerCallable = innerCallable;
-    this.tracerFactory = tracerFactory;
-    this.spanName = spanName;
-    this.apiTracerContext = null;
-    this.resourceNameExtractor = null;
-  }
+
 
   public TracedUnaryCallable(
       UnaryCallable<RequestT, ResponseT> innerCallable,
@@ -86,15 +77,10 @@ public TracedUnaryCallable(
    */
   @Override
   public ApiFuture<ResponseT> futureCall(RequestT request, ApiCallContext context) {
-    ApiTracer tracer;
-    if (apiTracerContext != null) {
-      tracer =
-          tracerFactory.newTracer(
-              context.getTracer(),
-              apiTracerContext.withResourceNameExtractor(request, resourceNameExtractor));
-    } else {
-      tracer = tracerFactory.newTracer(context.getTracer(), spanName, OperationType.Unary);
-    }
+    ApiTracer tracer =
+        tracerFactory.newTracer(
+            context.getTracer(),
+            apiTracerContext.withResourceNameExtractor(request, resourceNameExtractor));
     TraceFinisher<ResponseT> finisher = new TraceFinisher<>(tracer);
 
     try {

sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/TracedCallableTest.java

@@ -46,6 +46,8 @@
 import com.google.api.gax.rpc.UnaryCallSettings;
 import com.google.api.gax.rpc.UnaryCallable;
 import com.google.api.gax.rpc.testing.FakeCallContext;
+import com.google.api.gax.rpc.LibraryMetadata;
+import com.google.api.gax.tracing.ApiTracerContext.Transport;
 import com.google.api.gax.tracing.ApiTracerFactory.OperationType;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -56,6 +58,13 @@
 @ExtendWith(MockitoExtension.class)
 class TracedCallableTest {
   private static final SpanName SPAN_NAME = SpanName.of("FakeClient", "FakeRpc");
+  private static final ApiTracerContext TRACER_CONTEXT =
+      ApiTracerContext.newBuilder()
+          .setFullMethodName("FakeClient/FakeRpc")
+          .setTransport(Transport.GRPC)
+          .setLibraryMetadata(LibraryMetadata.empty())
+          .setOperationType(OperationType.Unary)
+          .build();
 
   @Mock private ApiTracerFactory tracerFactory;
   private ApiTracer parentTracer;
@@ -71,8 +80,7 @@ void setUp() {
     parentTracer = BaseApiTracer.getInstance();
 
     // Wire the mock tracer factory
-    when(tracerFactory.newTracer(
-            any(ApiTracer.class), any(SpanName.class), eq(OperationType.Unary)))
+    when(tracerFactory.newTracer(any(ApiTracer.class), any(ApiTracerContext.class)))
         .thenReturn(tracer);
 
     // Wire the mock inner callable
@@ -87,7 +95,7 @@ public UnaryCallable<String, String> setupTracedUnaryCallable(
       UnaryCallSettings<Object, Object> callSettings) {
     UnaryCallable<String, String> callable =
         Callables.retrying(innerCallable, callSettings, clientContext);
-    return new TracedUnaryCallable<>(callable, tracerFactory, SPAN_NAME);
+    return new TracedUnaryCallable<>(callable, tracerFactory, TRACER_CONTEXT, null);
   }
 
   @Test
@@ -102,7 +110,7 @@ void testNonRetriedCallable() throws Exception {
 
     ApiFuture<String> future = callable.futureCall("Is your refrigerator running?", callContext);
 
-    verify(tracerFactory, times(1)).newTracer(parentTracer, SPAN_NAME, OperationType.Unary);
+    verify(tracerFactory, times(1)).newTracer(parentTracer, TRACER_CONTEXT);
     verify(tracer, times(1)).attemptStarted(anyString(), anyInt());
     verify(tracer, times(1)).attemptSucceeded();
     verify(tracer, times(1)).operationSucceeded();

sdk-platform-java/gax-java/gax/src/test/java/com/google/api/gax/tracing/TracedUnaryCallableTest.java

@@ -73,21 +73,14 @@ class TracedUnaryCallableTest {
   private TracedUnaryCallable<String, String> tracedUnaryCallable;
   private FakeCallContext callContext;
 
-  void init(boolean useContext) {
+  void init() {
     parentTracer = BaseApiTracer.getInstance();
 
     // Wire the mock tracer factory
-    if (useContext) {
-      when(tracerFactory.newTracer(any(ApiTracer.class), any(ApiTracerContext.class)))
-          .thenReturn(tracer);
-      tracedUnaryCallable =
-          new TracedUnaryCallable<>(innerCallable, tracerFactory, TRACER_CONTEXT, null);
-    } else {
-      when(tracerFactory.newTracer(
-              any(ApiTracer.class), any(SpanName.class), eq(OperationType.Unary)))
-          .thenReturn(tracer);
-      tracedUnaryCallable = new TracedUnaryCallable<>(innerCallable, tracerFactory, SPAN_NAME);
-    }
+    when(tracerFactory.newTracer(any(ApiTracer.class), any(ApiTracerContext.class)))
+        .thenReturn(tracer);
+    tracedUnaryCallable =
+        new TracedUnaryCallable<>(innerCallable, tracerFactory, TRACER_CONTEXT, null);
 
     // Wire the mock inner callable
     innerResult = SettableApiFuture.create();
@@ -96,16 +89,11 @@ void init(boolean useContext) {
     callContext = FakeCallContext.createDefault();
   }
 
-  @ParameterizedTest
-  @ValueSource(booleans = {false, true})
-  void testTracerCreated(boolean useContext) {
-    init(useContext);
+  @Test
+  void testTracerCreated() {
+    init();
     tracedUnaryCallable.futureCall("test", callContext);
-    if (useContext) {
-      verify(tracerFactory, times(1)).newTracer(parentTracer, TRACER_CONTEXT);
-    } else {
-      verify(tracerFactory, times(1)).newTracer(parentTracer, SPAN_NAME, OperationType.Unary);
-    }
+    verify(tracerFactory, times(1)).newTracer(parentTracer, TRACER_CONTEXT);
   }
 
   @Test
@@ -131,40 +119,36 @@ void testOperationTypeIsSet() {
     assertThat(contextCaptor.getValue().operationType()).isEqualTo(OperationType.Unary);
   }
 
-  @ParameterizedTest
-  @ValueSource(booleans = {false, true})
-  void testOperationFinish(boolean useContext) {
-    init(useContext);
+  @Test
+  void testOperationFinish() {
+    init();
     innerResult.set("successful result");
     tracedUnaryCallable.futureCall("test", callContext);
 
     verify(tracer, times(1)).operationSucceeded();
   }
 
-  @ParameterizedTest
-  @ValueSource(booleans = {false, true})
-  void testOperationCancelled(boolean useContext) {
-    init(useContext);
+  @Test
+  void testOperationCancelled() {
+    init();
     innerResult.cancel(true);
     tracedUnaryCallable.futureCall("test", callContext);
     verify(tracer, times(1)).operationCancelled();
   }
 
-  @ParameterizedTest
-  @ValueSource(booleans = {false, true})
-  void testOperationFailed(boolean useContext) {
-    init(useContext);
+  @Test
+  void testOperationFailed() {
+    init();
     RuntimeException fakeError = new RuntimeException("fake error");
     innerResult.setException(fakeError);
     tracedUnaryCallable.futureCall("test", callContext);
 
     verify(tracer, times(1)).operationFailed(fakeError);
   }
 
-  @ParameterizedTest
-  @ValueSource(booleans = {false, true})
-  void testSyncError(boolean useContext) {
-    init(useContext);
+  @Test
+  void testSyncError() {
+    init();
     RuntimeException fakeError = new RuntimeException("fake error");
 
     // Reset the irrelevant expectations from setup. (only needed to silence the warnings).