From af730832eb788cb38daaed176c046eb2ec3e5251 Mon Sep 17 00:00:00 2001 From: Jonah George Date: Thu, 17 May 2018 08:40:14 -0400 Subject: [PATCH 1/2] [Fixes #22] Span tags overwrite for the same key --- src/Jaeger/Span.php | 9 ++++----- tests/SpanTest.php | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 tests/SpanTest.php diff --git a/src/Jaeger/Span.php b/src/Jaeger/Span.php index 98e7537..fec998d 100644 --- a/src/Jaeger/Span.php +++ b/src/Jaeger/Span.php @@ -40,7 +40,7 @@ class Span implements OpenTracing\Span private $logs; /** @var BinaryAnnotation[] */ - public $tags; + public $tags = []; /** @var bool */ private $debug = false; @@ -63,7 +63,6 @@ public function __construct( $this->peer = null; $this->component = null; - $this->tags = []; $this->logs = []; foreach ($tags as $key => $value) { $this->setTag($key, $value); @@ -176,7 +175,7 @@ public function setTags(array $tags) } } - public function setTag($key, $value) + public function setTag($key, $value): Span { // if ($key == SAMPLING_PRIORITY) { // } @@ -191,7 +190,7 @@ public function setTag($key, $value) if (!$handled) { $tag = $this->makeStringTag($key, (string) $value); - $this->tags[] = $tag; + $this->tags[$key] = $tag; } } @@ -297,7 +296,7 @@ public function __toString(): string ); } - public function getTags() + public function getTags(): array { return $this->tags; } diff --git a/tests/SpanTest.php b/tests/SpanTest.php new file mode 100644 index 0000000..eeaf434 --- /dev/null +++ b/tests/SpanTest.php @@ -0,0 +1,33 @@ +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); + } +} \ No newline at end of file From 39c1132e3fd798addb521897e9c53c2d40387481 Mon Sep 17 00:00:00 2001 From: Jonah George Date: Thu, 17 May 2018 08:54:54 -0400 Subject: [PATCH 2/2] [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; }