Tune up the Samplers

.gitignore

@@ -1,7 +1,9 @@
-.settings
-.buildpath
-.project
-.idea
+# Please do not use this ignore file to define platform specific files.
+#
+# For these purposes create a global .gitignore file, which is a list of rules
+# for ignoring files in every Git repository on your computer.
+#
+# https://help.github.com/articles/ignoring-files/#create-a-global-gitignore
 
 jaeger-idl
 vendor

README.md

@@ -2,20 +2,20 @@
 
 # Jaeger Bindings for PHP OpenTracing API
 
-This is a client-side library that can be used to instrument PHP apps for distributed trace collection, and to send those traces to Jaeger. See the [OpenTracing PHP API](https://github.com/opentracing/opentracing-php) for additional detail.
+This is a client-side library that can be used to instrument PHP apps for distributed trace collection,
+and to send those traces to Jaeger. See the [OpenTracing PHP API](https://github.com/opentracing/opentracing-php)
+for additional detail.
 
 ## Contributing and Developing
 
 Please see [CONTRIBUTING.md](./CONTRIBUTING.md).
 
 ## Installation
 
-```json
-{
-  "require": {
-    "jonahgeorge/jaeger-client-php": "0.3.0"
-  }
-}
+Install [Composer](https://getcomposer.org) in a common location or in your project. Then run the installer as follows:
+
+```bash
+php composer.phar require jonahgeorge/jaeger-client-php
 ```
 
 ## Getting Started
@@ -54,7 +54,6 @@ Tests are located in the `tests` directory. See [tests/README.md](./tests/README
 
 ## Roadmap
 
-- [Support Span logging](https://github.com/jonahgeorge/jaeger-client-php/issues/1)
 - [Support Span baggage](https://github.com/jonahgeorge/jaeger-client-php/issues/5)
 - [Support PHP Version 5.6](https://github.com/jonahgeorge/jaeger-client-php/issues/11)
 - [Support Tracer metrics](https://github.com/jonahgeorge/jaeger-client-php/issues/12)

src/Jaeger/Codec/TextCodec.php

@@ -161,7 +161,7 @@ private function spanContextFromString($value): array
         $parts = explode(':', $value);
 
         if (count($parts) != 4) {
-            throw new Exception('Malformed tracer state string');
+            throw new Exception('Malformed tracer state string.');
         }
 
         return [

src/Jaeger/Config.php

@@ -54,7 +54,7 @@ public function __construct(array $config, string $serviceName = null, LoggerInt
 
         $this->serviceName = $config['service_name'] ?? $serviceName;
         if ($this->serviceName === null) {
-            throw new Exception('service_name required in the config or param');
+            throw new Exception('service_name required in the config or param.');
         }
 
         $this->logger = $logger ?: new NullLogger();

src/Jaeger/Sampler/ConstSampler.php

@@ -8,21 +8,28 @@
 
 /**
  * ConstSampler always returns the same decision.
+ *
+ * @package Jaeger\Sampler
  */
 class ConstSampler implements SamplerInterface
 {
     /**
+     * Whether or not the new trace should be sampled.
+     *
      * @var bool
      */
     private $decision;
 
     /**
+     * A list of the sampler tags.
+     *
      * @var array
      */
     private $tags = [];
 
     /**
      * ConstSampler constructor.
+     *
      * @param bool $decision
      */
     public function __construct(bool $decision = true)
@@ -31,20 +38,31 @@ public function __construct(bool $decision = true)
             SAMPLER_TYPE_TAG_KEY => SAMPLER_TYPE_CONST,
             SAMPLER_PARAM_TAG_KEY => $decision,
         ];
+
         $this->decision = $decision;
     }
 
     /**
-     * @param string $traceId
-     * @param string $operation
+     * {@inheritdoc}
+     *
+     * @param string $traceId   The traceId on the span.
+     * @param string $operation The operation name set on the span.
      * @return array
      */
     public function isSampled(string $traceId, string $operation = ''): array
     {
-        return array($this->decision, $this->tags);
+        return [$this->decision, $this->tags];
     }
 
+    /**
+     * {@inheritdoc}
+     *
+     * Only implemented to satisfy the sampler interface.
+     *
+     * @return void
+     */
     public function close()
     {
+        // nothing to do
     }
 }

src/Jaeger/Sampler/ProbabilisticSampler.php

@@ -2,36 +2,45 @@
 
 namespace Jaeger\Sampler;
 
-use Exception;
+use OutOfBoundsException;
 use const Jaeger\SAMPLER_PARAM_TAG_KEY;
 use const Jaeger\SAMPLER_TYPE_PROBABILISTIC;
 use const Jaeger\SAMPLER_TYPE_TAG_KEY;
 
 /**
  * A sampler that randomly samples a certain percentage of traces specified
  * by the samplingRate, in the range between 0.0 and 1.0.
+ *
+ * @package Jaeger\Sampler
  */
 class ProbabilisticSampler implements SamplerInterface
 {
     /**
+     * The sampling rate rate between 0.0 and 1.0.
+     *
      * @var float
      */
     private $rate;
 
     /**
+     * A list of the sampler tags.
+     *
      * @var array
      */
     private $tags = [];
 
     /**
-     * @var float|int
+     * The boundary of the sample sampling rate.
+     *
+     * @var float
      */
     private $boundary;
 
     /**
      * ProbabilisticSampler constructor.
+     *
      * @param float $rate
-     * @throws Exception
+     * @throws OutOfBoundsException
      */
     public function __construct(float $rate)
     {
@@ -41,24 +50,34 @@ public function __construct(float $rate)
         ];
 
         if ($rate <= 0.0 || $rate >= 1.0) {
-            throw new Exception('Sampling rate must be between 0.0 and 1.0');
+            throw new OutOfBoundsException('Sampling rate must be between 0.0 and 1.0.');
         }
 
         $this->rate = $rate;
         $this->boundary = $rate * PHP_INT_MAX;
     }
 
     /**
-     * @param string $traceId
-     * @param string $operation
+     * {@inheritdoc}
+     *
+     * @param string $traceId   The traceId on the span.
+     * @param string $operation The operation name set on the span.
      * @return array
      */
     public function isSampled(string $traceId, string $operation = ''): array
     {
         return [($traceId < $this->boundary), $this->tags];
     }
 
+    /**
+     * {@inheritdoc}
+     *
+     * Only implemented to satisfy the sampler interface.
+     *
+     * @return void
+     */
     public function close()
     {
+        // nothing to do
     }
 }

src/Jaeger/Sampler/SamplerInterface.php

@@ -2,8 +2,28 @@
 
 namespace Jaeger\Sampler;
 
+/**
+ * Sampler is responsible for deciding if a new trace should be sampled and captured for storage.
+ *
+ * @package Jaeger\Sampler
+ */
 interface SamplerInterface
 {
+    /**
+     * Whether or not the new trace should be sampled.
+     *
+     * Implementations should return an array in the format [$decision, $tags].
+     *
+     * @param string $traceId   The traceId on the span.
+     * @param string $operation The operation name set on the span.
+     * @return array
+     */
     public function isSampled(string $traceId, string $operation);
+
+    /**
+     * Release any resources used by the sampler.
+     *
+     * @return void
+     */
     public function close();
 }

tests/Jaeger/Codec/TextCodecTest.php

@@ -43,7 +43,7 @@ public function testSpanContextParsingFromHeader()
 
     /**
      * @expectedException \Exception
-     * @expectedExceptionMessage Malformed tracer state string
+     * @expectedExceptionMessage Malformed tracer state string.
      */
     public function testInvalidSpanContextParsingFromHeader()
     {

tests/Jaeger/ConfigTest.php

@@ -49,7 +49,7 @@ function testCreateTracer()
 
     /**
      * @expectedException Exception
-     * @expectedExceptionMessage service_name required in the config or param
+     * @expectedExceptionMessage service_name required in the config or param.
      */
     function testThrowExceptionWhenServiceNameIsNotDefined()
     {

tests/Jaeger/Sampler/ConstSamplerTest.php

@@ -4,32 +4,40 @@
 
 use Jaeger\Sampler\ConstSampler;
 use PHPUnit\Framework\TestCase;
+use const Jaeger\SAMPLER_PARAM_TAG_KEY;
+use const Jaeger\SAMPLER_TYPE_CONST;
+use const Jaeger\SAMPLER_TYPE_TAG_KEY;
 
 class ConstSamplerTest extends TestCase
 {
-    private function getTags($type, $param)
+    /**
+     * @test
+     * @dataProvider samplerProvider
+     * @param bool $decision
+     * @param mixed $traceId
+     */
+    public function shouldDetermineWhetherOrTraceShouldBeSampled($decision, $traceId)
     {
-        return [
-            'sampler.type' => $type,
-            'sampler.param' => $param,
-        ];
-    }
+        $sampler = new ConstSampler($decision);
 
-    public function testConstSampler()
-    {
-        $sampler = new ConstSampler(True);
-        list($sampled, $tags) = $sampler->isSampled(1);
-        $this->assertTrue($sampled);
+        list($sampled, $tags) = $sampler->isSampled($traceId);
 
-        list($sampled, $tags) = $sampler->isSampled(PHP_INT_MAX);
-        $this->assertTrue($sampled);
+        $this->assertEquals($decision, $sampled);
+        $this->assertEquals([
+            SAMPLER_TYPE_TAG_KEY  => SAMPLER_TYPE_CONST,
+            SAMPLER_PARAM_TAG_KEY => $decision,
+        ], $tags);
 
-        $sampler = new ConstSampler(False);
-        list($sampled, $tags) = $sampler->isSampled(1);
-        $this->assertFalse($sampled);
+        $sampler->close();
+    }
 
-        list($sampled, $tags) = $sampler->isSampled(PHP_INT_MAX);
-        $this->assertFalse($sampled);
-        $this->assertEquals($tags, $this->getTags('const', False));
+    public function samplerProvider()
+    {
+        return [
+            [true,  1],
+            [true,  PHP_INT_MAX],
+            [false, 1],
+            [false, PHP_INT_MAX],
+        ];
     }
 }

tests/Jaeger/Sampler/ProbablisticSamplerTest.php

@@ -4,27 +4,61 @@
 
 use Jaeger\Sampler\ProbabilisticSampler;
 use PHPUnit\Framework\TestCase;
+use const Jaeger\SAMPLER_PARAM_TAG_KEY;
+use const Jaeger\SAMPLER_TYPE_PROBABILISTIC;
+use const Jaeger\SAMPLER_TYPE_TAG_KEY;
 
 class ProbablisticSamplerTest extends TestCase
 {
-    private function getTags($type, $param)
+    /**
+     * @test
+     * @dataProvider samplerProvider
+     * @param float $rate
+     * @param mixed $traceId
+     * @param bool $decision
+     */
+    public function shouldDetermineWhetherOrTraceShouldBeSampled($rate, $traceId, $decision)
+    {
+        $sampler = new ProbabilisticSampler($rate);
+
+        list($sampled, $tags) = $sampler->isSampled($traceId);
+
+        $this->assertEquals($decision, $sampled);
+        $this->assertEquals([
+            SAMPLER_TYPE_TAG_KEY  => SAMPLER_TYPE_PROBABILISTIC,
+            SAMPLER_PARAM_TAG_KEY => $rate,
+        ], $tags);
+
+        $sampler->close();
+    }
+
+    public function samplerProvider()
     {
         return [
-            'sampler.type' => $type,
-            'sampler.param' => $param,
+            [0.5, PHP_INT_MIN + 10, true],
+            [0.5, PHP_INT_MAX - 10, false],
         ];
     }
 
-    public function testProbabilisticSampler()
+    /**
+     * @test
+     * @dataProvider rateProvider
+     * @param mixed $rate
+     * @expectedException \OutOfBoundsException
+     * @expectedExceptionMessage Sampling rate must be between 0.0 and 1.0.
+     */
+    public function shouldThrowOutOfBoundsExceptionInCaseOfInvalidRate($rate)
     {
-        $sampler = new ProbabilisticSampler(0.5);
-
-        list($sampled, $tags) = $sampler->isSampled(PHP_INT_MIN + 10);
-        $this->assertTrue($sampled);
-        $this->assertEquals($tags, $this->getTags('probabilistic', 0.5));
+        new ProbabilisticSampler($rate);
+    }
 
-        list($sampled, $tags) = $sampler->isSampled(PHP_INT_MAX - 10);
-        $this->assertFalse($sampled);
-        $sampler->close();
+    public function rateProvider()
+    {
+        return [
+            [1.1],
+            [-0.1],
+            [PHP_INT_MAX],
+            [PHP_INT_MIN],
+        ];
     }
 }