refactor: remove the usages of SpanName constructor of TracedUnaryCallable (#12948)

This PR removes the usages of SpanName constructor of
TracedUnaryCallable, and migrates all usages to use ApiTracerContext
instead.

The constructor is now marked as `@ObsoleteApi` and it should be removed
once the usages in downstream libraries (java-bigtable) are removed.

---------

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

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);
   }
 
   @Override

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

@@ -33,6 +33,7 @@
 import com.google.api.core.ApiFutures;
 import com.google.api.core.BetaApi;
 import com.google.api.core.InternalApi;
+import com.google.api.core.ObsoleteApi;
 import com.google.api.gax.rpc.ApiCallContext;
 import com.google.api.gax.rpc.ResourceNameExtractor;
 import com.google.api.gax.rpc.UnaryCallable;
@@ -54,6 +55,7 @@ public class TracedUnaryCallable<RequestT, ResponseT> extends UnaryCallable<Requ
   @Nullable private final ApiTracerContext apiTracerContext;
   @Nullable private final ResourceNameExtractor<RequestT> resourceNameExtractor;
 
+  @ObsoleteApi("Use constructor with ApiTracerContext instead")
   public TracedUnaryCallable(
       UnaryCallable<RequestT, ResponseT> innerCallable,
       ApiTracerFactory tracerFactory,
@@ -78,6 +80,13 @@ public TracedUnaryCallable(
     this.resourceNameExtractor = resourceNameExtractor;
   }
 
+  TracedUnaryCallable(
+      UnaryCallable<RequestT, ResponseT> innerCallable,
+      ApiTracerFactory tracerFactory,
+      ApiTracerContext apiTracerContext) {
+    this(innerCallable, tracerFactory, apiTracerContext, null);
+  }
+
   /**
    * Calls the wrapped {@link UnaryCallable} within the context of a new trace.
    *

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

@@ -32,7 +32,6 @@
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.anyString;
-import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -43,9 +42,11 @@
 import com.google.api.gax.rpc.ApiCallContext;
 import com.google.api.gax.rpc.Callables;
 import com.google.api.gax.rpc.ClientContext;
+import com.google.api.gax.rpc.LibraryMetadata;
 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.tracing.ApiTracerContext.Transport;
 import com.google.api.gax.tracing.ApiTracerFactory.OperationType;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -55,7 +56,14 @@
 
 @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 +79,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 +94,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);
   }
 
   @Test
@@ -102,7 +109,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

@@ -47,15 +47,13 @@
 import com.google.api.gax.tracing.ApiTracerFactory.OperationType;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @ExtendWith(MockitoExtension.class)
 class TracedUnaryCallableTest {
-  private static final SpanName SPAN_NAME = SpanName.of("FakeClient", "FakeRpc");
+
   private static final ApiTracerContext TRACER_CONTEXT =
       ApiTracerContext.newBuilder()
           .setFullMethodName("FakeClient/FakeRpc")
@@ -73,21 +71,13 @@ 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);
 
     // Wire the mock inner callable
     innerResult = SettableApiFuture.create();
@@ -96,29 +86,23 @@ 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
   void testOperationTypeIsSet() {
     when(tracerFactory.newTracer(any(ApiTracer.class), any(ApiTracerContext.class)))
         .thenReturn(tracer);
-    tracedUnaryCallable =
-        new TracedUnaryCallable<>(innerCallable, tracerFactory, TRACER_CONTEXT, null);
+    tracedUnaryCallable = new TracedUnaryCallable<>(innerCallable, tracerFactory, TRACER_CONTEXT);
     ApiTracerContext contextWithWrongType =
         TRACER_CONTEXT.toBuilder().setOperationType(OperationType.BidiStreaming).build();
 
     tracedUnaryCallable =
-        new TracedUnaryCallable<>(innerCallable, tracerFactory, contextWithWrongType, null);
+        new TracedUnaryCallable<>(innerCallable, tracerFactory, contextWithWrongType);
 
     innerResult = SettableApiFuture.create();
     when(innerCallable.futureCall(anyString(), any(ApiCallContext.class))).thenReturn(innerResult);
@@ -131,40 +115,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).