diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 0083401f..0cbd7f5c 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -63,6 +63,10 @@ jobs: - name: Setup Problem Matchers for PHPUnit run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" + - name: Update PHPUnit + run: composer require --dev phpunit/phpunit ^9.3.9 --no-update + if: matrix.php == '8.0' && matrix.dependencies == 'lowest' + - name: Install dependencies uses: ramsey/composer-install@v1 with: @@ -112,7 +116,7 @@ jobs: run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" - name: Remove optional packages - run: composer remove doctrine/dbal doctrine/doctrine-bundle symfony/messenger symfony/twig-bundle symfony/cache --dev --no-update + run: composer remove doctrine/dbal doctrine/doctrine-bundle symfony/messenger symfony/twig-bundle symfony/cache symfony/http-client --dev --no-update - name: Install dependencies uses: ramsey/composer-install@v1 diff --git a/CHANGELOG.md b/CHANGELOG.md index e77863f3..d56470cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- Add support for tracing of the Symfony HTTP client requests (#606) + ## 4.3.0 (2022-05-30) - Fix compatibility issue with Symfony >= 6.1.0 (#635) - Add `TracingDriverConnectionInterface::getNativeConnection()` method to get the original driver connection (#597) diff --git a/composer.json b/composer.json index fd124a43..537f5239 100644 --- a/composer.json +++ b/composer.json @@ -54,6 +54,7 @@ "symfony/cache": "^4.4.20||^5.0.11||^6.0", "symfony/dom-crawler": "^4.4.20||^5.0.11||^6.0", "symfony/framework-bundle": "^4.4.20||^5.0.11||^6.0", + "symfony/http-client": "^4.4.20||^5.0.11||^6.0", "symfony/messenger": "^4.4.20||^5.0.11||^6.0", "symfony/monolog-bundle": "^3.4", "symfony/phpunit-bridge": "^5.2.6||^6.0", diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index cda2318b..aa34e31c 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -105,6 +105,11 @@ parameters: count: 1 path: src/DependencyInjection/SentryExtension.php + - + message: "#^Parameter \\#2 \\$config of method Sentry\\\\SentryBundle\\\\DependencyInjection\\\\SentryExtension\\:\\:registerHttpClientTracingConfiguration\\(\\) expects array\\, mixed given\\.$#" + count: 1 + path: src/DependencyInjection/SentryExtension.php + - message: "#^Parameter \\#2 \\$config of method Sentry\\\\SentryBundle\\\\DependencyInjection\\\\SentryExtension\\:\\:registerMessengerListenerConfiguration\\(\\) expects array\\, mixed given\\.$#" count: 1 @@ -122,7 +127,7 @@ parameters: - message: "#^Parameter \\#2 \\$config of method Symfony\\\\Component\\\\DependencyInjection\\\\Extension\\\\Extension\\:\\:isConfigEnabled\\(\\) expects array, mixed given\\.$#" - count: 3 + count: 4 path: src/DependencyInjection/SentryExtension.php - @@ -235,6 +240,31 @@ parameters: count: 1 path: src/Tracing/Doctrine/DBAL/TracingStatementForV3.php + - + message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\HttpClient\\\\AbstractTraceableHttpClient\\:\\:request\\(\\) has parameter \\$options with no value type specified in iterable type array\\.$#" + count: 1 + path: src/Tracing/HttpClient/AbstractTraceableHttpClient.php + + - + message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\HttpClient\\\\AbstractTraceableResponse\\:\\:toArray\\(\\) return type has no value type specified in iterable type array\\.$#" + count: 1 + path: src/Tracing/HttpClient/AbstractTraceableResponse.php + + - + message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\HttpClient\\\\TraceableHttpClientForV6\\:\\:withOptions\\(\\) has parameter \\$options with no value type specified in iterable type array\\.$#" + count: 1 + path: src/Tracing/HttpClient/TraceableHttpClientForV6.php + + - + message: "#^Call to an undefined method Symfony\\\\Contracts\\\\HttpClient\\\\ResponseInterface\\:\\:toStream\\(\\)\\.$#" + count: 1 + path: src/Tracing/HttpClient/TraceableResponseForV5.php + + - + message: "#^Call to an undefined method Symfony\\\\Contracts\\\\HttpClient\\\\ResponseInterface\\:\\:toStream\\(\\)\\.$#" + count: 1 + path: src/Tracing/HttpClient/TraceableResponseForV6.php + - message: "#^Cannot access offset 'sample_rate' on mixed\\.$#" count: 1 @@ -410,3 +440,12 @@ parameters: count: 1 path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php + - + message: "#^Parameter \\#1 \\$responses of method Sentry\\\\SentryBundle\\\\Tracing\\\\HttpClient\\\\AbstractTraceableHttpClient\\:\\:stream\\(\\) expects iterable\\<\\(int\\|string\\), Symfony\\\\Contracts\\\\HttpClient\\\\ResponseInterface\\>\\|Symfony\\\\Contracts\\\\HttpClient\\\\ResponseInterface, stdClass given\\.$#" + count: 1 + path: tests/Tracing/HttpClient/TraceableHttpClientTest.php + + - + message: "#^Parameter \\#2 \\$responses of static method Sentry\\\\SentryBundle\\\\Tracing\\\\HttpClient\\\\AbstractTraceableResponse\\:\\:stream\\(\\) expects iterable\\, array\\ given\\.$#" + count: 1 + path: tests/Tracing/HttpClient/TraceableResponseTest.php diff --git a/phpstan.neon b/phpstan.neon index df2e90c3..b3ba7ee8 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -12,6 +12,8 @@ parameters: - src/aliases.php - src/Tracing/Doctrine/DBAL/TracingStatementForV2.php - src/Tracing/Doctrine/DBAL/TracingDriverForV2.php + - src/Tracing/HttpClient/TraceableHttpClientForV4.php + - src/Tracing/HttpClient/TraceableHttpClientForV5.php - tests/End2End/App - tests/Tracing/Doctrine/DBAL/TracingDriverForV2Test.php - tests/EventListener/Fixtures/UserWithoutIdentifierStub.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 5ef7a1e3..32016223 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -50,6 +50,16 @@ $params + + + toStream + + + + + toStream + + TracingDriverForV2 @@ -63,4 +73,23 @@ PostResponseEvent + + + $container->getParameter('sentry.tracing.cache.enabled') + + + + + $container->getParameter('sentry.tracing.enabled') + $container->getParameter('sentry.tracing.http_client.enabled') + + + + + $container->getParameter('sentry.tracing.enabled') + $container->getParameter('sentry.tracing.dbal.enabled') + $container->getParameter('sentry.tracing.dbal.connections') + $container->getParameter('doctrine.connections') + + diff --git a/psalm.xml b/psalm.xml index 39fe8712..89240539 100644 --- a/psalm.xml +++ b/psalm.xml @@ -11,6 +11,8 @@ + + diff --git a/src/DependencyInjection/Compiler/HttpClientTracingPass.php b/src/DependencyInjection/Compiler/HttpClientTracingPass.php new file mode 100644 index 00000000..2464532b --- /dev/null +++ b/src/DependencyInjection/Compiler/HttpClientTracingPass.php @@ -0,0 +1,32 @@ +getParameter('sentry.tracing.enabled') || !$container->getParameter('sentry.tracing.http_client.enabled')) { + return; + } + + foreach ($container->findTaggedServiceIds('http_client.client') as $id => $tags) { + $container->register('.sentry.traceable.' . $id, TraceableHttpClient::class) + ->setDecoratedService($id) + ->setArgument(0, new Reference('.sentry.traceable.' . $id . '.inner')) + ->setArgument(1, new Reference(HubInterface::class)) + ->addTag('kernel.reset', ['method' => 'reset']); + } + } +} diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 6e5a5902..05d06550 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -14,6 +14,7 @@ use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; +use Symfony\Component\HttpClient\HttpClient; use Symfony\Component\Messenger\MessageBusInterface; final class Configuration implements ConfigurationInterface @@ -184,6 +185,9 @@ private function addDistributedTracingSection(ArrayNodeDefinition $rootNode): vo ->arrayNode('cache') ->{class_exists(CacheItem::class) ? 'canBeDisabled' : 'canBeEnabled'}() ->end() + ->arrayNode('http_client') + ->{class_exists(HttpClient::class) ? 'canBeDisabled' : 'canBeEnabled'}() + ->end() ->arrayNode('console') ->addDefaultsIfNotSet() ->fixXmlConfig('excluded_command') diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index 1b2bc394..e60e7a84 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -37,6 +37,7 @@ use Symfony\Component\DependencyInjection\Loader; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\ErrorHandler\Error\FatalError; +use Symfony\Component\HttpClient\HttpClient; use Symfony\Component\HttpKernel\DependencyInjection\ConfigurableExtension; final class SentryExtension extends ConfigurableExtension @@ -74,6 +75,7 @@ protected function loadInternal(array $mergedConfig, ContainerBuilder $container $this->registerDbalTracingConfiguration($container, $mergedConfig['tracing']); $this->registerTwigTracingConfiguration($container, $mergedConfig['tracing']); $this->registerCacheTracingConfiguration($container, $mergedConfig['tracing']); + $this->registerHttpClientTracingConfiguration($container, $mergedConfig['tracing']); } /** @@ -247,6 +249,21 @@ private function registerCacheTracingConfiguration(ContainerBuilder $container, $container->setParameter('sentry.tracing.cache.enabled', $isConfigEnabled); } + /** + * @param array $config + */ + private function registerHttpClientTracingConfiguration(ContainerBuilder $container, array $config): void + { + $isConfigEnabled = $this->isConfigEnabled($container, $config) + && $this->isConfigEnabled($container, $config['http_client']); + + if ($isConfigEnabled && !class_exists(HttpClient::class)) { + throw new LogicException('Http client tracing support cannot be enabled because the symfony/http-client Composer package is not installed.'); + } + + $container->setParameter('sentry.tracing.http_client.enabled', $isConfigEnabled); + } + /** * @param string[] $integrations * @param array $config diff --git a/src/Resources/config/schema/sentry-1.0.xsd b/src/Resources/config/schema/sentry-1.0.xsd index 79a6aac4..9240c5c6 100644 --- a/src/Resources/config/schema/sentry-1.0.xsd +++ b/src/Resources/config/schema/sentry-1.0.xsd @@ -91,6 +91,7 @@ + @@ -117,4 +118,8 @@ + + + + diff --git a/src/SentryBundle.php b/src/SentryBundle.php index 4098f8b3..130999eb 100644 --- a/src/SentryBundle.php +++ b/src/SentryBundle.php @@ -6,6 +6,7 @@ use Sentry\SentryBundle\DependencyInjection\Compiler\CacheTracingPass; use Sentry\SentryBundle\DependencyInjection\Compiler\DbalTracingPass; +use Sentry\SentryBundle\DependencyInjection\Compiler\HttpClientTracingPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; @@ -19,5 +20,6 @@ public function build(ContainerBuilder $container): void $container->addCompilerPass(new DbalTracingPass()); $container->addCompilerPass(new CacheTracingPass()); + $container->addCompilerPass(new HttpClientTracingPass()); } } diff --git a/src/Tracing/HttpClient/AbstractTraceableHttpClient.php b/src/Tracing/HttpClient/AbstractTraceableHttpClient.php new file mode 100644 index 00000000..3ad27ad4 --- /dev/null +++ b/src/Tracing/HttpClient/AbstractTraceableHttpClient.php @@ -0,0 +1,98 @@ +client = $client; + $this->hub = $hub; + } + + /** + * {@inheritdoc} + */ + public function request(string $method, string $url, array $options = []): ResponseInterface + { + $span = null; + $parent = $this->hub->getSpan(); + + if (null !== $parent) { + $headers = $options['headers'] ?? []; + $headers['sentry-trace'] = $parent->toTraceparent(); + $options['headers'] = $headers; + + $context = new SpanContext(); + $context->setOp('http.client'); + $context->setDescription('HTTP ' . $method); + $context->setTags([ + 'http.method' => $method, + 'http.url' => $url, + ]); + + $span = $parent->startChild($context); + } + + return new TraceableResponse($this->client, $this->client->request($method, $url, $options), $span); + } + + /** + * {@inheritdoc} + */ + public function stream($responses, float $timeout = null): ResponseStreamInterface + { + if ($responses instanceof AbstractTraceableResponse) { + $responses = [$responses]; + } elseif (!is_iterable($responses)) { + throw new \TypeError(sprintf('"%s()" expects parameter 1 to be an iterable of TraceableResponse objects, "%s" given.', __METHOD__, get_debug_type($responses))); + } + + return new ResponseStream(AbstractTraceableResponse::stream($this->client, $responses, $timeout)); + } + + public function reset(): void + { + if ($this->client instanceof ResetInterface) { + $this->client->reset(); + } + } + + /** + * {@inheritdoc} + */ + public function setLogger(LoggerInterface $logger): void + { + if ($this->client instanceof LoggerAwareInterface) { + $this->client->setLogger($logger); + } + } +} diff --git a/src/Tracing/HttpClient/AbstractTraceableResponse.php b/src/Tracing/HttpClient/AbstractTraceableResponse.php new file mode 100644 index 00000000..71571294 --- /dev/null +++ b/src/Tracing/HttpClient/AbstractTraceableResponse.php @@ -0,0 +1,131 @@ +client = $client; + $this->response = $response; + $this->span = $span; + } + + public function __destruct() + { + try { + if (method_exists($this->response, '__destruct')) { + $this->response->__destruct(); + } + } finally { + $this->finishSpan(); + } + } + + public function __sleep(): array + { + throw new \BadMethodCallException('Serializing instances of this class is forbidden.'); + } + + public function __wakeup() + { + throw new \BadMethodCallException('Unserializing instances of this class is forbidden.'); + } + + public function getStatusCode(): int + { + return $this->response->getStatusCode(); + } + + public function getHeaders(bool $throw = true): array + { + return $this->response->getHeaders($throw); + } + + public function getContent(bool $throw = true): string + { + try { + return $this->response->getContent($throw); + } finally { + $this->finishSpan(); + } + } + + public function toArray(bool $throw = true): array + { + try { + return $this->response->toArray($throw); + } finally { + $this->finishSpan(); + } + } + + public function cancel(): void + { + $this->response->cancel(); + $this->finishSpan(); + } + + /** + * @internal + * + * @param iterable $responses + * + * @return \Generator + */ + public static function stream(HttpClientInterface $client, iterable $responses, ?float $timeout): \Generator + { + /** @var \SplObjectStorage $traceableMap */ + $traceableMap = new \SplObjectStorage(); + $wrappedResponses = []; + + foreach ($responses as $response) { + if (!$response instanceof self) { + throw new \TypeError(sprintf('"%s::stream()" expects parameter 1 to be an iterable of TraceableResponse objects, "%s" given.', TraceableHttpClient::class, get_debug_type($response))); + } + + $traceableMap[$response->response] = $response; + $wrappedResponses[] = $response->response; + } + + foreach ($client->stream($wrappedResponses, $timeout) as $response => $chunk) { + $traceableResponse = $traceableMap[$response]; + $traceableResponse->finishSpan(); + + yield $traceableResponse => $chunk; + } + } + + private function finishSpan(): void + { + if (null !== $this->span) { + $this->span->finish(); + $this->span = null; + } + } +} diff --git a/src/Tracing/HttpClient/TraceableHttpClientForV4.php b/src/Tracing/HttpClient/TraceableHttpClientForV4.php new file mode 100644 index 00000000..63e31dd9 --- /dev/null +++ b/src/Tracing/HttpClient/TraceableHttpClientForV4.php @@ -0,0 +1,12 @@ +client = $this->client->withOptions($options); + + return $clone; + } +} diff --git a/src/Tracing/HttpClient/TraceableHttpClientForV6.php b/src/Tracing/HttpClient/TraceableHttpClientForV6.php new file mode 100644 index 00000000..ab9e0c8e --- /dev/null +++ b/src/Tracing/HttpClient/TraceableHttpClientForV6.php @@ -0,0 +1,22 @@ +client = $this->client->withOptions($options); + + return $clone; + } +} diff --git a/src/Tracing/HttpClient/TraceableResponseForV4.php b/src/Tracing/HttpClient/TraceableResponseForV4.php new file mode 100644 index 00000000..0cbaedad --- /dev/null +++ b/src/Tracing/HttpClient/TraceableResponseForV4.php @@ -0,0 +1,19 @@ +response->getInfo($type); + } +} diff --git a/src/Tracing/HttpClient/TraceableResponseForV5.php b/src/Tracing/HttpClient/TraceableResponseForV5.php new file mode 100644 index 00000000..0ca572a2 --- /dev/null +++ b/src/Tracing/HttpClient/TraceableResponseForV5.php @@ -0,0 +1,31 @@ +response->getInfo($type); + } + + /** + * {@inheritdoc} + */ + public function toStream(bool $throw = true) + { + return $this->response->toStream($throw); + } +} diff --git a/src/Tracing/HttpClient/TraceableResponseForV6.php b/src/Tracing/HttpClient/TraceableResponseForV6.php new file mode 100644 index 00000000..43dbbbf2 --- /dev/null +++ b/src/Tracing/HttpClient/TraceableResponseForV6.php @@ -0,0 +1,29 @@ +response->getInfo($type); + } + + /** + * {@inheritdoc} + */ + public function toStream(bool $throw = true) + { + return $this->response->toStream($throw); + } +} diff --git a/src/aliases.php b/src/aliases.php index 5d0894aa..839a55c9 100644 --- a/src/aliases.php +++ b/src/aliases.php @@ -11,12 +11,24 @@ use Sentry\SentryBundle\Tracing\Cache\TraceableTagAwareCacheAdapter; use Sentry\SentryBundle\Tracing\Cache\TraceableTagAwareCacheAdapterForV2; use Sentry\SentryBundle\Tracing\Cache\TraceableTagAwareCacheAdapterForV3; +use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriver; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverForV2; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverForV3; +use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingStatement; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingStatementForV2; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingStatementForV3; +use Sentry\SentryBundle\Tracing\HttpClient\TraceableHttpClient; +use Sentry\SentryBundle\Tracing\HttpClient\TraceableHttpClientForV4; +use Sentry\SentryBundle\Tracing\HttpClient\TraceableHttpClientForV5; +use Sentry\SentryBundle\Tracing\HttpClient\TraceableHttpClientForV6; +use Sentry\SentryBundle\Tracing\HttpClient\TraceableResponse; +use Sentry\SentryBundle\Tracing\HttpClient\TraceableResponseForV4; +use Sentry\SentryBundle\Tracing\HttpClient\TraceableResponseForV5; +use Sentry\SentryBundle\Tracing\HttpClient\TraceableResponseForV6; use Symfony\Component\Cache\Adapter\AdapterInterface; use Symfony\Component\Cache\DoctrineProvider; +use Symfony\Component\HttpClient\Response\StreamableInterface; +use Symfony\Contracts\HttpClient\ResponseInterface; if (interface_exists(AdapterInterface::class)) { if (!class_exists(DoctrineProvider::class, false) && version_compare(\PHP_VERSION, '8.0.0', '>=')) { @@ -38,12 +50,25 @@ class_alias(TraceableTagAwareCacheAdapterForV2::class, TraceableTagAwareCacheAda } } -if (!class_exists('Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingStatement')) { +if (!class_exists(TracingStatement::class)) { if (class_exists(Result::class)) { - class_alias(TracingStatementForV3::class, 'Sentry\\SentryBundle\\Tracing\\Doctrine\\DBAL\\TracingStatement'); - class_alias(TracingDriverForV3::class, 'Sentry\\SentryBundle\\Tracing\\Doctrine\\DBAL\\TracingDriver'); + class_alias(TracingStatementForV3::class, TracingStatement::class); + class_alias(TracingDriverForV3::class, TracingDriver::class); } elseif (interface_exists(Result::class)) { - class_alias(TracingStatementForV2::class, 'Sentry\\SentryBundle\\Tracing\\Doctrine\\DBAL\\TracingStatement'); - class_alias(TracingDriverForV2::class, 'Sentry\\SentryBundle\\Tracing\\Doctrine\\DBAL\\TracingDriver'); + class_alias(TracingStatementForV2::class, TracingStatement::class); + class_alias(TracingDriverForV2::class, TracingDriver::class); + } +} + +if (!class_exists(TraceableResponse::class) && interface_exists(ResponseInterface::class)) { + if (!interface_exists(StreamableInterface::class)) { + class_alias(TraceableResponseForV4::class, TraceableResponse::class); + class_alias(TraceableHttpClientForV4::class, TraceableHttpClient::class); + } elseif (version_compare(\PHP_VERSION, '8.0', '>=')) { + class_alias(TraceableResponseForV6::class, TraceableResponse::class); + class_alias(TraceableHttpClientForV6::class, TraceableHttpClient::class); + } else { + class_alias(TraceableResponseForV5::class, TraceableResponse::class); + class_alias(TraceableHttpClientForV5::class, TraceableHttpClient::class); } } diff --git a/tests/DependencyInjection/Compiler/HttpClientTracingPassTest.php b/tests/DependencyInjection/Compiler/HttpClientTracingPassTest.php new file mode 100644 index 00000000..9f3d12a7 --- /dev/null +++ b/tests/DependencyInjection/Compiler/HttpClientTracingPassTest.php @@ -0,0 +1,85 @@ +createContainerBuilder(true, true); + $container->compile(); + + $this->assertSame(TraceableHttpClient::class, $container->findDefinition('http.client')->getClass()); + } + + /** + * @dataProvider processDoesNothingIfConditionsForEnablingTracingAreMissingDataProvider + */ + public function testProcessDoesNothingIfConditionsForEnablingTracingAreMissing(bool $isTracingEnabled, bool $isHttpClientTracingEnabled): void + { + $container = $this->createContainerBuilder($isTracingEnabled, $isHttpClientTracingEnabled); + $container->compile(); + + $this->assertSame(HttpClient::class, $container->getDefinition('http.client')->getClass()); + } + + /** + * @return \Generator + */ + public function processDoesNothingIfConditionsForEnablingTracingAreMissingDataProvider(): \Generator + { + yield [ + true, + false, + ]; + + yield [ + false, + false, + ]; + + yield [ + false, + true, + ]; + } + + private function createContainerBuilder(bool $isTracingEnabled, bool $isHttpClientTracingEnabled): ContainerBuilder + { + $container = new ContainerBuilder(); + $container->addCompilerPass(new HttpClientTracingPass()); + $container->setParameter('sentry.tracing.enabled', $isTracingEnabled); + $container->setParameter('sentry.tracing.http_client.enabled', $isHttpClientTracingEnabled); + + $container->register(HubInterface::class, HubInterface::class) + ->setPublic(true); + + $container->register('http.client', HttpClient::class) + ->setPublic(true) + ->addTag('http_client.client'); + + return $container; + } + + private static function isHttpClientPackageInstalled(): bool + { + return interface_exists(HttpClientInterface::class); + } +} diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index 5f73cfde..9b853130 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -12,6 +12,7 @@ use Symfony\Component\Cache\CacheItem; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Config\Definition\Processor; +use Symfony\Component\HttpClient\HttpClient; use Symfony\Component\Messenger\MessageBusInterface; final class ConfigurationTest extends TestCase @@ -52,6 +53,9 @@ public function testProcessConfigurationWithDefaultConfiguration(): void 'cache' => [ 'enabled' => class_exists(CacheItem::class), ], + 'http_client' => [ + 'enabled' => class_exists(HttpClient::class), + ], 'console' => [ 'excluded_commands' => ['messenger:consume'], ], diff --git a/tests/DependencyInjection/Fixtures/php/full.php b/tests/DependencyInjection/Fixtures/php/full.php index ae7f861e..82c71ef8 100644 --- a/tests/DependencyInjection/Fixtures/php/full.php +++ b/tests/DependencyInjection/Fixtures/php/full.php @@ -51,6 +51,9 @@ 'enabled' => false, 'connections' => ['default'], ], + 'http_client' => [ + 'enabled' => false, + ], 'twig' => [ 'enabled' => false, ], diff --git a/tests/DependencyInjection/Fixtures/php/http_client_tracing_enabled.php b/tests/DependencyInjection/Fixtures/php/http_client_tracing_enabled.php new file mode 100644 index 00000000..639e2f9a --- /dev/null +++ b/tests/DependencyInjection/Fixtures/php/http_client_tracing_enabled.php @@ -0,0 +1,14 @@ +loadFromExtension('sentry', [ + 'tracing' => [ + 'http_client' => [ + 'enabled' => true, + ], + ], +]); diff --git a/tests/DependencyInjection/Fixtures/xml/full.xml b/tests/DependencyInjection/Fixtures/xml/full.xml index cab3d83a..e9240f89 100644 --- a/tests/DependencyInjection/Fixtures/xml/full.xml +++ b/tests/DependencyInjection/Fixtures/xml/full.xml @@ -49,6 +49,7 @@ + app:command diff --git a/tests/DependencyInjection/Fixtures/xml/http_client_tracing_enabled.xml b/tests/DependencyInjection/Fixtures/xml/http_client_tracing_enabled.xml new file mode 100644 index 00000000..cb0621ab --- /dev/null +++ b/tests/DependencyInjection/Fixtures/xml/http_client_tracing_enabled.xml @@ -0,0 +1,14 @@ + + + + + + + + + + diff --git a/tests/DependencyInjection/Fixtures/yml/full.yml b/tests/DependencyInjection/Fixtures/yml/full.yml index 19981f21..e3c86290 100644 --- a/tests/DependencyInjection/Fixtures/yml/full.yml +++ b/tests/DependencyInjection/Fixtures/yml/full.yml @@ -50,6 +50,8 @@ sentry: enabled: false cache: enabled: false + http_client: + enabled: false console: excluded_commands: - app:command diff --git a/tests/DependencyInjection/Fixtures/yml/http_client_tracing_enabled.yml b/tests/DependencyInjection/Fixtures/yml/http_client_tracing_enabled.yml new file mode 100644 index 00000000..50d56ede --- /dev/null +++ b/tests/DependencyInjection/Fixtures/yml/http_client_tracing_enabled.yml @@ -0,0 +1,4 @@ +sentry: + tracing: + http_client: + enabled: true diff --git a/tests/DependencyInjection/SentryExtensionTest.php b/tests/DependencyInjection/SentryExtensionTest.php index eaafea6d..e907e512 100644 --- a/tests/DependencyInjection/SentryExtensionTest.php +++ b/tests/DependencyInjection/SentryExtensionTest.php @@ -35,6 +35,7 @@ use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\ErrorHandler\Error\FatalError; +use Symfony\Component\HttpClient\HttpClient; use Symfony\Component\HttpKernel\KernelEvents; use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent; use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent; @@ -380,6 +381,18 @@ public function testTwigTracingExtensionIsConfiguredWhenTwigTracingIsEnabled(): $this->assertTrue($container->hasDefinition(TwigTracingExtension::class)); } + public function testHttpClientTracingExtensionIsConfiguredWhenHttpClientTracingIsEnabled(): void + { + if (!class_exists(HttpClient::class)) { + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('Http client tracing support cannot be enabled because the symfony/http-client Composer package is not installed.'); + } + + $container = $this->createContainerFromFixture('http_client_tracing_enabled'); + + $this->assertTrue($container->getParameter('sentry.tracing.http_client.enabled')); + } + public function testTwigTracingExtensionIsRemovedWhenTwigTracingIsDisabled(): void { $container = $this->createContainerFromFixture('full'); diff --git a/tests/Tracing/HttpClient/TraceableHttpClientTest.php b/tests/Tracing/HttpClient/TraceableHttpClientTest.php new file mode 100644 index 00000000..ba9da822 --- /dev/null +++ b/tests/Tracing/HttpClient/TraceableHttpClientTest.php @@ -0,0 +1,204 @@ +hub = $this->createMock(HubInterface::class); + $this->decoratedHttpClient = $this->createMock(TestableHttpClientInterface::class); + $this->httpClient = new TraceableHttpClient($this->decoratedHttpClient, $this->hub); + } + + public function testRequest(): void + { + $transaction = new Transaction(new TransactionContext()); + $transaction->initSpanRecorder(); + + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn($transaction); + + $mockResponse = new MockResponse(); + $decoratedHttpClient = new MockHttpClient($mockResponse); + $httpClient = new TraceableHttpClient($decoratedHttpClient, $this->hub); + $response = $httpClient->request('GET', 'https://www.example.com/test-page'); + + $this->assertInstanceOf(AbstractTraceableResponse::class, $response); + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame('GET', $response->getInfo('http_method')); + $this->assertSame('https://www.example.com/test-page', $response->getInfo('url')); + $this->assertSame(['sentry-trace: ' . $transaction->toTraceparent()], $mockResponse->getRequestOptions()['normalized_headers']['sentry-trace']); + $this->assertNotNull($transaction->getSpanRecorder()); + + $spans = $transaction->getSpanRecorder()->getSpans(); + $expectedTags = [ + 'http.method' => 'GET', + 'http.url' => 'https://www.example.com/test-page', + ]; + + $this->assertCount(2, $spans); + $this->assertNull($spans[1]->getEndTimestamp()); + $this->assertSame('http.client', $spans[1]->getOp()); + $this->assertSame('HTTP GET', $spans[1]->getDescription()); + $this->assertSame($expectedTags, $spans[1]->getTags()); + } + + public function testStream(): void + { + $transaction = new Transaction(new TransactionContext()); + $transaction->initSpanRecorder(); + + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn($transaction); + + $decoratedHttpClient = new MockHttpClient(new MockResponse(['foo', 'bar'])); + $httpClient = new TraceableHttpClient($decoratedHttpClient, $this->hub); + $response = $httpClient->request('GET', 'https://www.example.com/test-page'); + $chunks = []; + + foreach ($httpClient->stream($response) as $chunkResponse => $chunk) { + $this->assertSame($response, $chunkResponse); + + $chunks[] = $chunk->getContent(); + } + + $this->assertNotNull($transaction->getSpanRecorder()); + + $spans = $transaction->getSpanRecorder()->getSpans(); + $expectedTags = [ + 'http.method' => 'GET', + 'http.url' => 'https://www.example.com/test-page', + ]; + + $this->assertSame('foobar', implode('', $chunks)); + $this->assertCount(2, $spans); + $this->assertNotNull($spans[1]->getEndTimestamp()); + $this->assertSame('http.client', $spans[1]->getOp()); + $this->assertSame('HTTP GET', $spans[1]->getDescription()); + $this->assertSame($expectedTags, $spans[1]->getTags()); + + $loopIndex = 0; + + foreach ($httpClient->stream($response) as $chunk) { + ++$loopIndex; + } + + $this->assertSame(1, $loopIndex); + } + + public function testStreamThrowsExceptionIfResponsesArgumentIsInvalid(): void + { + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('"Sentry\\SentryBundle\\Tracing\\HttpClient\\AbstractTraceableHttpClient::stream()" expects parameter 1 to be an iterable of TraceableResponse objects, "stdClass" given.'); + + $this->httpClient->stream(new \stdClass()); + } + + public function testSetLogger(): void + { + $logger = new NullLogger(); + + $this->decoratedHttpClient->expects($this->once()) + ->method('setLogger') + ->with($logger); + + $this->httpClient->setLogger($logger); + } + + public function testReset(): void + { + $this->decoratedHttpClient->expects($this->once()) + ->method('reset'); + + $this->httpClient->reset(); + } + + public function testWithOptions(): void + { + if (!method_exists(MockHttpClient::class, 'withOptions')) { + self::markTestSkipped(); + } + + $transaction = new Transaction(new TransactionContext()); + $transaction->initSpanRecorder(); + + $this->hub->expects($this->exactly(2)) + ->method('getSpan') + ->willReturn($transaction); + + $responses = [ + new MockResponse(), + new MockResponse(), + ]; + + $decoratedHttpClient = new MockHttpClient($responses, 'https://www.example.com'); + $httpClient1 = new TraceableHttpClient($decoratedHttpClient, $this->hub); + $httpClient2 = $httpClient1->withOptions(['base_uri' => 'https://www.example.org']); + + $this->assertNotSame($httpClient1, $httpClient2); + + $response = $httpClient1->request('GET', 'test-page'); + + $this->assertInstanceOf(AbstractTraceableResponse::class, $response); + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame('GET', $response->getInfo('http_method')); + $this->assertSame('https://www.example.com/test-page', $response->getInfo('url')); + + $response = $httpClient2->request('GET', 'test-page'); + + $this->assertInstanceOf(AbstractTraceableResponse::class, $response); + $this->assertSame(200, $response->getStatusCode()); + $this->assertSame('GET', $response->getInfo('http_method')); + $this->assertSame('https://www.example.org/test-page', $response->getInfo('url')); + } + + private static function isHttpClientPackageInstalled(): bool + { + return interface_exists(HttpClientInterface::class); + } +} + +interface TestableHttpClientInterface extends HttpClientInterface, LoggerAwareInterface, ResetInterface +{ +} diff --git a/tests/Tracing/HttpClient/TraceableResponseTest.php b/tests/Tracing/HttpClient/TraceableResponseTest.php new file mode 100644 index 00000000..e67616d5 --- /dev/null +++ b/tests/Tracing/HttpClient/TraceableResponseTest.php @@ -0,0 +1,141 @@ +client = $this->createMock(HttpClientInterface::class); + $this->hub = $this->createMock(HubInterface::class); + } + + public function testInstanceCannotBeSerialized(): void + { + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('Serializing instances of this class is forbidden.'); + + serialize(new TraceableResponse($this->client, new MockResponse(), null)); + } + + public function testInstanceCannotBeUnserialized(): void + { + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('Unserializing instances of this class is forbidden.'); + + unserialize(sprintf('O:%u:"%s":0:{}', \strlen(TraceableResponse::class), TraceableResponse::class)); + } + + public function testDestructor(): void + { + $transaction = new Transaction(new TransactionContext(), $this->hub); + $context = new SpanContext(); + $span = $transaction->startChild($context); + $response = new TraceableResponse($this->client, new MockResponse(), $span); + + // Call gc to invoke destructors at the right time. + unset($response); + + gc_mem_caches(); + gc_collect_cycles(); + + $this->assertNotNull($span->getEndTimestamp()); + } + + public function testGetStatusCode(): void + { + $response = new TraceableResponse($this->client, new MockResponse(), null); + + $this->assertSame(200, $response->getStatusCode()); + } + + public function testGetHeaders(): void + { + $expectedHeaders = ['content-length' => ['0']]; + $response = new TraceableResponse($this->client, new MockResponse('', ['response_headers' => $expectedHeaders]), null); + + $this->assertSame($expectedHeaders, $response->getHeaders()); + } + + public function testGetContent(): void + { + $span = new Span(); + $httpClient = new MockHttpClient(new MockResponse('foobar')); + $response = new TraceableResponse($httpClient, $httpClient->request('GET', 'https://www.example.org/'), $span); + + $this->assertSame('foobar', $response->getContent()); + $this->assertNotNull($span->getEndTimestamp()); + } + + public function testToArray(): void + { + $span = new Span(); + $httpClient = new MockHttpClient(new MockResponse('{"foo":"bar"}')); + $response = new TraceableResponse($this->client, $httpClient->request('GET', 'https://www.example.org/'), $span); + + $this->assertSame(['foo' => 'bar'], $response->toArray()); + $this->assertNotNull($span->getEndTimestamp()); + } + + public function testCancel(): void + { + $span = new Span(); + $response = new TraceableResponse($this->client, new MockResponse(), $span); + + $response->cancel(); + + $this->assertTrue($response->getInfo('canceled')); + $this->assertNotNull($span->getEndTimestamp()); + } + + public function testGetInfo(): void + { + $response = new TraceableResponse($this->client, new MockResponse(), null); + + $this->assertSame(200, $response->getInfo('http_code')); + } + + public function testToStream(): void + { + $httpClient = new MockHttpClient(new MockResponse('foobar')); + $response = new TraceableResponse($this->client, $httpClient->request('GET', 'https://www.example.org/'), null); + + if (!method_exists($response, 'toStream')) { + $this->markTestSkipped('The TraceableResponse::toStream() method is not supported'); + } + + $this->assertSame('foobar', stream_get_contents($response->toStream())); + } + + public function testStreamThrowsExceptionIfResponsesArgumentIsInvalid(): void + { + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('"Sentry\\SentryBundle\\Tracing\\HttpClient\\TraceableHttpClient::stream()" expects parameter 1 to be an iterable of TraceableResponse objects, "stdClass" given.'); + + iterator_to_array(TraceableResponse::stream($this->client, [new \stdClass()], null)); + } +}