fix(Gax): address tracing review feedback and span lifecycle fixes

Gax/src/GapicClientTrait.php

@@ -72,9 +72,9 @@ trait GapicClientTrait
 
     private ?TransportInterface $transport = null;
     private ?HeaderCredentialsInterface $credentialsWrapper = null;
-    private ?TracerProviderInterface $openTelemetryTracerProvider = null;
     private array $telemetryOptions = [];
-    private string $apiEndpoint = '';
+    private ?string $serverAddress = null;
+    private ?int $serverPort = null;
     /** @var RetrySettings[] $retrySettings */
     private array $retrySettings = [];
     private string $serviceName = '';
@@ -269,6 +269,8 @@ protected function getCredentialsWrapper()
      *           The code generator version of the GAPIC library.
      *     @type callable $clientCertSource
      *           A callable which returns the client cert as a string.
+     *     @type TracerProviderInterface|null $openTelemetryTracerProvider
+     *           An OpenTelemetry TracerProvider to use for tracing API calls.
      * }
      * @throws ValidationException
      */
@@ -378,30 +380,31 @@ private function setClientOptions(array $options)
             );
         }
 
-        $this->apiEndpoint = $options['apiEndpoint'];
-        $this->openTelemetryTracerProvider = $options['openTelemetryTracerProvider'] ?? null;
-        $telemetryOptions = [
+        $this->telemetryOptions = [
             'openTelemetryTracerProvider' => $options['openTelemetryTracerProvider'] ?? null,
-            'clientVersion' => $options['libVersion'] ?? null,
+            'clientVersion' => $options['gapicVersion'] ?? null,
         ];
-        $this->telemetryOptions = $telemetryOptions;
+        if (!empty($this->telemetryOptions['openTelemetryTracerProvider']) && $options['apiEndpoint'] !== '') {
+            try {
+                [$serverAddress, $serverPort] = self::normalizeServiceAddress($options['apiEndpoint']);
+                $this->serverAddress = $serverAddress;
+                $this->serverPort = (int) $serverPort;
+            } catch (ValidationException $e) {
+                // Ignore invalid apiEndpoint formats when setting span attributes
+            }
+        }
 
         $transport = $options['transport'] ?: self::defaultTransport();
-        if ($transport instanceof TransportInterface) {
-            if (method_exists($transport, 'setTelemetryOptions')) {
-                $transport->setTelemetryOptions($telemetryOptions);
-            }
-            $this->transport = $transport;
-        } else {
-            $this->transport = $this->createTransport(
+        $this->transport = $transport instanceof TransportInterface
+            ? $transport
+            : $this->createTransport(
                 $options['apiEndpoint'],
                 $transport,
                 $options['transportConfig'],
                 $options['clientCertSource'],
                 $hasEmulator,
-                $telemetryOptions
+                $this->telemetryOptions
             );
-        }
     }
 
     /**
@@ -775,14 +778,12 @@ private function createCallStack(array $callConstructionOptions)
             $callStack = $fn($callStack);
         }
 
-        if ($this->openTelemetryTracerProvider) {
-            [$serverAddress, $serverPort] = self::normalizeServiceAddress($this->apiEndpoint);
+        if (!empty($this->telemetryOptions['openTelemetryTracerProvider'])) {
             $systemName = $this->transport instanceof GrpcTransport ? 'grpc' : 'http';
             $callStack = new TracingMiddleware(
                 $callStack,
-                $this->openTelemetryTracerProvider,
-                $serverAddress,
-                (int) $serverPort,
+                $this->serverAddress,
+                $this->serverPort,
                 $systemName,
                 $this->telemetryOptions
             );

Gax/src/Middleware/TracingMiddleware.php

@@ -39,7 +39,6 @@
 use GuzzleHttp\Promise\PromiseInterface;
 use OpenTelemetry\API\Trace\SpanKind;
 use OpenTelemetry\API\Trace\StatusCode;
-use OpenTelemetry\API\Trace\TracerProviderInterface;
 use Throwable;
 
 /**
@@ -53,39 +52,35 @@ class TracingMiddleware implements MiddlewareInterface
 
     /** @var MiddlewareInterface|callable */
     private $nextHandler;
-    private string $serverAddress;
-    private int $serverPort;
     private string $systemName;
 
     /**
      * @param MiddlewareInterface|callable $nextHandler
-     * @param TracerProviderInterface|null $openTelemetryTracerProvider
-     * @param string $serverAddress
-     * @param int $serverPort
+     * @param string|null $serverAddress
+     * @param int|null $serverPort
      * @param string $systemName
      * @param array $telemetryOptions
      */
     public function __construct(
         $nextHandler,
-        ?TracerProviderInterface $openTelemetryTracerProvider = null,
-        string $serverAddress = '',
-        int $serverPort = 443,
+        ?string $serverAddress = null,
+        ?int $serverPort = null,
         string $systemName = 'grpc',
         array $telemetryOptions = []
     ) {
         $this->nextHandler = $nextHandler;
-        $this->serverAddress = $serverAddress;
+        $this->serverAddress = $serverAddress ?: null;
         $this->serverPort = $serverPort;
         $this->systemName = $systemName;
-        $this->initTelemetry($telemetryOptions, $openTelemetryTracerProvider);
+        $this->setTelemetryOptions($telemetryOptions);
     }
 
     /**
      * {@inheritdoc}
      */
     public function __invoke(Call $call, array $options)
     {
-        if (!$this->openTelemetryTracerProvider) {
+        if (!$this->openTelemetryTracerProvider || $call->getCallType() !== Call::UNARY_CALL) {
             return ($this->nextHandler)($call, $options);
         }
 
@@ -129,12 +124,17 @@ public function __invoke(Call $call, array $options)
                     function () use ($result, $span) {
                         $waitScope = $span->activate();
                         try {
-                            $result->wait();
+                            $result->wait(false);
                         } finally {
                             $waitScope->detach();
                         }
                     },
-                    [$result, 'cancel']
+                    function () use ($result, $span) {
+                        $span->setStatus(StatusCode::STATUS_ERROR, 'Call cancelled');
+                        $span->setAttribute(SpanAttributes::ERROR_TYPE, 'CANCELLED');
+                        $span->end();
+                        $result->cancel();
+                    }
                 );
 
                 $result->then(

Gax/src/Options/ClientOptions.php

@@ -173,6 +173,8 @@ class ClientOptions implements ArrayAccess, OptionsInterface
      *          The API key to be used for the client.
      *     @type null|false|LoggerInterface
      *           A PSR-3 compliant logger.
+     *     @type TracerProviderInterface|null $openTelemetryTracerProvider
+     *           An OpenTelemetry TracerProvider to use for tracing API calls.
      * }
      */
     public function __construct(array $options)

Gax/src/Telemetry/SpanAttributes.php

@@ -43,8 +43,7 @@ final class SpanAttributes
     public const RPC_SYSTEM_NAME = 'rpc.system.name';
     public const RPC_RESPONSE_STATUS_CODE = 'rpc.response.status_code';
 
-    // HTTP & Network attributes
-    public const HTTP_RESPONSE_STATUS_CODE = 'http.response.status_code';
+    // Network attributes
     public const SERVER_ADDRESS = 'server.address';
     public const SERVER_PORT = 'server.port';
 

Gax/src/Telemetry/TelemetryTrait.php

@@ -34,11 +34,12 @@
 namespace Google\ApiCore\Telemetry;
 
 use Google\ApiCore\ApiException;
+use Google\ApiCore\ServiceAddressTrait;
+use Google\ApiCore\ValidationException;
 use OpenTelemetry\API\Trace\SpanInterface;
 use OpenTelemetry\API\Trace\SpanKind;
 use OpenTelemetry\API\Trace\StatusCode;
 use OpenTelemetry\API\Trace\TracerProviderInterface;
-use Psr\Http\Message\ResponseInterface;
 use Throwable;
 
 /**
@@ -48,42 +49,36 @@
  */
 trait TelemetryTrait
 {
+    use ServiceAddressTrait;
+
     private ?TracerProviderInterface $openTelemetryTracerProvider = null;
     private ?string $clientVersion = null;
+    private ?string $serverAddress = null;
+    private ?int $serverPort = null;
 
     /**
      * Sets telemetry options and initializes tracing properties.
      *
      * @param array $telemetryOptions
-     * @param TracerProviderInterface|null $openTelemetryTracerProvider
+     * @param string|null $apiEndpoint
      * @return $this
      */
-    public function setTelemetryOptions(
-        array $telemetryOptions,
-        ?TracerProviderInterface $openTelemetryTracerProvider = null
-    ): self {
-        $this->initTelemetry($telemetryOptions, $openTelemetryTracerProvider);
+    private function setTelemetryOptions(array $telemetryOptions, ?string $apiEndpoint = null): self
+    {
+        $this->openTelemetryTracerProvider = $telemetryOptions['openTelemetryTracerProvider'] ?? null;
+        $this->clientVersion = $telemetryOptions['clientVersion'] ?? null;
+        if ($this->openTelemetryTracerProvider && $apiEndpoint) {
+            try {
+                [$addr, $port] = self::normalizeServiceAddress($apiEndpoint);
+                $this->serverAddress = $addr;
+                $this->serverPort = (int) $port;
+            } catch (ValidationException $e) {
+                // Ignore invalid apiEndpoint formats when setting span attributes
+            }
+        }
         return $this;
     }
 
-    /**
-     * Initializes telemetry properties from an options array and optional tracer provider.
-     *
-     * @param array $telemetryOptions
-     * @param TracerProviderInterface|null $openTelemetryTracerProvider
-     */
-    private function initTelemetry(
-        array $telemetryOptions,
-        ?TracerProviderInterface $openTelemetryTracerProvider = null
-    ): void {
-        $this->openTelemetryTracerProvider = $openTelemetryTracerProvider
-            ?? $telemetryOptions['openTelemetryTracerProvider']
-            ?? null;
-        $this->clientVersion = $telemetryOptions['clientVersion']
-            ?? $telemetryOptions['libVersion']
-            ?? null;
-    }
-
     /**
      * Returns default telemetry config options for transport build methods.
      *
@@ -97,19 +92,6 @@ private static function getTelemetryDefaultConfig(): array
         ];
     }
 
-    /**
-     * Returns the telemetry options populated from this instance.
-     *
-     * @return array
-     */
-    private function getTelemetryOptions(): array
-    {
-        return [
-            'openTelemetryTracerProvider' => $this->openTelemetryTracerProvider,
-            'clientVersion' => $this->clientVersion,
-        ];
-    }
-
     /**
      * Builds and starts a span with standard client metadata attributes.
      *
@@ -153,26 +135,19 @@ private function recordException(?SpanInterface $span, Throwable $e, bool $end =
             return;
         }
 
-        $statusCode = null;
-        if (method_exists($e, 'getResponse') && $e->getResponse() instanceof ResponseInterface) {
-            $statusCode = $e->getResponse()->getStatusCode();
-            $span->setAttribute(SpanAttributes::HTTP_RESPONSE_STATUS_CODE, $statusCode);
-        }
-
-        $errorType = null;
-        if ($statusCode !== null) {
-            $errorType = (string) $statusCode;
-        } elseif ($e instanceof ApiException && $e->getStatus()) {
-            $errorType = $e->getStatus();
+        if ($e instanceof ApiException) {
+            $errorType = $e->getReason() ?: $e->getStatus() ?: get_class($e);
+            $message = $e->getBasicMessage() ?? $e->getMessage();
         } else {
             $errorType = get_class($e);
+            $message = $e->getMessage();
         }
 
         $span->recordException($e);
-        $span->setStatus(StatusCode::STATUS_ERROR, $e->getMessage());
+        $span->setStatus(StatusCode::STATUS_ERROR, $message);
         $span->setAttribute(SpanAttributes::ERROR_TYPE, $errorType);
         $span->setAttribute(SpanAttributes::EXCEPTION_TYPE, get_class($e));
-        $span->setAttribute(SpanAttributes::STATUS_MESSAGE, $e->getMessage());
+        $span->setAttribute(SpanAttributes::STATUS_MESSAGE, $message);
 
         if ($end) {
             $span->end();