Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(http-server): adds middleware instrumentation. #162

Merged
merged 20 commits into from
Jul 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
1d18a24
feat(http-server): adds middleware and request handler instrumentation.
jcchavezs Jun 24, 2020
bd11c76
tests(http-server): adds integration test for the Middleware.
jcchavezs Jun 24, 2020
14fcc62
docs(http-server): improves documentation.
jcchavezs Jun 24, 2020
1f462b0
chore(http-server): makes linter happy.
jcchavezs Jun 24, 2020
31faf67
chore(http-server): removes RequestHandler as middleware is enough fo…
jcchavezs Jun 26, 2020
754340d
chore: uses span error API.
jcchavezs Jul 6, 2020
9fa99f1
chore(http-server): decouples client/server tracing from PSR implemen…
jcchavezs Jul 13, 2020
64cb9f9
tests(http-server): adds test for next span in the PSR15 server middl…
jcchavezs Jul 13, 2020
d737d34
chore(http-server): small tweaks to tests.
jcchavezs Jul 14, 2020
d15581c
tests(http-server): improves test naming for client and middleware.
jcchavezs Jul 15, 2020
b3866a9
chore: moves RequestHeaders into propagation folder, also adds typing…
jcchavezs Jul 15, 2020
398d4cb
feat: adds request/response objects to HTTP instrumentation.
jcchavezs Jul 20, 2020
71275e2
docs(http-server): improves docs.
jcchavezs Jul 20, 2020
a38152e
chore(http-server): makes Request/Response interfaces abstract classe…
jcchavezs Jul 21, 2020
d51b511
chore(http-server): removes Parser::spanName to reduce the API surface.
jcchavezs Jul 21, 2020
bc571dc
chore(http-server): do not tag success status code.
jcchavezs Jul 21, 2020
2b0c3a2
chore(http-server): removed the need to pass a parser when intended t…
jcchavezs Jul 21, 2020
8b09a44
chore(http-server): adds default implementation for request/response …
jcchavezs Jul 21, 2020
31bfded
chore(http-server): removes getRoute as now we use abstract classes a…
jcchavezs Jul 24, 2020
0406c52
docs(http-server): last tweaks to documentation.
jcchavezs Jul 24, 2020
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
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,8 @@ request. To do this, simply pass null to `openScope`.

## Instrumentation

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

## 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": "[email protected]"
}
],
"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
6 changes: 1 addition & 5 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,7 @@ 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
-
# SpanCustomizer is definitively not null
message: '#Parameter \#3 \$ of callable callable\(.*, Zipkin\\Propagation\\TraceContext, Zipkin\\SpanCustomizer\): void expects Zipkin\\SpanCustomizer, Zipkin\\SpanCustomizerShield\|null given.#'
path: src/Zipkin/Tracer
path: src/Zipkin/Tracer
40 changes: 30 additions & 10 deletions src/Zipkin/Instrumentation/Http/Client/DefaultParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,48 @@
use Zipkin\Tags;
use Zipkin\SpanCustomizer;
use Zipkin\Propagation\TraceContext;
use Throwable;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\RequestInterface;
use Zipkin\Instrumentation\Http\Response;
use Zipkin\Instrumentation\Http\Request;
use Zipkin\Instrumentation\Http\Client\Parser;

/**
* DefaultParser contains the basic logic for turning request/response information
* into span name and tags. Implementors can use this as a base parser to reduce
* boilerplate.
*/
class DefaultParser implements Parser
{
public function spanName(RequestInterface $request): string
/**
* spanName returns an appropiate span name based on the request,
* usually the HTTP method is enough (e.g GET or POST).
*/
protected function spanName(Request $request): string
{
return $request->getMethod();
}

public function request(RequestInterface $request, TraceContext $context, SpanCustomizer $span): void
/**
* {@inhertidoc}
*/
public function request(Request $request, TraceContext $context, SpanCustomizer $span): void
{
$span->setName($this->spanName($request));
$span->tag(Tags\HTTP_METHOD, $request->getMethod());
$span->tag(Tags\HTTP_PATH, $request->getUri()->getPath() ?: "/");
$span->tag(Tags\HTTP_PATH, $request->getPath() ?: "/");
}

public function response(ResponseInterface $response, TraceContext $context, SpanCustomizer $span): void
/**
* {@inhertidoc}
*/
public function response(Response $response, TraceContext $context, SpanCustomizer $span): void
{
$span->tag(Tags\HTTP_STATUS_CODE, (string) $response->getStatusCode());
if ($response->getStatusCode() > 399) {
$span->tag(Tags\ERROR, (string) $response->getStatusCode());
$statusCode = $response->getStatusCode();
if ($statusCode < 200 || $statusCode > 299) {
$span->tag(Tags\HTTP_STATUS_CODE, (string) $statusCode);
}

if ($statusCode > 399) {
Copy link
Member

Choose a reason for hiding this comment

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

$span->tag(Tags\ERROR, (string) $statusCode);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
namespace Zipkin\Instrumentation\Http\Client;

use Zipkin\Tracing;
use Psr\Http\Message\RequestInterface;
use Zipkin\Instrumentation\Http\Request;

/**
* ClientTracing includes all the elements needed to instrument a
* HTTP client.
*/
class ClientTracing
class HttpClientTracing
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the name @adriancole

{
/**
* @var Tracing
Expand All @@ -25,13 +25,9 @@ class ClientTracing

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

Expand All @@ -51,7 +47,7 @@ public function getTracing(): Tracing
}

/**
* @return (callable(RequestInterface):?bool)|null
* @return (callable(Request):?bool)|null
*/
public function getRequestSampler(): ?callable
{
Expand Down
15 changes: 4 additions & 11 deletions src/Zipkin/Instrumentation/Http/Client/Parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,23 @@

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

/**
* Parser includes the methods to obtain meaningful span information
* out of HTTP request/response elements.
*/
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;

/**
* request parses the incoming data related to a request in order to add
* relevant information to the span under the SpanCustomizer interface.
*
* 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 $request, TraceContext $context, SpanCustomizer $span): void;


/**
Expand All @@ -40,5 +33,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 $response, TraceContext $context, SpanCustomizer $span): void;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@

declare(strict_types=1);

namespace Zipkin\Instrumentation\Http\Client;
namespace Zipkin\Instrumentation\Http\Client\Psr18;

use Zipkin\Tracer;
use Zipkin\SpanCustomizerShield;
use Zipkin\Propagation\TraceContext;
use Zipkin\Kind;
use Zipkin\Instrumentation\Http\Client\Psr18\Propagation\RequestHeaders;
use Zipkin\Instrumentation\Http\Client\Parser;
use Zipkin\Instrumentation\Http\Client\HttpClientTracing;
use Throwable;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\RequestInterface;
Expand Down Expand Up @@ -36,15 +39,13 @@ final class Client implements ClientInterface
private $parser;

/**
* @var (callable(RequestInterface):?bool)|null
* @var (callable(Request):?bool)|null
*/
private $requestSampler;

public function __construct(
ClientInterface $delegate,
ClientTracing $tracing
) {
$this->delegate = $delegate;
public function __construct(ClientInterface $client, HttpClientTracing $tracing)
{
$this->delegate = $client;
$this->injector = $tracing->getTracing()->getPropagation()->getInjector(new RequestHeaders());
$this->tracer = $tracing->getTracing()->getTracer();
$this->parser = $tracing->getParser();
Expand All @@ -53,12 +54,13 @@ public function __construct(

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

Expand All @@ -76,13 +78,12 @@ public function sendRequest(RequestInterface $request): ResponseInterface
// If span is NOOP it does not make sense to add customizations
// to it like tags or annotations.
$spanCustomizer = new SpanCustomizerShield($span);
$span->setName($this->parser->spanName($request));
$this->parser->request($request, $span->getContext(), $spanCustomizer);
$this->parser->request($parsedRequest, $span->getContext(), $spanCustomizer);

try {
$span->start();
$response = $this->delegate->sendRequest($request);
$this->parser->response($response, $span->getContext(), $spanCustomizer);
$this->parser->response(new Response($response, $parsedRequest), $span->getContext(), $spanCustomizer);
return $response;
} catch (Throwable $e) {
$span->setError($e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

declare(strict_types=1);

namespace Zipkin\Instrumentation\Http\Client;
namespace Zipkin\Instrumentation\Http\Client\Psr18\Propagation;

use Zipkin\Propagation\RequestHeaders as BaseRequestHeaders;
use Zipkin\Propagation\RemoteSetter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ In this example we use Guzzle 7 but any HTTP client supporting PSR18 clients wil
```php
use GuzzleHttp\Client;
use GuzzleHttp\Psr7\Request;
use Zipkin\Instrumentation\Http\Client\Client as ZipkinClient;
use Zipkin\Instrumentation\Http\Client\ClientTracing;
use Zipkin\Instrumentation\Http\Client\HttpClientTracing;
use Zipkin\Instrumentation\Http\Client\Psr\Client as ZipkinClient;

$tracing = TracingBuilder::create()
->havingLocalServiceName('my_service')
->build();

$httpClientTracing = new ClientTracing($tracing);
$httpClientTracing = new HttpClientTracing($tracing);
...

$httpClient = new ZipkinClient(new Client, $httpClientTracing);
Expand Down
63 changes: 63 additions & 0 deletions src/Zipkin/Instrumentation/Http/Client/Psr18/Request.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

declare(strict_types=1);

namespace Zipkin\Instrumentation\Http\Client\Psr18;

use Zipkin\Instrumentation\Http\Client\Request as ClientRequest;
use Psr\Http\Message\RequestInterface;

final class Request extends ClientRequest
{
/**
* @var RequestInterface
*/
private $delegate;

public function __construct(RequestInterface $delegate)
{
$this->delegate = $delegate;
}

/**
* {@inheritdoc}
*/
public function getMethod(): string
{
return $this->delegate->getMethod();
}

/**
* {@inheritdoc}
*/
public function getPath(): ?string
{
return $this->delegate->getUri()->getPath();
}

/**
* {@inheritdoc}
*/
public function getUrl(): string
{
return $this->delegate->getUri()->__toString();
}

/**
* {@inheritdoc}
*/
public function getHeader(string $name): ?string
{
return $this->delegate->getHeaderLine($name) ?: null;
}

/**
* {@inheritdoc}
*
* @return RequestInterface
*/
public function unwrap()
{
return $this->delegate;
}
}
Loading