Skip to content

Commit

Permalink
interfaces improvements (#36)
Browse files Browse the repository at this point in the history
* add SpanProcessorInterface, rename ExporterInterface and TransportInterface, add getDescription on SampleInterface as described in specifications

* fix failing tests and improve Dockerfile for phan and test in order to use docker caching
  • Loading branch information
beniamin authored and bobstrecansky committed Dec 23, 2019
1 parent 2aeb5d1 commit cf85f7d
Show file tree
Hide file tree
Showing 17 changed files with 113 additions and 29 deletions.
2 changes: 2 additions & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/vendor
/.git
5 changes: 2 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@
},
"autoload-dev": {
"psr-4": {
"": "tests"
},
"classmap": ["tests"]
"Opentelemetry\\Tests\\": "test/"
}
},
"require-dev": {
"phpunit/phpunit": "^7.5.16",
Expand Down
8 changes: 7 additions & 1 deletion resources/Dockerfile.phan
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ 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
COPY . /usr/src/myapp

WORKDIR /usr/src/myapp

COPY ./composer.json ./
COPY ./composer.lock ./
RUN composer install

COPY . ./

ENTRYPOINT ["./vendor/bin/phan"]
8 changes: 7 additions & 1 deletion resources/Dockerfile.test
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ 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
COPY . /usr/src/myapp

WORKDIR /usr/src/myapp

COPY ./composer.json ./
COPY ./composer.lock ./
RUN composer install

COPY . ./

ENTRYPOINT ["./vendor/phpunit/phpunit/phpunit", "--colors=always"]
9 changes: 7 additions & 2 deletions src/Exporter/BasisExporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@

namespace OpenTelemetry\Exporter;

use OpenTelemetry\Exporter;
use OpenTelemetry\Exporter\ExporterInterface;
use OpenTelemetry\Tracing\Span;

class BasisExporter extends Exporter
class BasisExporter implements ExporterInterface
{
public function convertSpan(Span $span) : array
{
Expand All @@ -20,4 +20,9 @@ public function convertSpan(Span $span) : array
'body' => serialize($span),
];
}

public function export(iterable $spans): int
{
return ExporterInterface::SUCCESS;
}
}
5 changes: 2 additions & 3 deletions src/Exporter.php → src/Exporter/ExporterInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,16 @@

declare(strict_types=1);

namespace OpenTelemetry;
namespace OpenTelemetry\Exporter;

use OpenTelemetry\Tracing\Span;
use OpenTelemetry\Tracing\Tracer;

/**
* A simple Exporter interface
*
* @package OpenTelemetry
*/
interface Exporter
interface ExporterInterface
{
/**
* Possible return values as outlined in the OpenTelemetry spec
Expand Down
12 changes: 7 additions & 5 deletions src/Exporter/ZipkinExporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@

namespace OpenTelemetry\Exporter;

use OpenTelemetry\Exporter;
use OpenTelemetry\Exporter\ExporterInterface;
use OpenTelemetry\Tracing\Span;

/**
* Class ZipkinExporter - implements the export interface for data transfer via Zipkin protocol
* @package OpenTelemetry\Exporter
*/
class ZipkinExporter implements Exporter
class ZipkinExporter implements ExporterInterface
{

/**
* @var endpoint to send Spans to
* @var $endpoint array to send Spans to
*/
private $endpoint;

Expand All @@ -35,6 +35,8 @@ public function export(iterable $spans) : int
/* todo: format into JSON paylod for zipkin:
* @see https://github.com/census-ecosystem/opencensus-php-exporter-zipkin/blob/master/src/ZipkinExporter.php#L143
*/

return ExporterInterface::SUCCESS;
}

/**
Expand Down Expand Up @@ -83,9 +85,9 @@ private function convertSpan(Span $span) : array
/**
* Gets the configured endpoint for the Zipkin exporter
*
* @return endpoint
* @return array |null
*/
public function getEndpoint()
public function getEndpoint(): ?array
{
return $this->endpoint;
}
Expand Down
26 changes: 26 additions & 0 deletions src/SpanProcessorInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace OpenTelemetry;

use OpenTelemetry\Tracing\Span;

interface SpanProcessorInterface
{
/**
* This method is called when a span is started.
* This method is called synchronously on the thread that started the span,
* therefore it should not block or throw exceptions.
*/
public function onStart(Span $span): void;

/**
* This method is called when a span is ended.
* This method is called synchronously on the execution thread,
* therefore it should not block or throw an exception.
*/
public function onEnd(Span $span): void;

/* The spec mentions a shutdown() function. We don't see this as necessary;
* if an Exporter needs to clean up, it can use a destructor.
*/
}
10 changes: 8 additions & 2 deletions src/Tracing/Sampler/AlwaysOffSampler.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php
namespace OpenTelemetry\Tracing\Sampler;

/**
* This implementation of the SamplerInterface always returns false.
* Example:
Expand All @@ -15,8 +16,13 @@ class AlwaysOffSampler implements SamplerInterface
*
* @return bool
*/
public function shouldSample()
public function shouldSample(): bool
{
return false;
}
}

public function getDescription(): string
{
return self::class;
}
}
8 changes: 7 additions & 1 deletion src/Tracing/Sampler/AlwaysOnSampler.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php
namespace OpenTelemetry\Tracing\Sampler;

/**
* This implementation of the SamplerInterface always returns true.
* Example:
Expand All @@ -15,8 +16,13 @@ class AlwaysOnSampler implements SamplerInterface
*
* @return bool
*/
public function shouldSample()
public function shouldSample(): bool
{
return true;
}

public function getDescription(): string
{
return self::class;
}
}
13 changes: 12 additions & 1 deletion src/Tracing/Sampler/SamplerInterface.php
Original file line number Diff line number Diff line change
@@ -1,14 +1,25 @@
<?php
namespace OpenTelemetry\Tracing\Sampler;

/**
* This interface is used to organize sampling logic.
*/
interface SamplerInterface
{
const FLAG_SAMPLED = 1;

/**
* Returns true if we should sample the request.
*
* @return bool
*/
public function shouldSample();
public function shouldSample(): bool;

/**
* Returns the sampler name or short description with the configuration.
* This may be displayed on debug pages or in the logs.
* Example: "ProbabilitySampler{0.000100}"
* @return string
*/
public function getDescription(): string;
}
8 changes: 5 additions & 3 deletions src/Tracing/Span.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class Span

private $attributes = [];
private $events = [];
private $link = [];

// todo: missing span kind
// https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-tracing.md#spankind
Expand All @@ -44,7 +45,7 @@ public function __construct(string $name, SpanContext $spanContext, SpanContext
$this->start = microtime(true);
$this->statusCode = Status::OK;
$this->statusDescription = null;
$this->link = addLinks();
$this->link = $this->addLinks();
}

public function getContext(): SpanContext
Expand All @@ -58,8 +59,9 @@ public function getParentContext(): ?SpanContext
return $this->parentSpanContext !== null ? clone $this->parentSpanContext : null;
}

public function addLinks()
{;
public function addLinks(): array
{
return [];
}


Expand Down
5 changes: 5 additions & 0 deletions src/Tracing/SpanContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ public static function fork(string $traceId)
return self::restore($traceId, bin2hex(random_bytes(8)));
}

public static function restore(string $traceId, string $spanId)
{
return new self($traceId, $spanId, '', []);
}

public function __construct(string $traceId, string $spanId, string $traceFlags, array $traceState)
{
$this->traceId = $traceId;
Expand Down
2 changes: 1 addition & 1 deletion src/Transport.php → src/TransportInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace OpenTelemetry;

interface Transport
interface TransportInterface
{
public function write(array $data) : bool;
}
13 changes: 9 additions & 4 deletions tests/TracingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

declare(strict_types=1);

namespace OpenTelemetry\Tests;

use OpenTelemetry\Exporter\BasisExporter;
use OpenTelemetry\Exporter\ZipkinExporter;
use OpenTelemetry\Tracing\Builder;
Expand All @@ -10,6 +12,7 @@
use OpenTelemetry\Tracing\Tracer;
use OpenTelemetry\Transport\TarantoolQueueTransport;
use PHPUnit\Framework\TestCase;
use ReflectionMethod;

class TracingTest extends TestCase
{
Expand Down Expand Up @@ -188,9 +191,7 @@ public function testEventRegistration()
public function testBuilder()
{
$spanContext = SpanContext::generate();
$tracer = Builder::create()
->setSpanContext($spanContext)
->getTracer();
$tracer = new Tracer($spanContext);

$this->assertInstanceOf(Tracer::class, $tracer);
$this->assertEquals($tracer->getActiveSpan()->getContext(), $spanContext);
Expand Down Expand Up @@ -267,8 +268,12 @@ public function testZipkinConverter()
$event = $span->addEvent('validators.list', [ 'job' => 'stage.updateTime' ]);
$span->end();

$method = new ReflectionMethod(ZipkinExporter::class, 'convertSpan');
$method->setAccessible(true);

$exporter = new ZipkinExporter();
$row = $exporter->convertSpan($span);

$row = $method->invokeArgs($exporter, ['span' => $span]);
$this->assertSame($row['name'], $span->getName());

$this->assertSame($row['tags'], $span->getAttributes());
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/Tracing/Sampler/AlwaysOffSamplerTest.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php
require __DIR__.'/../../../../vendor/autoload.php';
namespace OpenTelemetry\Tests\Unit\Tracing\Sampler;

use OpenTelemetry\Tracing\Sampler\AlwaysOffSampler;
use PHPUnit\Framework\TestCase;
class AlwaysOffSamplerTest extends TestCase
Expand Down
5 changes: 4 additions & 1 deletion tests/unit/Tracing/Sampler/AlwaysOnSamplerTest.php
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
<?php
require __DIR__.'/../../../../vendor/autoload.php';

namespace OpenTelemetry\Tests\Unit\Tracing\Sampler;

use OpenTelemetry\Tracing\Sampler\AlwaysOnSampler;
use PHPUnit\Framework\TestCase;

class AlwaysOnTest extends TestCase
{
public function testAlwaysOnSampler()
Expand Down

0 comments on commit cf85f7d

Please sign in to comment.