Merge pull request #33 from jcchavezs/simplfy_propagation

chore: refactors the propagation bootstrapping to make it simpler and…
jcchavezs · May 19, 2020 · 565f3be · 565f3be
2 parents 66fa610 + fe0780f
commit 565f3be
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 49 deletions.
diff --git a/src/ZipkinOpenTracing/Tracer.php b/src/ZipkinOpenTracing/Tracer.php
@@ -2,25 +2,23 @@
 
 namespace ZipkinOpenTracing;
 
+use Zipkin\Timestamp;
 use OpenTracing\Formats;
 use OpenTracing\Reference;
-use OpenTracing\SpanContext as OTSpanContext;
+use Zipkin\Propagation\Map;
 use OpenTracing\StartSpanOptions;
+use Zipkin\Tracer as ZipkinTracer;
 use OpenTracing\Tracer as OTTracer;
-use Zipkin\Propagation\Getter;
-use Zipkin\Propagation\Map;
-use Zipkin\Propagation\Propagation as ZipkinPropagation;
-use Zipkin\Propagation\RequestHeaders;
-use Zipkin\Propagation\SamplingFlags;
-use Zipkin\Propagation\Setter;
 use Zipkin\Propagation\TraceContext;
-use Zipkin\Timestamp;
-use Zipkin\Tracer as ZipkinTracer;
 use Zipkin\Tracing as ZipkinTracing;
-use ZipkinOpenTracing\PartialSpanContext as ZipkinOpenPartialTracingContext;
+use Zipkin\Propagation\SamplingFlags;
+use Zipkin\Propagation\RequestHeaders;
+use OpenTracing\Exceptions\UnsupportedFormat;
+use OpenTracing\SpanContext as OTSpanContext;
 use ZipkinOpenTracing\Span as ZipkinOpenTracingSpan;
 use ZipkinOpenTracing\NoopSpan as ZipkinOpenTracingNoopSpan;
 use ZipkinOpenTracing\SpanContext as ZipkinOpenTracingContext;
+use ZipkinOpenTracing\PartialSpanContext as ZipkinOpenPartialTracingContext;
 
 final class Tracer implements OTTracer
 {
@@ -30,14 +28,28 @@ final class Tracer implements OTTracer
     private $tracer;
 
     /**
-     * @var ZipkinPropagation
+     * @var callable[]|array
+     */
+    private $injectors;
+
+    /**
+     * @var callable[]|array
      */
-    private $propagation;
+    private $extractors;
 
     public function __construct(ZipkinTracing $tracing)
     {
+        $propagation = $tracing->getPropagation();
+        $this->injectors = [
+            Formats\TEXT_MAP => $propagation->getInjector(new Map()),
+            Formats\HTTP_HEADERS => $propagation->getInjector(new RequestHeaders())
+        ];
+        $this->extractors = [
+            Formats\TEXT_MAP => $propagation->getExtractor(new Map()),
+            Formats\HTTP_HEADERS => $propagation->getExtractor(new RequestHeaders())
+        ];
+
         $this->tracer = $tracing->getTracer();
-        $this->propagation = $tracing->getPropagation();
         $this->scopeManager = new ScopeManager();
     }
 
@@ -137,8 +149,7 @@ public function startSpan($operationName, $options = [])
     public function inject(OTSpanContext $spanContext, $format, &$carrier)
     {
         if ($spanContext instanceof ZipkinOpenTracingContext) {
-            $setter = $this->getSetterByFormat($format);
-            $injector = $this->propagation->getInjector($setter);
+            $injector = $this->getInjector($format);
             return $injector($spanContext->getContext(), $carrier);
         }
 
@@ -154,8 +165,7 @@ public function inject(OTSpanContext $spanContext, $format, &$carrier)
      */
     public function extract($format, $carrier)
     {
-        $getter = $this->getGetterByFormat($format);
-        $extractor =  $this->propagation->getExtractor($getter);
+        $extractor = $this->getExtractor($format);
         $extractedContext = $extractor($carrier);
 
         if ($extractedContext instanceof TraceContext) {
@@ -182,38 +192,30 @@ public function flush()
 
     /**
      * @param string $format
-     * @return Setter
-     * @throws \UnexpectedValueException
+     * @return callable
+     * @throws UnsupportedFormat
      */
-    private function getSetterByFormat($format)
+    private function getInjector($format)
     {
-        if ($format === Formats\TEXT_MAP) {
-            return new Map();
-        }
-
-        if ($format === Formats\HTTP_HEADERS) {
-            return new RequestHeaders();
+        if (array_key_exists($format, $this->injectors)) {
+            return $this->injectors[$format];
         }
 
-        throw new \UnexpectedValueException(\sprintf('Format %s not implemented', $format));
+        throw new UnsupportedFormat(\sprintf('Format %s not implemented', $format));
     }
 
     /**
      * @param string $format
-     * @return Getter
-     * @throws \UnexpectedValueException
+     * @return callable
+     * @throws UnsupportedFormat
      */
-    private function getGetterByFormat($format)
+    private function getExtractor($format)
     {
-        if ($format === Formats\TEXT_MAP) {
-            return new Map();
-        }
-
-        if ($format === Formats\HTTP_HEADERS) {
-            return new RequestHeaders();
+        if (array_key_exists($format, $this->extractors)) {
+            return $this->extractors[$format];
         }
 
-        throw new \UnexpectedValueException(\sprintf('Format %s not implemented', $format));
+        throw new UnsupportedFormat($format);
     }
 
     private function hasParentInOptions(StartSpanOptions $options)

diff --git a/tests/Unit/TracerTest.php b/tests/Unit/TracerTest.php
@@ -2,20 +2,21 @@
 
 namespace ZipkinOpenTracing\Tests\Unit;
 
-use PHPUnit_Framework_TestCase;
-use Prophecy\Argument;
-use Prophecy\Argument\Token\AnyValuesToken;
-use Zipkin\Propagation\DefaultSamplingFlags;
 use Zipkin\Sampler;
+use Prophecy\Argument;
+use OpenTracing\Formats;
 use Zipkin\TracingBuilder;
-use ZipkinOpenTracing\NoopSpan;
-use ZipkinOpenTracing\PartialSpanContext;
 use ZipkinOpenTracing\Span;
-use ZipkinOpenTracing\SpanContext;
 use ZipkinOpenTracing\Tracer;
-use OpenTracing\Formats;
-use Zipkin\Propagation\TraceContext;
+use PHPUnit_Framework_TestCase;
+use ZipkinOpenTracing\NoopSpan;
 use Zipkin\Samplers\BinarySampler;
+use ZipkinOpenTracing\SpanContext;
+use Zipkin\Propagation\TraceContext;
+use ZipkinOpenTracing\PartialSpanContext;
+use Prophecy\Argument\Token\AnyValuesToken;
+use Zipkin\Propagation\DefaultSamplingFlags;
+use OpenTracing\Exceptions\UnsupportedFormat;
 
 final class TracerTest extends PHPUnit_Framework_TestCase
 {
@@ -129,7 +130,7 @@ public function testGetActiveSpan()
         $this->assertEquals($expectedSpan, $actualSpan);
     }
 
-    public function testExtractContextHeaderSuccess()
+    public function testExtractContextFromRequestHeadersSuccess()
     {
         $tracing = TracingBuilder::create()->build();
         $tracer = new Tracer($tracing);
@@ -153,18 +154,30 @@ public function testExtractContextHeaderSuccess()
         );
     }
 
+    public function testInjectContextWithUnkownFormatFails()
+    {
+        $this->expectException(UnsupportedFormat::class);
+        $tracing = TracingBuilder::create()->build();
+        $tracer = new Tracer($tracing);
+        $span = $tracer->startSpan("test");
+
+        $headers = new Request();
+        $tracer->inject($span->getContext(), 'unknown_format', $headers);
+    }
+
     /**
      * @dataProvider samplers
      */
-    public function testInjectContextHeaderSuccess(Sampler $sampler)
+    public function testInjectContextToRequestHeadersSuccess(Sampler $sampler)
     {
         $tracing = TracingBuilder::create()->havingSampler($sampler)->build();
         $tracer = new Tracer($tracing);
         $span = $tracer->startSpan("test");
 
         $headers = new Request();
         $tracer->inject($span->getContext(), Formats\HTTP_HEADERS, $headers);
-        $headers->hasHeader('x-trace-id');
+        $this->assertTrue($headers->hasHeader('x-b3-traceid'));
+        $this->assertTrue($headers->hasHeader('x-b3-spanid'));
     }
 
     public function samplers()