From 146d03698b0fb1ed1ebabe237935c95ae85b7bc1 Mon Sep 17 00:00:00 2001 From: Pawel Filipczak Date: Tue, 29 Oct 2024 15:03:20 +0100 Subject: [PATCH 1/6] auto instrumentation for curl (#1420) --- composer.json | 3 + src/Instrumentation/Curl/.gitattributes | 12 + src/Instrumentation/Curl/.gitignore | 1 + src/Instrumentation/Curl/.php-cs-fixer.php | 43 ++ src/Instrumentation/Curl/README.md | 27 ++ src/Instrumentation/Curl/_register.php | 18 + src/Instrumentation/Curl/composer.json | 55 +++ src/Instrumentation/Curl/phpstan.neon.dist | 9 + src/Instrumentation/Curl/phpunit.xml.dist | 47 +++ src/Instrumentation/Curl/psalm.xml.dist | 17 + .../Curl/src/CurlInstrumentation.php | 385 ++++++++++++++++++ .../Integration/CurlInstrumentationTest.php | 129 ++++++ .../CurlMultiInstrumentationTest.php | 124 ++++++ src/Instrumentation/Curl/tests/Unit/.gitkeep | 0 14 files changed, 870 insertions(+) create mode 100644 src/Instrumentation/Curl/.gitattributes create mode 100644 src/Instrumentation/Curl/.gitignore create mode 100644 src/Instrumentation/Curl/.php-cs-fixer.php create mode 100644 src/Instrumentation/Curl/README.md create mode 100644 src/Instrumentation/Curl/_register.php create mode 100644 src/Instrumentation/Curl/composer.json create mode 100644 src/Instrumentation/Curl/phpstan.neon.dist create mode 100644 src/Instrumentation/Curl/phpunit.xml.dist create mode 100644 src/Instrumentation/Curl/psalm.xml.dist create mode 100644 src/Instrumentation/Curl/src/CurlInstrumentation.php create mode 100644 src/Instrumentation/Curl/tests/Integration/CurlInstrumentationTest.php create mode 100644 src/Instrumentation/Curl/tests/Integration/CurlMultiInstrumentationTest.php create mode 100644 src/Instrumentation/Curl/tests/Unit/.gitkeep diff --git a/composer.json b/composer.json index 7ac30bc1f..c60c5fd22 100644 --- a/composer.json +++ b/composer.json @@ -17,6 +17,7 @@ "psr-4": { "OpenTelemetry\\Contrib\\Aws\\": "src/Aws/src", "OpenTelemetry\\Contrib\\Context\\Swoole\\": "src/Context/Swoole/src", + "OpenTelemetry\\Contrib\\Instrumentation\\Curl\\": "src/Instrumentation/Curl/src", "OpenTelemetry\\Contrib\\Instrumentation\\ExtAmqp\\": "src/Instrumentation/ExtAmqp/src", "OpenTelemetry\\Contrib\\Instrumentation\\ExtRdKafka\\": "src/Instrumentation/ExtRdKafka/src", "OpenTelemetry\\Contrib\\Instrumentation\\Laravel\\": "src/Instrumentation/Laravel/src", @@ -39,6 +40,7 @@ "OpenTelemetry\\Contrib\\Symfony\\": "src/Symfony/src" }, "files": [ + "src/Instrumentation/Curl/_register.php", "src/Instrumentation/ExtAmqp/_register.php", "src/Instrumentation/ExtRdKafka/_register.php", "src/Instrumentation/HttpAsyncClient/_register.php", @@ -59,6 +61,7 @@ "open-telemetry/contrib-aws": "self.version", "open-telemetry/contrib-sdk-bundle": "self.version", "open-telemetry/context-swoole": "self.version", + "open-telemetry/opentelemetry-auto-curl": "self.version", "open-telemetry/opentelemetry-auto-laravel": "self.version", "open-telemetry/opentelemetry-auto-http-async": "self.version", "open-telemetry/opentelemetry-auto-io": "self.version", diff --git a/src/Instrumentation/Curl/.gitattributes b/src/Instrumentation/Curl/.gitattributes new file mode 100644 index 000000000..1676cf825 --- /dev/null +++ b/src/Instrumentation/Curl/.gitattributes @@ -0,0 +1,12 @@ +* text=auto + +*.md diff=markdown +*.php diff=php + +/.gitattributes export-ignore +/.gitignore export-ignore +/.php-cs-fixer.php export-ignore +/phpstan.neon.dist export-ignore +/phpunit.xml.dist export-ignore +/psalm.xml.dist export-ignore +/tests export-ignore diff --git a/src/Instrumentation/Curl/.gitignore b/src/Instrumentation/Curl/.gitignore new file mode 100644 index 000000000..57872d0f1 --- /dev/null +++ b/src/Instrumentation/Curl/.gitignore @@ -0,0 +1 @@ +/vendor/ diff --git a/src/Instrumentation/Curl/.php-cs-fixer.php b/src/Instrumentation/Curl/.php-cs-fixer.php new file mode 100644 index 000000000..e35fa078c --- /dev/null +++ b/src/Instrumentation/Curl/.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, + 'type_declaration_spaces' => true, + 'include' => true, + 'lowercase_cast' => true, + 'new_with_parentheses' => 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, + 'blank_lines_before_namespace' => true, + 'single_quote' => true, + 'trailing_comma_in_multiline' => true, + ]) + ->setRiskyAllowed(true) + ->setFinder($finder); + diff --git a/src/Instrumentation/Curl/README.md b/src/Instrumentation/Curl/README.md new file mode 100644 index 000000000..2aa324352 --- /dev/null +++ b/src/Instrumentation/Curl/README.md @@ -0,0 +1,27 @@ +[![Releases](https://img.shields.io/badge/releases-purple)](https://github.com/opentelemetry-php/contrib-auto-curl/releases) +[![Issues](https://img.shields.io/badge/issues-pink)](https://github.com/open-telemetry/opentelemetry-php/issues) +[![Source](https://img.shields.io/badge/source-contrib-green)](https://github.com/open-telemetry/opentelemetry-php-contrib/tree/main/src/Instrumentation/curl) +[![Mirror](https://img.shields.io/badge/mirror-opentelemetry--php--contrib-blue)](https://github.com/opentelemetry-php/contrib-auto-curl) +[![Latest Version](http://poser.pugx.org/open-telemetry/opentelemetry-auto-curl/v/unstable)](https://packagist.org/packages/open-telemetry/opentelemetry-auto-curl/) +[![Stable](http://poser.pugx.org/open-telemetry/opentelemetry-auto-curl/v/stable)](https://packagist.org/packages/open-telemetry/opentelemetry-auto-curl/) + +This is a read-only subtree split of https://github.com/open-telemetry/opentelemetry-php-contrib. + +# OpenTelemetry curl auto-instrumentation + +Please read https://opentelemetry.io/docs/instrumentation/php/automatic/ for instructions on how to +install and configure the extension and SDK. + +## Overview +Auto-instrumentation hooks are registered via composer, and client kind spans will automatically be created when calling `curl_exec` or `curl_multi_exec` functions. + +## Limitations +The curl_multi instrumentation is not resilient to shortcomings in the application and requires proper implementation. If the application does not call the curl_multi_info_read function, the instrumentation will be unable to measure the execution time for individual requests-time will be aggregated for all transfers. Similarly, error detection will be impacted, as the error code information will be missing in this case. In case of encountered issues, it is recommended to review the application code and adjust it to match example #1 provided in [curl_multi_exec documentation](https://www.php.net/manual/en/function.curl-multi-exec.php). + +## Configuration + +The extension can be disabled via [runtime configuration](https://opentelemetry.io/docs/instrumentation/php/sdk/#configuration): + +```shell +OTEL_PHP_DISABLED_INSTRUMENTATIONS=curl +``` diff --git a/src/Instrumentation/Curl/_register.php b/src/Instrumentation/Curl/_register.php new file mode 100644 index 000000000..b079334a4 --- /dev/null +++ b/src/Instrumentation/Curl/_register.php @@ -0,0 +1,18 @@ + + + + + + + src + + + + + + + + + + + + + tests/Unit + + + tests/Integration + + + + diff --git a/src/Instrumentation/Curl/psalm.xml.dist b/src/Instrumentation/Curl/psalm.xml.dist new file mode 100644 index 000000000..5a04b34d7 --- /dev/null +++ b/src/Instrumentation/Curl/psalm.xml.dist @@ -0,0 +1,17 @@ + + + + + + + + + + diff --git a/src/Instrumentation/Curl/src/CurlInstrumentation.php b/src/Instrumentation/Curl/src/CurlInstrumentation.php new file mode 100644 index 000000000..9a79c52ff --- /dev/null +++ b/src/Instrumentation/Curl/src/CurlInstrumentation.php @@ -0,0 +1,385 @@ + */ + $curlHandleToAttributes = new WeakMap(); + + /** @var WeakMap > + * + * curlMultiToHandle -> array('started'=>bool, + * 'handles'=> + * WeakMap[CurlHandle] => { + * 'finished' => bool, + * 'span' => WeakReference + * } + * ) + */ + $curlMultiToHandle = new WeakMap(); + + $instrumentation = new CachedInstrumentation( + 'io.opentelemetry.contrib.php.curl', + null, + 'https://opentelemetry.io/schemas/1.24.0' + ); + + hook( + null, + 'curl_init', + pre: null, + post: static function ($obj, array $params, mixed $retVal) use ($curlHandleToAttributes) { + if ($retVal instanceof CurlHandle) { + $curlHandleToAttributes[$retVal] = [TraceAttributes::HTTP_REQUEST_METHOD => 'GET']; + if ($params[0] !== null) { + $curlHandleToAttributes[$retVal][TraceAttributes::URL_FULL] = self::redactUrlString($params[0]); + } + } + } + ); + + hook( + null, + 'curl_setopt', + pre: null, + post: static function ($obj, array $params, mixed $retVal) use ($curlHandleToAttributes) { + if ($retVal != true) { + return; + } + + $attr = self::getAttributeFromCurlOption($params[1], $params[2]); + if ($attr) { + $handleAttributes = &$curlHandleToAttributes[$params[0]]; + $handleAttributes[$attr[0]] = $attr[1]; + } + } + ); + + hook( + null, + 'curl_setopt_array', + pre: null, + post: static function ($obj, array $params, mixed $retVal) use ($curlHandleToAttributes) { + if ($retVal != true) { + return; + } + + foreach ($params[1] as $option => $value) { + $attr = self::getAttributeFromCurlOption($option, $value); + if ($attr) { + $handleAttributes = &$curlHandleToAttributes[$params[0]]; + $handleAttributes[$attr[0]] = $attr[1]; + } + } + } + ); + + hook( + null, + 'curl_close', + pre: static function ($obj, array $params) use ($curlHandleToAttributes) { + if ($params[0] instanceof CurlHandle) { + $curlHandleToAttributes->offsetUnset($params[0]); + } + }, + post: null + ); + + hook( + null, + 'curl_copy_handle', + pre: null, + post: static function ($obj, array $params, mixed $retVal) use ($curlHandleToAttributes) { + if ($params[0] instanceof CurlHandle && $retVal instanceof CurlHandle) { + $curlHandleToAttributes[$retVal] = $curlHandleToAttributes[$params[0]]; + } + } + ); + + hook( + null, + 'curl_reset', + pre: static function ($obj, array $params) use ($curlHandleToAttributes) { + if ($params[0] instanceof CurlHandle) { + $curlHandleToAttributes[$params[0]] = [TraceAttributes::HTTP_REQUEST_METHOD => 'GET']; + } + }, + post: null + ); + + hook( + null, + 'curl_exec', + pre: static function ($obj, array $params, ?string $class, ?string $function, ?string $filename, ?int $lineno) use ($instrumentation, $curlHandleToAttributes) { + /** @psalm-suppress ArgumentTypeCoercion */ + if (!($params[0] instanceof CurlHandle)) { + return; + } + + $spanName = array_key_exists(TraceAttributes::HTTP_REQUEST_METHOD, $curlHandleToAttributes[$params[0]]) ? $curlHandleToAttributes[$params[0]][TraceAttributes::HTTP_REQUEST_METHOD] : 'curl_exec'; + + $builder = $instrumentation->tracer() + ->spanBuilder($spanName) + ->setSpanKind(SpanKind::KIND_CLIENT) + ->setAttribute(TraceAttributes::CODE_FUNCTION, $function) + ->setAttribute(TraceAttributes::CODE_FILEPATH, $filename) + ->setAttribute(TraceAttributes::CODE_LINENO, $lineno) + ->setAttributes($curlHandleToAttributes[$params[0]]); + + $parent = Context::getCurrent(); + $span = $builder->startSpan(); + Context::storage()->attach($span->storeInContext($parent)); + }, + post: static function ($obj, array $params, mixed $retVal) use ($curlHandleToAttributes) { + $scope = Context::storage()->scope(); + if (!$scope) { + return; + } + + $scope->detach(); + $span = Span::fromContext($scope->context()); + + if ($retVal !== false) { + if ($params[0] instanceof CurlHandle) { + self::setAttributesFromCurlGetInfo($params[0], $span); + } + } else { + if ($params[0] instanceof CurlHandle) { + $errno = curl_errno($params[0]); + if ($errno != 0) { + $errorDescription = curl_strerror($errno) . ' (' . $errno . ')'; + $span->setStatus(StatusCode::STATUS_ERROR, $errorDescription); + } + $span->setAttribute(TraceAttributes::ERROR_TYPE, 'cURL error (' . $errno . ')'); + } + } + + $span->end(); + } + ); + + hook( + null, + 'curl_multi_init', + pre: null, + post: static function ($obj, array $params, mixed $retVal) use ($curlMultiToHandle) { + if ($retVal instanceof CurlMultiHandle) { + $curlMultiToHandle[$retVal] = ['started' => false, 'handles' => new WeakMap()]; + } + } + ); + + // curl_multi_add_handle(CurlMultiHandle $multi_handle, CurlHandle $handle): int, Returns 0 on success, or one of the CURLM_XXX error codes. + hook( + null, + 'curl_multi_add_handle', + pre: null, + post: static function ($obj, array $params, mixed $retVal) use ($curlMultiToHandle) { + if ($retVal == 0) { + $mHandle = &$curlMultiToHandle[$params[0]]; + $mHandle['handles'][$params[1]] = ['finished' => false, 'span' => null]; + } + } + ); + + // curl_multi_remove_handle(CurlMultiHandle $multi_handle, CurlHandle $handle): int, Returns 0 on success, or one of the CURLM_XXX error codes. + hook( + null, + 'curl_multi_remove_handle', + pre: null, + post: static function ($obj, array $params, mixed $retVal) use ($curlMultiToHandle) { + if ($retVal == 0) { + $curlMultiToHandle[$params[0]]['handles']->offsetUnset($params[1]); + } + } + ); + + // curl_multi_close(CurlMultiHandle $multi_handle): void + hook( + null, + 'curl_multi_close', + pre: static function ($obj, array $params) use ($curlMultiToHandle) { + if ($params[0] instanceof CurlMultiHandle) { + $curlMultiToHandle->offsetUnset($params[0]); + } + }, + post: null + ); + + // curl_multi_exec(CurlMultiHandle $multi_handle, int &$still_running): int + hook( + null, + 'curl_multi_exec', + pre: null, + post: static function ($obj, array $params, mixed $retVal) use ($curlMultiToHandle, $instrumentation, $curlHandleToAttributes) { + if ($retVal == CURLM_OK) { + $mHandle = &$curlMultiToHandle[$params[0]]; + + $handles = &$mHandle['handles']; + + if (!$mHandle['started']) { // on first call to curl_multi_exec we're marking it's a transfer start for all curl handles attached to multi handle + $parent = Context::getCurrent(); + foreach ($handles as $cHandle => &$metadata) { + $spanName = array_key_exists(TraceAttributes::HTTP_REQUEST_METHOD, $curlHandleToAttributes[$cHandle]) ? $curlHandleToAttributes[$cHandle][TraceAttributes::HTTP_REQUEST_METHOD] : 'curl_multi_exec'; + $builder = $instrumentation->tracer() + ->spanBuilder($spanName) + ->setSpanKind(SpanKind::KIND_CLIENT) + ->setAttribute(TraceAttributes::CODE_FUNCTION, 'curl_multi_exec') + ->setAttributes($curlHandleToAttributes[$cHandle]); + + $span = $builder->startSpan(); + Context::storage()->attach($span->storeInContext($parent)); + $metadata['span'] = WeakReference::create($span); + } + $mHandle['started'] = true; + } + + $isRunning = $params[1]; + if ($isRunning == 0) { + + // it is the last call to multi - in case curl_multi_info_read might not not be called anytime, we need to finish all spans left + foreach ($handles as $cHandle => &$metadata) { + if ($metadata['finished'] == false) { + $metadata['finished'] = true; + self::finishMultiSpan(CURLE_OK, $cHandle, $metadata['span']->get()); // there is no way to get information if it was OK or not without calling curl_multi_info_read + } + } + + $mHandle['started'] = false; // reset multihandle started state in case it will be reused + // https://curl.se/libcurl/c/libcurl-multi.html If you want to reuse an easy handle that was added to the multi handle for transfer, you must first remove it from the multi stack and then re-add it again (possibly after having altered some options at your own choice). + unset($mHandle['handles']); + $mHandle['handles'] = new WeakMap(); + } + } + } + ); + + // curl_multi_info_read(CurlMultiHandle $multi_handle, int &$queued_messages = null): array|false + hook( + null, + 'curl_multi_info_read', + pre: null, + post: static function ($obj, array $params, mixed $retVal) use ($curlMultiToHandle) { + $mHandle = &$curlMultiToHandle[$params[0]]; + + if ($retVal != false) { + if ($retVal['msg'] == CURLMSG_DONE) { + if (!$mHandle['handles']->offsetExists($retVal['handle'])) { + return; + } + + $currentHandle = &$mHandle['handles'][$retVal['handle']]; + if ($currentHandle['finished']) { + return; + } + + $currentHandle['finished'] = true; + self::finishMultiSpan($retVal['result'], $retVal['handle'], $currentHandle['span']->get()); + } + } + } + ); + } + + private static function finishMultiSpan(int $curlResult, CurlHandle $curlHandle, SpanInterface $span) + { + if ($curlResult == CURLE_OK) { + self::setAttributesFromCurlGetInfo($curlHandle, $span); + } else { + $errorDescription = curl_strerror($curlResult) . ' (' . $curlResult . ')'; + $span->setStatus(StatusCode::STATUS_ERROR, $errorDescription); + $span->setAttribute(TraceAttributes::ERROR_TYPE, 'cURL error (' . $curlResult . ')'); + } + $span->end(); + } + + private static function redactUrlString(string $fullUrl) + { + $urlParts = parse_url($fullUrl); + if ($urlParts == false) { + return; + } + + $scheme = isset($urlParts['scheme']) ? $urlParts['scheme'] . '://' : ''; + $host = isset($urlParts['host']) ? $urlParts['host'] : ''; + $port = isset($urlParts['port']) ? ':' . $urlParts['port'] : ''; + $user = isset($urlParts['user']) ? 'REDACTED' : ''; + $pass = isset($urlParts['pass']) ? ':' . 'REDACTED' : ''; + $pass = ($user || $pass) ? "$pass@" : ''; + $path = isset($urlParts['path']) ? $urlParts['path'] : ''; + $query = isset($urlParts['query']) ? '?' . $urlParts['query'] : ''; + $fragment = isset($urlParts['fragment']) ? '#' . $urlParts['fragment'] : ''; + + return $scheme . $user . $pass . $host . $port . $path . $query . $fragment; + } + + private static function getAttributeFromCurlOption(int $option, mixed $value): ?array + { + switch ($option) { + case CURLOPT_CUSTOMREQUEST: + return [TraceAttributes::HTTP_REQUEST_METHOD, $value]; + case CURLOPT_HTTPGET: + // Based on https://github.com/curl/curl/blob/curl-7_73_0/lib/setopt.c#L841 + return [TraceAttributes::HTTP_REQUEST_METHOD, 'GET']; + case CURLOPT_POST: + return [TraceAttributes::HTTP_REQUEST_METHOD, ($value == 1 ? 'POST' : 'GET')]; + case CURLOPT_POSTFIELDS: + // Based on https://github.com/curl/curl/blob/curl-7_73_0/lib/setopt.c#L269 + return [TraceAttributes::HTTP_REQUEST_METHOD, 'POST']; + case CURLOPT_PUT: + return [TraceAttributes::HTTP_REQUEST_METHOD, ($value == 1 ? 'PUT' : 'GET')]; + case CURLOPT_NOBODY: + // Based on https://github.com/curl/curl/blob/curl-7_73_0/lib/setopt.c#L269 + return [TraceAttributes::HTTP_REQUEST_METHOD, ($value == 1 ? 'HEAD' : 'GET')]; + case CURLOPT_URL: + return [TraceAttributes::URL_FULL, self::redactUrlString($value)]; + case CURLOPT_USERAGENT: + return [TraceAttributes::USER_AGENT_ORIGINAL, $value]; + } + + return null; + } + + private static function setAttributesFromCurlGetInfo(CurlHandle $handle, SpanInterface $span) + { + $info = curl_getinfo($handle); + if (array_key_exists('http_code', $info)) { + $span->setAttribute(TraceAttributes::HTTP_RESPONSE_STATUS_CODE, $info['http_code']); + } + if (array_key_exists('download_content_length', $info) && $info['download_content_length'] > -1) { + $span->setAttribute(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH, $info['download_content_length']); + } + if (array_key_exists('upload_content_length', $info) && $info['upload_content_length'] > -1) { + $span->setAttribute(TraceAttributes::HTTP_REQUEST_BODY_SIZE, $info['upload_content_length']); + } + if (array_key_exists('scheme', $info)) { + $span->setAttribute(TraceAttributes::URL_SCHEME, $info['scheme']); + } + if (array_key_exists('primary_ip', $info)) { + $span->setAttribute(TraceAttributes::SERVER_ADDRESS, $info['primary_ip']); + } + if (array_key_exists('primary_port', $info)) { + $span->setAttribute(TraceAttributes::SERVER_PORT, $info['primary_port']); + } + } +} diff --git a/src/Instrumentation/Curl/tests/Integration/CurlInstrumentationTest.php b/src/Instrumentation/Curl/tests/Integration/CurlInstrumentationTest.php new file mode 100644 index 000000000..129517978 --- /dev/null +++ b/src/Instrumentation/Curl/tests/Integration/CurlInstrumentationTest.php @@ -0,0 +1,129 @@ + */ + private ArrayObject $storage; + + public function setUp(): void + { + $this->storage = new ArrayObject(); + $tracerProvider = new TracerProvider( + new SimpleSpanProcessor( + new InMemoryExporter($this->storage) + ) + ); + + $this->scope = Configurator::create() + ->withTracerProvider($tracerProvider) + ->activate(); + } + + public function tearDown(): void + { + $this->scope->detach(); + } + + public function test_curl_reset(): void + { + $ch = curl_init(); + curl_setopt($ch, CURLOPT_POST, 1); + curl_setopt($ch, CURLOPT_URL, 'http://gugugaga.gugugaga/'); + curl_reset($ch); + curl_exec($ch); + + $this->assertCount(1, $this->storage); + $span = $this->storage->offsetGet(0); + $this->assertSame('GET', $span->getName()); + $this->assertSame('Error', $span->getStatus()->getCode()); + $this->assertSame('URL using bad/illegal format or missing URL (3)', $span->getStatus()->getDescription()); + } + + public function test_curl_setopt(): void + { + $ch = curl_init(); + curl_setopt($ch, CURLOPT_POST, 1); + curl_setopt($ch, CURLOPT_URL, 'http://gugugaga.gugugaga/'); + curl_exec($ch); + + $this->assertCount(1, $this->storage); + $span = $this->storage->offsetGet(0); + $this->assertSame('POST', $span->getName()); + $this->assertSame('Error', $span->getStatus()->getCode()); + $this->assertSame('Couldn\'t resolve host name (6)', $span->getStatus()->getDescription()); + } + + public function test_curl_setopt_array(): void + { + $ch = curl_init(); + curl_setopt_array($ch, [CURLOPT_POST => 1, CURLOPT_URL => 'http://gugugaga.gugugaga/']); + curl_exec($ch); + + $this->assertCount(1, $this->storage); + $span = $this->storage->offsetGet(0); + $this->assertSame('POST', $span->getName()); + $this->assertSame('Error', $span->getStatus()->getCode()); + $this->assertSame('Couldn\'t resolve host name (6)', $span->getStatus()->getDescription()); + } + + public function test_curl_copy_handle(): void + { + $ch = curl_init('http://gugugaga.gugugaga/'); + curl_setopt($ch, CURLOPT_POST, 1); + + $ch_copy = curl_copy_handle($ch); + curl_close($ch); + + curl_exec($ch_copy); + + $this->assertCount(1, $this->storage); + $span = $this->storage->offsetGet(0); + $this->assertSame('POST', $span->getName()); + $this->assertSame('Error', $span->getStatus()->getCode()); + $this->assertSame('Couldn\'t resolve host name (6)', $span->getStatus()->getDescription()); + } + + public function test_curl_exec_with_error(): void + { + $ch = curl_init('http://gugugaga.gugugaga/'); + curl_exec($ch); + + $this->assertCount(1, $this->storage); + $span = $this->storage->offsetGet(0); + $this->assertSame('GET', $span->getName()); + $this->assertSame('Error', $span->getStatus()->getCode()); + $this->assertSame('Couldn\'t resolve host name (6)', $span->getStatus()->getDescription()); + $this->assertEquals('cURL error (6)', $span->getAttributes()->get(TraceAttributes::ERROR_TYPE)); + $this->assertEquals('GET', $span->getAttributes()->get(TraceAttributes::HTTP_REQUEST_METHOD)); + $this->assertEquals('http://gugugaga.gugugaga/', $span->getAttributes()->get(TraceAttributes::URL_FULL)); + } + + public function test_curl_exec(): void + { + $ch = curl_init('http://example.com/'); + curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); + curl_exec($ch); + + $this->assertCount(1, $this->storage); + $span = $this->storage->offsetGet(0); + $this->assertSame('GET', $span->getName()); + $this->assertEquals(200, $span->getAttributes()->get(TraceAttributes::HTTP_RESPONSE_STATUS_CODE)); + $this->assertEqualsIgnoringCase('http', $span->getAttributes()->get(TraceAttributes::URL_SCHEME)); + $this->assertEquals(80, $span->getAttributes()->get(TraceAttributes::SERVER_PORT)); + } +} diff --git a/src/Instrumentation/Curl/tests/Integration/CurlMultiInstrumentationTest.php b/src/Instrumentation/Curl/tests/Integration/CurlMultiInstrumentationTest.php new file mode 100644 index 000000000..2f07bbb25 --- /dev/null +++ b/src/Instrumentation/Curl/tests/Integration/CurlMultiInstrumentationTest.php @@ -0,0 +1,124 @@ + */ + private ArrayObject $storage; + + public function setUp(): void + { + $this->storage = new ArrayObject(); + $tracerProvider = new TracerProvider( + new SimpleSpanProcessor( + new InMemoryExporter($this->storage) + ) + ); + + $this->scope = Configurator::create() + ->withTracerProvider($tracerProvider) + ->activate(); + } + + public function tearDown(): void + { + $this->scope->detach(); + } + + public function test_curl_multi() + { + $mh = curl_multi_init(); + $ch1 = curl_init('http://example.com/'); + curl_setopt($ch1, CURLOPT_RETURNTRANSFER, 1); + + $ch2 = curl_copy_handle($ch1); + + curl_multi_add_handle($mh, $ch1); + curl_multi_add_handle($mh, $ch2); + + $running = null; + do { + curl_multi_exec($mh, $running); + + while (($info = curl_multi_info_read($mh)) !== false) { + } + } while ($running); + + curl_multi_remove_handle($mh, $ch1); + curl_multi_remove_handle($mh, $ch2); + curl_multi_close($mh); + + $this->assertCount(2, $this->storage); + foreach ([0, 1] as $offset) { + $span = $this->storage->offsetGet($offset); + $this->assertSame('GET', $span->getName()); + $this->assertEquals(200, $span->getAttributes()->get(TraceAttributes::HTTP_RESPONSE_STATUS_CODE)); + $this->assertEqualsIgnoringCase('http', $span->getAttributes()->get(TraceAttributes::URL_SCHEME)); + $this->assertEquals(80, $span->getAttributes()->get(TraceAttributes::SERVER_PORT)); + } + } + + public function test_curl_multi_error() + { + $mh = curl_multi_init(); + $ch1 = curl_init('unknown://scheme.com/'); + + curl_multi_add_handle($mh, $ch1); + + $running = null; + do { + curl_multi_exec($mh, $running); + + while (($info = curl_multi_info_read($mh)) !== false) { + } + } while ($running); + + curl_multi_close($mh); + + $this->assertCount(1, $this->storage); + $span = $this->storage->offsetGet(0); + $this->assertEquals('curl_multi_exec', $span->getAttributes()->get(TraceAttributes::CODE_FUNCTION)); + $this->assertEquals('unknown://scheme.com/', actual: $span->getAttributes()->get(TraceAttributes::URL_FULL)); + $this->assertSame('GET', $span->getName()); + } + + public function test_curl_multi_remove_handle() + { + $mh = curl_multi_init(); + $ch1 = curl_init('unknown://scheme.com/'); + $ch2 = curl_init('other://scheme.com/'); + + curl_multi_add_handle($mh, $ch1); + curl_multi_add_handle($mh, $ch2); + + curl_multi_remove_handle($mh, $ch1); + + $running = null; + do { + curl_multi_exec($mh, $running); + + while (($info = curl_multi_info_read($mh)) !== false) { + } + } while ($running); + + curl_multi_close($mh); + + $this->assertCount(1, $this->storage); + $span = $this->storage->offsetGet(0); + $this->assertEquals('other://scheme.com/', actual: $span->getAttributes()->get(TraceAttributes::URL_FULL)); + } +} diff --git a/src/Instrumentation/Curl/tests/Unit/.gitkeep b/src/Instrumentation/Curl/tests/Unit/.gitkeep new file mode 100644 index 000000000..e69de29bb From 7ef418875510b84d993787246b971a785d82900b Mon Sep 17 00:00:00 2001 From: Pawel Filipczak Date: Tue, 29 Oct 2024 16:09:10 +0100 Subject: [PATCH 2/6] Added Curl to php workflow --- .github/workflows/php.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index ee1d2ee4f..b000ebe6f 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -22,6 +22,7 @@ jobs: project: [ 'Aws', 'Context/Swoole', + 'Instrumentation/Curl', 'Instrumentation/CakePHP', 'Instrumentation/CodeIgniter', 'Instrumentation/ExtAmqp', @@ -85,6 +86,12 @@ jobs: php-version: 8.0 - project: 'Instrumentation/IO' php-version: 8.1 + - project: 'Instrumentation/Curl' + php-version: 7.4 + - project: 'Instrumentation/Curl' + php-version: 8.0 + - project: 'Instrumentation/Curl' + php-version: 8.1 - project: 'Instrumentation/PDO' php-version: 7.4 - project: 'Instrumentation/PDO' From 679c6fc80d6e2feffe2f8e852ba3f7291da13ede Mon Sep 17 00:00:00 2001 From: Pawel Filipczak Date: Tue, 29 Oct 2024 22:11:43 +0100 Subject: [PATCH 3/6] cleanup and optimisations --- .../Curl/src/CurlInstrumentation.php | 46 +++++++++---------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/src/Instrumentation/Curl/src/CurlInstrumentation.php b/src/Instrumentation/Curl/src/CurlInstrumentation.php index 9a79c52ff..b0deab87c 100644 --- a/src/Instrumentation/Curl/src/CurlInstrumentation.php +++ b/src/Instrumentation/Curl/src/CurlInstrumentation.php @@ -67,10 +67,9 @@ public static function register(): void return; } - $attr = self::getAttributeFromCurlOption($params[1], $params[2]); - if ($attr) { - $handleAttributes = &$curlHandleToAttributes[$params[0]]; - $handleAttributes[$attr[0]] = $attr[1]; + $attribute = self::getAttributeFromCurlOption($params[1], $params[2]); + if ($attribute) { + $curlHandleToAttributes[$params[0]][$attribute[0]] = $attribute[1]; } } ); @@ -85,10 +84,9 @@ public static function register(): void } foreach ($params[1] as $option => $value) { - $attr = self::getAttributeFromCurlOption($option, $value); - if ($attr) { - $handleAttributes = &$curlHandleToAttributes[$params[0]]; - $handleAttributes[$attr[0]] = $attr[1]; + $attribute = self::getAttributeFromCurlOption($option, $value); + if ($attribute) { + $curlHandleToAttributes[$params[0]][$attribute[0]] = $attribute[1]; } } } @@ -131,12 +129,11 @@ public static function register(): void null, 'curl_exec', pre: static function ($obj, array $params, ?string $class, ?string $function, ?string $filename, ?int $lineno) use ($instrumentation, $curlHandleToAttributes) { - /** @psalm-suppress ArgumentTypeCoercion */ if (!($params[0] instanceof CurlHandle)) { return; } - $spanName = array_key_exists(TraceAttributes::HTTP_REQUEST_METHOD, $curlHandleToAttributes[$params[0]]) ? $curlHandleToAttributes[$params[0]][TraceAttributes::HTTP_REQUEST_METHOD] : 'curl_exec'; + $spanName = $curlHandleToAttributes[$params[0]][TraceAttributes::HTTP_REQUEST_METHOD] ?? 'curl_exec'; $builder = $instrumentation->tracer() ->spanBuilder($spanName) @@ -196,8 +193,7 @@ public static function register(): void pre: null, post: static function ($obj, array $params, mixed $retVal) use ($curlMultiToHandle) { if ($retVal == 0) { - $mHandle = &$curlMultiToHandle[$params[0]]; - $mHandle['handles'][$params[1]] = ['finished' => false, 'span' => null]; + $curlMultiToHandle[$params[0]]['handles'][$params[1]] = ['finished' => false, 'span' => null]; } } ); @@ -240,7 +236,7 @@ public static function register(): void if (!$mHandle['started']) { // on first call to curl_multi_exec we're marking it's a transfer start for all curl handles attached to multi handle $parent = Context::getCurrent(); foreach ($handles as $cHandle => &$metadata) { - $spanName = array_key_exists(TraceAttributes::HTTP_REQUEST_METHOD, $curlHandleToAttributes[$cHandle]) ? $curlHandleToAttributes[$cHandle][TraceAttributes::HTTP_REQUEST_METHOD] : 'curl_multi_exec'; + $spanName = $curlHandleToAttributes[$cHandle][TraceAttributes::HTTP_REQUEST_METHOD] ?? 'curl_multi_exec'; $builder = $instrumentation->tracer() ->spanBuilder($spanName) ->setSpanKind(SpanKind::KIND_CLIENT) @@ -363,23 +359,23 @@ private static function getAttributeFromCurlOption(int $option, mixed $value): ? private static function setAttributesFromCurlGetInfo(CurlHandle $handle, SpanInterface $span) { $info = curl_getinfo($handle); - if (array_key_exists('http_code', $info)) { - $span->setAttribute(TraceAttributes::HTTP_RESPONSE_STATUS_CODE, $info['http_code']); + if (($value = $info['http_code'] ?? null) != 0) { + $span->setAttribute(TraceAttributes::HTTP_RESPONSE_STATUS_CODE, $value); } - if (array_key_exists('download_content_length', $info) && $info['download_content_length'] > -1) { - $span->setAttribute(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH, $info['download_content_length']); + if (($value = $info['download_content_length'] ?? null) > -1) { + $span->setAttribute(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH, $value); } - if (array_key_exists('upload_content_length', $info) && $info['upload_content_length'] > -1) { - $span->setAttribute(TraceAttributes::HTTP_REQUEST_BODY_SIZE, $info['upload_content_length']); + if (($value = $info['upload_content_length'] ?? null) > -1) { + $span->setAttribute(TraceAttributes::HTTP_REQUEST_BODY_SIZE, $value); } - if (array_key_exists('scheme', $info)) { - $span->setAttribute(TraceAttributes::URL_SCHEME, $info['scheme']); + if ($value = $info['scheme'] ?? null) { + $span->setAttribute(TraceAttributes::URL_SCHEME, $value); } - if (array_key_exists('primary_ip', $info)) { - $span->setAttribute(TraceAttributes::SERVER_ADDRESS, $info['primary_ip']); + if ($value = $info['primary_ip'] ?? null) { + $span->setAttribute(TraceAttributes::SERVER_ADDRESS, $value); } - if (array_key_exists('primary_port', $info)) { - $span->setAttribute(TraceAttributes::SERVER_PORT, $info['primary_port']); + if ($value = $info['primary_port'] ?? null) { + $span->setAttribute(TraceAttributes::SERVER_PORT, $value); } } } From cd99cc9bd1a9b83c187456bdd43167f067be47e1 Mon Sep 17 00:00:00 2001 From: Pawel Filipczak Date: Wed, 30 Oct 2024 08:43:07 +0100 Subject: [PATCH 4/6] Update .github/workflows/php.yml Co-authored-by: Brett McBride --- .github/workflows/php.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml index b000ebe6f..2fe06941e 100644 --- a/.github/workflows/php.yml +++ b/.github/workflows/php.yml @@ -22,9 +22,9 @@ jobs: project: [ 'Aws', 'Context/Swoole', - 'Instrumentation/Curl', 'Instrumentation/CakePHP', 'Instrumentation/CodeIgniter', + 'Instrumentation/Curl', 'Instrumentation/ExtAmqp', 'Instrumentation/ExtRdKafka', 'Instrumentation/Guzzle', From 1475f4d1bdaf1a653f63289af7e938f70ac148ed Mon Sep 17 00:00:00 2001 From: Pawel Filipczak Date: Wed, 30 Oct 2024 09:25:55 +0100 Subject: [PATCH 5/6] Updated gitsplit --- .gitsplit.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitsplit.yml b/.gitsplit.yml index db0a0dd02..06edf178c 100644 --- a/.gitsplit.yml +++ b/.gitsplit.yml @@ -14,6 +14,8 @@ splits: target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-sdk-bundle.git" - prefix: "src/Instrumentation/CodeIgniter" target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-auto-codeigniter.git" + - prefix: "src/Instrumentation/Curl" + target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-auto-curl.git" - prefix: "src/Instrumentation/ExtAmqp" target: "https://${GH_TOKEN}@github.com/opentelemetry-php/contrib-auto-ext-amqp.git" - prefix: "src/Instrumentation/ExtRdKafka" From aeac36d8e7b597d6aae19f66fbfe6178a3894001 Mon Sep 17 00:00:00 2001 From: Pawel Filipczak Date: Wed, 30 Oct 2024 13:39:00 +0100 Subject: [PATCH 6/6] Fixed phpstan issues --- .../Curl/src/CurlInstrumentation.php | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Instrumentation/Curl/src/CurlInstrumentation.php b/src/Instrumentation/Curl/src/CurlInstrumentation.php index b0deab87c..c3ef45229 100644 --- a/src/Instrumentation/Curl/src/CurlInstrumentation.php +++ b/src/Instrumentation/Curl/src/CurlInstrumentation.php @@ -51,8 +51,8 @@ public static function register(): void post: static function ($obj, array $params, mixed $retVal) use ($curlHandleToAttributes) { if ($retVal instanceof CurlHandle) { $curlHandleToAttributes[$retVal] = [TraceAttributes::HTTP_REQUEST_METHOD => 'GET']; - if ($params[0] !== null) { - $curlHandleToAttributes[$retVal][TraceAttributes::URL_FULL] = self::redactUrlString($params[0]); + if (($handle = $params[0] ?? null) !== null) { + $curlHandleToAttributes[$retVal][TraceAttributes::URL_FULL] = self::redactUrlString($handle); } } } @@ -96,7 +96,7 @@ public static function register(): void null, 'curl_close', pre: static function ($obj, array $params) use ($curlHandleToAttributes) { - if ($params[0] instanceof CurlHandle) { + if (count($params) > 0 && $params[0] instanceof CurlHandle) { $curlHandleToAttributes->offsetUnset($params[0]); } }, @@ -118,7 +118,7 @@ public static function register(): void null, 'curl_reset', pre: static function ($obj, array $params) use ($curlHandleToAttributes) { - if ($params[0] instanceof CurlHandle) { + if (count($params) > 0 && $params[0] instanceof CurlHandle) { $curlHandleToAttributes[$params[0]] = [TraceAttributes::HTTP_REQUEST_METHOD => 'GET']; } }, @@ -147,7 +147,7 @@ public static function register(): void $span = $builder->startSpan(); Context::storage()->attach($span->storeInContext($parent)); }, - post: static function ($obj, array $params, mixed $retVal) use ($curlHandleToAttributes) { + post: static function ($obj, array $params, mixed $retVal) { $scope = Context::storage()->scope(); if (!$scope) { return; @@ -359,22 +359,22 @@ private static function getAttributeFromCurlOption(int $option, mixed $value): ? private static function setAttributesFromCurlGetInfo(CurlHandle $handle, SpanInterface $span) { $info = curl_getinfo($handle); - if (($value = $info['http_code'] ?? null) != 0) { + if (($value = $info['http_code']) != 0) { $span->setAttribute(TraceAttributes::HTTP_RESPONSE_STATUS_CODE, $value); } - if (($value = $info['download_content_length'] ?? null) > -1) { + if (($value = $info['download_content_length']) > -1) { $span->setAttribute(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH, $value); } - if (($value = $info['upload_content_length'] ?? null) > -1) { + if (($value = $info['upload_content_length']) > -1) { $span->setAttribute(TraceAttributes::HTTP_REQUEST_BODY_SIZE, $value); } - if ($value = $info['scheme'] ?? null) { + if (!empty($value = $info['scheme'])) { $span->setAttribute(TraceAttributes::URL_SCHEME, $value); } - if ($value = $info['primary_ip'] ?? null) { + if (!empty($value = $info['primary_ip'])) { $span->setAttribute(TraceAttributes::SERVER_ADDRESS, $value); } - if ($value = $info['primary_port'] ?? null) { + if (($value = $info['primary_port']) != 0) { $span->setAttribute(TraceAttributes::SERVER_PORT, $value); } }