Removes span to array as it is deprecated (#212)

* chore: removes Span::toArray as it's been deprecated in 2.0. * chore: removes unused default span name constant. * chore: fix lint. * tests: adds tests for log reporter. * chore: upgrades phpstan. * chore: removes comma from constructor. * chore: makes the new line OS dependent in the LogSerializer.
openzipkin · Sep 13, 2021 · 05888a7 · 05888a7
1 parent f49cf80
commit 05888a7
Show file tree

Hide file tree

Showing 10 changed files with 181 additions and 146 deletions.
diff --git a/composer.json b/composer.json
@@ -20,18 +20,18 @@
         "issues": "https://github.com/openzipkin/zipkin-php/issues"
     },
     "require": {
-        "php": "^7.3 || ^8.0",
+        "php": "^7.4 || ^8.0",
         "ext-curl": "*",
         "psr/http-message": "~1.0",
         "psr/log": "^1.0"
     },
     "require-dev": {
-        "nyholm/psr7": "^1.4",
         "jcchavezs/httptest": "~0.2",
         "middlewares/fast-route": "^2.0",
         "middlewares/request-handler": "^2.0",
+        "nyholm/psr7": "^1.4",
         "phpspec/prophecy-phpunit": "^2.0",
-        "phpstan/phpstan": "~0.12.88",
+        "phpstan/phpstan": "^0.12.26",
         "phpunit/phpunit": "~9",
         "psr/http-client": "^1.0",
         "psr/http-server-middleware": "^1.0",

diff --git a/src/Zipkin/Endpoint.php b/src/Zipkin/Endpoint.php
@@ -8,8 +8,6 @@
 
 final class Endpoint
 {
-    public const DEFAULT_SERVICE_NAME = 'unknown';
-
     /**
      * Service name in lowercase, such as "memcache" or "zipkin-web"
      * Conventionally, when the service name isn't known, service_name = "unknown".
@@ -101,28 +99,4 @@ public function withServiceName(string $serviceName): Endpoint
     {
         return new self($serviceName, $this->ipv4, $this->ipv6, $this->port);
     }
-
-    /**
-     * @deprecated
-     */
-    public function toArray(): array
-    {
-        $endpoint = [
-            'serviceName' => $this->serviceName,
-        ];
-
-        if ($this->ipv4) {
-            $endpoint['ipv4'] = $this->ipv4;
-        }
-
-        if ($this->port) {
-            $endpoint['port'] = $this->port;
-        }
-
-        if ($this->ipv6) {
-            $endpoint['ipv6'] = $this->ipv6;
-        }
-
-        return $endpoint;
-    }
 }
diff --git a/src/Zipkin/Recording/Span.php b/src/Zipkin/Recording/Span.php
@@ -280,54 +280,4 @@ public function finish(int $finishTimestamp = null): void
 
         $this->finished = true;
     }
-
-    /**
-     * @deprecated
-     */
-    public function toArray(): array
-    {
-        $spanAsArray = [
-            'id' => $this->spanId,
-            'name' => $this->name,
-            'traceId' => $this->traceId,
-            'timestamp' => $this->timestamp,
-            'duration' => $this->duration,
-            'localEndpoint' => $this->localEndpoint->toArray(),
-        ];
-
-        if ($this->parentId !== null) {
-            $spanAsArray['parentId'] = $this->parentId;
-        }
-
-        if ($this->debug === true) {
-            $spanAsArray['debug'] = true;
-        }
-
-        if ($this->shared === true) {
-            $spanAsArray['shared'] = true;
-        }
-
-        if ($this->kind !== null) {
-            $spanAsArray['kind'] = $this->kind;
-        }
-
-        if ($this->remoteEndpoint !== null) {
-            $spanAsArray['remoteEndpoint'] = $this->remoteEndpoint->toArray();
-        }
-
-        if (!empty($this->annotations)) {
-            $spanAsArray['annotations'] = $this->annotations;
-        }
-
-        if (!empty($this->tags)) {
-            $spanAsArray['tags'] = $this->tags;
-        }
-
-        return $spanAsArray;
-    }
-
-    public function __toString(): string
-    {
-        return \json_encode($this->toArray()) ?: '';
-    }
 }
diff --git a/src/Zipkin/Reporters/Log.php b/src/Zipkin/Reporters/Log.php
@@ -12,18 +12,21 @@ final class Log implements Reporter
 {
     private LoggerInterface $logger;
 
-    public function __construct(LoggerInterface $logger)
-    {
+    private SpanSerializer $serializer;
+
+    public function __construct(
+        LoggerInterface $logger,
+        SpanSerializer $serializer = null
+    ) {
         $this->logger = $logger;
+        $this->serializer = $serializer ?? new JsonV2Serializer();
     }
 
     /**
      * @param Span[] $spans
      */
     public function report(array $spans): void
     {
-        foreach ($spans as $span) {
-            $this->logger->info($span->__toString());
-        }
+        $this->logger->info($this->serializer->serialize($spans));
     }
 }
diff --git a/src/Zipkin/Reporters/Log/LogSerializer.php b/src/Zipkin/Reporters/Log/LogSerializer.php
@@ -0,0 +1,60 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Zipkin\Reporters\Log;
+
+use Zipkin\Reporters\SpanSerializer;
+use Zipkin\Recording\ReadbackSpan;
+
+final class LogSerializer implements SpanSerializer
+{
+    /**
+     * @param ReadbackSpan[]|array $spans
+     */
+    public function serialize(array $spans): string
+    {
+        $spansAsArray = array_map([self::class, 'serializeSpan'], $spans);
+
+        return implode(PHP_EOL, $spansAsArray);
+    }
+
+    private function serializeSpan(ReadbackSpan $span): string
+    {
+        $serialized = [];
+        $serialized[] = sprintf("Name: %s", $span->getName());
+        $serialized[] = sprintf("TraceID: %s", $span->getTraceId());
+        $serialized[] = sprintf("SpanID: %s", $span->getSpanId());
+        if (!is_null($parentID = $span->getParentId())) {
+            $serialized[] = sprintf("StartTime: %s", $parentID);
+        }
+        $serialized[] = sprintf("Timestamp: %s", $span->getTimestamp());
+        $serialized[] = sprintf("Duration: %s", $span->getDuration());
+        $serialized[] = sprintf("Kind: %s", $span->getKind());
+
+        $serialized[] = sprintf("LocalEndpoint: %s", $span->getLocalEndpoint()->getServiceName());
+
+        if (\count($tags = $span->getTags()) > 0) {
+            $serialized[] = "Tags:";
+
+            foreach ($tags as $key => $value) {
+                $serialized[] = sprintf("    %s: %s", $key, $value);
+            }
+        }
+
+        if (\count($annotations = $span->getAnnotations()) > 0) {
+            $serialized[] = "Annotations:";
+
+            foreach ($annotations as $annotation) {
+                $serialized[] = sprintf("    - timestamp: %s", $annotation["timestamp"]);
+                $serialized[] = sprintf("      value: %s", $annotation["value"]);
+            }
+        }
+
+        if (!is_null($remoteEndpoint = $span->getRemoteEndpoint())) {
+            $serialized[] = sprintf("RemoteEndpoint: %s", $remoteEndpoint->getServiceName());
+        }
+
+        return implode(PHP_EOL, $serialized);
+    }
+}
diff --git a/tests/Integration/Instrumentation/Http/Server/MiddlewareTest.php b/tests/Integration/Instrumentation/Http/Server/MiddlewareTest.php
@@ -4,24 +4,25 @@
 
 namespace ZipkinTests\Integration\Instrumentation\Http\Server;
 
-use function FastRoute\simpleDispatcher;
 use Zipkin\TracingBuilder;
 use Zipkin\SpanCustomizer;
 use Zipkin\Samplers\BinarySampler;
 use Zipkin\Reporters\InMemory;
+use Zipkin\Recording\ReadbackSpan;
 use Zipkin\Propagation\TraceContext;
 
 use Zipkin\Instrumentation\Http\Server\Request;
 use Zipkin\Instrumentation\Http\Server\Psr15\Middleware;
-use Zipkin\Instrumentation\Http\Server\HttpServerParser;
 use Zipkin\Instrumentation\Http\Server\HttpServerTracing;
+use Zipkin\Instrumentation\Http\Server\HttpServerParser;
 use Zipkin\Instrumentation\Http\Server\DefaultHttpServerParser;
 use RingCentral\Psr7\Response;
 use PHPUnit\Framework\TestCase;
 use Middlewares\Utils\Factory;
 use Middlewares\Utils\Dispatcher;
 use Middlewares;
 use FastRoute\RouteCollector;
+use function FastRoute\simpleDispatcher;
 
 final class MiddlewareTest extends TestCase
 {
@@ -79,13 +80,16 @@ public function request(Request $request, TraceContext $context, SpanCustomizer
 
         $this->assertCount(1, $spans);
 
-        $span = $spans[0]->toArray();
+        /**
+         * @var ReadbackSpan $span
+         */
+        $span = $spans[0];
 
-        $this->assertEquals('GET', $span['name']);
+        $this->assertEquals('GET', $span->getName());
         $this->assertEquals([
             'http.method' => 'GET',
             'http.path' => '/users/abc123',
             'user_id' => 'abc123',
-        ], $span['tags']);
+        ], $span->getTags());
     }
 }
diff --git a/tests/Unit/Instrumentation/Http/Server/Psr15/MiddlewareTest.php b/tests/Unit/Instrumentation/Http/Server/Psr15/MiddlewareTest.php
@@ -8,6 +8,7 @@
 use Zipkin\Span;
 use Zipkin\Samplers\BinarySampler;
 use Zipkin\Reporters\InMemory;
+use Zipkin\Recording\ReadbackSpan;
 use Zipkin\Propagation\TraceContext;
 use Zipkin\Propagation\DefaultSamplingFlags;
 use Zipkin\Instrumentation\Http\Server\Psr15\Middleware;
@@ -34,7 +35,7 @@ private static function createTracing(callable $requestSampler = null): array
         $tracer = $tracing->getTracer();
 
         return [
-            new HttpServerTracing($tracing, new DefaultHttpServerParser, $requestSampler),
+            new HttpServerTracing($tracing, new DefaultHttpServerParser(), $requestSampler),
             static function () use ($tracer, $reporter): array {
                 $tracer->flush();
                 return $reporter->flush();
@@ -83,13 +84,16 @@ public function testMiddlewareHandlesRequestSuccessfully()
 
         $this->assertCount(1, $spans);
 
-        $span = $spans[0]->toArray();
+        /**
+         * @var ReadbackSpan $span
+         */
+        $span = $spans[0];
 
-        $this->assertEquals('GET', $span['name']);
+        $this->assertEquals('GET', $span->getName());
         $this->assertEquals([
             'http.method' => 'GET',
             'http.path' => '/',
-        ], $span['tags']);
+        ], $span->getTags());
     }
 
     public function testMiddlewareParsesRequestSuccessfullyWithNon2xx()
@@ -108,15 +112,18 @@ public function testMiddlewareParsesRequestSuccessfullyWithNon2xx()
 
         $this->assertCount(1, $spans);
 
-        $span = $spans[0]->toArray();
+        /**
+         * @var ReadbackSpan $span
+         */
+        $span = $spans[0];
 
-        $this->assertEquals('GET', $span['name']);
+        $this->assertEquals('GET', $span->getName());
         $this->assertEquals([
             'http.method' => 'GET',
             'http.path' => '/',
             'http.status_code' => '404',
             'error' => '404'
-        ], $span['tags']);
+        ], $span->getTags());
     }
 
     public function testMiddlewareKeepsContextForJoinSpan()

diff --git a/tests/Unit/Recording/SpanTest.php b/tests/Unit/Recording/SpanTest.php
@@ -2,13 +2,13 @@
 
 namespace ZipkinTests\Unit\Recording;
 
-use PHPUnit\Framework\TestCase;
+use Zipkin\Recording\Span;
+use Zipkin\Propagation\TraceContext;
+use Zipkin\Propagation\DefaultSamplingFlags;
 use Zipkin\Kind;
 use Zipkin\Endpoint;
-use Zipkin\Propagation\DefaultSamplingFlags;
-use Zipkin\Recording\Span;
+use PHPUnit\Framework\TestCase;
 use function Zipkin\Timestamp\now;
-use Zipkin\Propagation\TraceContext;
 
 final class SpanTest extends TestCase
 {
@@ -29,50 +29,4 @@ public function testStartSpanSuccess()
         $span->start($timestamp);
         $this->assertEquals($timestamp, $span->getTimestamp());
     }
-
-    public function testConvertingSpanToArrayHasTheExpectedValues()
-    {
-        $spanId = 'e463f94de30144fa';
-        $traceId = 'e463f94de30144fa';
-        $parentId = 'e463f94de30144fb';
-
-        $context = TraceContext::create($traceId, $spanId, $parentId);
-
-        $localEndpoint = Endpoint::create('test_service_name', '127.0.0.1', null, 3333);
-        $span = Span::createFromContext($context, $localEndpoint);
-        $timestamp = now();
-
-        $span->start($timestamp);
-        $span->setKind(Kind\CLIENT);
-        $span->setName('test_name');
-        $span->annotate($timestamp, 'test_annotation');
-        $span->tag('test_key', 'test_value');
-        $span->finish($timestamp + 100);
-
-        $expectedSpanArray = [
-            'id' => $spanId,
-            'kind' => 'CLIENT',
-            'traceId' => $traceId,
-            'parentId' => $parentId,
-            'timestamp' => $timestamp,
-            'name' => 'test_name',
-            'duration' => 100,
-            'localEndpoint' => [
-                'serviceName' => 'test_service_name',
-                'ipv4' => '127.0.0.1',
-                'port' => 3333,
-            ],
-            'annotations' => [
-                [
-                    'value' => 'test_annotation',
-                    'timestamp' => $timestamp,
-                ],
-            ],
-            'tags' => [
-                'test_key' => 'test_value',
-            ],
-        ];
-
-        $this->assertEquals($expectedSpanArray, $span->toArray());
-    }
 }