From 0991f72146e1df0f8c84e8d2a5fed3f649f782cf Mon Sep 17 00:00:00 2001 From: Beniamin Date: Tue, 7 Jan 2020 15:39:14 +0200 Subject: [PATCH] TracerFactory implementation (#38) * implement TracerFactory, begin migration to recommended library layout, use docker-compose and make to simplify development * add tests for all TracerFactory method, move TraceFactory configuration in constructor * run phan in travis-ci build * rename namespace Tracing into Trace --- .phpcs.xml | 6 ++ .travis.yml | 2 + Makefile | 13 +++ docker-compose.yaml | 8 ++ docker/Dockerfile | 11 +++ examples/AlwaysOffTraceExample.php | 11 ++- examples/AlwaysOnTraceExample.php | 29 +++--- src/{Tracing => Context}/SpanContext.php | 4 +- src/Exporter/BasisExporter.php | 3 +- src/Exporter/ExporterInterface.php | 2 +- src/Exporter/ZipkinExporter.php | 3 +- src/{Tracing => Trace}/Event.php | 2 +- .../Sampler/AlwaysOffSampler.php | 2 +- .../Sampler/AlwaysOnSampler.php | 2 +- .../Sampler/SamplerInterface.php | 2 +- src/{Tracing => Trace}/Span.php | 12 +-- src/{ => Trace}/SpanProcessorInterface.php | 4 +- src/{Tracing => Trace}/Status.php | 2 +- src/{Tracing => Trace}/Tracer.php | 4 +- src/Trace/TracerFactory.php | 74 +++++++++++++++ tests/TracingTest.php | 8 +- tests/unit/Trace/TracerFactoryTest.php | 94 +++++++++++++++++++ .../Tracing/Sampler/AlwaysOffSamplerTest.php | 2 +- .../Tracing/Sampler/AlwaysOnSamplerTest.php | 2 +- 24 files changed, 254 insertions(+), 48 deletions(-) create mode 100644 .phpcs.xml create mode 100644 Makefile create mode 100644 docker-compose.yaml create mode 100644 docker/Dockerfile rename src/{Tracing => Context}/SpanContext.php (96%) rename src/{Tracing => Trace}/Event.php (97%) rename src/{Tracing => Trace}/Sampler/AlwaysOffSampler.php (92%) rename src/{Tracing => Trace}/Sampler/AlwaysOnSampler.php (92%) rename src/{Tracing => Trace}/Sampler/SamplerInterface.php (92%) rename src/{Tracing => Trace}/Span.php (97%) rename src/{ => Trace}/SpanProcessorInterface.php (91%) rename src/{Tracing => Trace}/Status.php (99%) rename src/{Tracing => Trace}/Tracer.php (97%) create mode 100644 src/Trace/TracerFactory.php create mode 100644 tests/unit/Trace/TracerFactoryTest.php diff --git a/.phpcs.xml b/.phpcs.xml new file mode 100644 index 000000000..12aaa66e0 --- /dev/null +++ b/.phpcs.xml @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file diff --git a/.travis.yml b/.travis.yml index 38c718e2f..0d814c9f2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,10 +13,12 @@ cache: - $HOME/.composer/cache before_script: + - pecl install ast-1.0.4 - travis_retry composer update ${COMPOSER_FLAGS} --no-interaction --prefer-dist script: - vendor/bin/phpunit --coverage-text --coverage-clover=coverage.clover + - vendor/bin/phan after_script: - | diff --git a/Makefile b/Makefile new file mode 100644 index 000000000..7a9f5d3c8 --- /dev/null +++ b/Makefile @@ -0,0 +1,13 @@ +DC_RUN_PHP = docker-compose run php + +install: + $(DC_RUN_PHP) composer install +test: + $(DC_RUN_PHP) php ./vendor/bin/phpunit --colors=always +phan: + $(DC_RUN_PHP) php ./vendor/bin/phan +examples: FORCE + $(DC_RUN_PHP) php ./examples/AlwaysOnTraceExample.php +bash: + $(DC_RUN_PHP) bash +FORCE: \ No newline at end of file diff --git a/docker-compose.yaml b/docker-compose.yaml new file mode 100644 index 000000000..407bbb54d --- /dev/null +++ b/docker-compose.yaml @@ -0,0 +1,8 @@ +version: '3.7' +services: + php: + build: + context: . + dockerfile: docker/Dockerfile + volumes: + - ./:/usr/src/myapp diff --git a/docker/Dockerfile b/docker/Dockerfile new file mode 100644 index 000000000..8a124efc0 --- /dev/null +++ b/docker/Dockerfile @@ -0,0 +1,11 @@ +FROM php:7.1-buster + +RUN apt-get -y update && apt-get -y install git zip && \ +curl -sS https://getcomposer.org/installer | php && \ +mv composer.phar /usr/local/bin/composer && \ +chmod +x /usr/local/bin/composer && \ + +pecl install ast-1.0.4 && \ +echo "extension=ast.so" >> /usr/local/etc/php/conf.d/docker-php-ext-ast.ini + +WORKDIR /usr/src/myapp \ No newline at end of file diff --git a/examples/AlwaysOffTraceExample.php b/examples/AlwaysOffTraceExample.php index 9ac7197c3..9aeed93df 100644 --- a/examples/AlwaysOffTraceExample.php +++ b/examples/AlwaysOffTraceExample.php @@ -1,14 +1,15 @@ getTracer('io.opentelemetry.contrib.php'); // start a span, register some events $span = $tracer->createSpan('session.generate'); diff --git a/examples/AlwaysOnTraceExample.php b/examples/AlwaysOnTraceExample.php index 1ee9b72f6..7f917478b 100644 --- a/examples/AlwaysOnTraceExample.php +++ b/examples/AlwaysOnTraceExample.php @@ -1,30 +1,29 @@ getTracer('io.opentelemetry.contrib.php'); - // start a span, register some events - $span = $tracer->createSpan('session.generate'); - $span->setAttributes(['remote_ip' => '1.2.3.4']); - $span->setAttribute('country', 'USA'); + // start a span, register some events + $span = $tracer->createSpan('session.generate'); + $span->setAttributes(['remote_ip' => '1.2.3.4']); + $span->setAttribute('country', 'USA'); - $span->addEvent('found_login', [ + $span->addEvent('found_login', [ 'id' => 12345, 'username' => 'otuser', - ]); - $span->addEvent('generated_session', [ + ]); + $span->addEvent('generated_session', [ 'id' => md5(microtime(true)) - ]); + ]); - $span->end(); // pass status as an optional argument - print_r($span); // print the span as a resulting output + $span->end(); // pass status as an optional argument + print_r($span); // print the span as a resulting output } else { echo "Sampling is not enabled"; } diff --git a/src/Tracing/SpanContext.php b/src/Context/SpanContext.php similarity index 96% rename from src/Tracing/SpanContext.php rename to src/Context/SpanContext.php index 7e90c9929..1597c6839 100644 --- a/src/Tracing/SpanContext.php +++ b/src/Context/SpanContext.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace OpenTelemetry\Tracing; +namespace OpenTelemetry\Context; class SpanContext { @@ -60,6 +60,6 @@ public function IsValid() : bool /* TODO : Finish this function */ public function IsRemote() : bool { - return false; + return false; } } diff --git a/src/Exporter/BasisExporter.php b/src/Exporter/BasisExporter.php index 7a9abee37..fe46d6705 100644 --- a/src/Exporter/BasisExporter.php +++ b/src/Exporter/BasisExporter.php @@ -4,8 +4,7 @@ namespace OpenTelemetry\Exporter; -use OpenTelemetry\Exporter\ExporterInterface; -use OpenTelemetry\Tracing\Span; +use OpenTelemetry\Trace\Span; class BasisExporter implements ExporterInterface { diff --git a/src/Exporter/ExporterInterface.php b/src/Exporter/ExporterInterface.php index 0cfcfcd05..c36ae7657 100644 --- a/src/Exporter/ExporterInterface.php +++ b/src/Exporter/ExporterInterface.php @@ -4,7 +4,7 @@ namespace OpenTelemetry\Exporter; -use OpenTelemetry\Tracing\Span; +use OpenTelemetry\Trace\Span; /** * A simple Exporter interface diff --git a/src/Exporter/ZipkinExporter.php b/src/Exporter/ZipkinExporter.php index 7135f809c..199b37670 100644 --- a/src/Exporter/ZipkinExporter.php +++ b/src/Exporter/ZipkinExporter.php @@ -4,8 +4,7 @@ namespace OpenTelemetry\Exporter; -use OpenTelemetry\Exporter\ExporterInterface; -use OpenTelemetry\Tracing\Span; +use OpenTelemetry\Trace\Span; /** * Class ZipkinExporter - implements the export interface for data transfer via Zipkin protocol diff --git a/src/Tracing/Event.php b/src/Trace/Event.php similarity index 97% rename from src/Tracing/Event.php rename to src/Trace/Event.php index 4a0463713..b87a0a563 100644 --- a/src/Tracing/Event.php +++ b/src/Trace/Event.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace OpenTelemetry\Tracing; +namespace OpenTelemetry\Trace; class Event { diff --git a/src/Tracing/Sampler/AlwaysOffSampler.php b/src/Trace/Sampler/AlwaysOffSampler.php similarity index 92% rename from src/Tracing/Sampler/AlwaysOffSampler.php rename to src/Trace/Sampler/AlwaysOffSampler.php index 699ed9e3a..fa77b1c9f 100644 --- a/src/Tracing/Sampler/AlwaysOffSampler.php +++ b/src/Trace/Sampler/AlwaysOffSampler.php @@ -1,5 +1,5 @@ https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-tracing.md#spankind - // Links + // Links // A Span may be linked to zero or more other Spans (defined by SpanContext) that are causally related. Links can point to SpanContexts inside a single Trace - // or across different Traces. Links can be used to represent batched operations where a Span was initiated by multiple initiating Spans, + // or across different Traces. Links can be used to represent batched operations where a Span was initiated by multiple initiating Spans, // each representing a single incoming item being processed in the batch. // https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/overview.md#links-between-spans @@ -89,7 +89,7 @@ public function getStatus(): Status return Status::new($this->statusCode, $this->statusDescription); } - // I think this is too simple, see: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-tracing.md#isrecording + // I think this is too simple, see: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-tracing.md#isrecording // -> This had an update this past month: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-tracing.md#isrecording public function isRecording(): bool { @@ -181,4 +181,4 @@ private function throwIfNotRecording() throw new Exception("Span is readonly"); } } -} +} \ No newline at end of file diff --git a/src/SpanProcessorInterface.php b/src/Trace/SpanProcessorInterface.php similarity index 91% rename from src/SpanProcessorInterface.php rename to src/Trace/SpanProcessorInterface.php index 454e7e7f4..c963d7ea6 100644 --- a/src/SpanProcessorInterface.php +++ b/src/Trace/SpanProcessorInterface.php @@ -1,8 +1,8 @@ spanProcessors = $spanProcessors; + } + + /** + * @param SpanProcessorInterface[] $spanProcessors + * + * @return static + */ + public static function getInstance(array $spanProcessors = []): self + { + if (self::$instance instanceof TracerFactory) { + return self::$instance; + } + + $instance = new TracerFactory($spanProcessors); + + return self::$instance = $instance; + } + + public function getTracer(string $name, string $version = ""): Tracer + { + + if ($this->tracers[$name] instanceof Tracer) { + return $this->tracers[$name]; + } + + $spanContext = SpanContext::generate(); + return $this->tracers[$name] = new Tracer($spanContext); + } +} \ No newline at end of file diff --git a/tests/TracingTest.php b/tests/TracingTest.php index 3d71b1de8..b2de47b5b 100644 --- a/tests/TracingTest.php +++ b/tests/TracingTest.php @@ -6,11 +6,9 @@ use OpenTelemetry\Exporter\BasisExporter; use OpenTelemetry\Exporter\ZipkinExporter; -use OpenTelemetry\Tracing\Builder; -use OpenTelemetry\Tracing\SpanContext; -use OpenTelemetry\Tracing\Status; -use OpenTelemetry\Tracing\Tracer; -use OpenTelemetry\Transport\TarantoolQueueTransport; +use OpenTelemetry\Context\SpanContext; +use OpenTelemetry\Trace\Status; +use OpenTelemetry\Trace\Tracer; use PHPUnit\Framework\TestCase; use ReflectionMethod; diff --git a/tests/unit/Trace/TracerFactoryTest.php b/tests/unit/Trace/TracerFactoryTest.php new file mode 100644 index 000000000..52de8fd95 --- /dev/null +++ b/tests/unit/Trace/TracerFactoryTest.php @@ -0,0 +1,94 @@ +setAccessible(true); + $refProperty->setValue(null); + } + + /** + * @test + * @expectedException Error + */ + public function shouldNotBeAbleToInstantiateDirectly() + { + new TracerFactory(); + } + + /** + * @test + */ + public function gettingSameTracerMultipleTimesShouldReturnSameObject() + { + $tracer1 = TracerFactory::getInstance()->getTracer('test_tracer'); + $tracer2 = TracerFactory::getInstance()->getTracer('test_tracer'); + + $this->assertTrue($tracer1 === $tracer2); + } + + /** + * @test + */ + public function callingSameInstanceMultipleTimesShouldReturnSameFactoryObject() + { + $instance1 = TracerFactory::getInstance(); + $instance2 = TracerFactory::getInstance(); + + $this->assertTrue($instance1 === $instance2); + } + + /** + * @test + */ + public function shouldInstantiateWithoutErrorIfConfigurationIsOk() + { + $factory = TracerFactory::getInstance([ + $this->createMock(SpanProcessorInterface::class), + $this->createMock(SpanProcessorInterface::class) + ]); + + $this->assertInstanceOf(TracerFactory::class, $factory); + } + + /** + * @test + * @dataProvider wrongConfigurationDataProvider + * @expectedException InvalidArgumentException + */ + public function shouldThrowExceptionIfConfigurationParamsAreInvalid($spanProcessors) + { + TracerFactory::getInstance($spanProcessors); + } + + public function wrongConfigurationDataProvider() + { + return [ + 'array of numbers' => [ + 'spanProcessors' => [1, -1, 0.1, -0.1, 0] + ], + 'array of strings' => [ + 'spanProcessors' => ['aaa', 'bbb', ''] + ], + 'array of standardObjects' => [ + 'spanProcessors' => [new StdClass(), new StdClass()] + ], + 'array of boolean' => [ + 'spanProcessors' => [true, false, null] + ] + ]; + } +} \ No newline at end of file diff --git a/tests/unit/Tracing/Sampler/AlwaysOffSamplerTest.php b/tests/unit/Tracing/Sampler/AlwaysOffSamplerTest.php index 1f9bc1a1b..dd8ba8277 100644 --- a/tests/unit/Tracing/Sampler/AlwaysOffSamplerTest.php +++ b/tests/unit/Tracing/Sampler/AlwaysOffSamplerTest.php @@ -1,7 +1,7 @@