From 39c1132e3fd798addb521897e9c53c2d40387481 Mon Sep 17 00:00:00 2001
From: Jonah George <jonah.george@me.com>
Date: Thu, 17 May 2018 08:54:54 -0400
Subject: [PATCH] [Fixes #24] Correctly set tags for RPC calls

---
 src/Jaeger/LocalAgentSender.php | 42 +++++++++++++++++++++++++++++----
 src/Jaeger/Span.php             | 20 +++++++++++++---
 src/Jaeger/SpanContext.php      |  4 ++--
 3 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/src/Jaeger/LocalAgentSender.php b/src/Jaeger/LocalAgentSender.php
index b0753e0..fe8e260 100644
--- a/src/Jaeger/LocalAgentSender.php
+++ b/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
+        ]);
+    }
 }
diff --git a/src/Jaeger/Span.php b/src/Jaeger/Span.php
index fec998d..f486cb6 100644
--- a/src/Jaeger/Span.php
+++ b/src/Jaeger/Span.php
@@ -30,10 +30,14 @@ 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;
 
@@ -182,9 +186,9 @@ public function setTag($key, $value): Span
 
         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);
             }
 
@@ -250,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
      *
diff --git a/src/Jaeger/SpanContext.php b/src/Jaeger/SpanContext.php
index 493c487..4847e15 100644
--- a/src/Jaeger/SpanContext.php
+++ b/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;
     }