Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Sep 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions src/Contrib/Jaeger/AgentExporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -52,7 +49,7 @@ public function doExport(iterable $spans): int
);
}

return SpanExporterInterface::STATUS_SUCCESS;
return true;
}

/** @inheritDoc */
Expand Down
7 changes: 2 additions & 5 deletions src/Contrib/Jaeger/HttpCollectorExporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
6 changes: 3 additions & 3 deletions src/Contrib/Newrelic/Exporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
}

Expand Down
10 changes: 5 additions & 5 deletions src/Contrib/OtlpGrpc/Exporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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()]);
}
Expand All @@ -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 = [
Expand All @@ -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
Expand Down
10 changes: 5 additions & 5 deletions src/Contrib/OtlpHttp/Exporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

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

Expand Down
6 changes: 3 additions & 3 deletions src/Contrib/Zipkin/Exporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
}

Expand Down
6 changes: 3 additions & 3 deletions src/Contrib/ZipkinToNewrelic/Exporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
}

Expand Down
6 changes: 3 additions & 3 deletions src/SDK/Trace/Behavior/SpanExporterDecoratorTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
9 changes: 3 additions & 6 deletions src/SDK/Trace/Behavior/SpanExporterTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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 */
}
6 changes: 3 additions & 3 deletions src/SDK/Trace/SpanExporter/ConsoleSpanExporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions src/SDK/Trace/SpanExporter/InMemoryExporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 15 additions & 19 deletions src/SDK/Trace/SpanExporter/LoggerDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
);
}
}
}
6 changes: 3 additions & 3 deletions src/SDK/Trace/SpanExporter/LoggerExporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
9 changes: 1 addition & 8 deletions src/SDK/Trace/SpanExporterInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading