From cd75a733d2090444c01a89e899c45f959c8d5004 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Carlos=20Ch=C3=A1vez?= Date: Sun, 12 Nov 2017 17:55:43 +0100 Subject: [PATCH 1/3] Removes carriers. --- README.md | 41 ++++++++--------- composer.json | 6 +-- src/OpenTracing/Carriers/HttpHeaders.php | 58 ------------------------ src/OpenTracing/Carriers/TextMap.php | 44 ------------------ src/OpenTracing/Formats.php | 23 ++++++++++ src/OpenTracing/NoopTracer.php | 7 +-- src/OpenTracing/Propagation/Formats.php | 18 -------- src/OpenTracing/Propagation/Reader.php | 15 ------ src/OpenTracing/Propagation/Writer.php | 24 ---------- src/OpenTracing/Tracer.php | 19 ++++---- tests/Unit/Carriers/HttpHeadersTest.php | 37 --------------- tests/Unit/Carriers/TextMapTest.php | 34 -------------- 12 files changed, 54 insertions(+), 272 deletions(-) delete mode 100644 src/OpenTracing/Carriers/HttpHeaders.php delete mode 100644 src/OpenTracing/Carriers/TextMap.php create mode 100644 src/OpenTracing/Formats.php delete mode 100644 src/OpenTracing/Propagation/Formats.php delete mode 100644 src/OpenTracing/Propagation/Reader.php delete mode 100644 src/OpenTracing/Propagation/Writer.php delete mode 100644 tests/Unit/Carriers/HttpHeadersTest.php delete mode 100644 tests/Unit/Carriers/TextMapTest.php diff --git a/README.md b/README.md index 4736ae3..2a80389 100644 --- a/README.md +++ b/README.md @@ -46,13 +46,14 @@ The simplest starting point is to set the global tracer. As early as possible, d To start a new `Span`, you can use the `StartSpanFromContext` method. ```php - use Psr\Http\Message\RequestInterface; + use OpenTracing\Formats; + use OpenTracing\GlobalTracer; ... $spanContext = GlobalTracer::get()->extract( - Propagator::HTTP_HEADERS, - HttpHeaders::withHeaders($request->getHeaders()) + Formats\HTTP_HEADERS, + $request->getHeaders() ); function doSomething(SpanContext $spanContext, ...) { @@ -93,7 +94,7 @@ reference. $span = GlobalTracer::get()->startSpan( 'my_span', [ - 'child_of' => $spanContext, + 'child_of' => $parentSpan, ] ); @@ -105,17 +106,16 @@ reference. ### Serializing to the wire ```php - use OpenTracing\Carriers\HttpHeaders as HttpHeadersCarrier; - use OpenTracing\Context; use OpenTracing\GlobalTracer; + use OpenTracing\Formats; ... $tracer = GlobalTracer::get(); $spanContext = $tracer->extract( - Propagator::HTTP_HEADERS, - HttpHeaders::withHeaders($request->getHeaders()) + Formats\HTTP_HEADERS, + $request->getHeaders() ); try { @@ -123,14 +123,15 @@ reference. $client = new GuzzleHttp\Client; - $request = new \GuzzleHttp\Psr7\Request('GET', 'http://myservice'); + $headers = []; $tracer->inject( - $span->context(), - Propagator::HTTP_HEADERS, - HttpHeadersCarrier::withHeaders($request->getHeaders()) - ) - + $span->getContext(), + Formats\HTTP_HEADERS, + $headers + ); + + $request = new \GuzzleHttp\Psr7\Request('GET', 'http://myservice', $headers); $client->send($request); ... } catch (\Exception $e) { @@ -144,12 +145,12 @@ reference. When using http header for context propagation you can use either the `Request` or the `$_SERVER` variable: ```php - use OpenTracing\Carriers\HttpHeaders; use OpenTracing\GlobalTracer; + use OpenTracing\Formats; $request = Request::createFromGlobals(); $tracer = GlobalTracer::get(); - $spanContext = $tracer->extract(Propagator::HTTP_HEADERS, HttpHeaders::fromRequest($request)); + $spanContext = $tracer->extract(Formats\HTTP_HEADERS, $request->getHeaders()); $tracer->startSpan('my_span', [ 'child_of' => $spanContext, ]); @@ -164,8 +165,6 @@ cause problems for Tracer implementations. This is why the PHP API contains a `flush` method that allows to trigger a span sending out of process. ```php -createSpan('my_span', [ 'child_of' => $spanContext, 'tags' => ['foo' => 'bar'], @@ -215,7 +210,7 @@ Tracers will throw an exception if the requested format is not handled by them. - `Tracer::FORMAT_HTTP_HEADERS` should represent the span context as HTTP header lines in an array list. For two context details "Span-Id" and "Trace-Id", the result would be `['Span-Id: abc123', 'Trace-Id: def456']`. This definition can be - passed directly to curl and file_get_contents. + passed directly to `curl` and `file_get_contents`. - `Tracer::FORMAT_BINARY` makes no assumptions about the data format other than it is proprietary and each Tracer can handle it as it wants. diff --git a/composer.json b/composer.json index e81752a..08eeb17 100644 --- a/composer.json +++ b/composer.json @@ -11,19 +11,17 @@ ], "minimum-stability": "stable", "require": { - "php": "^5.6||^7.0", - "psr/http-message": "^1.0" + "php": "^5.6||^7.0" }, "require-dev": { "phpunit/phpunit": "~5.7.19", - "guzzlehttp/psr7": "^1.4", "squizlabs/php_codesniffer": "3.*" }, "autoload": { "psr-4": { "OpenTracing\\": "./src/OpenTracing/" }, - "files": ["./src/OpenTracing/Ext/Tags.php", "./src/OpenTracing/Propagation/Formats.php"] + "files": ["./src/OpenTracing/Ext/Tags.php", "./src/OpenTracing/Formats.php"] }, "autoload-dev": { "psr-4": { diff --git a/src/OpenTracing/Carriers/HttpHeaders.php b/src/OpenTracing/Carriers/HttpHeaders.php deleted file mode 100644 index c15b73a..0000000 --- a/src/OpenTracing/Carriers/HttpHeaders.php +++ /dev/null @@ -1,58 +0,0 @@ - $value) { - $this->items[(string) $key] = (string) $value; - } - } - - /** - * @param RequestInterface $request - * @return HttpHeaders - */ - public static function fromRequest(RequestInterface $request) - { - return new self( - array_map(function ($values) { - return $values[0]; - }, $request->getHeaders()) - ); - } - - /** - * @param array|string[] $headers - * @return HttpHeaders - */ - public static function fromHeaders(array $headers) - { - return new self($headers); - } - - /** - * {@inheritdoc} - */ - public function set($key, $value) - { - $this->items[(string) $key] = (string) $value; - } - - /** - * {@inheritdoc} - */ - public function getIterator() - { - return new ArrayIterator($this->items); - } -} diff --git a/src/OpenTracing/Carriers/TextMap.php b/src/OpenTracing/Carriers/TextMap.php deleted file mode 100644 index c4f549e..0000000 --- a/src/OpenTracing/Carriers/TextMap.php +++ /dev/null @@ -1,44 +0,0 @@ - $value) { - $this->items[(string) $key] = (string) $value; - } - } - - /** - * @param array|string[] $textMap - * @return TextMap - */ - public static function fromArray(array $textMap) - { - return new self($textMap); - } - - /** - * {@inheritdoc} - */ - public function set($key, $value) - { - $this->items[(string) $key] = (string) $value; - } - - /** - * {@inheritdoc} - */ - public function getIterator() - { - return new ArrayIterator($this->items); - } -} diff --git a/src/OpenTracing/Formats.php b/src/OpenTracing/Formats.php new file mode 100644 index 0000000..137a5a2 --- /dev/null +++ b/src/OpenTracing/Formats.php @@ -0,0 +1,23 @@ + 'bar']; - - public function testCreationWithHeadersHasTheExpectedValues() - { - $headers = self::TEST_HEADERS; - $httpHeaders = HttpHeaders::fromHeaders($headers); - - foreach ($httpHeaders as $key => $value) { - $this->assertEquals('foo', $key); - $this->assertEquals('bar', $value); - } - } - - public function testCreationFromRequestHasTheExpectedValues() - { - $request = new Request('GET', '', self::TEST_HEADERS); - $httpHeaders = HttpHeaders::fromRequest($request); - - foreach ($httpHeaders as $key => $value) { - $this->assertEquals('foo', $key); - $this->assertEquals('bar', $value); - } - } -} diff --git a/tests/Unit/Carriers/TextMapTest.php b/tests/Unit/Carriers/TextMapTest.php deleted file mode 100644 index d8fe402..0000000 --- a/tests/Unit/Carriers/TextMapTest.php +++ /dev/null @@ -1,34 +0,0 @@ - 'bar']; - $textMap = TextMap::fromArray($items); - - foreach ($textMap as $key => $value) { - $this->assertEquals('foo', $key); - $this->assertEquals('bar', $value); - } - } - - public function testSettingAKeyValuePairSuccess() - { - $textMap = TextMap::fromArray([]); - $textMap->set('foo', 'bar'); - - foreach ($textMap as $key => $value) { - $this->assertEquals('foo', $key); - $this->assertEquals('bar', $value); - } - } -} From 17d6748692e260e967ac8459a9c311d6df02ad63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Carlos=20Ch=C3=A1vez?= Date: Mon, 13 Nov 2017 18:15:40 +0100 Subject: [PATCH 2/3] Adds documentation about expectations for carriers on different formats. --- src/OpenTracing/Formats.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/OpenTracing/Formats.php b/src/OpenTracing/Formats.php index 137a5a2..7ffa970 100644 --- a/src/OpenTracing/Formats.php +++ b/src/OpenTracing/Formats.php @@ -9,6 +9,11 @@ /** * Used for an arbitrary string-to-string map with an unrestricted character set for both keys and values + * + * Unlike `HTTP_HEADERS`, the `TEXT_MAP` format does not restrict the key or + * value character sets in any way. + * + * For both Tracer::inject() and Tracer::extract() the carrier must be a `array|ArrayObject`. */ const TEXT_MAP = 'text_map'; @@ -17,6 +22,23 @@ * In practice, since there is such "diversity" in the way that HTTP headers are treated in the wild, it is strongly * recommended that Tracer implementations use a limited HTTP header key space and escape values conservatively. * + * Unlike `TEXT_MAP`, the `HTTP_HEADERS` format requires that the keys and values be valid as HTTP headers as-is + * (i.e., character casing may be unstable and special characters are disallowed in keys, values should be + * URL-escaped, etc). + * + * For both Tracer::inject() and Tracer::extract() the carrier must be a `array|ArrayObject`. + * + * For example, Tracer::inject(): + * + * $headers = [] + * $tracer->inject($span->getContext(), Formats\HTTP_HEADERS, $headers) + * $request = new GuzzleHttp\Psr7\Request($uri, $body, $headers); + * + * Or Tracer::extract(): + * + * $headers = $request->getHeaders() + * $clientContext = $tracer->extract(Formats\HTTP_HEADERS, $headers) + * * @see http://www.php-fig.org/psr/psr-7/#12-http-headers * @see http://php.net/manual/en/function.getallheaders.php */ From e016672bc57c40b38a23d4d88fa9a32216bc7d67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Carlos=20Ch=C3=A1vez?= Date: Wed, 15 Nov 2017 08:56:51 +0100 Subject: [PATCH 3/3] Adds expected type for binary format. --- src/OpenTracing/Formats.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/OpenTracing/Formats.php b/src/OpenTracing/Formats.php index 7ffa970..ce8d844 100644 --- a/src/OpenTracing/Formats.php +++ b/src/OpenTracing/Formats.php @@ -4,6 +4,8 @@ /** * Used a (single) arbitrary binary blob representing a SpanContext + * + * For both Tracer::inject() and Tracer::extract() the carrier must be a `string`. */ const BINARY = 'binary';