Change SpanExporter::export() return value to boolean (#827)

open-telemetry · Sep 18, 2022 · 15ebaeb · 15ebaeb
1 parent b551294
commit 15ebaeb
Show file tree

Hide file tree

Showing 23 changed files with 100 additions and 153 deletions.
diff --git a/src/Contrib/Jaeger/AgentExporter.php b/src/Contrib/Jaeger/AgentExporter.php
@@ -39,10 +39,7 @@ public function closeAgentConnection(): void
         $this->jaegerTransport->close();
     }
 
-    /**
-     * @psalm-return SpanExporterInterface::STATUS_*
-     */
-    public function doExport(iterable $spans): int
+    public function doExport(iterable $spans): bool
     {
         // UDP Transport begins here after converting to thrift format span
         foreach ($spans as $span) {
@@ -52,7 +49,7 @@ public function doExport(iterable $spans): int
             );
         }
 
-        return SpanExporterInterface::STATUS_SUCCESS;
+        return true;
     }
 
     /** @inheritDoc */

diff --git a/src/Contrib/Jaeger/HttpCollectorExporter.php b/src/Contrib/Jaeger/HttpCollectorExporter.php
@@ -39,14 +39,11 @@ public function __construct(
         );
     }
 
-    /**
-     * @psalm-return SpanExporterInterface::STATUS_*
-     */
-    public function doExport(iterable $spans): int
+    public function doExport(iterable $spans): bool
     {
         $this->sender->send($spans);
 
-        return SpanExporterInterface::STATUS_SUCCESS;
+        return true;
     }
 
     /** @inheritDoc */

diff --git a/src/Contrib/Newrelic/Exporter.php b/src/Contrib/Newrelic/Exporter.php
@@ -97,11 +97,11 @@ public function export(iterable $spans, ?CancellationInterface $cancellation = n
     {
         return $this->transport
             ->send($this->serializeTrace($spans), 'application/json', $cancellation)
-            ->map(static fn (): int => 0)
-            ->catch(static function (Throwable $throwable): int {
+            ->map(static fn (): bool => true)
+            ->catch(static function (Throwable $throwable): bool {
                 self::logError('Export failure', ['exception' => $throwable]);
 
-                return 1;
+                return false;
             });
     }
 

diff --git a/src/Contrib/OtlpGrpc/Exporter.php b/src/Contrib/OtlpGrpc/Exporter.php
@@ -105,7 +105,7 @@ public function getClientOptions(): array
      * @inheritDoc
      * @psalm-suppress UndefinedConstant
      */
-    protected function doExport(iterable $spans): int
+    protected function doExport(iterable $spans): bool
     {
         $request = (new SpanConverter())->convert($spans);
 
@@ -120,7 +120,7 @@ protected function doExport(iterable $spans): int
                     'error' => $response->getPartialSuccess()->getErrorMessage(),
                 ]);
 
-                return self::STATUS_FAILED_NOT_RETRYABLE;
+                return false;
             } elseif ($response->getPartialSuccess()->getErrorMessage()) {
                 self::logWarning('Export warning', ['server_message' => $response->getPartialSuccess()->getErrorMessage()]);
             }
@@ -129,7 +129,7 @@ protected function doExport(iterable $spans): int
         if ($status->code === \Grpc\STATUS_OK) {
             self::logDebug('Exported span(s)', ['spans' => $request->getResourceSpans()]);
 
-            return self::STATUS_SUCCESS;
+            return true;
         }
 
         $error = [
@@ -150,12 +150,12 @@ protected function doExport(iterable $spans): int
         ], true)) {
             self::logWarning('Retryable error exporting grpc span', ['error' => $error]);
 
-            return self::STATUS_FAILED_RETRYABLE;
+            return false;
         }
 
         self::logError('Error exporting grpc span', ['error' => $error]);
 
-        return self::STATUS_FAILED_NOT_RETRYABLE;
+        return false;
     }
 
     public function setHeader($key, $value): void

diff --git a/src/Contrib/OtlpHttp/Exporter.php b/src/Contrib/OtlpHttp/Exporter.php
@@ -98,7 +98,7 @@ public function export(iterable $spans, ?CancellationInterface $cancellation = n
     {
         return $this->transport
             ->send((new SpanConverter())->convert($spans)->serializeToString(), 'application/x-protobuf', $cancellation)
-            ->map(static function (string $payload): int {
+            ->map(static function (string $payload): bool {
                 $serviceResponse = new ExportTraceServiceResponse();
                 $serviceResponse->mergeFromString($payload);
 
@@ -109,18 +109,18 @@ public function export(iterable $spans, ?CancellationInterface $cancellation = n
                         'error_message' => $partialSuccess->getErrorMessage(),
                     ]);
 
-                    return 1;
+                    return false;
                 }
                 if ($partialSuccess !== null && $partialSuccess->getErrorMessage()) {
                     self::logWarning('Export success with warnings/suggestions', ['error_message' => $partialSuccess->getErrorMessage()]);
                 }
 
-                return 0;
+                return true;
             })
-            ->catch(static function (Throwable $throwable): int {
+            ->catch(static function (Throwable $throwable): bool {
                 self::logError('Export failure', ['exception' => $throwable]);
 
-                return 1;
+                return false;
             });
     }
 

diff --git a/src/Contrib/Zipkin/Exporter.php b/src/Contrib/Zipkin/Exporter.php
@@ -70,11 +70,11 @@ public function export(iterable $spans, ?CancellationInterface $cancellation = n
     {
         return $this->transport
             ->send($this->serializeTrace($spans), 'application/json', $cancellation)
-            ->map(static fn (): int => 0)
-            ->catch(static function (Throwable $throwable): int {
+            ->map(static fn (): bool => true)
+            ->catch(static function (Throwable $throwable): bool {
                 self::logError('Export failure', ['exception' => $throwable]);
 
-                return 1;
+                return false;
             });
     }
 

diff --git a/src/Contrib/ZipkinToNewrelic/Exporter.php b/src/Contrib/ZipkinToNewrelic/Exporter.php
@@ -84,11 +84,11 @@ public function export(iterable $spans, ?CancellationInterface $cancellation = n
     {
         return $this->transport
             ->send($this->serializeTrace($spans), 'application/json', $cancellation)
-            ->map(static fn (): int => 0)
-            ->catch(static function (Throwable $throwable): int {
+            ->map(static fn (): bool => true)
+            ->catch(static function (Throwable $throwable): bool {
                 self::logError('Export failure', ['exception' => $throwable]);
 
-                return 1;
+                return false;
             });
     }
 

diff --git a/src/SDK/Trace/Behavior/SpanExporterDecoratorTrait.php b/src/SDK/Trace/Behavior/SpanExporterDecoratorTrait.php
@@ -15,20 +15,20 @@ trait SpanExporterDecoratorTrait
 
     /**
      * @param iterable<SpanDataInterface> $spans
-     * @return FutureInterface<int>
+     * @return FutureInterface<bool>
      */
     public function export(iterable $spans, ?CancellationInterface $cancellation = null): FutureInterface
     {
         $spans = $this->beforeExport($spans);
         $response = $this->decorated->export($spans, $cancellation);
-        $response->map(fn (int $result) => $this->afterExport($spans, $result));
+        $response->map(fn (bool $result) => $this->afterExport($spans, $result));
 
         return $response;
     }
 
     abstract protected function beforeExport(iterable $spans): iterable;
 
-    abstract protected function afterExport(iterable $spans, int $exporterResponse): void;
+    abstract protected function afterExport(iterable $spans, bool $exportSuccess): void;
 
     public function shutdown(?CancellationInterface $cancellation = null): bool
     {

diff --git a/src/SDK/Trace/Behavior/SpanExporterTrait.php b/src/SDK/Trace/Behavior/SpanExporterTrait.php
@@ -8,7 +8,6 @@
 use OpenTelemetry\SDK\Common\Future\CompletedFuture;
 use OpenTelemetry\SDK\Common\Future\FutureInterface;
 use OpenTelemetry\SDK\Trace\SpanDataInterface;
-use OpenTelemetry\SDK\Trace\SpanExporterInterface;
 
 trait SpanExporterTrait
 {
@@ -32,21 +31,19 @@ abstract public static function fromConnectionString(string $endpointUrl, string
 
     /**
      * @param iterable<SpanDataInterface> $spans
-     * @return FutureInterface<int>
+     * @return FutureInterface<bool>
      */
     public function export(iterable $spans, ?CancellationInterface $cancellation = null): FutureInterface
     {
         if (!$this->running) {
-            return new CompletedFuture(SpanExporterInterface::STATUS_FAILED_NOT_RETRYABLE);
+            return new CompletedFuture(false);
         }
 
         return new CompletedFuture($this->doExport($spans)); /** @phpstan-ignore-line */
     }
 
     /**
      * @param iterable<SpanDataInterface> $spans Batch of spans to export
-     *
-     * @psalm-return SpanExporterInterface::STATUS_*
      */
-    abstract protected function doExport(iterable $spans): int; /** @phpstan-ignore-line */
+    abstract protected function doExport(iterable $spans): bool; /** @phpstan-ignore-line */
 }
diff --git a/src/SDK/Trace/SpanExporter/ConsoleSpanExporter.php b/src/SDK/Trace/SpanExporter/ConsoleSpanExporter.php
@@ -21,7 +21,7 @@ public function __construct(?SpanConverterInterface $converter = null)
     }
 
     /** @inheritDoc */
-    public function doExport(iterable $spans): int
+    public function doExport(iterable $spans): bool
     {
         try {
             foreach ($spans as $span) {
@@ -32,10 +32,10 @@ public function doExport(iterable $spans): int
                 );
             }
         } catch (Throwable $t) {
-            return SpanExporterInterface::STATUS_FAILED_NOT_RETRYABLE;
+            return false;
         }
 
-        return SpanExporterInterface::STATUS_SUCCESS;
+        return true;
     }
 
     public static function fromConnectionString(string $endpointUrl = null, string $name = null, $args = null)

diff --git a/src/SDK/Trace/SpanExporter/InMemoryExporter.php b/src/SDK/Trace/SpanExporter/InMemoryExporter.php
@@ -24,13 +24,13 @@ public static function fromConnectionString(string $endpointUrl = null, string $
         return new self();
     }
 
-    protected function doExport(iterable $spans): int
+    protected function doExport(iterable $spans): bool
     {
         foreach ($spans as $span) {
             $this->storage[] = $span;
         }
 
-        return SpanExporterInterface::STATUS_SUCCESS;
+        return true;
     }
 
     public function getSpans(): array

diff --git a/src/SDK/Trace/SpanExporter/LoggerDecorator.php b/src/SDK/Trace/SpanExporter/LoggerDecorator.php
@@ -21,18 +21,6 @@ class LoggerDecorator implements SpanExporterInterface, LoggerAwareInterface
     use UsesSpanConverterTrait;
     use LoggerAwareTrait;
 
-    private const RESPONSE_MAPPING = [
-        SpanExporterInterface::STATUS_SUCCESS =>  LogLevel::INFO,
-        SpanExporterInterface::STATUS_FAILED_RETRYABLE =>  LogLevel::ERROR,
-        SpanExporterInterface::STATUS_FAILED_NOT_RETRYABLE =>  LogLevel::ALERT,
-    ];
-
-    private const MESSAGE_MAPPING = [
-        SpanExporterInterface::STATUS_SUCCESS =>  'Status Success',
-        SpanExporterInterface::STATUS_FAILED_RETRYABLE =>  'Status Failed Retryable',
-        SpanExporterInterface::STATUS_FAILED_NOT_RETRYABLE =>  'Status Failed Not Retryable',
-    ];
-
     public function __construct(
         SpanExporterInterface $decorated,
         ?LoggerInterface $logger = null,
@@ -57,14 +45,22 @@ protected function beforeExport(iterable $spans): iterable
 
     /**
      * @param iterable $spans
-     * @param int $exporterResponse
+     * @param bool $exportSuccess
      */
-    protected function afterExport(iterable $spans, int $exporterResponse): void
+    protected function afterExport(iterable $spans, bool $exportSuccess): void
     {
-        $this->log(
-            self::MESSAGE_MAPPING[$exporterResponse],
-            $this->getSpanConverter()->convert($spans),
-            self::RESPONSE_MAPPING[$exporterResponse]
-        );
+        if ($exportSuccess) {
+            $this->log(
+                'Status Success',
+                $this->getSpanConverter()->convert($spans),
+                LogLevel::INFO,
+            );
+        } else {
+            $this->log(
+                'Status Failed Retryable',
+                $this->getSpanConverter()->convert($spans),
+                LogLevel::ERROR,
+            );
+        }
     }
 }
diff --git a/src/SDK/Trace/SpanExporter/LoggerExporter.php b/src/SDK/Trace/SpanExporter/LoggerExporter.php
@@ -51,15 +51,15 @@ public function __construct(
     }
 
     /** @inheritDoc */
-    public function doExport(iterable $spans): int
+    public function doExport(iterable $spans): bool
     {
         try {
             $this->doLog($spans);
         } catch (Throwable $t) {
-            return SpanExporterInterface::STATUS_FAILED_NOT_RETRYABLE;
+            return false;
         }
 
-        return SpanExporterInterface::STATUS_SUCCESS;
+        return true;
     }
 
     /**

diff --git a/src/SDK/Trace/SpanExporterInterface.php b/src/SDK/Trace/SpanExporterInterface.php
@@ -12,21 +12,14 @@
  */
 interface SpanExporterInterface
 {
-    /**
-     * Possible return values as outlined in the OpenTelemetry spec
-     */
-    public const STATUS_SUCCESS = 0;
-    public const STATUS_FAILED_NOT_RETRYABLE = 1;
-    public const STATUS_FAILED_RETRYABLE = 2;
-
     public static function fromConnectionString(string $endpointUrl, string $name, string $args);
 
     /**
      * @param iterable<SpanDataInterface> $spans Batch of spans to export
      *
      * @see https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/specification/trace/sdk.md#exportbatch
      *
-     * @psalm-return FutureInterface<int>
+     * @psalm-return FutureInterface<bool>
      */
     public function export(iterable $spans, ?CancellationInterface $cancellation = null): FutureInterface;