Merge pull request #65 from jcchavezs/guards_malformed_propagated_con…

…text Guards for not to fail when receiving malformed trace information.
openzipkin · Mar 6, 2018 · 17caf54 · 17caf54
2 parents d8847d2 + 525dac3
commit 17caf54
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 16 deletions.
diff --git a/src/Zipkin/Propagation/B3.php b/src/Zipkin/Propagation/B3.php
@@ -2,6 +2,10 @@
 
 namespace Zipkin\Propagation;
 
+use InvalidArgumentException;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+use Zipkin\Propagation\Exceptions\InvalidTraceContextArgument;
 use Zipkin\Propagation\TraceContext;
 
 /**
@@ -34,7 +38,17 @@ final class B3 implements Propagation
      * '1' implies sampled and is a request to override collection-tier sampling policy.
      */
     const FLAGS_NAME = 'X-B3-Flags';
-
+
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
+    public function __construct(LoggerInterface $logger = null)
+    {
+        $this->logger = $logger ?: new NullLogger();
+    }
+
     /**
      * @return array|string[]
      */
@@ -83,7 +97,6 @@ public function getExtractor(Getter $getter)
         /**
          * @param mixed $carrier
          * @return TraceContext|SamplingFlags
-         * @throws \InvalidArgumentException
          */
         return function ($carrier) use ($getter) {
             $sampledString = $getter->get($carrier, self::SAMPLED_NAME);
@@ -116,7 +129,16 @@ public function getExtractor(Getter $getter)
 
             $parentSpanId = $getter->get($carrier, self::PARENT_SPAN_ID_NAME);
 
-            return TraceContext::create($traceId, $spanId, $parentSpanId, $isSampled, $isDebug);
+            try {
+                return TraceContext::create($traceId, $spanId, $parentSpanId, $isSampled, $isDebug);
+            } catch (InvalidTraceContextArgument $e) {
+                $this->logger->debug(sprintf(
+                    'Failed to extract propagated context: %s',
+                    $e->getMessage()
+                ));
+
+                return DefaultSamplingFlags::createAsEmpty();
+            }
         };
     }
 }
diff --git a/src/Zipkin/Propagation/Exceptions/InvalidTraceContextArgument.php b/src/Zipkin/Propagation/Exceptions/InvalidTraceContextArgument.php
@@ -0,0 +1,33 @@
+<?php
+
+namespace Zipkin\Propagation\Exceptions;
+
+use InvalidArgumentException;
+
+final class InvalidTraceContextArgument extends InvalidArgumentException
+{
+    public static function forTraceId($traceId)
+    {
+        return new self(sprintf('Invalid trace id, got %s', $traceId));
+    }
+
+    public static function forSpanId($spanId)
+    {
+        return new self(sprintf('Invalid span id, got %s', $spanId));
+    }
+
+    public static function forParentSpanId($parentId)
+    {
+        return new self(sprintf('Invalid parent span id, got %s', $parentId));
+    }
+
+    public static function forSampled($isSampled)
+    {
+        return new self(sprintf('is Sampled should be boolean or null, got %s', gettype($isSampled)));
+    }
+
+    public static function forDebug($isDebug)
+    {
+        return new self(sprintf('isDebug should be boolean, got %s', gettype($isDebug)));
+    }
+}
diff --git a/src/Zipkin/Propagation/TraceContext.php b/src/Zipkin/Propagation/TraceContext.php
@@ -2,9 +2,9 @@
 
 namespace Zipkin\Propagation;
 
-use InvalidArgumentException;
 use Zipkin\Propagation\DefaultSamplingFlags;
 use Zipkin\Propagation\SamplingFlags;
+use Zipkin\Propagation\Exceptions\InvalidTraceContextArgument;
 
 final class TraceContext implements SamplingFlags
 {
@@ -56,7 +56,7 @@ private function __construct($traceId, $spanId, $parentId, $isSampled, $isDebug,
      * @param bool $isDebug
      * @param bool $usesTraceId128bits
      * @return TraceContext
-     * @throws \InvalidArgumentException
+     * @throws InvalidTraceContextArgument
      */
     public static function create(
         $traceId,
@@ -67,25 +67,23 @@ public static function create(
         $usesTraceId128bits = false
     ) {
         if (!Id\isValidTraceId($traceId)) {
-            throw new InvalidArgumentException(sprintf('Invalid trace id, got %s', $traceId));
+            throw InvalidTraceContextArgument::forTraceId($traceId);
         }
 
         if (!Id\isValidSpanId($spanId)) {
-            throw new InvalidArgumentException(sprintf('Invalid span id, got %s', $spanId));
+            throw InvalidTraceContextArgument::forSpanId($spanId);
         }
 
         if ($parentId !== null && !Id\isValidSpanId($parentId)) {
-            throw new InvalidArgumentException(sprintf('Invalid parent span id, got %s', $parentId));
+            throw InvalidTraceContextArgument::forParentSpanId($parentId);
         }
 
         if ($isSampled !== null && $isSampled !== (bool) $isSampled) {
-            throw new InvalidArgumentException(
-                sprintf('is Sampled should be boolean or null, got %s', gettype($isSampled))
-            );
+            throw InvalidTraceContextArgument::forSampled($isSampled);
         }
 
         if ($isDebug !== (bool) $isDebug) {
-            throw new InvalidArgumentException(sprintf('isDebug should be boolean, got %s', gettype($isDebug)));
+            throw InvalidTraceContextArgument::forDebug($isDebug);
         }
 
         return new self($traceId, $spanId, $parentId, $isSampled, $isDebug, $usesTraceId128bits);

diff --git a/tests/Unit/Propagation/TraceContextTest.php b/tests/Unit/Propagation/TraceContextTest.php
@@ -2,8 +2,8 @@
 
 namespace ZipkinTests\Unit\Propagation;
 
-use InvalidArgumentException;
 use PHPUnit_Framework_TestCase;
+use Zipkin\Propagation\Exceptions\InvalidTraceContextArgument;
 use Zipkin\Propagation\DefaultSamplingFlags;
 use Zipkin\Propagation\TraceContext;
 
@@ -115,7 +115,7 @@ public function testChangeSampledSuccess($sampled, $debug)
      */
     public function testCreateFailsDueToInvalidId($sampled, $debug)
     {
-        $this->expectException(InvalidArgumentException::class);
+        $this->expectException(InvalidTraceContextArgument::class);
         $this->expectExceptionMessage('Invalid trace id, got invalid_bd7a977555f6b982');
 
         TraceContext::create(
@@ -132,7 +132,7 @@ public function testCreateFailsDueToInvalidId($sampled, $debug)
      */
     public function testCreateFailsDueToInvalidSpanId($sampled, $debug)
     {
-        $this->expectException(InvalidArgumentException::class);
+        $this->expectException(InvalidTraceContextArgument::class);
         $this->expectExceptionMessage('Invalid span id, got invalid_be2d01e33cc78d97');
 
         TraceContext::create(
@@ -149,7 +149,7 @@ public function testCreateFailsDueToInvalidSpanId($sampled, $debug)
      */
     public function testCreateFailsDueToInvalidParentSpanId($sampled, $debug)
     {
-        $this->expectException(InvalidArgumentException::class);
+        $this->expectException(InvalidTraceContextArgument::class);
         $this->expectExceptionMessage('Invalid parent span id, got invalid_bd7a977555f6b982');
 
         TraceContext::create(