chore(http-server): decouples client/server tracing from PSR implemen…

…tation.

phpstan.neon

@@ -36,4 +36,10 @@ parameters:
 		-
 			# 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
+		-
+			message: '#Method .* with no typehint specified.#'                      
+			path: src/Zipkin/Instrumentation/Http/Server
+		-
+			message: '#Method .* with no typehint specified.#'                      
+			path: src/Zipkin/Instrumentation/Http/Client

src/Zipkin/Instrumentation/Http/Client/ClientTracing.php → src/Zipkin/Instrumentation/Http/Client/HttpClientTracing.php

@@ -11,7 +11,7 @@
  * ClientTracing includes all the elements needed to instrument a
  * HTTP client.
  */
-class ClientTracing
+class HttpClientTracing
 {
     /**
      * @var Tracing
@@ -25,23 +25,19 @@ class ClientTracing
 
     /**
      * function that decides to sample or not an unsampled
-     * request. The signature is:
-     *
-     * <pre>
-     * function (RequestInterface $request): ?bool {}
-     * </pre>
+     * request.
      *
      * @var callable(RequestInterface):?bool|null
      */
     private $requestSampler;
 
     public function __construct(
         Tracing $tracing,
-        Parser $parser = null,
+        Parser $parser,
         callable $requestSampler = null
     ) {
         $this->tracing = $tracing;
-        $this->parser = $parser ?? new DefaultParser;
+        $this->parser = $parser;
         $this->requestSampler = $requestSampler;
     }
 

src/Zipkin/Instrumentation/Http/Client/Parser.php

@@ -6,21 +6,25 @@
 
 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.
+ *
+ * $request and $response objects are not typed on purpose as different
+ * frameworks can use different request models (e.g. symfony uses a list
+ * parameters including method, url and options). Since PHP does not support
+ * generics this is the only way to make this interface reusable but
+ * implementors have to make sure the request/response objects have the right
+ * type.
  */
 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
@@ -29,7 +33,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;
 
 
     /**
@@ -40,5 +44,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;
 }

src/Zipkin/Instrumentation/Http/Client/Client.php → src/Zipkin/Instrumentation/Http/Client/Psr18/Client.php

@@ -2,12 +2,14 @@
 
 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\Parser;
+use Zipkin\Instrumentation\Http\Client\HttpClientTracing;
 use Throwable;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\RequestInterface;
@@ -42,7 +44,7 @@ final class Client implements ClientInterface
 
     public function __construct(
         ClientInterface $delegate,
-        ClientTracing $tracing
+        HttpClientTracing $tracing
     ) {
         $this->delegate = $delegate;
         $this->injector = $tracing->getTracing()->getPropagation()->getInjector(new RequestHeaders());

src/Zipkin/Instrumentation/Http/Client/Psr18/DefaultParser.php

@@ -0,0 +1,52 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Zipkin\Instrumentation\Http\Client\Psr18;
+
+use Zipkin\Tags;
+use Zipkin\SpanCustomizer;
+use Zipkin\Propagation\TraceContext;
+use Zipkin\Instrumentation\Http\Client\Parser;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\RequestInterface;
+
+/**
+ * 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($request): string
+    {
+        self::assertRequestType($request);
+        return $request->getMethod();
+    }
+
+    public function request($request, TraceContext $context, SpanCustomizer $span): void
+    {
+        self::assertRequestType($request);
+        $span->tag(Tags\HTTP_METHOD, $request->getMethod());
+        $span->tag(Tags\HTTP_PATH, $request->getUri()->getPath() ?: "/");
+    }
+
+    public function response($response, TraceContext $context, SpanCustomizer $span): void
+    {
+        self::assertResponseType($response);
+        $span->tag(Tags\HTTP_STATUS_CODE, (string) $response->getStatusCode());
+        if ($response->getStatusCode() > 399) {
+            $span->tag(Tags\ERROR, (string) $response->getStatusCode());
+        }
+    }
+
+    // Makes sure the type for request object is correct.
+    private static function assertRequestType(RequestInterface $request): void
+    {
+    }
+
+    // Makes sure the type for response object is correct.
+    private static function assertResponseType(ResponseInterface $response): void
+    {
+    }
+}

src/Zipkin/Instrumentation/Http/Client/README.md → src/Zipkin/Instrumentation/Http/Client/Psr18/README.md

@@ -17,14 +17,15 @@ 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;
+use Zipkin\Instrumentation\Http\Client\Psr\DefaultParser;
 
 $tracing = TracingBuilder::create()
             ->havingLocalServiceName('my_service')
             ->build();
 
-$httpClientTracing = new ClientTracing($tracing);
+$httpClientTracing = new HttpClientTracing($tracing, new DefaultParser);
 ...
 
 $httpClient = new ZipkinClient(new Client, $httpClientTracing);

src/Zipkin/Instrumentation/Http/Client/RequestHeaders.php → src/Zipkin/Instrumentation/Http/Client/Psr18/RequestHeaders.php

@@ -2,7 +2,7 @@
 
 declare(strict_types=1);
 
-namespace Zipkin\Instrumentation\Http\Client;
+namespace Zipkin\Instrumentation\Http\Client\Psr18;
 
 use Zipkin\Propagation\RequestHeaders as BaseRequestHeaders;
 use Zipkin\Propagation\RemoteSetter;

src/Zipkin/Instrumentation/Http/Server/ServerTracing.php → src/Zipkin/Instrumentation/Http/Server/HttpServerTracing.php

@@ -10,7 +10,7 @@
  * Tracing includes all the elements needed to trace a HTTP server
  * middleware or request handler.
  */
-class ServerTracing
+class HttpServerTracing
 {
     /**
      * @var Tracing
@@ -24,23 +24,19 @@ class ServerTracing
 
     /**
      * function that decides to sample or not an unsampled
-     * request. The signature is:
+     * request.
      *
-     * <pre>
-     * function (RequestInterface $request): ?bool {}
-     * </pre>
-     *
-     * @var callable|null
+     * @var (callable(mixed):?bool)|null
      */
     private $requestSampler;
 
     public function __construct(
         Tracing $tracing,
-        Parser $parser = null,
+        Parser $parser,
         callable $requestSampler = null
     ) {
         $this->tracing = $tracing;
-        $this->parser = $parser ?? new DefaultParser;
+        $this->parser = $parser;
         $this->requestSampler = $requestSampler;
     }
 
@@ -50,11 +46,7 @@ public function getTracing(): Tracing
     }
 
     /**
-     * @return callable|null with the signature:
-     *
-     * <pre>
-     * function (RequestInterface $request): ?bool
-     * </pre>
+     * @return (callable(mixed):?bool)|null
      */
     public function getRequestSampler(): ?callable
     {

src/Zipkin/Instrumentation/Http/Server/Parser.php

@@ -6,14 +6,17 @@
 
 use Zipkin\SpanCustomizer;
 use Zipkin\Propagation\TraceContext;
-use Throwable;
-use Psr\Http\Message\ServerRequestInterface;
-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.
+ *
+ * $request and $response objects are not typed on purpose as different
+ * frameworks can use different request models (e.g. symfony uses a list
+ * parameters including method, url and options). Since PHP does not support
+ * generics this is the only way to make this interface reusable but
+ * implementors have to make sure the request/response objects have the right
+ * type.
  */
 interface Parser
 {
@@ -22,7 +25,7 @@ interface Parser
      * usually the HTTP method is enough (e.g GET or POST) but ideally
      * the http.route is desired (e.g. /user/{user_id}).
      */
-    public function spanName(ServerRequestInterface $request): string;
+    public function spanName($request): string;
 
     /**
      * request parses the incoming data related to a request in order to add
@@ -31,7 +34,7 @@ public function spanName(ServerRequestInterface $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(ServerRequestInterface $request, TraceContext $context, SpanCustomizer $span): void;
+    public function request($request, TraceContext $context, SpanCustomizer $span): void;
 
 
     /**
@@ -42,5 +45,5 @@ public function request(ServerRequestInterface $request, TraceContext $context,
      * 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;
 }