From 78d4ed9b23fb5ed28317660bc3d8f39f70101465 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Thu, 1 Dec 2022 01:17:10 +1100 Subject: [PATCH] finish off psr-18 auto-instrumentation (#88) - add tests - replace enum with class for php8.0 compatibility - set up auto-registration via composer autoloader - mention sdk autoloading in readme - document some todos/questions --- .github/workflows/php.yml | 3 + .gitsplit.yml | 2 + examples/instrumentation/Psr18/request.php | 1 - src/Instrumentation/Psr18/.php-cs-fixer.php | 43 +++++++ src/Instrumentation/Psr18/README.md | 32 +++++ src/Instrumentation/Psr18/_register.php | 5 + src/Instrumentation/Psr18/client_tracing.php | 105 ---------------- src/Instrumentation/Psr18/composer.json | 36 ++++++ src/Instrumentation/Psr18/phpstan.neon.dist | 9 ++ src/Instrumentation/Psr18/phpunit.xml.dist | 47 +++++++ src/Instrumentation/Psr18/psalm.xml.dist | 15 +++ .../Psr18/{ => src}/HeadersPropagator.php | 11 +- .../Psr18/src/Psr18Instrumentation.php | 117 ++++++++++++++++++ .../Integration/Psr18InstrumentationTest.php | 98 +++++++++++++++ src/Instrumentation/Psr18/tests/Unit/.gitkeep | 0 15 files changed, 415 insertions(+), 109 deletions(-) create mode 100644 src/Instrumentation/Psr18/.php-cs-fixer.php create mode 100644 src/Instrumentation/Psr18/README.md create mode 100644 src/Instrumentation/Psr18/_register.php delete mode 100644 src/Instrumentation/Psr18/client_tracing.php create mode 100644 src/Instrumentation/Psr18/composer.json create mode 100644 src/Instrumentation/Psr18/phpstan.neon.dist create mode 100644 src/Instrumentation/Psr18/phpunit.xml.dist create mode 100644 src/Instrumentation/Psr18/psalm.xml.dist rename src/Instrumentation/Psr18/{ => src}/HeadersPropagator.php (61%) create mode 100644 src/Instrumentation/Psr18/src/Psr18Instrumentation.php create mode 100644 src/Instrumentation/Psr18/tests/Integration/Psr18InstrumentationTest.php create mode 100644 src/Instrumentation/Psr18/tests/Unit/.gitkeep diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index f7bca9aa..554e2d42 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -21,6 +21,7 @@ jobs: 'Context/Swoole', 'Instrumentation/Slim', 'Instrumentation/Psr15', + 'Instrumentation/Psr18', 'Symfony' ] exclude: @@ -28,6 +29,8 @@ jobs: php-version: 7.4 - project: 'Instrumentation/Psr15' php-version: 7.4 + - project: 'Instrumentation/Psr18' + php-version: 7.4 steps: - uses: actions/checkout@v3 diff --git a/.gitsplit.yml b/.gitsplit.yml index 3cce91a9..0d97e22f 100644 --- a/.gitsplit.yml +++ b/.gitsplit.yml @@ -12,6 +12,8 @@ splits: target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-sdk-bundle.git" - prefix: "src/Instrumentation/Psr15" target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-auto-psr15.git" + - prefix: "src/Instrumentation/Psr18" + target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-auto-psr18.git" - prefix: "src/Instrumentation/Slim" target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-auto-slim.git" - prefix: "src/Context/Swoole" diff --git a/examples/instrumentation/Psr18/request.php b/examples/instrumentation/Psr18/request.php index 663198de..336e9016 100644 --- a/examples/instrumentation/Psr18/request.php +++ b/examples/instrumentation/Psr18/request.php @@ -14,7 +14,6 @@ use OpenTelemetry\SDK\Trace\TracerProvider; require_once dirname(__DIR__, 3) . '/vendor/autoload.php'; -require_once dirname(__DIR__, 3) . '/src/Instrumentation/Psr18/client_tracing.php'; $tracerProvider = new TracerProvider( new BatchSpanProcessor(new ConsoleSpanExporter(), ClockFactory::getDefault()), diff --git a/src/Instrumentation/Psr18/.php-cs-fixer.php b/src/Instrumentation/Psr18/.php-cs-fixer.php new file mode 100644 index 00000000..248b4b9a --- /dev/null +++ b/src/Instrumentation/Psr18/.php-cs-fixer.php @@ -0,0 +1,43 @@ +exclude('vendor') + ->exclude('var/cache') + ->in(__DIR__); + +$config = new PhpCsFixer\Config(); +return $config->setRules([ + 'concat_space' => ['spacing' => 'one'], + 'declare_equal_normalize' => ['space' => 'none'], + 'is_null' => true, + 'modernize_types_casting' => true, + 'ordered_imports' => true, + 'php_unit_construct' => true, + 'single_line_comment_style' => true, + 'yoda_style' => false, + '@PSR2' => true, + 'array_syntax' => ['syntax' => 'short'], + 'blank_line_after_opening_tag' => true, + 'blank_line_before_statement' => true, + 'cast_spaces' => true, + 'declare_strict_types' => true, + 'function_typehint_space' => true, + 'include' => true, + 'lowercase_cast' => true, + 'new_with_braces' => true, + 'no_extra_blank_lines' => true, + 'no_leading_import_slash' => true, + 'echo_tag_syntax' => true, + 'no_unused_imports' => true, + 'no_useless_else' => true, + 'no_useless_return' => true, + 'phpdoc_order' => true, + 'phpdoc_scalar' => true, + 'phpdoc_types' => true, + 'short_scalar_cast' => true, + 'single_blank_line_before_namespace' => true, + 'single_quote' => true, + 'trailing_comma_in_multiline' => true, + ]) + ->setRiskyAllowed(true) + ->setFinder($finder); + diff --git a/src/Instrumentation/Psr18/README.md b/src/Instrumentation/Psr18/README.md new file mode 100644 index 00000000..447263bc --- /dev/null +++ b/src/Instrumentation/Psr18/README.md @@ -0,0 +1,32 @@ +# OpenTelemetry PSR-18 auto-instrumentation + +## Requirements + +* OpenTelemetry extension +* OpenTelemetry SDK an exporter (required to actually export traces) +* A psr-18 client +* (optional) OpenTelemetry [SDK Autoloading](https://github.com/open-telemetry/opentelemetry-php/blob/main/examples/autoload_sdk.php) configured + +## Overview +Auto-instrumentation hooks are registered via composer, which will: + +* create spans automatically for each PSR-18 request that is sent +* add a `traceparent` header to the request to facilitate distributed tracing. + +## Manual configuration +If you are not using SDK autoloading, you will need to create and register a `TracerProvider` early in your application's lifecycle: + +```php +withTracerProvider($tracerProvider) + ->activate(); + +$client->sendRequest($request); + +$scope->detach(); +$tracerProvider->shutdown(); +``` diff --git a/src/Instrumentation/Psr18/_register.php b/src/Instrumentation/Psr18/_register.php new file mode 100644 index 00000000..256eb93e --- /dev/null +++ b/src/Instrumentation/Psr18/_register.php @@ -0,0 +1,5 @@ +attach(Context::getCurrent()); - - return null; - } - - static $instrumentation; - $instrumentation ??= new CachedInstrumentation('io.opentelemetry.contrib.php.psr18', schemaUrl: TraceAttributes::SCHEMA_URL); - $propagator = Instrumentation\Globals::propagator(); - $parentContext = Context::getCurrent(); - - $spanBuilder = $instrumentation - ->tracer() - ->spanBuilder(sprintf('HTTP %s', $request->getMethod())) - ->setParent($parentContext) - ->setSpanKind(SpanKind::KIND_CLIENT) - ->setAttribute(TraceAttributes::HTTP_URL, (string) $request->getUri()) - ->setAttribute(TraceAttributes::HTTP_METHOD, $request->getMethod()) - ->setAttribute(TraceAttributes::HTTP_FLAVOR, $request->getProtocolVersion()) - ->setAttribute(TraceAttributes::HTTP_USER_AGENT, $request->getHeaderLine('User-Agent')) - ->setAttribute(TraceAttributes::HTTP_REQUEST_CONTENT_LENGTH, $request->getHeaderLine('Content-Length')) - ->setAttribute(TraceAttributes::NET_PEER_NAME, $request->getUri()->getHost()) - ->setAttribute(TraceAttributes::NET_PEER_PORT, $request->getUri()->getPort()) - ->setAttribute(TraceAttributes::CODE_FUNCTION, $function) - ->setAttribute(TraceAttributes::CODE_NAMESPACE, $class) - ->setAttribute(TraceAttributes::CODE_FILEPATH, $filename) - ->setAttribute(TraceAttributes::CODE_LINENO, $lineno) - ; - - foreach ($propagator->fields() as $field) { - $request = $request->withoutHeader($field); - } - foreach ((array) (get_cfg_var('otel.instrumentation.http.request_headers') ?: []) as $header) { - if ($request->hasHeader($header)) { - $spanBuilder->setAttribute(sprintf('http.request.header.%s', strtr(strtolower($header), ['-' => '_'])), $request->getHeader($header)); - } - } - - $span = $spanBuilder->startSpan(); - $context = $span->storeInContext($parentContext); - $propagator->inject($request, HeadersPropagator::Instance, $context); - - Context::storage()->attach($context); - - return [$request]; - }, - static function (ClientInterface $client, array $params, ?ResponseInterface $response, ?Throwable $exception): void { - $scope = Context::storage()->scope(); - $scope?->detach(); - - if (!$scope || $scope->context() === Context::getCurrent()) { - return; - } - - $span = Span::fromContext($scope->context()); - - if ($response) { - $span->setAttribute(TraceAttributes::HTTP_STATUS_CODE, $response->getStatusCode()); - $span->setAttribute(TraceAttributes::HTTP_FLAVOR, $response->getProtocolVersion()); - $span->setAttribute(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH, $response->getHeaderLine('Content-Length')); - - foreach ((array) (get_cfg_var('otel.instrumentation.http.response_headers') ?: []) as $header) { - if ($response->hasHeader($header)) { - $span->setAttribute(sprintf('http.response.header.%s', strtr(strtolower($header), ['-' => '_'])), $response->getHeader($header)); - } - } - if ($response->getStatusCode() >= 400 && $response->getStatusCode() < 600) { - $span->setStatus(StatusCode::STATUS_ERROR); - } - } - if ($exception) { - $span->recordException($exception, [TraceAttributes::EXCEPTION_ESCAPED => true]); - $span->setStatus(StatusCode::STATUS_ERROR, $exception->getMessage()); - } - - $span->end(); - }, -); diff --git a/src/Instrumentation/Psr18/composer.json b/src/Instrumentation/Psr18/composer.json new file mode 100644 index 00000000..90c9f46b --- /dev/null +++ b/src/Instrumentation/Psr18/composer.json @@ -0,0 +1,36 @@ +{ + "name": "open-telemetry/opentelemetry-auto-psr18", + "description": "OpenTelemetry auto-instrumentation for psr18 http clients.", + "keywords": ["opentelemetry", "otel", "open-telemetry", "tracing", "psr18", "instrumentation"], + "type": "library", + "homepage": "https://opentelemetry.io/docs/php", + "readme": "./README.md", + "license": "Apache-2.0", + "minimum-stability": "dev", + "require": { + "php": "^8.0", + "ext-otel_instrumentation": "*", + "open-telemetry/api": "^0", + "psr/http-client": "^1" + }, + "autoload": { + "psr-4": { + "OpenTelemetry\\Contrib\\Instrumentation\\Psr18\\": "src/" + }, + "files": [ + "_register.php" + ] + }, + "require-dev": { + "friendsofphp/php-cs-fixer": "^3", + "nyholm/psr7": "*", + "phan/phan": "^5.0", + "php-http/mock-client": "*", + "phpstan/phpstan": "^1.1", + "phpstan/phpstan-phpunit": "^1.0", + "psalm/plugin-phpunit": "^0.16", + "open-telemetry/sdk": "^0", + "phpunit/phpunit": "^9.5", + "vimeo/psalm": "^4.0" + } +} diff --git a/src/Instrumentation/Psr18/phpstan.neon.dist b/src/Instrumentation/Psr18/phpstan.neon.dist new file mode 100644 index 00000000..ed94c13d --- /dev/null +++ b/src/Instrumentation/Psr18/phpstan.neon.dist @@ -0,0 +1,9 @@ +includes: + - vendor/phpstan/phpstan-phpunit/extension.neon + +parameters: + tmpDir: var/cache/phpstan + level: 5 + paths: + - src + - tests diff --git a/src/Instrumentation/Psr18/phpunit.xml.dist b/src/Instrumentation/Psr18/phpunit.xml.dist new file mode 100644 index 00000000..d8548853 --- /dev/null +++ b/src/Instrumentation/Psr18/phpunit.xml.dist @@ -0,0 +1,47 @@ + + + + + + + src + + + + + + + + + + + + + tests/Unit + + + tests/Integration + + + + diff --git a/src/Instrumentation/Psr18/psalm.xml.dist b/src/Instrumentation/Psr18/psalm.xml.dist new file mode 100644 index 00000000..15571171 --- /dev/null +++ b/src/Instrumentation/Psr18/psalm.xml.dist @@ -0,0 +1,15 @@ + + + + + + + + + + diff --git a/src/Instrumentation/Psr18/HeadersPropagator.php b/src/Instrumentation/Psr18/src/HeadersPropagator.php similarity index 61% rename from src/Instrumentation/Psr18/HeadersPropagator.php rename to src/Instrumentation/Psr18/src/HeadersPropagator.php index 25ba14da..fecac839 100644 --- a/src/Instrumentation/Psr18/HeadersPropagator.php +++ b/src/Instrumentation/Psr18/src/HeadersPropagator.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace OpenTelemetry\Instrumentation\Psr18; +namespace OpenTelemetry\Contrib\Instrumentation\Psr18; use function assert; use OpenTelemetry\Context\Propagation\PropagationSetterInterface; @@ -11,9 +11,14 @@ /** * @internal */ -enum HeadersPropagator implements PropagationSetterInterface +class HeadersPropagator implements PropagationSetterInterface { - case Instance; + public static function instance(): self + { + static $instance; + + return $instance ??= new self(); + } public function set(&$carrier, string $key, string $value): void { diff --git a/src/Instrumentation/Psr18/src/Psr18Instrumentation.php b/src/Instrumentation/Psr18/src/Psr18Instrumentation.php new file mode 100644 index 00000000..beee4fc4 --- /dev/null +++ b/src/Instrumentation/Psr18/src/Psr18Instrumentation.php @@ -0,0 +1,117 @@ +attach(Context::getCurrent()); + + return null; + } + + $propagator = Instrumentation\Globals::propagator(); + $parentContext = Context::getCurrent(); + + $spanBuilder = $instrumentation + ->tracer() + ->spanBuilder(sprintf('HTTP %s', $request->getMethod())) + ->setParent($parentContext) + ->setSpanKind(SpanKind::KIND_CLIENT) + ->setAttribute(TraceAttributes::HTTP_URL, (string) $request->getUri()) + ->setAttribute(TraceAttributes::HTTP_METHOD, $request->getMethod()) + ->setAttribute(TraceAttributes::HTTP_FLAVOR, $request->getProtocolVersion()) + ->setAttribute(TraceAttributes::HTTP_USER_AGENT, $request->getHeaderLine('User-Agent')) + ->setAttribute(TraceAttributes::HTTP_REQUEST_CONTENT_LENGTH, $request->getHeaderLine('Content-Length')) + ->setAttribute(TraceAttributes::NET_PEER_NAME, $request->getUri()->getHost()) + ->setAttribute(TraceAttributes::NET_PEER_PORT, $request->getUri()->getPort()) + ->setAttribute(TraceAttributes::CODE_FUNCTION, $function) + ->setAttribute(TraceAttributes::CODE_NAMESPACE, $class) + ->setAttribute(TraceAttributes::CODE_FILEPATH, $filename) + ->setAttribute(TraceAttributes::CODE_LINENO, $lineno) + ; + + foreach ($propagator->fields() as $field) { + $request = $request->withoutHeader($field); + } + //@todo could we use SDK Configuration to retrieve this, and move into a key such as OTEL_PHP_xxx? + foreach ((array) (get_cfg_var('otel.instrumentation.http.request_headers') ?: []) as $header) { + if ($request->hasHeader($header)) { + $spanBuilder->setAttribute( + sprintf('http.request.header.%s', strtr(strtolower($header), ['-' => '_'])), + $request->getHeader($header) + ); + } + } + + $span = $spanBuilder->startSpan(); + $context = $span->storeInContext($parentContext); + $propagator->inject($request, HeadersPropagator::instance(), $context); + + Context::storage()->attach($context); + + return [$request]; + }, + post: static function (ClientInterface $client, array $params, ?ResponseInterface $response, ?Throwable $exception): void { + $scope = Context::storage()->scope(); + $scope?->detach(); + + //@todo do we need the second part of this 'or'? + if (!$scope || $scope->context() === Context::getCurrent()) { + return; + } + + $span = Span::fromContext($scope->context()); + + if ($response) { + $span->setAttribute(TraceAttributes::HTTP_STATUS_CODE, $response->getStatusCode()); + $span->setAttribute(TraceAttributes::HTTP_FLAVOR, $response->getProtocolVersion()); + $span->setAttribute(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH, $response->getHeaderLine('Content-Length')); + + foreach ((array) (get_cfg_var('otel.instrumentation.http.response_headers') ?: []) as $header) { + if ($response->hasHeader($header)) { + $span->setAttribute(sprintf('http.response.header.%s', strtr(strtolower($header), ['-' => '_'])), $response->getHeader($header)); + } + } + if ($response->getStatusCode() >= 400 && $response->getStatusCode() < 600) { + $span->setStatus(StatusCode::STATUS_ERROR); + } + } + if ($exception) { + $span->recordException($exception, [TraceAttributes::EXCEPTION_ESCAPED => true]); + $span->setStatus(StatusCode::STATUS_ERROR, $exception->getMessage()); + } + + $span->end(); + }, + ); + } +} diff --git a/src/Instrumentation/Psr18/tests/Integration/Psr18InstrumentationTest.php b/src/Instrumentation/Psr18/tests/Integration/Psr18InstrumentationTest.php new file mode 100644 index 00000000..5e00dbd8 --- /dev/null +++ b/src/Instrumentation/Psr18/tests/Integration/Psr18InstrumentationTest.php @@ -0,0 +1,98 @@ +storage = new ArrayObject(); + $this->tracerProvider = new TracerProvider( + new SimpleSpanProcessor( + new InMemoryExporter($this->storage) + ) + ); + $this->client = $this->createMock(ClientInterface::class); + + $this->scope = Configurator::create() + ->withTracerProvider($this->tracerProvider) + ->withPropagator(TraceContextPropagator::getInstance()) + ->activate(); + } + + public function tearDown(): void + { + $this->scope->detach(); + } + + /** + * @dataProvider requestProvider + */ + public function test_send_request(string $method, string $uri, int $statusCode): void + { + $request = new Request( + $method, + $uri, + [], + 'body', + '1.1', + ); + $response = new Response($statusCode); + + $this->assertCount(0, $this->storage); + $this->client + ->expects($this->once()) + ->method('sendRequest') + ->with($this->callback(function (RequestInterface $request) { + $this->assertTrue($request->hasHeader('traceparent'), 'traceparent has been injected into request'); + $this->assertNotNull($request->getHeaderLine('traceparent')); + + return true; + })) + ->willReturn($response); + $this->client->sendRequest($request); + $this->assertCount(1, $this->storage); + + /** @var ImmutableSpan $span */ + $span = $this->storage[0]; + + $this->assertStringContainsString($method, $span->getName()); + $this->assertTrue($span->getAttributes()->has(TraceAttributes::HTTP_URL)); + $this->assertSame($uri, $span->getAttributes()->get(TraceAttributes::HTTP_URL)); + $this->assertTrue($span->getAttributes()->has(TraceAttributes::HTTP_METHOD)); + $this->assertSame($method, $span->getAttributes()->get(TraceAttributes::HTTP_METHOD)); + $this->assertTrue($span->getAttributes()->has(TraceAttributes::HTTP_STATUS_CODE)); + $this->assertSame($statusCode, $span->getAttributes()->get(TraceAttributes::HTTP_STATUS_CODE)); + } + + public function requestProvider(): array + { + return [ + ['GET', 'http://example.com/foo', 200], + ['POST', 'https://example.com/bar', 401], + ]; + } +} diff --git a/src/Instrumentation/Psr18/tests/Unit/.gitkeep b/src/Instrumentation/Psr18/tests/Unit/.gitkeep new file mode 100644 index 00000000..e69de29b