From 7ecf88f0f1d6f1dce4033fba1efd9d169b7ac1c9 Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Wed, 22 Aug 2018 14:04:57 +0300 Subject: [PATCH 1/6] Throw OutOfBoundsException on ProbabilisticSampler --- src/Jaeger/Sampler/ConstSampler.php | 24 ++++++++++++++--- src/Jaeger/Sampler/ProbabilisticSampler.php | 29 +++++++++++++++++---- src/Jaeger/Sampler/SamplerInterface.php | 20 ++++++++++++++ 3 files changed, 65 insertions(+), 8 deletions(-) diff --git a/src/Jaeger/Sampler/ConstSampler.php b/src/Jaeger/Sampler/ConstSampler.php index fae21d1..8bf67ac 100644 --- a/src/Jaeger/Sampler/ConstSampler.php +++ b/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; /** + * Trace 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 } } diff --git a/src/Jaeger/Sampler/ProbabilisticSampler.php b/src/Jaeger/Sampler/ProbabilisticSampler.php index 1b0697f..25c5963 100644 --- a/src/Jaeger/Sampler/ProbabilisticSampler.php +++ b/src/Jaeger/Sampler/ProbabilisticSampler.php @@ -2,7 +2,7 @@ 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; @@ -10,28 +10,37 @@ /** * 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 trace tags. + * * @var array */ private $tags = []; /** + * The boundary of the sample sampling rate. + * * @var float|int */ private $boundary; /** * ProbabilisticSampler constructor. + * * @param float $rate - * @throws Exception + * @throws OutOfBoundsException */ public function __construct(float $rate) { @@ -41,7 +50,7 @@ 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; @@ -49,8 +58,10 @@ public function __construct(float $rate) } /** - * @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 @@ -58,7 +69,15 @@ 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 } } diff --git a/src/Jaeger/Sampler/SamplerInterface.php b/src/Jaeger/Sampler/SamplerInterface.php index 8308da7..b6d44e7 100644 --- a/src/Jaeger/Sampler/SamplerInterface.php +++ b/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(); } From a4ecb7012f32f2dbc42240d87a1a6bd24e4c15f4 Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Wed, 22 Aug 2018 14:06:13 +0300 Subject: [PATCH 2/6] Bring all error messages to a general view --- src/Jaeger/Codec/TextCodec.php | 2 +- src/Jaeger/Config.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Jaeger/Codec/TextCodec.php b/src/Jaeger/Codec/TextCodec.php index 1f6661d..cc1e48c 100644 --- a/src/Jaeger/Codec/TextCodec.php +++ b/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 [ diff --git a/src/Jaeger/Config.php b/src/Jaeger/Config.php index adc68a0..0e05319 100644 --- a/src/Jaeger/Config.php +++ b/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(); From 3d30aa49fb3980149c400b65ed659162e8c1bdf8 Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Wed, 22 Aug 2018 14:06:46 +0300 Subject: [PATCH 3/6] Amended Sampler's tests --- src/Jaeger/Sampler/ConstSampler.php | 2 +- src/Jaeger/Sampler/ProbabilisticSampler.php | 4 +- tests/Jaeger/Codec/TextCodecTest.php | 2 +- tests/Jaeger/ConfigTest.php | 2 +- tests/Jaeger/Sampler/ConstSamplerTest.php | 46 +++++++++------ .../Sampler/ProbablisticSamplerTest.php | 58 +++++++++++++++---- 6 files changed, 78 insertions(+), 36 deletions(-) diff --git a/src/Jaeger/Sampler/ConstSampler.php b/src/Jaeger/Sampler/ConstSampler.php index 8bf67ac..7c6ba3d 100644 --- a/src/Jaeger/Sampler/ConstSampler.php +++ b/src/Jaeger/Sampler/ConstSampler.php @@ -21,7 +21,7 @@ class ConstSampler implements SamplerInterface private $decision; /** - * Trace tags. + * A list of the sampler tags. * * @var array */ diff --git a/src/Jaeger/Sampler/ProbabilisticSampler.php b/src/Jaeger/Sampler/ProbabilisticSampler.php index 25c5963..331bb7c 100644 --- a/src/Jaeger/Sampler/ProbabilisticSampler.php +++ b/src/Jaeger/Sampler/ProbabilisticSampler.php @@ -23,7 +23,7 @@ class ProbabilisticSampler implements SamplerInterface private $rate; /** - * A list of the trace tags. + * A list of the sampler tags. * * @var array */ @@ -32,7 +32,7 @@ class ProbabilisticSampler implements SamplerInterface /** * The boundary of the sample sampling rate. * - * @var float|int + * @var float */ private $boundary; diff --git a/tests/Jaeger/Codec/TextCodecTest.php b/tests/Jaeger/Codec/TextCodecTest.php index 38b76f2..754ef52 100644 --- a/tests/Jaeger/Codec/TextCodecTest.php +++ b/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() { diff --git a/tests/Jaeger/ConfigTest.php b/tests/Jaeger/ConfigTest.php index a4449ca..5d05717 100644 --- a/tests/Jaeger/ConfigTest.php +++ b/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() { diff --git a/tests/Jaeger/Sampler/ConstSamplerTest.php b/tests/Jaeger/Sampler/ConstSamplerTest.php index 1268fd6..85cb805 100644 --- a/tests/Jaeger/Sampler/ConstSamplerTest.php +++ b/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], + ]; } } diff --git a/tests/Jaeger/Sampler/ProbablisticSamplerTest.php b/tests/Jaeger/Sampler/ProbablisticSamplerTest.php index 524dfd5..210c204 100644 --- a/tests/Jaeger/Sampler/ProbablisticSamplerTest.php +++ b/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], + ]; } } From 3c7ba2c98a892374da2aa9f2e14bb767eecc91c7 Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Wed, 22 Aug 2018 14:27:00 +0300 Subject: [PATCH 4/6] Update docs [ci skip] --- README.md | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index c3baa06..aeec3e1 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,9 @@ # 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 @@ -10,12 +12,10 @@ 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) From 6bd8d23b4b4379862fffa205228297efffa990e8 Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Wed, 22 Aug 2018 14:27:20 +0300 Subject: [PATCH 5/6] Update .gitignore [ci skip] --- .gitignore | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 7877103..07dc555 100644 --- a/.gitignore +++ b/.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 From 424d046857fe32a45af27cbc8727424ce2f4a4be Mon Sep 17 00:00:00 2001 From: Serghei Iakovlev Date: Thu, 23 Aug 2018 10:38:53 +0300 Subject: [PATCH 6/6] Correct install instructions [ci skip] --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index aeec3e1..c4077ca 100644 --- a/README.md +++ b/README.md @@ -12,10 +12,10 @@ Please see [CONTRIBUTING.md](./CONTRIBUTING.md). ## Installation -Install [Composer](https://getcomposer.org) in a common location or in your project. Then run the installer as follows: +Jaeger client can be installed via Composer: ```bash -php composer.phar require jonahgeorge/jaeger-client-php +composer require jonahgeorge/jaeger-client-php ``` ## Getting Started