Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ request. To do this, simply pass null to `openScope`.
## Instrumentation

- [PSR18 HTTP Client](src/Zipkin/Instrumentation/Http/Client)
- [PSR15 HTTP Server](src/Zipkin/Instrumentation/Http/Server)

## Tests

Expand Down
15 changes: 12 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,36 @@
"description": "A Zipkin instrumentation for PHP",
"keywords": [
"zipkin",
"distributed-tracing",
"tracing",
"openzipkin"
],
"license": "MIT",
"license": "Apache-2.0",
"authors": [
{
"name": "José Carlos Chávez",
"email": "jcchavezs@gmail.com"
}
],
"homepage": "https://github.com/openzipkin/zipkin-php",
"support": {
"issues": "https://github.com/openzipkin/zipkin-php/issues"
},
"require": {
"php": "^7.1",
"ext-curl": "*",
"psr/http-message": "~1.0",
"psr/log": "^1.0"
},
"require-dev": {
"guzzlehttp/psr7": "^1.4",
"guzzlehttp/psr7": "^1.6",
"jcchavezs/httptest": "~0.2",
"phpstan/phpstan": "~0.12.28",
"phpunit/phpunit": "~7.5.20",
"psr/http-client": "^1.0",
"psr/http-server-middleware": "^1.0",
"middlewares/fast-route": "^1.2.1",
"middlewares/request-handler": "^1.4.0",
"squizlabs/php_codesniffer": "3.*"
},
"config": {
Expand Down Expand Up @@ -60,7 +68,8 @@
"static-check": "phpstan analyse src --level 8"
},
"suggest": {
"psr/http-client": "Allows to instrument HTTP clients following PSR18."
"psr/http-client": "Allows to instrument HTTP clients following PSR18.",
"psr/http-server-middleware": "Allows to instrument HTTP servers via middlewares following PSR15."
},
"extra": {
"branch-alias": {
Expand Down
17 changes: 13 additions & 4 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,16 @@ parameters:
# In general types are desired but carrier is an special case
message: '#has parameter \$carrier with no typehint specified#'
path: src/Zipkin/*
-
# Somehow this started failing
message: '#Constant .* already defined#'
path: src/Zipkin
-
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore this changes.

message: '#Method .* with no typehint specified.#'
path: src/Zipkin/Instrumentation/Http/Server
-
message: '#Method .* with no typehint specified.#'
path: src/Zipkin/Instrumentation/Http/Client
#-
# # Somehow this started failing
# message: '#Constant .* already defined#'
# path: src/Zipkin
-
message: '#should return static#'
path: src/Zipkin/Instrumentation/Http/Client/Psr/Request.php
83 changes: 61 additions & 22 deletions src/Zipkin/Instrumentation/Http/Client/ClientTracing.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
namespace Zipkin\Instrumentation\Http\Client;

use Zipkin\Tracing;
use Zipkin\Tracer;
use Zipkin\Span;
use Zipkin\Propagation\SamplingFlags;
use Zipkin\Instrumentation\Http\Client\Parser;

/**
* ClientTracing includes all the elements needed to instrument a
Expand All @@ -23,46 +27,81 @@ class ClientTracing
private $parser;

/**
* function that decides to sample or not an unsampled
* request. The signature is:
*
* <pre>
* function (RequestInterface $request): ?bool {}
* </pre>
*
* @var callable|null
* @var callable
*/
private $requestSampler;
private $nextSpanResolver;

/**
* @param Tracing $tracing
* @param Parser $parser HTTP client parser to obtain meaningful information from
* request and response and tag the span accordingly.
* @param callable(mixed):?bool $requestSampler function that decides to sample or not an unsampled
* request.
* @param callable(mixed):?SamplingFlags $traceContextObtainer function that obtains the context from the
* request. It is mostly used in event loop scenaries where the global scope can't be used.
*/
public function __construct(
Tracing $tracing,
Parser $parser = null,
callable $requestSampler = null
callable $requestSampler = null,
callable $traceContextObtainer = null
) {
$this->tracing = $tracing;
$this->parser = $parser ?? new DefaultParser;
$this->requestSampler = $requestSampler;
$this->parser = $parser ?? new NoopParser;
$this->nextSpanResolver = self::buildNextSpanResolver(
$tracing->getTracer(),
$requestSampler,
$traceContextObtainer
);
}

public function getTracing(): Tracing
{
return $this->tracing;
}

public function getParser(): Parser
{
return $this->parser;
}

/**
* @return callable with the signature:
*
* <pre>
* function (RequestInterface $request): ?bool
* </pre>
* @return callable(mixed):Span the next span handler which creates an appropriate span based on the current scope,
* and the incoming request.
*/
public function getRequestSampler(): ?callable
public function getNextSpanResolver(): callable
{
return $this->requestSampler;
return $this->nextSpanResolver;
}

public function getParser(): Parser
{
return $this->parser;
private static function buildNextSpanResolver(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is important change.

Tracer $tracer,
?callable $requestSampler,
?callable $traceContextObtainer
): callable {
return static function ($request) use ($tracer, $requestSampler, $traceContextObtainer): Span {
if ($traceContextObtainer !== null) {
// in this case, the trace context is meant to be obtained from the request
$traceContext = ($traceContextObtainer)($request);

if ($requestSampler !== null) {
return $tracer->nextSpanWithSampler(
$requestSampler,
[$request],
$traceContext
);
}
return $tracer->nextSpan($traceContext);
}

if ($requestSampler !== null) {
return $tracer->nextSpanWithSampler(
$requestSampler,
[$request]
);
}

return $tracer->nextSpan();
};
}
}
34 changes: 0 additions & 34 deletions src/Zipkin/Instrumentation/Http/Client/DefaultParser.php

This file was deleted.

29 changes: 29 additions & 0 deletions src/Zipkin/Instrumentation/Http/Client/NoopParser.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

declare(strict_types=1);

namespace Zipkin\Instrumentation\Http\Client;

use Zipkin\SpanCustomizer;
use Zipkin\Propagation\TraceContext;

/**
* NoopParser is a noop implementation of a parser that parses nothing as it does not know anything about the request.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure I should provide this but at least it is convenient for tests.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if not sure keep it in tests directory :P

* It is responsibility of the library instrumentation to provide a default parser as this should never be used in
* production.
*/
final class NoopParser implements Parser
{
public function spanName($request): string
{
return '';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this because null isn't a thing?

}

public function request($request, TraceContext $context, SpanCustomizer $span): void
{
}

public function response($response, TraceContext $context, SpanCustomizer $span): void
{
}
}
18 changes: 10 additions & 8 deletions src/Zipkin/Instrumentation/Http/Client/Parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,23 @@

use Zipkin\SpanCustomizer;
use Zipkin\Propagation\TraceContext;
use Throwable;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\RequestInterface;

/**
* Parser includes the methods to obtain meaningful span information
* out of HTTP request/response elements.
* Parser includes the methods to obtain meaningful span information out of
* HTTP request/response elements.
*
* $request and $response in methods are not typed on purpose as they
* should be flexible to accept different types defined by the implementations
* of this interface. Since PHP does not support generics this is the only way
* to go.
*/
interface Parser
{
/**
* spanName returns an appropiate span name based on the request,
* usually the HTTP method is good enough (e.g GET or POST).
*/
public function spanName(RequestInterface $request): string;
public function spanName($request): string;

/**
* request parses the incoming data related to a request in order to add
Expand All @@ -29,7 +31,7 @@ public function spanName(RequestInterface $request): string;
* Basic data being tagged is HTTP method, HTTP path but other information
* such as query parameters can be added to enrich the span information.
*/
public function request(RequestInterface $request, TraceContext $context, SpanCustomizer $span): void;
public function request($request, TraceContext $context, SpanCustomizer $span): void;


/**
Expand All @@ -40,5 +42,5 @@ public function request(RequestInterface $request, TraceContext $context, SpanCu
* as any response header or more granular information based on the response
* payload can be added.
*/
public function response(ResponseInterface $response, TraceContext $context, SpanCustomizer $span): void;
public function response($response, TraceContext $context, SpanCustomizer $span): void;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

declare(strict_types=1);

namespace Zipkin\Instrumentation\Http\Client;
namespace Zipkin\Instrumentation\Http\Client\Psr;

use Zipkin\Tracer;
use Zipkin\SpanCustomizerShield;
use Zipkin\Span;
use Zipkin\Propagation\TraceContext;
use Zipkin\Kind;
use Zipkin\Instrumentation\Http\Client\Parser;
use Zipkin\Instrumentation\Http\Client\ClientTracing;
use Throwable;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\RequestInterface;
Expand All @@ -20,47 +23,36 @@ final class Client implements ClientInterface
private $delegate;

/**
* @var callable
* @var callable(TraceContext,mixed):void
*/
private $injector;

/**
* @var Tracer
*/
private $tracer;

/**
* @var Parser
*/
private $parser;

/**
* @var callable|null
* @var callable(mixed):Span
*/
private $requestSampler;
private $nextSpanResolver;

public function __construct(
ClientInterface $delegate,
ClientTracing $tracing
) {
$this->delegate = $delegate;
$this->injector = $tracing->getTracing()->getPropagation()->getInjector(new RequestHeaders());
$this->tracer = $tracing->getTracing()->getTracer();
$this->parser = $tracing->getParser();
$this->requestSampler = $tracing->getRequestSampler();
$this->nextSpanResolver = $tracing->getNextSpanResolver();
}

public function sendRequest(RequestInterface $request): ResponseInterface
{
if ($this->requestSampler === null) {
$span = $this->tracer->nextSpan();
} else {
$span = $this->tracer->nextSpanWithSampler(
$this->requestSampler,
[$request]
);
}

/**
* @var Span $span
*/
$span = ($this->nextSpanResolver)($request);
($this->injector)($span->getContext(), $request);

if ($span->isNoop()) {
Expand Down
Loading