Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracing: symfony http client #606

Merged
merged 31 commits into from Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
bc0592b
http client tracing
alekitto Mar 22, 2022
c680429
sentry sentry-trace header in tracing http client
alekitto Mar 23, 2022
166c478
add test on http client request
alekitto Mar 23, 2022
5ce0a9d
use newer phpunit on php 8
alekitto Mar 23, 2022
9d42874
require php 8.1 in psalm
alekitto Mar 23, 2022
0ee2c44
Revert "require php 8.1 in psalm"
alekitto Apr 12, 2022
0648814
fix for review
alekitto Apr 13, 2022
73132c2
add a couple of tests and changelog line
alekitto Apr 14, 2022
095abc3
use http.url and http.method tags, instead of url and method
alekitto Apr 15, 2022
d4c749f
http.request -> http.client
alekitto May 2, 2022
12490c4
add tests for traceable response classes
alekitto Jun 22, 2022
6760d1b
style fixes
alekitto Jun 22, 2022
79695c3
create fixtures for destructible and streamable responses
alekitto Jun 22, 2022
c4687c1
use ->assert instead of self::assert in phpunit tests
alekitto Jun 22, 2022
38faf38
mark test classes as final
alekitto Jun 22, 2022
0e2e979
rename variable
alekitto Jun 22, 2022
b00333f
test http-client tracing enabled/disabled
alekitto Jun 22, 2022
4a28c72
add span description
alekitto Jun 22, 2022
0d63111
update CHANGELOG.md
alekitto Jun 23, 2022
fd9f433
Add a test for the `TraceableHttpClient::withOptions()` method
ste93cry Jul 4, 2022
39eaa7f
fix withOptions test for v4
alekitto Jul 5, 2022
993c494
add test for http-client stream method
alekitto Jul 5, 2022
f2f5adc
Mark the `TraceableHttpClientFor*` classes as `@internal`
ste93cry Jul 6, 2022
e732d59
Minor methods and variables names changes
ste93cry Jul 6, 2022
c6d5f2b
Improve the test for the `TraceableHttpClient::stream()` method
ste93cry Jul 6, 2022
8a824d7
Improve the tests for the `TraceableResponse` class
ste93cry Jul 7, 2022
27d9d53
fix tests for lower deps
alekitto Jul 11, 2022
dcea7b5
fix static analysis
alekitto Jul 11, 2022
71ba653
add error conditions tests for stream methods
alekitto Jul 12, 2022
0ae2524
Minor improvements to the tests
ste93cry Jul 14, 2022
d9a3a71
Improve the tests for the `HttpClientTracingPass` class
ste93cry Jul 14, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/workflows/tests.yaml
Expand Up @@ -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
ste93cry marked this conversation as resolved.
Show resolved Hide resolved
if: matrix.php == '8.0' && matrix.dependencies == 'lowest'

- name: Install dependencies
uses: ramsey/composer-install@v1
with:
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions composer.json
Expand Up @@ -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",
Expand Down
36 changes: 35 additions & 1 deletion phpstan-baseline.neon
Expand Up @@ -122,7 +122,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

-
Expand All @@ -135,6 +135,11 @@ parameters:
count: 1
path: src/DependencyInjection/SentryExtension.php

-
message: "#^Parameter \\#2 \\$config of method Sentry\\\\SentryBundle\\\\DependencyInjection\\\\SentryExtension\\:\\:registerHttpClientTracingConfiguration\\(\\) expects array<string, mixed>, mixed given\\.$#"
count: 1
path: src/DependencyInjection/SentryExtension.php

-
message: "#^Parameter \\#2 \\$value of method Symfony\\\\Component\\\\DependencyInjection\\\\Container\\:\\:setParameter\\(\\) expects array\\|bool\\|float\\|int\\|string\\|UnitEnum\\|null, mixed given\\.$#"
count: 2
Expand Down Expand Up @@ -265,6 +270,26 @@ parameters:
count: 1
path: tests/DependencyInjection/SentryExtensionTest.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\\\\AbstractTraceableResponse::stream\\(\\) has parameter \\$responses with no value type specified in iterable type iterable.$#"
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: "#^Class Symfony\\\\Component\\\\Debug\\\\Exception\\\\FatalErrorException not found\\.$#"
count: 1
Expand Down Expand Up @@ -410,3 +435,12 @@ parameters:
count: 1
path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.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
2 changes: 2 additions & 0 deletions phpstan.neon
Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions psalm-baseline.xml
Expand Up @@ -50,6 +50,16 @@
<code>$params</code>
</MoreSpecificImplementedParamType>
</file>
<file src="src/Tracing/HttpClient/TraceableResponseForV5.php">
<UndefinedInterfaceMethod occurrences="1">
<code>toStream</code>
</UndefinedInterfaceMethod>
</file>
<file src="src/Tracing/HttpClient/TraceableResponseForV6.php">
<UndefinedInterfaceMethod occurrences="1">
<code>toStream</code>
</UndefinedInterfaceMethod>
</file>
<file src="src/aliases.php">
<MissingDependency occurrences="1">
<code>TracingDriverForV2</code>
Expand All @@ -63,4 +73,23 @@
<code>PostResponseEvent</code>
</UndefinedClass>
</file>
<file src="src/DependencyInjection/Compiler/CacheTracingPass.php">
<UndefinedDocblockClass occurrences="1">
<code>$container-&gt;getParameter('sentry.tracing.cache.enabled')</code>
</UndefinedDocblockClass>
</file>
<file src="src/DependencyInjection/Compiler/HttpClientTracingPass.php">
<UndefinedDocblockClass occurrences="2">
<code>$container-&gt;getParameter('sentry.tracing.enabled')</code>
<code>$container-&gt;getParameter('sentry.tracing.http_client.enabled')</code>
</UndefinedDocblockClass>
</file>
<file src="src/DependencyInjection/Compiler/DbalTracingPass.php">
<UndefinedDocblockClass occurrences="4">
<code>$container-&gt;getParameter('sentry.tracing.enabled')</code>
<code>$container-&gt;getParameter('sentry.tracing.dbal.enabled')</code>
<code>$container-&gt;getParameter('sentry.tracing.dbal.connections')</code>
<code>$container-&gt;getParameter('doctrine.connections')</code>
</UndefinedDocblockClass>
</file>
</files>
2 changes: 2 additions & 0 deletions psalm.xml
Expand Up @@ -11,6 +11,8 @@
<directory name="src" />
<ignoreFiles>
<file name="src/Tracing/Doctrine/DBAL/TracingStatementForV2.php" />
<file name="src/Tracing/HttpClient/TraceableHttpClientForV4.php" />
<file name="src/Tracing/HttpClient/TraceableHttpClientForV5.php" />
<directory name="vendor" />
</ignoreFiles>
</projectFiles>
Expand Down
37 changes: 37 additions & 0 deletions src/DependencyInjection/Compiler/HttpClientTracingPass.php
@@ -0,0 +1,37 @@
<?php

declare(strict_types=1);

namespace Sentry\SentryBundle\DependencyInjection\Compiler;

use Sentry\SentryBundle\Tracing\HttpClient\TraceableHttpClient;
use Sentry\State\HubInterface;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;

final class HttpClientTracingPass implements CompilerPassInterface
{
/**
* {@inheritdoc}
*/
public function process(ContainerBuilder $container): void
{
if (
!$container->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)
->setArguments([
new Reference('.sentry.traceable.' . $id . '.inner'),
new Reference(HubInterface::class),
])
->addTag('kernel.reset', ['method' => 'reset'])
->setDecoratedService($id);
}
}
}
4 changes: 4 additions & 0 deletions src/DependencyInjection/Configuration.php
Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand Down
17 changes: 17 additions & 0 deletions src/DependencyInjection/SentryExtension.php
Expand Up @@ -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
Expand Down Expand Up @@ -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']);
}

/**
Expand Down Expand Up @@ -247,6 +249,21 @@ private function registerCacheTracingConfiguration(ContainerBuilder $container,
$container->setParameter('sentry.tracing.cache.enabled', $isConfigEnabled);
}

/**
* @param array<string, mixed> $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.');
ste93cry marked this conversation as resolved.
Show resolved Hide resolved
}

$container->setParameter('sentry.tracing.http_client.enabled', $isConfigEnabled);
}

/**
* @param string[] $integrations
* @param array<string, mixed> $config
Expand Down
5 changes: 5 additions & 0 deletions src/Resources/config/schema/sentry-1.0.xsd
Expand Up @@ -91,6 +91,7 @@
<xsd:element name="twig" type="tracing-twig" minOccurs="0" maxOccurs="1" />
<xsd:element name="cache" type="tracing-cache" minOccurs="0" maxOccurs="1" />
<xsd:element name="console" type="tracing-console" minOccurs="0" maxOccurs="1" />
<xsd:element name="http-client" type="tracing-http-client" minOccurs="0" maxOccurs="1" />
</xsd:choice>

<xsd:attribute name="enabled" type="xsd:boolean" default="true"/>
Expand All @@ -117,4 +118,8 @@
<xsd:element name="excluded-command" type="xsd:string" minOccurs="0" maxOccurs="unbounded" />
</xsd:sequence>
</xsd:complexType>

<xsd:complexType name="tracing-http-client">
<xsd:attribute name="enabled" type="xsd:boolean" />
</xsd:complexType>
</xsd:schema>
2 changes: 2 additions & 0 deletions src/SentryBundle.php
Expand Up @@ -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;

Expand All @@ -19,5 +20,6 @@ public function build(ContainerBuilder $container): void

$container->addCompilerPass(new DbalTracingPass());
$container->addCompilerPass(new CacheTracingPass());
$container->addCompilerPass(new HttpClientTracingPass());
}
}
97 changes: 97 additions & 0 deletions src/Tracing/HttpClient/AbstractTraceableHttpClient.php
@@ -0,0 +1,97 @@
<?php

declare(strict_types=1);

namespace Sentry\SentryBundle\Tracing\HttpClient;

use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerInterface;
use Sentry\State\HubInterface;
use Sentry\Tracing\SpanContext;
use Symfony\Component\HttpClient\Response\ResponseStream;
use Symfony\Contracts\HttpClient\HttpClientInterface;
use Symfony\Contracts\HttpClient\ResponseInterface;
use Symfony\Contracts\HttpClient\ResponseStreamInterface;
use Symfony\Contracts\Service\ResetInterface;

/**
* This is an implementation of the {@see HttpClientInterface} that decorates
* an existing http client to support distributed tracing capabilities.
*
* @internal
*/
abstract class AbstractTraceableHttpClient implements HttpClientInterface, ResetInterface, LoggerAwareInterface
{
/**
* @var HttpClientInterface
*/
protected $client;

/**
* @var HubInterface
*/
private $hub;

public function __construct(HttpClientInterface $client, HubInterface $hub)
{
$this->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();
ste93cry marked this conversation as resolved.
Show resolved Hide resolved
$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);
}
}
}