Merge pull request #123 from jans-jeroen/feature/add_missing_tags

Add missing tags when using Jaeger.thrift dispatch modes
jonahgeorge · Jul 19, 2022 · 9135beb · 9135beb
2 parents 3d56b89 + 8095fb7
commit 9135beb
Show file tree

Hide file tree

Showing 3 changed files with 147 additions and 10 deletions.
diff --git a/src/Jaeger/Mapper/SpanToJaegerMapper.php b/src/Jaeger/Mapper/SpanToJaegerMapper.php
@@ -10,6 +10,10 @@
 use Jaeger\Thrift\Tag;
 use Jaeger\Thrift\TagType;
 use const OpenTracing\Tags\COMPONENT;
+use const OpenTracing\Tags\PEER_HOST_IPV4;
+use const OpenTracing\Tags\PEER_PORT;
+use const OpenTracing\Tags\PEER_SERVICE;
+use const OpenTracing\Tags\SPAN_KIND;
 
 class SpanToJaegerMapper
 {
@@ -47,6 +51,43 @@ public function mapSpanToJaeger(Span $span) : JaegerThriftSpan
             "vStr" => $span->getComponent() ?? $span->getTracer()->getServiceName(),
         ]);
 
+        // Handle special tags
+        $peerService = $span->peer['service_name'] ?? null;
+        if ($peerService !== null) {
+            $tags[] = new Tag([
+                "key" => PEER_SERVICE,
+                "vType" => TagType::STRING,
+                "vStr" => $peerService,
+            ]);
+        }
+
+        $peerHostIpv4 = $span->peer['ipv4'] ?? null;
+        if ($peerHostIpv4 !== null) {
+            $tags[] = new Tag([
+                "key" => PEER_HOST_IPV4,
+                "vType" => TagType::STRING,
+                "vStr" => $peerHostIpv4,
+            ]);
+        }
+
+        $peerPort = $span->peer['port'] ?? null;
+        if ($peerPort !== null) {
+            $tags[] = new Tag([
+                "key" => PEER_PORT,
+                "vType" => TagType::LONG,
+                "vLong" => $peerPort,
+            ]);
+        }
+
+        $spanKind = $span->getKind();
+        if ($spanKind !== null) {
+            $tags[] = new Tag([
+                "key" => SPAN_KIND,
+                "vType" => TagType::STRING,
+                "vStr" => $spanKind,
+            ]);
+        }
+
         /** @var BinaryAnnotation[] $binaryAnnotationTags */
         $binaryAnnotationTags = $span->getTags();
         foreach ($binaryAnnotationTags as $binaryAnnotationTag) {

diff --git a/src/Jaeger/Span.php b/src/Jaeger/Span.php
@@ -13,6 +13,8 @@
 use const OpenTracing\Tags\PEER_PORT;
 use const OpenTracing\Tags\PEER_SERVICE;
 use const OpenTracing\Tags\SPAN_KIND;
+use const OpenTracing\Tags\SPAN_KIND_MESSAGE_BUS_CONSUMER;
+use const OpenTracing\Tags\SPAN_KIND_MESSAGE_BUS_PRODUCER;
 use const OpenTracing\Tags\SPAN_KIND_RPC_CLIENT;
 use const OpenTracing\Tags\SPAN_KIND_RPC_SERVER;
 
@@ -294,13 +296,28 @@ private function setComponent($value): bool
      */
     private function setSpanKind($value): bool
     {
-        if ($value === null || $value === SPAN_KIND_RPC_CLIENT || $value === SPAN_KIND_RPC_SERVER) {
+        $validSpanKinds = [
+            SPAN_KIND_RPC_CLIENT,
+            SPAN_KIND_RPC_SERVER,
+            SPAN_KIND_MESSAGE_BUS_CONSUMER,
+            SPAN_KIND_MESSAGE_BUS_PRODUCER,
+        ];
+
+        if ($value === null || in_array($value, $validSpanKinds, true)) {
             $this->kind = $value;
             return true;
         }
         return false;
     }
 
+    /**
+     * @return string|null
+     */
+    public function getKind(): ?string
+    {
+        return $this->kind;
+    }
+
     /**
      * @return bool
      */

diff --git a/tests/Jaeger/Mapper/SpanToJaegerMapperTest.php b/tests/Jaeger/Mapper/SpanToJaegerMapperTest.php
@@ -1,12 +1,19 @@
 <?php
 
-
+use Jaeger\Mapper\SpanToJaegerMapper;
 use Jaeger\Reporter\NullReporter;
 use Jaeger\Sampler\ConstSampler;
 use Jaeger\Span;
 use Jaeger\SpanContext;
+use Jaeger\Thrift\TagType;
 use Jaeger\Tracer;
 use const Jaeger\SAMPLED_FLAG;
+use const OpenTracing\Tags\COMPONENT;
+use const OpenTracing\Tags\PEER_HOST_IPV4;
+use const OpenTracing\Tags\PEER_PORT;
+use const OpenTracing\Tags\PEER_SERVICE;
+use const OpenTracing\Tags\SPAN_KIND;
+use const OpenTracing\Tags\SPAN_KIND_RPC_CLIENT;
 
 class SpanToJaegerMapperTest extends \PHPUnit\Framework\TestCase
 {
@@ -27,7 +34,7 @@ class SpanToJaegerMapperTest extends \PHPUnit\Framework\TestCase
     public function setUp(): void
     {
         $this->tracer = new Tracer($this->serviceName, new NullReporter, new ConstSampler);
-        $this->context = new SpanContext(0, 0,0, SAMPLED_FLAG);
+        $this->context = new SpanContext(0, 0, 0, SAMPLED_FLAG);
     }
 
     /**
@@ -51,40 +58,112 @@ public function shouldProperlyInitializeAtConstructTime(): void
             "tag-string" => "hello-world"
         ]);
 
-        $mapper = new \Jaeger\Mapper\SpanToJaegerMapper();
+        $mapper = new SpanToJaegerMapper();
         $thriftSpan = $mapper->mapSpanToJaeger($span);
 
         $index = 0;
         $this->assertEquals($thriftSpan->tags[$index]->key, "component");
-        $this->assertEquals($thriftSpan->tags[$index]->vType, \Jaeger\Thrift\TagType::STRING);
+        $this->assertEquals($thriftSpan->tags[$index]->vType, TagType::STRING);
         $this->assertEquals($thriftSpan->tags[$index]->vStr, $this->serviceName);
         $index++;
 
         $this->assertEquals($thriftSpan->tags[$index]->key, "tag-bool1");
-        $this->assertEquals($thriftSpan->tags[$index]->vType, \Jaeger\Thrift\TagType::BOOL);
+        $this->assertEquals($thriftSpan->tags[$index]->vType, TagType::BOOL);
         $this->assertEquals($thriftSpan->tags[$index]->vBool, true);
         $index++;
 
         $this->assertEquals($thriftSpan->tags[$index]->key, "tag-bool2");
-        $this->assertEquals($thriftSpan->tags[$index]->vType, \Jaeger\Thrift\TagType::BOOL);
+        $this->assertEquals($thriftSpan->tags[$index]->vType, TagType::BOOL);
         $this->assertEquals($thriftSpan->tags[$index]->vBool, false);
         $index++;
 
         $this->assertEquals($thriftSpan->tags[$index]->key, "tag-int");
-        $this->assertEquals($thriftSpan->tags[$index]->vType, \Jaeger\Thrift\TagType::LONG);
+        $this->assertEquals($thriftSpan->tags[$index]->vType, TagType::LONG);
         $this->assertEquals($thriftSpan->tags[$index]->vLong, 1234567);
         $index++;
 
         $this->assertEquals($thriftSpan->tags[$index]->key, "tag-float");
-        $this->assertEquals($thriftSpan->tags[$index]->vType, \Jaeger\Thrift\TagType::DOUBLE);
+        $this->assertEquals($thriftSpan->tags[$index]->vType, TagType::DOUBLE);
         $this->assertEquals($thriftSpan->tags[$index]->vDouble, 1.23456);
         $index++;
 
         $this->assertEquals($thriftSpan->tags[$index]->key, "tag-string");
-        $this->assertEquals($thriftSpan->tags[$index]->vType, \Jaeger\Thrift\TagType::STRING);
+        $this->assertEquals($thriftSpan->tags[$index]->vType, TagType::STRING);
         $this->assertEquals($thriftSpan->tags[$index]->vStr, "hello-world");
         $index++;
+    }
+
+    /**
+     * @dataProvider specialTagProvider
+     * @param array<string, mixed> $tags
+     * @return void
+     */
+    public function testSpecialTagsAreAdded(array $tags): void
+    {
+        $span = new Span($this->context, $this->tracer, 'test-operation');
+        $span->setTags($tags);
+
+        // The component tag is always added, even if it's not specified in tags
+        $expectedTagValues = array_merge([COMPONENT => $this->serviceName], $tags);
+
+        $mapper = new SpanToJaegerMapper();
+        $thriftSpan = $mapper->mapSpanToJaeger($span);
 
+        $foundTags = [];
+
+        foreach ($thriftSpan->tags as $tag) {
+            $foundTags[] = $tag->key;
+
+            switch ($tag->key) {
+                case PEER_SERVICE:
+                case PEER_HOST_IPV4:
+                case SPAN_KIND:
+                case COMPONENT:
+                    $this->assertEquals(TagType::STRING, $tag->vType, 'Incorrect tag value type');
+                    $this->assertEquals($expectedTagValues[$tag->key], $tag->vStr, 'Incorrect tag value');
+                    break;
+                case PEER_PORT:
+                    $this->assertEquals(TagType::LONG, $tag->vType, 'Incorrect tag value type');
+                    $this->assertEquals($expectedTagValues[$tag->key], $tag->vLong, 'Incorrect tag value');
+                    break;
+            }
+        }
+
+        $this->assertEqualsCanonicalizing(array_keys($expectedTagValues), $foundTags, 'Some of the tags are missing');
     }
 
+    public function specialTagProvider(): array
+    {
+        return [
+            [
+                [
+                    'bool_tag' => true,
+                    PEER_SERVICE => 'my_service',
+                    PEER_HOST_IPV4 => '127.0.0.1',
+                    PEER_PORT => 443,
+                    SPAN_KIND => SPAN_KIND_RPC_CLIENT,
+                    COMPONENT => 'grpc',
+                ],
+            ],
+            [
+                [
+                    'int_tag' => 5,
+                    PEER_HOST_IPV4 => '192.168.0.1',
+                    PEER_PORT => 80,
+                ],
+            ],
+            [
+                [
+                    'string_tag' => 'testing-tag',
+                    PEER_PORT => 80,
+                    COMPONENT => 'grpc',
+                ],
+            ],
+            [
+                [
+                    'string_tag' => 'testing-tag',
+                ],
+            ],
+        ];
+    }
 }