Merge pull request #25 from jonahgeorge/bugfixes

Bugfixes

src/Jaeger/LocalAgentSender.php

@@ -7,12 +7,16 @@
 use Jaeger\ThriftGen\BinaryAnnotation;
 use Jaeger\ThriftGen\Endpoint;
 use Jaeger\ThriftGen\Span;
+use const OpenTracing\Ext\Tags\COMPONENT;
 use Thrift\Protocol\TCompactProtocol;
 use Thrift\Transport\TBufferedTransport;
 use Thrift\Transport\TSocket;
 
 class LocalAgentSender
 {
+    const CLIENT_ADDR = "ca";
+    const SERVER_ADDR = "sa";
+
     /** @var Span[] */
     private $spans = [];
 
@@ -131,12 +135,28 @@ private function makeZipkinBatch(array $spans): array
 
     private function addZipkinAnnotations(\Jaeger\Span $span, $endpoint)
     {
-        $tag = $this->makeLocalComponentTag(
-            $span->getComponent() ?? $span->getTracer()->getServiceName(),
-            $endpoint
-        );
+        if ($span->isRpc()) {
+            $isClient = $span->isRpcClient();
+
+            if ($span->peer) {
+                $host = $this->makeEndpoint(
+                    $span->peer['ipv4'] ?? 0,
+                    $span->peer['port'] ?? 0,
+                    $span->peer['service_name'] ?? '');
+
+                $key = ($isClient) ? self::SERVER_ADDR : self::CLIENT_ADDR;
+
+                $peer = $this->makePeerAddressTag($key, $host);
+                $span->tags[$key] = $peer;
+            }
+        } else {
+            $tag = $this->makeLocalComponentTag(
+                $span->getComponent() ?? $span->getTracer()->getServiceName(),
+                $endpoint
+            );
 
-        $span->tags[] = $tag;
+            $span->tags[COMPONENT] = $tag;
+        }
     }
 
     private function makeLocalComponentTag(string $componentName, $endpoint): BinaryAnnotation
@@ -170,4 +190,16 @@ private function ipv4ToInt($ipv4): int
 
         return ip2long($ipv4);
     }
+
+    // Used for Zipkin binary annotations like CA/SA (client/server address).
+    // They are modeled as Boolean type with '0x01' as the value.
+    private function makePeerAddressTag($key, $host)
+    {
+        return new BinaryAnnotation([
+            "key" => $key,
+            "value" => '0x01',
+            "annotation_type" => AnnotationType::BOOL,
+            "host" => $host
+        ]);
+    }
 }

src/Jaeger/Span.php

@@ -30,17 +30,21 @@ class Span implements OpenTracing\Span
     /** @var float */
     private $endTime;
 
+    /**
+     * SPAN_RPC_CLIENT
+     * @var null|string
+     */
     private $kind;
 
     /** @var array|null */
-    private $peer;
+    public $peer;
 
     private $component;
 
     private $logs;
 
     /** @var BinaryAnnotation[] */
-    public $tags;
+    public $tags = [];
 
     /** @var bool */
     private $debug = false;
@@ -63,7 +67,6 @@ public function __construct(
         $this->peer = null;
         $this->component = null;
 
-        $this->tags = [];
         $this->logs = [];
         foreach ($tags as $key => $value) {
             $this->setTag($key, $value);
@@ -176,22 +179,22 @@ public function setTags(array $tags)
         }
     }
 
-    public function setTag($key, $value)
+    public function setTag($key, $value): Span
     {
 //        if ($key == SAMPLING_PRIORITY) {
 //        }
 
         if ($this->isSampled()) {
             $special = self::SPECIAL_TAGS[$key] ?? null;
-            $handled = False;
+            $handled = false;
 
-            if ($special !== null && is_callable($special)) {
+            if ($special !== null && is_callable([$this, $special])) {
                 $handled = $this->$special($value);
             }
 
             if (!$handled) {
                 $tag = $this->makeStringTag($key, (string) $value);
-                $this->tags[] = $tag;
+                $this->tags[$key] = $tag;
             }
         }
 
@@ -251,6 +254,16 @@ private function setPeerService($value): bool
         return true;
     }
 
+    public function isRpc(): bool
+    {
+        return $this->kind == SPAN_KIND_RPC_CLIENT || $this->kind == SPAN_KIND_RPC_SERVER;
+    }
+
+    public function isRpcClient(): bool
+    {
+        return $this->kind == SPAN_KIND_RPC_CLIENT;
+    }
+
     /**
      * Adds a log record to the span
      *
@@ -297,7 +310,7 @@ public function __toString(): string
         );
     }
 
-    public function getTags()
+    public function getTags(): array
     {
         return $this->tags;
     }

src/Jaeger/SpanContext.php

@@ -14,13 +14,13 @@ class SpanContext implements OpenTracing\SpanContext
     private $baggage;
     private $debugId;
 
-    public function __construct($traceId, $spanId, $parentId, $flags, $baggage = null)
+    public function __construct($traceId, $spanId, $parentId, $flags, $baggage = [])
     {
         $this->traceId = $traceId;
         $this->spanId = $spanId;
         $this->parentId = $parentId;
         $this->flags = $flags;
-        $this->baggage = $baggage ?? [];
+        $this->baggage = $baggage;
         $this->debugId = null;
     }
 

tests/SpanTest.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace Jaeger;
+
+use Jaeger\Reporter\NullReporter;
+use Jaeger\Sampler\ConstSampler;
+use PHPUnit\Framework\TestCase;
+
+class SpanTest extends TestCase
+{
+    private $tracer;
+    private $context;
+
+    function setUp()
+    {
+        $this->tracer = new Tracer('test-service', new NullReporter, new ConstSampler);
+        $this->context = new SpanContext(0, 0,0, SAMPLED_FLAG);
+    }
+
+    function testSetTag_TagsKeysAreUnique()
+    {
+        // Given
+        $span = new Span($this->context, $this->tracer, 'test-operation');
+
+        // When
+        $span->setTag('foo', 'test-component-1');
+        $span->setTag('foo', 'test-component-2');
+
+        // Then
+        $this->assertEquals( 1, count($span->getTags()));
+        $this->assertEquals( 'test-component-2', $span->getTags()['foo']->value);
+    }
+}