feat: add sampler configuration (#8)

contributors: @mmross
auxmoney · Mar 19, 2020 · ec1d5e0 · ec1d5e0
1 parent 67c3bd1
commit ec1d5e0
Show file tree

Hide file tree

Showing 8 changed files with 235 additions and 12 deletions.
diff --git a/Factory/SamplerFactory.php b/Factory/SamplerFactory.php
@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Auxmoney\OpentracingBundle\Factory;
+
+use Exception;
+use Zipkin\Sampler;
+
+interface SamplerFactory
+{
+    /**
+     * @throws Exception
+     * @param mixed $samplerValue
+     */
+    public function createSampler(string $samplerClass, $samplerValue): Sampler;
+}
diff --git a/Factory/ZipkinSamplerFactory.php b/Factory/ZipkinSamplerFactory.php
@@ -0,0 +1,50 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Auxmoney\OpentracingBundle\Factory;
+
+use Exception;
+use Zipkin\Sampler;
+use Zipkin\Samplers\BinarySampler;
+use Zipkin\Samplers\PercentageSampler;
+
+final class ZipkinSamplerFactory implements SamplerFactory
+{
+    /**
+     * @inheritDoc
+     * @SuppressWarnings(PHPMD.StaticAccess)
+     */
+    public function createSampler(string $samplerClass, $samplerValue): Sampler
+    {
+        if ($samplerClass === BinarySampler::class) {
+            return $this->createBinarySampler($samplerValue);
+        }
+
+        if ($samplerClass === PercentageSampler::class) {
+            return PercentageSampler::create($samplerValue);
+        }
+
+        throw new Exception(sprintf('unknown sampler class %s given', $samplerClass));
+    }
+
+    /**
+     * @param mixed $samplerValue
+     * @SuppressWarnings(PHPMD.StaticAccess)
+     * @throws Exception
+     */
+    private function createBinarySampler($samplerValue): BinarySampler
+    {
+        if (!is_bool($samplerValue)) {
+            throw new Exception(
+                sprintf('sampler value for the binary sampler must be an boolean, %s given', gettype($samplerValue))
+            );
+        }
+
+        if ($samplerValue) {
+            return BinarySampler::createAsAlwaysSample();
+        }
+
+        return BinarySampler::createAsNeverSample();
+    }
+}
diff --git a/Factory/ZipkinTracerFactory.php b/Factory/ZipkinTracerFactory.php
@@ -10,33 +10,44 @@
 use Psr\Log\LoggerInterface;
 use Zipkin\Endpoint;
 use Zipkin\Reporters\Http;
-use Zipkin\Samplers\BinarySampler;
 use Zipkin\TracingBuilder;
 use ZipkinOpenTracing\Tracer as ZipkinTracer;
 
 final class ZipkinTracerFactory implements TracerFactory
 {
     private $logger;
     private $agentHostResolver;
+    private $samplerFactory;
 
-    public function __construct(AgentHostResolver $agentHostResolver, LoggerInterface $logger)
-    {
+    public function __construct(
+        AgentHostResolver $agentHostResolver,
+        SamplerFactory $samplerFactory,
+        LoggerInterface $logger
+    ) {
         $this->agentHostResolver = $agentHostResolver;
         $this->logger = $logger;
+        $this->samplerFactory = $samplerFactory;
     }
 
     /**
      * @SuppressWarnings(PHPMD.StaticAccess)
+     * @param mixed $samplerValue
      */
-    public function create(string $projectName, string $agentHost, string $agentPort): Tracer
-    {
+    public function create(
+        string $projectName,
+        string $agentHost,
+        string $agentPort,
+        string $samplerClass,
+        $samplerValue
+    ): Tracer {
         $tracer = new NoopTracer();
 
         try {
             $this->agentHostResolver->ensureAgentHostIsResolvable($agentHost);
             $endpoint = Endpoint::create($projectName, gethostbyname($agentHost), null, (int) $agentPort);
             $reporter = new Http();
-            $sampler = BinarySampler::createAsAlwaysSample();
+            $samplerValue = json_decode($samplerValue);
+            $sampler = $this->samplerFactory->createSampler($samplerClass, $samplerValue);
             $tracing = TracingBuilder::create()
                 ->havingLocalEndpoint($endpoint)
                 ->havingSampler($sampler)

diff --git a/README.md b/README.md
@@ -11,3 +11,25 @@
 This symfony bundle provides a tracer implementation for [Zipkin](https://zipkin.io/) for the [auxmoney OpentracingBundle](https://github.com/auxmoney/OpentracingBundle-core).
 
 Please have a look at [the central documentation](https://github.com/auxmoney/OpentracingBundle-core) for installation and usage instructions.
+
+## Configuration
+
+You can optionally configure environment variables, however, the default configuration will sample every request.
+If you cannot change environment variables in your project, you can alternatively overwrite the container parameters directly.
+
+| environment variable | container parameter | type | default | description |
+|---|---|---|---|---|
+| AUXMONEY_OPENTRACING_SAMPLER_CLASS | auxmoney_opentracing.sampler.class | `string` | `Zipkin\Samplers\BinarySampler` | class of the using sampler, see [existing samplers](#existing-samplers) |
+| AUXMONEY_OPENTRACING_SAMPLER_VALUE | auxmoney_opentracing.sampler.value | `string` | `'true'` | must be a JSON decodable string, for the configuration of the sampler |
+
+### Existing Samplers
+
+* constant sampler
+    * Class: `Zipkin\Samplers\BinarySampler` 
+    * possible values: `'true'` / `'false'`
+    * Description: you activate or deactivate the tracing
+
+* percentage sampler
+    * Class: `Zipkin\Samplers\PercentageSampler` 
+    * possible values: Rate min `'0.00'` - max `'1.00'`
+    * Description: you activate the tracing for the given rate
diff --git a/Resources/config/services.yaml b/Resources/config/services.yaml
@@ -1,5 +1,7 @@
 parameters:
   env(AUXMONEY_OPENTRACING_AGENT_PORT): '9411'
+  env(AUXMONEY_OPENTRACING_SAMPLER_CLASS): Zipkin\Samplers\BinarySampler
+  env(AUXMONEY_OPENTRACING_SAMPLER_VALUE): 'true'
 
 services:
   _defaults:
@@ -9,3 +11,6 @@ services:
 
   Auxmoney\OpentracingBundle\Factory\TracerFactory:
     class: Auxmoney\OpentracingBundle\Factory\ZipkinTracerFactory
+
+  Auxmoney\OpentracingBundle\Factory\SamplerFactory:
+    class: Auxmoney\OpentracingBundle\Factory\ZipkinSamplerFactory
diff --git a/Tests/Factory/ZipkinSamplerFactoryTest.php b/Tests/Factory/ZipkinSamplerFactoryTest.php
@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Auxmoney\OpentracingBundle\Factory;
+
+use Exception;
+use PHPUnit\Framework\TestCase;
+use Zipkin\Samplers\BinarySampler;
+use Zipkin\Samplers\PercentageSampler;
+
+class ZipkinSamplerFactoryTest  extends TestCase
+{
+    private $subject;
+
+    public function setUp()
+    {
+        $this->subject = new ZipkinSamplerFactory();
+    }
+
+    /**
+     * @throws Exception
+     */
+    public function testCreateActiveBinarySampler(): void
+    {
+        $binarySampler = $this->subject->createSampler(BinarySampler::class, true);
+        self::assertTrue($binarySampler->isSampled('dummyTraceId'));
+    }
+
+    /**
+     * @throws Exception
+     */
+    public function testCreateInactiveBinarySampler(): void
+    {
+        $binarySampler = $this->subject->createSampler(BinarySampler::class, false);
+        self::assertFalse($binarySampler->isSampled('dummyTraceId'));
+    }
+
+    /**
+     * @throws Exception
+     */
+    public function testCreateBinarySamplerWithoutStringValue(): void
+    {
+        self::expectException(Exception::class);
+        self::expectExceptionMessage('sampler value for the binary sampler must be an boolean, string given');
+        $this->subject->createSampler(BinarySampler::class, 'true');
+    }
+
+    /**
+     * @throws Exception
+     */
+    public function testCreate100PercentageSampler(): void
+    {
+        $percentageSampler = $this->subject->createSampler(PercentageSampler::class, 1.0);
+        self::assertTrue($percentageSampler->isSampled('dummyTraceId'));
+    }
+
+    /**
+     * @throws Exception
+     */
+    public function testCreate0PercentageSampler(): void
+    {
+        $percentageSampler = $this->subject->createSampler(PercentageSampler::class, 0.0);
+        self::assertFalse($percentageSampler->isSampled('dummyTraceId'));
+    }
+
+    /**
+     * @throws Exception
+     */
+    public function testCreateUnknownSampler(): void
+    {
+        self::expectException(Exception::class);
+        self::expectExceptionMessage('unknown sampler class unknown given');
+
+        $this->subject->createSampler('unknown', 0.0);
+    }
+}
diff --git a/Tests/Factory/ZipkinTracerFactoryTest.php b/Tests/Factory/ZipkinTracerFactoryTest.php
@@ -5,22 +5,27 @@
 namespace Auxmoney\OpentracingBundle\Tests\Factory;
 
 use Auxmoney\OpentracingBundle\Factory\AgentHostResolver;
+use Auxmoney\OpentracingBundle\Factory\SamplerFactory;
 use Auxmoney\OpentracingBundle\Factory\ZipkinTracerFactory;
 use OpenTracing\NoopTracer;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Psr\Log\LoggerInterface;
 use RuntimeException;
+use Zipkin\Samplers\BinarySampler;
 use ZipkinOpenTracing\Tracer;
 
 class ZipkinTracerFactoryTest extends TestCase
 {
+    private $samplerFactory;
     private $agentHostResolver;
     private $logger;
     private $projectName;
     private $agentHost;
     private $agentPort;
     private $subject;
+    private $samplerClass;
+    private $samplerValue;
 
     public function setUp()
     {
@@ -30,36 +35,71 @@ public function setUp()
         $this->projectName = 'project name';
         $this->agentHost = 'localhost';
         $this->agentPort = '9411';
+        $this->samplerClass = BinarySampler::class;
+        $this->samplerValue = 'true';
+        $this->samplerFactory = $this->prophesize(SamplerFactory::class);
 
-        $this->subject = new ZipkinTracerFactory($this->agentHostResolver->reveal(), $this->logger->reveal());
+        $this->subject = new ZipkinTracerFactory(
+            $this->agentHostResolver->reveal(),
+            $this->samplerFactory->reveal(),
+            $this->logger->reveal()
+        );
     }
 
     public function testCreateSuccess(): void
     {
         $this->agentHostResolver->ensureAgentHostIsResolvable('localhost')->shouldBeCalled();
         $this->logger->warning(Argument::type('string'))->shouldNotBeCalled();
+        $this->samplerFactory->createSampler($this->samplerClass, true)->shouldBeCalled()
+            ->willReturn(BinarySampler::createAsAlwaysSample());
 
-        self::assertInstanceOf(Tracer::class, $this->subject->create($this->projectName, $this->agentHost, $this->agentPort));
+        self::assertInstanceOf(
+            Tracer::class,
+            $this->subject->create(
+                $this->projectName,
+                $this->agentHost,
+                $this->agentPort,
+                $this->samplerClass,
+                $this->samplerValue
+            )
+        );
     }
 
     public function testCreateResolvingFailed(): void
     {
-        $this->agentHostResolver->ensureAgentHostIsResolvable('localhost')->shouldBeCalled()->willThrow(new RuntimeException('resolving failer'));
+        $this->agentHostResolver->ensureAgentHostIsResolvable('localhost')->shouldBeCalled()->willThrow(
+            new RuntimeException('resolving failer')
+        );
         $this->logger->warning(Argument::containingString('resolving failer'))->shouldBeCalledOnce();
+        $this->samplerFactory->createSampler($this->samplerClass, true)->shouldNotBeCalled();
 
         self::assertInstanceOf(
             NoopTracer::class,
-            $this->subject->create($this->projectName, $this->agentHost, $this->agentPort)
+            $this->subject->create(
+                $this->projectName,
+                $this->agentHost,
+                $this->agentPort,
+                $this->samplerClass,
+                $this->samplerValue
+            )
         );
     }
 
     public function testCreateException(): void
     {
+        $this->samplerFactory->createSampler($this->samplerClass, true)->shouldNotBeCalled()
+            ->willReturn(BinarySampler::createAsAlwaysSample());
         $this->logger->warning(Argument::containingString('Invalid port.'))->shouldBeCalledOnce();
 
         self::assertInstanceOf(
             NoopTracer::class,
-            $this->subject->create($this->projectName, 'localhost', '70000')
+            $this->subject->create(
+                $this->projectName,
+                'localhost',
+                '70000',
+                $this->samplerClass,
+                $this->samplerValue
+            )
         );
     }
 }
diff --git a/composer.json b/composer.json
@@ -18,7 +18,8 @@
   },
   "require": {
     "php": "^7.1.33",
-    "auxmoney/opentracing-bundle-core": "^0.4.0",
+    "ext-json": "*",
+    "auxmoney/opentracing-bundle-core": "^0.5",
     "opentracing/opentracing": "1.0.0-beta5@beta",
     "jcchavezs/zipkin-opentracing": "^0.1"
   },