From dbea7034ef1de26ada87d63ff4484458ae8bf86b Mon Sep 17 00:00:00 2001 From: Robert-Jan de Dreu Date: Fri, 22 Jan 2021 16:00:18 +0100 Subject: [PATCH 01/16] Add doctrine dbal tracing support that is optional to enable --- .../Compiler/ConnectDbalQueryListenerPass.php | 33 ++++++++++ src/DependencyInjection/Configuration.php | 1 + src/EventListener/Tracing/DbalListener.php | 62 +++++++++++++++++++ src/SentryBundle.php | 9 +++ 4 files changed, 105 insertions(+) create mode 100644 src/DependencyInjection/Compiler/ConnectDbalQueryListenerPass.php create mode 100644 src/EventListener/Tracing/DbalListener.php diff --git a/src/DependencyInjection/Compiler/ConnectDbalQueryListenerPass.php b/src/DependencyInjection/Compiler/ConnectDbalQueryListenerPass.php new file mode 100644 index 00000000..b527488c --- /dev/null +++ b/src/DependencyInjection/Compiler/ConnectDbalQueryListenerPass.php @@ -0,0 +1,33 @@ +getExtensionConfig('sentry'); + + $registerDbalListener = isset($config[0]['register_dbal_listener']) + ? $config[0]['register_dbal_listener'] + : false; + + if ($registerDbalListener && $container->hasDefinition('doctrine.dbal.logger')) { + $container->setDefinition( + 'doctrine.dbal.logger', + new Definition(DbalListener::class, [$container->getDefinition(HubInterface::class)]) + ); + } + } +} diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 69f94459..19278ec8 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -37,6 +37,7 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->end() ->booleanNode('register_error_listener')->defaultTrue()->end() + ->booleanNode('register_dbal_listener')->defaultFalse()->end() ->arrayNode('options') ->addDefaultsIfNotSet() ->fixXmlConfig('integration') diff --git a/src/EventListener/Tracing/DbalListener.php b/src/EventListener/Tracing/DbalListener.php new file mode 100644 index 00000000..2d248a5d --- /dev/null +++ b/src/EventListener/Tracing/DbalListener.php @@ -0,0 +1,62 @@ +hub = $hub; + } + + /** + * @param string $sql + */ + public function startQuery($sql, ?array $params = null, ?array $types = null) + { + $transaction = $this->hub->getTransaction(); + + if (!$transaction) { + return; + } + + $spanContext = new SpanContext(); + $spanContext->setOp('doctrine.query'); + $spanContext->setDescription($sql); + + $this->querySpan = $transaction->startChild($spanContext); + } + + public function stopQuery() + { + if (!$this->querySpan instanceof Span) { + return; + } + + $this->querySpan->finish(); + } +} diff --git a/src/SentryBundle.php b/src/SentryBundle.php index 673baad3..819c1d59 100644 --- a/src/SentryBundle.php +++ b/src/SentryBundle.php @@ -4,9 +4,18 @@ namespace Sentry\SentryBundle; +use Sentry\SentryBundle\DependencyInjection\Compiler\ConnectDbalQueryListenerPass; +use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; final class SentryBundle extends Bundle { public const SDK_IDENTIFIER = 'sentry.php.symfony'; + + public function build(ContainerBuilder $container): void + { + parent::build($container); + + $container->addCompilerPass(new ConnectDbalQueryListenerPass()); + } } From a2b4809f63d8c898164f06654b7dc8001f121d61 Mon Sep 17 00:00:00 2001 From: Robert-Jan de Dreu Date: Mon, 25 Jan 2021 14:26:33 +0100 Subject: [PATCH 02/16] Add option to set register_dbal_listener to ConfigurationTest --- composer.json | 3 ++- tests/DependencyInjection/ConfigurationTest.php | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index afc7ea97..2ec765d2 100644 --- a/composer.json +++ b/composer.json @@ -51,7 +51,8 @@ "vimeo/psalm": "^4.3" }, "suggest": { - "monolog/monolog": "Allow sending log messages to Sentry by using the included Monolog handler." + "monolog/monolog": "Allow sending log messages to Sentry by using the included Monolog handler.", + "doctrine/dbal": "Trace DBAL queries with Sentry Transactions by using the included DbalListener" }, "autoload": { "files": [ diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index 3aa3424e..79271507 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -17,6 +17,7 @@ public function testProcessConfigurationWithDefaultConfiguration(): void { $expectedBundleDefaultConfig = [ 'register_error_listener' => true, + 'register_dbal_listener' => false, 'options' => [ 'integrations' => [], 'prefixes' => [], From fd868739b9c1041ef93beba9426baf265b3d28f8 Mon Sep 17 00:00:00 2001 From: Robert-Jan de Dreu Date: Tue, 26 Jan 2021 12:47:33 +0100 Subject: [PATCH 03/16] Added tests to test the DbalListener and make sure dbal is installed --- .github/workflows/static-analysis.yaml | 9 ++ .github/workflows/tests.yaml | 1 + src/EventListener/Tracing/DbalListener.php | 3 +- .../Tracing/DbalListenerTest.php | 85 +++++++++++++++++++ 4 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 tests/EventListener/Tracing/DbalListenerTest.php diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml index 423cfa70..a4472bb9 100644 --- a/.github/workflows/static-analysis.yaml +++ b/.github/workflows/static-analysis.yaml @@ -23,6 +23,9 @@ jobs: - name: Install dependencies run: composer update --no-progress --no-interaction --prefer-dist + - name: Install suggested packages + run: composer suggests sentry/sentry-symfony --list | xargs -i composer require {} + - name: Run script run: composer phpcs @@ -39,6 +42,9 @@ jobs: - name: Install dependencies run: composer update --no-progress --no-interaction --prefer-dist + - name: Install suggested packages + run: composer suggests sentry/sentry-symfony --list | xargs -i composer require {} + - name: Run script run: composer phpstan @@ -55,5 +61,8 @@ jobs: - name: Install dependencies run: composer update --no-progress --no-interaction --prefer-dist + - name: Install suggested packages + run: composer suggests sentry/sentry-symfony --list | xargs -i composer require {} + - name: Run script run: composer psalm diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 7b78eb5a..98923cc7 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -58,6 +58,7 @@ jobs: - run: composer remove --dev symfony/messenger --no-update if: matrix.symfony_constraint == '3.4.*' || matrix.composer_option == '--prefer-lowest' - run: composer update --no-progress --ansi ${{ matrix.composer_option }} + - run: composer suggests sentry/sentry-symfony --list | xargs -i composer require {} - run: composer require sentry/sentry dev-develop if: matrix.sentry_constraint == 'dev-develop' - run: vendor/bin/phpunit --coverage-clover=coverage.xml diff --git a/src/EventListener/Tracing/DbalListener.php b/src/EventListener/Tracing/DbalListener.php index 2d248a5d..638afc9f 100644 --- a/src/EventListener/Tracing/DbalListener.php +++ b/src/EventListener/Tracing/DbalListener.php @@ -8,6 +8,7 @@ use Sentry\State\HubInterface; use Sentry\Tracing\Span; use Sentry\Tracing\SpanContext; +use Sentry\Tracing\Transaction; /** * Getting the logger, tied into dbal seems extremely hard. Cheating the system a bit by putting it in between the @@ -40,7 +41,7 @@ public function startQuery($sql, ?array $params = null, ?array $types = null) { $transaction = $this->hub->getTransaction(); - if (!$transaction) { + if (!$transaction instanceof Transaction) { return; } diff --git a/tests/EventListener/Tracing/DbalListenerTest.php b/tests/EventListener/Tracing/DbalListenerTest.php new file mode 100644 index 00000000..f7cc7cc9 --- /dev/null +++ b/tests/EventListener/Tracing/DbalListenerTest.php @@ -0,0 +1,85 @@ +hub = $this->createMock(HubInterface::class); + $this->listener = new DbalListener($this->hub); + } + + public function testThatDbalStartQueryIgnoresTracingWhenTransactionIsNotStarted(): void + { + $this->hub->expects($this->once()) + ->method('getTransaction') + ->willReturn(null); + + $this->listener->startQuery(''); + } + + public function testThatDbalStopQueryIgnoresTracingWhenChildSpanWasNotStarted(): void + { + $this->hub->expects($this->once()) + ->method('getTransaction') + ->willReturn(null); + + $this->listener->startQuery(''); + $this->listener->stopQuery(); + } + + public function testThatDbalStartQueryAttachesAChildSpanWhenTransactionStarted(): void + { + $transaction = new Transaction(new TransactionContext()); + $transaction->initSpanRecorder(); + + $this->hub->expects($this->once()) + ->method('getTransaction') + ->willReturn($transaction); + + $this->listener->startQuery(''); + + $spans = $transaction->getSpanRecorder()->getSpans(); + + $this->assertCount(2, $spans); + $this->assertNull($spans['1']->getEndTimestamp()); + } + + public function testThatDbalStopQueryFinishesTheChildSpanWhenChildSpanStarted(): void + { + $transaction = new Transaction(new TransactionContext()); + $transaction->initSpanRecorder(); + + $this->hub->expects($this->once()) + ->method('getTransaction') + ->willReturn($transaction); + + $this->listener->startQuery(''); + $this->listener->stopQuery(); + + $spans = $transaction->getSpanRecorder()->getSpans(); + + $this->assertCount(2, $spans); + $this->assertNotNull($spans['1']->getEndTimestamp()); + } +} From ec1fa5426e6934663a45a7ab936ec8e9e74410ca Mon Sep 17 00:00:00 2001 From: Robert-Jan de Dreu Date: Wed, 27 Jan 2021 09:27:24 +0100 Subject: [PATCH 04/16] Move tracing to its own main directory and apply comments review --- .github/workflows/static-analysis.yaml | 9 ------ .github/workflows/tests.yaml | 1 - composer.json | 3 +- .../Compiler/ConnectDbalQueryListenerPass.php | 17 +++++------ src/DependencyInjection/Configuration.php | 7 ++++- .../DbalSqlTracingLogger.php} | 28 ++++++++++--------- .../DependencyInjection/ConfigurationTest.php | 4 ++- .../DbalSqlTracingLoggerTest.php} | 10 +++---- 8 files changed, 38 insertions(+), 41 deletions(-) rename src/{EventListener/Tracing/DbalListener.php => Tracing/DbalSqlTracingLogger.php} (62%) rename tests/{EventListener/Tracing/DbalListenerTest.php => Tracing/DbalSqlTracingLoggerTest.php} (88%) diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml index a4472bb9..423cfa70 100644 --- a/.github/workflows/static-analysis.yaml +++ b/.github/workflows/static-analysis.yaml @@ -23,9 +23,6 @@ jobs: - name: Install dependencies run: composer update --no-progress --no-interaction --prefer-dist - - name: Install suggested packages - run: composer suggests sentry/sentry-symfony --list | xargs -i composer require {} - - name: Run script run: composer phpcs @@ -42,9 +39,6 @@ jobs: - name: Install dependencies run: composer update --no-progress --no-interaction --prefer-dist - - name: Install suggested packages - run: composer suggests sentry/sentry-symfony --list | xargs -i composer require {} - - name: Run script run: composer phpstan @@ -61,8 +55,5 @@ jobs: - name: Install dependencies run: composer update --no-progress --no-interaction --prefer-dist - - name: Install suggested packages - run: composer suggests sentry/sentry-symfony --list | xargs -i composer require {} - - name: Run script run: composer psalm diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 98923cc7..7b78eb5a 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -58,7 +58,6 @@ jobs: - run: composer remove --dev symfony/messenger --no-update if: matrix.symfony_constraint == '3.4.*' || matrix.composer_option == '--prefer-lowest' - run: composer update --no-progress --ansi ${{ matrix.composer_option }} - - run: composer suggests sentry/sentry-symfony --list | xargs -i composer require {} - run: composer require sentry/sentry dev-develop if: matrix.sentry_constraint == 'dev-develop' - run: vendor/bin/phpunit --coverage-clover=coverage.xml diff --git a/composer.json b/composer.json index 2ec765d2..b2e686ea 100644 --- a/composer.json +++ b/composer.json @@ -32,6 +32,7 @@ "symfony/security-core": "^3.4.43||^4.4.11||^5.0.11" }, "require-dev": { + "doctrine/dbal": "^2.0||^3.0", "friendsofphp/php-cs-fixer": "^2.17", "jangregor/phpstan-prophecy": "^0.8", "monolog/monolog": "^1.3||^2.0", @@ -52,7 +53,7 @@ }, "suggest": { "monolog/monolog": "Allow sending log messages to Sentry by using the included Monolog handler.", - "doctrine/dbal": "Trace DBAL queries with Sentry Transactions by using the included DbalListener" + "doctrine/dbal": "Allow distributed tracing of SQL queries using Sentry." }, "autoload": { "files": [ diff --git a/src/DependencyInjection/Compiler/ConnectDbalQueryListenerPass.php b/src/DependencyInjection/Compiler/ConnectDbalQueryListenerPass.php index b527488c..c1ff9e26 100644 --- a/src/DependencyInjection/Compiler/ConnectDbalQueryListenerPass.php +++ b/src/DependencyInjection/Compiler/ConnectDbalQueryListenerPass.php @@ -4,29 +4,26 @@ namespace Sentry\SentryBundle\DependencyInjection\Compiler; -use Sentry\SentryBundle\EventListener\Tracing\DbalListener; +use Sentry\SentryBundle\Tracing\DbalSqlTracingLogger; use Sentry\State\HubInterface; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; -class ConnectDbalQueryListenerPass implements CompilerPassInterface +final class ConnectDbalQueryListenerPass implements CompilerPassInterface { - /** - * @return void - */ - public function process(ContainerBuilder $container) + public function process(ContainerBuilder $container): void { $config = $container->getExtensionConfig('sentry'); - $registerDbalListener = isset($config[0]['register_dbal_listener']) - ? $config[0]['register_dbal_listener'] + $dbalTracingEnabled = isset($config[0]['tracing']['dbal_tracing']) + ? $config[0]['tracing']['dbal_tracing'] : false; - if ($registerDbalListener && $container->hasDefinition('doctrine.dbal.logger')) { + if ($dbalTracingEnabled && $container->hasDefinition('doctrine.dbal.logger')) { $container->setDefinition( 'doctrine.dbal.logger', - new Definition(DbalListener::class, [$container->getDefinition(HubInterface::class)]) + new Definition(DbalSqlTracingLogger::class, [$container->getDefinition(HubInterface::class)]) ); } } diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 19278ec8..a5e0a104 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -37,7 +37,12 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->end() ->booleanNode('register_error_listener')->defaultTrue()->end() - ->booleanNode('register_dbal_listener')->defaultFalse()->end() + ->arrayNode('tracing') + ->addDefaultsIfNotSet() + ->children() + ->booleanNode('dbal_tracing')->defaultFalse()->end() + ->end() + ->end() ->arrayNode('options') ->addDefaultsIfNotSet() ->fixXmlConfig('integration') diff --git a/src/EventListener/Tracing/DbalListener.php b/src/Tracing/DbalSqlTracingLogger.php similarity index 62% rename from src/EventListener/Tracing/DbalListener.php rename to src/Tracing/DbalSqlTracingLogger.php index 638afc9f..4fb2d1ca 100644 --- a/src/EventListener/Tracing/DbalListener.php +++ b/src/Tracing/DbalSqlTracingLogger.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Sentry\SentryBundle\EventListener\Tracing; +namespace Sentry\SentryBundle\Tracing; use Doctrine\DBAL\Logging\SQLLogger; use Sentry\State\HubInterface; @@ -10,11 +10,7 @@ use Sentry\Tracing\SpanContext; use Sentry\Tracing\Transaction; -/** - * Getting the logger, tied into dbal seems extremely hard. Cheating the system a bit by putting it in between the - * debug stack logger. - */ -final class DbalListener implements SQLLogger +final class DbalSqlTracingLogger implements SQLLogger { /** * @var HubInterface The current hub @@ -22,9 +18,9 @@ final class DbalListener implements SQLLogger private $hub; /** - * @var Span + * @var Span The span tracing the execution of a query */ - private $querySpan; + private $span; /** * @param HubInterface $hub The current hub @@ -35,7 +31,7 @@ public function __construct(HubInterface $hub) } /** - * @param string $sql + * {@inheritdoc} */ public function startQuery($sql, ?array $params = null, ?array $types = null) { @@ -46,18 +42,24 @@ public function startQuery($sql, ?array $params = null, ?array $types = null) } $spanContext = new SpanContext(); - $spanContext->setOp('doctrine.query'); + $spanContext->setOp('db.query'); $spanContext->setDescription($sql); + $spanContext->setData([ + 'db.system' => 'doctrine', + ]); - $this->querySpan = $transaction->startChild($spanContext); + $this->span = $transaction->startChild($spanContext); } + /** + * {@inheritdoc} + */ public function stopQuery() { - if (!$this->querySpan instanceof Span) { + if (null === $this->span) { return; } - $this->querySpan->finish(); + $this->span->finish(); } } diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index 79271507..5ede6452 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -17,7 +17,9 @@ public function testProcessConfigurationWithDefaultConfiguration(): void { $expectedBundleDefaultConfig = [ 'register_error_listener' => true, - 'register_dbal_listener' => false, + 'tracing' => [ + 'dbal_tracing' => false, + ], 'options' => [ 'integrations' => [], 'prefixes' => [], diff --git a/tests/EventListener/Tracing/DbalListenerTest.php b/tests/Tracing/DbalSqlTracingLoggerTest.php similarity index 88% rename from tests/EventListener/Tracing/DbalListenerTest.php rename to tests/Tracing/DbalSqlTracingLoggerTest.php index f7cc7cc9..0a49e2df 100644 --- a/tests/EventListener/Tracing/DbalListenerTest.php +++ b/tests/Tracing/DbalSqlTracingLoggerTest.php @@ -2,16 +2,16 @@ declare(strict_types=1); -namespace Sentry\SentryBundle\Tests\EventListener\Tracing; +namespace Sentry\SentryBundle\Tests\Tracing; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Sentry\SentryBundle\EventListener\Tracing\DbalListener; +use Sentry\SentryBundle\Tracing\DbalSqlTracingLogger; use Sentry\State\HubInterface; use Sentry\Tracing\Transaction; use Sentry\Tracing\TransactionContext; -final class DbalListenerTest extends TestCase +final class DbalSqlTracingLoggerTest extends TestCase { /** * @var MockObject&HubInterface @@ -19,14 +19,14 @@ final class DbalListenerTest extends TestCase private $hub; /** - * @var DbalListener + * @var \Sentry\SentryBundle\Tracing\DbalSqlTracingLogger */ private $listener; protected function setUp(): void { $this->hub = $this->createMock(HubInterface::class); - $this->listener = new DbalListener($this->hub); + $this->listener = new DbalSqlTracingLogger($this->hub); } public function testThatDbalStartQueryIgnoresTracingWhenTransactionIsNotStarted(): void From 603c9f2ac1b23b3e1ec2148f688fc93ad0e89828 Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Fri, 5 Feb 2021 10:00:13 +0100 Subject: [PATCH 05/16] Record the trace as a child of the current span rather than of the main transaction --- src/Tracing/DbalSqlTracingLogger.php | 14 +++---- tests/Tracing/DbalSqlTracingLoggerTest.php | 46 ++++++---------------- 2 files changed, 20 insertions(+), 40 deletions(-) diff --git a/src/Tracing/DbalSqlTracingLogger.php b/src/Tracing/DbalSqlTracingLogger.php index 4fb2d1ca..cda66df8 100644 --- a/src/Tracing/DbalSqlTracingLogger.php +++ b/src/Tracing/DbalSqlTracingLogger.php @@ -8,8 +8,10 @@ use Sentry\State\HubInterface; use Sentry\Tracing\Span; use Sentry\Tracing\SpanContext; -use Sentry\Tracing\Transaction; +/** + * Simple SQL logger that records a trace of each query and sends it to Sentry. + */ final class DbalSqlTracingLogger implements SQLLogger { /** @@ -35,20 +37,17 @@ public function __construct(HubInterface $hub) */ public function startQuery($sql, ?array $params = null, ?array $types = null) { - $transaction = $this->hub->getTransaction(); + $span = $this->hub->getSpan(); - if (!$transaction instanceof Transaction) { + if (null === $span) { return; } $spanContext = new SpanContext(); $spanContext->setOp('db.query'); $spanContext->setDescription($sql); - $spanContext->setData([ - 'db.system' => 'doctrine', - ]); - $this->span = $transaction->startChild($spanContext); + $this->span = $span->startChild($spanContext); } /** @@ -61,5 +60,6 @@ public function stopQuery() } $this->span->finish(); + $this->span = null; } } diff --git a/tests/Tracing/DbalSqlTracingLoggerTest.php b/tests/Tracing/DbalSqlTracingLoggerTest.php index 0a49e2df..1aee8487 100644 --- a/tests/Tracing/DbalSqlTracingLoggerTest.php +++ b/tests/Tracing/DbalSqlTracingLoggerTest.php @@ -19,67 +19,47 @@ final class DbalSqlTracingLoggerTest extends TestCase private $hub; /** - * @var \Sentry\SentryBundle\Tracing\DbalSqlTracingLogger + * @var DbalSqlTracingLogger */ - private $listener; + private $logger; protected function setUp(): void { $this->hub = $this->createMock(HubInterface::class); - $this->listener = new DbalSqlTracingLogger($this->hub); + $this->logger = new DbalSqlTracingLogger($this->hub); } - public function testThatDbalStartQueryIgnoresTracingWhenTransactionIsNotStarted(): void + public function testStopQueryDoesNothingIfTransactionDidNotStartTheChildSpan(): void { $this->hub->expects($this->once()) - ->method('getTransaction') + ->method('getSpan') ->willReturn(null); - $this->listener->startQuery(''); + $this->logger->startQuery('SELECT * FROM orders'); + $this->logger->stopQuery(); } - public function testThatDbalStopQueryIgnoresTracingWhenChildSpanWasNotStarted(): void - { - $this->hub->expects($this->once()) - ->method('getTransaction') - ->willReturn(null); - - $this->listener->startQuery(''); - $this->listener->stopQuery(); - } - - public function testThatDbalStartQueryAttachesAChildSpanWhenTransactionStarted(): void + public function testStopQueryFinishesTheChildSpan(): void { $transaction = new Transaction(new TransactionContext()); $transaction->initSpanRecorder(); $this->hub->expects($this->once()) - ->method('getTransaction') + ->method('getSpan') ->willReturn($transaction); - $this->listener->startQuery(''); + $this->logger->startQuery('SELECT * FROM orders'); $spans = $transaction->getSpanRecorder()->getSpans(); $this->assertCount(2, $spans); - $this->assertNull($spans['1']->getEndTimestamp()); - } - - public function testThatDbalStopQueryFinishesTheChildSpanWhenChildSpanStarted(): void - { - $transaction = new Transaction(new TransactionContext()); - $transaction->initSpanRecorder(); - - $this->hub->expects($this->once()) - ->method('getTransaction') - ->willReturn($transaction); + $this->assertNull($spans[1]->getEndTimestamp()); - $this->listener->startQuery(''); - $this->listener->stopQuery(); + $this->logger->stopQuery(); $spans = $transaction->getSpanRecorder()->getSpans(); $this->assertCount(2, $spans); - $this->assertNotNull($spans['1']->getEndTimestamp()); + $this->assertNotNull($spans[1]->getEndTimestamp()); } } From 3a295d80abadeec5467d34a7faa8817913a27599 Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Fri, 5 Feb 2021 10:03:04 +0100 Subject: [PATCH 06/16] Add missing CHANGELOG entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28c2e0ba..404716f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Add support for distributed tracing of SQL queries while using Doctrine DBAL (#426) - Added missing `capture-soft-fails` config schema option (#417) - Deprecate the `Sentry\SentryBundle\EventListener\ConsoleCommandListener` class in favor of its parent class `Sentry\SentryBundle\EventListener\ConsoleListener` (#429) From e7c987f8bdfb6221561701185d5b17b52543c7eb Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Fri, 5 Feb 2021 18:48:35 +0100 Subject: [PATCH 07/16] Fix CR issues --- composer.json | 3 +- .../Compiler/ConnectDbalQueryListenerPass.php | 30 ---- .../Compiler/DbalSqlTracingLoggerPass.php | 90 +++++++++++ src/DependencyInjection/Configuration.php | 33 +++- src/DependencyInjection/SentryExtension.php | 30 ++++ src/Resources/config/schema/sentry-1.0.xsd | 17 ++ src/Resources/config/services.xml | 4 + src/SentryBundle.php | 4 +- .../Compiler/DbalSqlTracingLoggerPassTest.php | 153 ++++++++++++++++++ .../DependencyInjection/ConfigurationTest.php | 9 +- .../Fixtures/php/dbal_tracing_disabled.php | 15 ++ .../DependencyInjection/Fixtures/php/full.php | 5 + .../Fixtures/xml/dbal_tracing_disabled.xml | 14 ++ .../DependencyInjection/Fixtures/xml/full.xml | 3 + .../Fixtures/yml/dbal_tracing_disabled.yml | 4 + .../DependencyInjection/Fixtures/yml/full.yml | 3 + .../SentryExtensionTest.php | 17 ++ 17 files changed, 390 insertions(+), 44 deletions(-) delete mode 100644 src/DependencyInjection/Compiler/ConnectDbalQueryListenerPass.php create mode 100644 src/DependencyInjection/Compiler/DbalSqlTracingLoggerPass.php create mode 100644 tests/DependencyInjection/Compiler/DbalSqlTracingLoggerPassTest.php create mode 100644 tests/DependencyInjection/Fixtures/php/dbal_tracing_disabled.php create mode 100644 tests/DependencyInjection/Fixtures/xml/dbal_tracing_disabled.xml create mode 100644 tests/DependencyInjection/Fixtures/yml/dbal_tracing_disabled.yml diff --git a/composer.json b/composer.json index b2e686ea..1fc15560 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,8 @@ "symfony/security-core": "^3.4.43||^4.4.11||^5.0.11" }, "require-dev": { - "doctrine/dbal": "^2.0||^3.0", + "doctrine/dbal": "^2.12||^3.0", + "doctrine/doctrine-bundle": "^2.2", "friendsofphp/php-cs-fixer": "^2.17", "jangregor/phpstan-prophecy": "^0.8", "monolog/monolog": "^1.3||^2.0", diff --git a/src/DependencyInjection/Compiler/ConnectDbalQueryListenerPass.php b/src/DependencyInjection/Compiler/ConnectDbalQueryListenerPass.php deleted file mode 100644 index c1ff9e26..00000000 --- a/src/DependencyInjection/Compiler/ConnectDbalQueryListenerPass.php +++ /dev/null @@ -1,30 +0,0 @@ -getExtensionConfig('sentry'); - - $dbalTracingEnabled = isset($config[0]['tracing']['dbal_tracing']) - ? $config[0]['tracing']['dbal_tracing'] - : false; - - if ($dbalTracingEnabled && $container->hasDefinition('doctrine.dbal.logger')) { - $container->setDefinition( - 'doctrine.dbal.logger', - new Definition(DbalSqlTracingLogger::class, [$container->getDefinition(HubInterface::class)]) - ); - } - } -} diff --git a/src/DependencyInjection/Compiler/DbalSqlTracingLoggerPass.php b/src/DependencyInjection/Compiler/DbalSqlTracingLoggerPass.php new file mode 100644 index 00000000..489f9614 --- /dev/null +++ b/src/DependencyInjection/Compiler/DbalSqlTracingLoggerPass.php @@ -0,0 +1,90 @@ +hasParameter('doctrine.connections')) { + return; + } + + $connections = $container->getParameter('doctrine.connections'); + $connectionsToTrace = $container->getParameter('sentry.tracing.dbal.connections'); + + foreach ($connectionsToTrace as $connectionName) { + if (!\in_array($connectionName, $connections, true)) { + continue; + } + + $configurationDefinition = $container->getDefinition(sprintf('doctrine.dbal.%s_connection.configuration', $connectionName)); + $setSQLLoggerMethodCall = $this->getSetSQLLoggerMethodCall($configurationDefinition); + + if (null === $setSQLLoggerMethodCall) { + $loggerDefinition = new Reference(DbalSqlTracingLogger::class); + } else { + $loggerDefinition = $this->addTracingLoggerToConnectionConfiguration($container, $setSQLLoggerMethodCall[1][0]); + } + + $configurationDefinition + ->removeMethodCall('setSQLLogger') + ->addMethodCall('setSQLLogger', [$loggerDefinition]); + } + } + + private function getSetSQLLoggerMethodCall(Definition $definition): ?array + { + foreach ($definition->getMethodCalls() as $methodCall) { + if ('setSQLLogger' === $methodCall[0]) { + return $methodCall; + } + } + + return null; + } + + /** + * @param Reference|Definition $loggerDefinition The service definition + * + * @return Reference|Definition + */ + private function addTracingLoggerToConnectionConfiguration(ContainerBuilder $container, $loggerDefinition) + { + if ($loggerDefinition instanceof Definition) { + if (LoggerChain::class === $loggerDefinition->getClass()) { + $loggerDefinition->setArgument(0, array_merge($loggerDefinition->getArgument(0), [new Reference(DbalSqlTracingLogger::class)])); + } else { + $loggerDefinition = new Definition(LoggerChain::class, [[ + $loggerDefinition, + new Reference(DbalSqlTracingLogger::class), + ]]); + } + } else { + $realLoggerDefinition = $container->findDefinition((string) $loggerDefinition); + + if (LoggerChain::class === $realLoggerDefinition->getClass()) { + $realLoggerDefinition->setArgument(0, array_merge($realLoggerDefinition->getArgument(0), [new Reference(DbalSqlTracingLogger::class)])); + } else { + $loggerDefinition = new Definition(LoggerChain::class, [[ + $loggerDefinition, + new Reference(DbalSqlTracingLogger::class), + ]]); + } + } + + return $loggerDefinition; + } +} diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index a5e0a104..4a8d161f 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -37,12 +37,6 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->end() ->booleanNode('register_error_listener')->defaultTrue()->end() - ->arrayNode('tracing') - ->addDefaultsIfNotSet() - ->children() - ->booleanNode('dbal_tracing')->defaultFalse()->end() - ->end() - ->end() ->arrayNode('options') ->addDefaultsIfNotSet() ->fixXmlConfig('integration') @@ -130,10 +124,10 @@ public function getConfigTreeBuilder(): TreeBuilder ->end() ->end() ->end() - ->end() - ; + ->end(); $this->addMessengerSection($rootNode); + $this->addDistributedTracingSection($rootNode); return $treeBuilder; } @@ -150,4 +144,27 @@ private function addMessengerSection(ArrayNodeDefinition $rootNode): void ->end() ->end(); } + + private function addDistributedTracingSection(ArrayNodeDefinition $rootNode): void + { + $rootNode + ->children() + ->arrayNode('tracing') + ->addDefaultsIfNotSet() + ->children() + ->arrayNode('dbal') + ->addDefaultsIfNotSet() + ->fixXmlConfig('connection') + ->canBeEnabled() + ->children() + ->arrayNode('connections') + ->scalarPrototype()->end() + ->defaultValue(['%doctrine.default_connection%']) + ->end() + ->end() + ->end() + ->end() + ->end() + ->end(); + } } diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index 302e432a..eda20000 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -4,7 +4,10 @@ namespace Sentry\SentryBundle\DependencyInjection; +use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; +use InvalidArgumentException; use Jean85\PrettyVersions; +use LogicException; use Sentry\Client; use Sentry\ClientBuilder; use Sentry\Integration\IgnoreErrorsIntegration; @@ -15,6 +18,7 @@ use Sentry\SentryBundle\EventListener\ErrorListener; use Sentry\SentryBundle\EventListener\MessengerListener; use Sentry\SentryBundle\SentryBundle; +use Sentry\SentryBundle\Tracing\DbalSqlTracingLogger; use Sentry\Serializer\RepresentationSerializer; use Sentry\Serializer\Serializer; use Sentry\Transport\TransportFactoryInterface; @@ -57,6 +61,7 @@ protected function loadInternal(array $mergedConfig, ContainerBuilder $container $this->registerConfiguration($container, $mergedConfig); $this->registerErrorListenerConfiguration($container, $mergedConfig); $this->registerMessengerListenerConfiguration($container, $mergedConfig['messenger']); + $this->registerTracingConfiguration($container, $mergedConfig['tracing']); } /** @@ -149,6 +154,22 @@ private function registerMessengerListenerConfiguration(ContainerBuilder $contai $container->getDefinition(MessengerListener::class)->setArgument(1, $config['capture_soft_fails']); } + /** + * @param array $config + */ + private function registerTracingConfiguration(ContainerBuilder $container, array $config): void + { + if ($this->isConfigEnabled($container, $config['dbal']) && !class_exists(DoctrineBundle::class)) { + throw new LogicException('DBAL tracing support cannot be enabled as the DoctrineBundle bundle is not installed. Try running "composer require doctrine/doctrine-bundle".'); + } + + $container->setParameter('sentry.tracing.dbal.connections', $config['dbal']['enabled'] ? $config['dbal']['connections'] : []); + + if (!$config['dbal']['enabled']) { + $container->removeDefinition(DbalSqlTracingLogger::class); + } + } + /** * @param string[] $integrations * @param array $config @@ -216,4 +237,13 @@ private function isIntegrationEnabled(string $integrationClass, array $integrati return false; } + + protected function isConfigEnabled(ContainerBuilder $container, array $config): bool + { + if (!isset($config['enabled'])) { + throw new InvalidArgumentException('The $config argument has no key named "enabled".'); + } + + return (bool) $container->getParameterBag()->resolveValue($config['enabled']); + } } diff --git a/src/Resources/config/schema/sentry-1.0.xsd b/src/Resources/config/schema/sentry-1.0.xsd index a98b4b36..1ea0cb2e 100644 --- a/src/Resources/config/schema/sentry-1.0.xsd +++ b/src/Resources/config/schema/sentry-1.0.xsd @@ -11,6 +11,7 @@ + @@ -79,4 +80,20 @@ + + + + + + + + + + + + + + + + diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index 9d22575b..515b2fb3 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -65,6 +65,10 @@ + + + + diff --git a/src/SentryBundle.php b/src/SentryBundle.php index 819c1d59..abfe2283 100644 --- a/src/SentryBundle.php +++ b/src/SentryBundle.php @@ -4,7 +4,7 @@ namespace Sentry\SentryBundle; -use Sentry\SentryBundle\DependencyInjection\Compiler\ConnectDbalQueryListenerPass; +use Sentry\SentryBundle\DependencyInjection\Compiler\DbalSqlTracingLoggerPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; @@ -16,6 +16,6 @@ public function build(ContainerBuilder $container): void { parent::build($container); - $container->addCompilerPass(new ConnectDbalQueryListenerPass()); + $container->addCompilerPass(new DbalSqlTracingLoggerPass()); } } diff --git a/tests/DependencyInjection/Compiler/DbalSqlTracingLoggerPassTest.php b/tests/DependencyInjection/Compiler/DbalSqlTracingLoggerPassTest.php new file mode 100644 index 00000000..bc4f3a84 --- /dev/null +++ b/tests/DependencyInjection/Compiler/DbalSqlTracingLoggerPassTest.php @@ -0,0 +1,153 @@ +createContainerBuilder(); + + $container->setParameter('doctrine.connections', ['connection_1', 'connection_2', 'connection_3', 'connection_4', 'connection_5']); + $container->setParameter('sentry.tracing.dbal.connections', ['connection_1', 'connection_2', 'connection_3', 'connection_4', 'connection_5']); + + $container + ->register('doctrine.dbal.logger', SQLLogger::class) + ->setPublic(true); + + $container + ->register('doctrine.dbal.logger.chain', LoggerChain::class) + ->setArgument(0, [new Reference('doctrine.dbal.logger')]) + ->setPublic(true); + + $container + ->register('doctrine.dbal.connection_1_connection.configuration', Configuration::class) + ->setPublic(true); + + $container + ->register('doctrine.dbal.connection_2_connection.configuration', Configuration::class) + ->addMethodCall('setSQLLogger', [new Reference('doctrine.dbal.logger')]) + ->setPublic(true); + + $container + ->register('doctrine.dbal.connection_3_connection.configuration', Configuration::class) + ->addMethodCall('setSQLLogger', [new Reference('doctrine.dbal.logger.chain')]) + ->setPublic(true); + + $container + ->register('doctrine.dbal.connection_4_connection.configuration', Configuration::class) + ->addMethodCall('setSQLLogger', [new Definition(LoggerChain::class, [[new Reference('doctrine.dbal.logger')]])]) + ->setPublic(true); + + $container + ->register('doctrine.dbal.connection_5_connection.configuration', Configuration::class) + ->addMethodCall('setSQLLogger', [new Definition(SQLLogger::class)]) + ->setPublic(true); + + $container->compile(); + + $this->assertEquals( + [['setSQLLogger', [new Reference(DbalSqlTracingLogger::class)]]], + $container->getDefinition('doctrine.dbal.connection_1_connection.configuration')->getMethodCalls() + ); + + $this->assertEquals( + [ + [ + 'setSQLLogger', + [ + new Definition(LoggerChain::class, [[ + new Reference('doctrine.dbal.logger'), + new Reference(DbalSqlTracingLogger::class), + ]]), + ], + ], + ], + $container->getDefinition('doctrine.dbal.connection_2_connection.configuration')->getMethodCalls() + ); + + $this->assertEquals( + [['setSQLLogger', [new Reference('doctrine.dbal.logger.chain')]]], + $container->getDefinition('doctrine.dbal.connection_3_connection.configuration')->getMethodCalls() + ); + + $this->assertEquals( + [ + [ + new Reference('doctrine.dbal.logger'), + new Reference(DbalSqlTracingLogger::class), + ], + ], + $container->getDefinition('doctrine.dbal.logger.chain')->getArguments() + ); + + $this->assertEquals( + [ + [ + 'setSQLLogger', + [ + new Definition(LoggerChain::class, [[ + new Reference('doctrine.dbal.logger'), + new Reference(DbalSqlTracingLogger::class), + ]]), + ], + ], + ], + $container->getDefinition('doctrine.dbal.connection_4_connection.configuration')->getMethodCalls() + ); + + $this->assertEquals( + [ + [ + 'setSQLLogger', + [ + new Definition(LoggerChain::class, [[ + new Definition(SQLLogger::class), + new Reference(DbalSqlTracingLogger::class), + ]]), + ], + ], + ], + $container->getDefinition('doctrine.dbal.connection_5_connection.configuration')->getMethodCalls() + ); + } + + private function createContainerBuilder(): ContainerBuilder + { + $container = new ContainerBuilder(); + $container->addCompilerPass(new DbalSqlTracingLoggerPass()); + + $container + ->register(DbalSqlTracingLogger::class, DbalSqlTracingLogger::class) + ->setPublic(true); + + return $container; + } +} diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index 5ede6452..7d945214 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -17,9 +17,6 @@ public function testProcessConfigurationWithDefaultConfiguration(): void { $expectedBundleDefaultConfig = [ 'register_error_listener' => true, - 'tracing' => [ - 'dbal_tracing' => false, - ], 'options' => [ 'integrations' => [], 'prefixes' => [], @@ -38,6 +35,12 @@ public function testProcessConfigurationWithDefaultConfiguration(): void 'enabled' => interface_exists(MessageBusInterface::class), 'capture_soft_fails' => true, ], + 'tracing' => [ + 'dbal' => [ + 'enabled' => false, + 'connections' => ['%doctrine.default_connection%'], + ], + ], ]; $this->assertSame($expectedBundleDefaultConfig, $this->processConfiguration([])); diff --git a/tests/DependencyInjection/Fixtures/php/dbal_tracing_disabled.php b/tests/DependencyInjection/Fixtures/php/dbal_tracing_disabled.php new file mode 100644 index 00000000..ed57d764 --- /dev/null +++ b/tests/DependencyInjection/Fixtures/php/dbal_tracing_disabled.php @@ -0,0 +1,15 @@ +loadFromExtension('sentry', [ + 'tracing' => [ + 'dbal' => [ + 'enabled' => false, + 'connections' => ['default'], + ], + ], +]); diff --git a/tests/DependencyInjection/Fixtures/php/full.php b/tests/DependencyInjection/Fixtures/php/full.php index 5c4c2207..3b7f8699 100644 --- a/tests/DependencyInjection/Fixtures/php/full.php +++ b/tests/DependencyInjection/Fixtures/php/full.php @@ -42,4 +42,9 @@ 'enabled' => true, 'capture_soft_fails' => false, ], + 'tracing' => [ + 'dbal' => [ + 'enabled' => true, + ], + ], ]); diff --git a/tests/DependencyInjection/Fixtures/xml/dbal_tracing_disabled.xml b/tests/DependencyInjection/Fixtures/xml/dbal_tracing_disabled.xml new file mode 100644 index 00000000..4e891b3a --- /dev/null +++ b/tests/DependencyInjection/Fixtures/xml/dbal_tracing_disabled.xml @@ -0,0 +1,14 @@ + + + + + + + + + + diff --git a/tests/DependencyInjection/Fixtures/xml/full.xml b/tests/DependencyInjection/Fixtures/xml/full.xml index 486231f1..418c8328 100644 --- a/tests/DependencyInjection/Fixtures/xml/full.xml +++ b/tests/DependencyInjection/Fixtures/xml/full.xml @@ -37,5 +37,8 @@ App\Sentry\Serializer\FooClassSerializer + + + diff --git a/tests/DependencyInjection/Fixtures/yml/dbal_tracing_disabled.yml b/tests/DependencyInjection/Fixtures/yml/dbal_tracing_disabled.yml new file mode 100644 index 00000000..b5a356f5 --- /dev/null +++ b/tests/DependencyInjection/Fixtures/yml/dbal_tracing_disabled.yml @@ -0,0 +1,4 @@ +sentry: + tracing: + dbal: + enabled: false diff --git a/tests/DependencyInjection/Fixtures/yml/full.yml b/tests/DependencyInjection/Fixtures/yml/full.yml index baae8e2e..b123c173 100644 --- a/tests/DependencyInjection/Fixtures/yml/full.yml +++ b/tests/DependencyInjection/Fixtures/yml/full.yml @@ -37,3 +37,6 @@ sentry: messenger: enabled: true capture_soft_fails: false + tracing: + dbal: + enabled: true diff --git a/tests/DependencyInjection/SentryExtensionTest.php b/tests/DependencyInjection/SentryExtensionTest.php index 0824621e..11f2e888 100644 --- a/tests/DependencyInjection/SentryExtensionTest.php +++ b/tests/DependencyInjection/SentryExtensionTest.php @@ -16,6 +16,7 @@ use Sentry\SentryBundle\EventListener\RequestListener; use Sentry\SentryBundle\EventListener\SubRequestListener; use Sentry\SentryBundle\SentryBundle; +use Sentry\SentryBundle\Tracing\DbalSqlTracingLogger; use Sentry\Serializer\RepresentationSerializer; use Sentry\Serializer\Serializer; use Sentry\Transport\TransportFactoryInterface; @@ -261,12 +262,28 @@ public function testIgnoreErrorsIntegrationIsNotAddedTwiceIfAlreadyConfigured(): $this->assertSame(1, $ignoreErrorsIntegrationsCount); } + public function testDbalSqlLoggerIsConfiguredWhenDbalTracingIsEnable(): void + { + $container = $this->createContainerFromFixture('full'); + + $this->assertTrue($container->hasDefinition(DbalSqlTracingLogger::class)); + } + + public function testDbalSqlLoggerIsRemovedWhenDbalTracingIsDisabled(): void + { + $container = $this->createContainerFromFixture('dbal_tracing_disabled'); + + $this->assertFalse($container->hasDefinition(DbalSqlTracingLogger::class)); + $this->assertEmpty($container->getParameter('sentry.tracing.dbal.connections')); + } + private function createContainerFromFixture(string $fixtureFile): ContainerBuilder { $container = new ContainerBuilder(new EnvPlaceholderParameterBag([ 'kernel.cache_dir' => __DIR__, 'kernel.build_dir' => __DIR__, 'kernel.project_dir' => __DIR__, + 'doctrine.default_connection' => 'default', ])); $container->registerExtension(new SentryExtension()); From d7b1e5f63537d928344937bd0e51be0b787425dc Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Sun, 7 Feb 2021 02:01:43 +0100 Subject: [PATCH 08/16] Fix broken build --- composer.json | 2 +- .../Compiler/DbalSqlTracingLoggerPass.php | 6 ++++++ src/DependencyInjection/Configuration.php | 3 ++- src/DependencyInjection/SentryExtension.php | 3 +++ src/Resources/config/schema/sentry-1.0.xsd | 2 -- src/Tracing/DbalSqlTracingLogger.php | 2 +- .../Fixtures/xml/dbal_tracing_disabled.xml | 4 +++- .../Fixtures/yml/dbal_tracing_disabled.yml | 2 ++ tests/DependencyInjection/SentryExtensionTest.php | 1 + 9 files changed, 19 insertions(+), 6 deletions(-) diff --git a/composer.json b/composer.json index 1fc15560..1b4ade81 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,7 @@ "symfony/security-core": "^3.4.43||^4.4.11||^5.0.11" }, "require-dev": { - "doctrine/dbal": "^2.12||^3.0", + "doctrine/dbal": "^2.10||^3.0", "doctrine/doctrine-bundle": "^2.2", "friendsofphp/php-cs-fixer": "^2.17", "jangregor/phpstan-prophecy": "^0.8", diff --git a/src/DependencyInjection/Compiler/DbalSqlTracingLoggerPass.php b/src/DependencyInjection/Compiler/DbalSqlTracingLoggerPass.php index 489f9614..986a99ff 100644 --- a/src/DependencyInjection/Compiler/DbalSqlTracingLoggerPass.php +++ b/src/DependencyInjection/Compiler/DbalSqlTracingLoggerPass.php @@ -22,7 +22,10 @@ public function process(ContainerBuilder $container): void return; } + /** @var string[] $connections */ $connections = $container->getParameter('doctrine.connections'); + + /** @var string[] $connectionsToTrace */ $connectionsToTrace = $container->getParameter('sentry.tracing.dbal.connections'); foreach ($connectionsToTrace as $connectionName) { @@ -45,6 +48,9 @@ public function process(ContainerBuilder $container): void } } + /** + * @return array{0: string, 1: mixed[]}|null + */ private function getSetSQLLoggerMethodCall(Definition $definition): ?array { foreach ($definition->getMethodCalls() as $methodCall) { diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 4a8d161f..3e560c29 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -4,6 +4,7 @@ namespace Sentry\SentryBundle\DependencyInjection; +use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; use Jean85\PrettyVersions; use Sentry\Options; use Sentry\SentryBundle\ErrorTypesParser; @@ -158,8 +159,8 @@ private function addDistributedTracingSection(ArrayNodeDefinition $rootNode): vo ->canBeEnabled() ->children() ->arrayNode('connections') + ->defaultValue(class_exists(DoctrineBundle::class) ? ['%doctrine.default_connection%'] : []) ->scalarPrototype()->end() - ->defaultValue(['%doctrine.default_connection%']) ->end() ->end() ->end() diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index eda20000..f89e6ec0 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -238,6 +238,9 @@ private function isIntegrationEnabled(string $integrationClass, array $integrati return false; } + /** + * @param array $config + */ protected function isConfigEnabled(ContainerBuilder $container, array $config): bool { if (!isset($config['enabled'])) { diff --git a/src/Resources/config/schema/sentry-1.0.xsd b/src/Resources/config/schema/sentry-1.0.xsd index 1ea0cb2e..d74c0dc1 100644 --- a/src/Resources/config/schema/sentry-1.0.xsd +++ b/src/Resources/config/schema/sentry-1.0.xsd @@ -85,8 +85,6 @@ - - diff --git a/src/Tracing/DbalSqlTracingLogger.php b/src/Tracing/DbalSqlTracingLogger.php index cda66df8..2adc36c2 100644 --- a/src/Tracing/DbalSqlTracingLogger.php +++ b/src/Tracing/DbalSqlTracingLogger.php @@ -20,7 +20,7 @@ final class DbalSqlTracingLogger implements SQLLogger private $hub; /** - * @var Span The span tracing the execution of a query + * @var Span|null The span tracing the execution of a query */ private $span; diff --git a/tests/DependencyInjection/Fixtures/xml/dbal_tracing_disabled.xml b/tests/DependencyInjection/Fixtures/xml/dbal_tracing_disabled.xml index 4e891b3a..08ed0e94 100644 --- a/tests/DependencyInjection/Fixtures/xml/dbal_tracing_disabled.xml +++ b/tests/DependencyInjection/Fixtures/xml/dbal_tracing_disabled.xml @@ -8,7 +8,9 @@ - + + default + diff --git a/tests/DependencyInjection/Fixtures/yml/dbal_tracing_disabled.yml b/tests/DependencyInjection/Fixtures/yml/dbal_tracing_disabled.yml index b5a356f5..3c819c2a 100644 --- a/tests/DependencyInjection/Fixtures/yml/dbal_tracing_disabled.yml +++ b/tests/DependencyInjection/Fixtures/yml/dbal_tracing_disabled.yml @@ -2,3 +2,5 @@ sentry: tracing: dbal: enabled: false + connections: + - default diff --git a/tests/DependencyInjection/SentryExtensionTest.php b/tests/DependencyInjection/SentryExtensionTest.php index 11f2e888..d504a5f2 100644 --- a/tests/DependencyInjection/SentryExtensionTest.php +++ b/tests/DependencyInjection/SentryExtensionTest.php @@ -284,6 +284,7 @@ private function createContainerFromFixture(string $fixtureFile): ContainerBuild 'kernel.build_dir' => __DIR__, 'kernel.project_dir' => __DIR__, 'doctrine.default_connection' => 'default', + 'doctrine.connections' => ['default'], ])); $container->registerExtension(new SentryExtension()); From 691fac7084aa3f7bdbaaa91ac3e8b29a9b90993f Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Mon, 8 Feb 2021 00:30:39 +0100 Subject: [PATCH 09/16] Add a DBAL driver that supports distributed tracing --- composer.json | 1 - .../Compiler/DbalSqlTracingLoggerPass.php | 96 ----- .../Compiler/DbalTracingDriverPass.php | 58 +++ src/DependencyInjection/SentryExtension.php | 29 +- src/Resources/config/services.xml | 2 +- src/SentryBundle.php | 4 +- src/Tracing/DbalSqlTracingLogger.php | 65 --- src/Tracing/Doctrine/DBAL/TracingDriver.php | 77 ++++ .../Doctrine/DBAL/TracingDriverConnection.php | 232 +++++++++++ .../Doctrine/DBAL/TracingDriverMiddleware.php | 39 ++ .../Compiler/DbalSqlTracingLoggerPassTest.php | 153 ------- .../Compiler/DbalTracingDriverPassTest.php | 103 +++++ .../SentryExtensionTest.php | 10 +- tests/Tracing/DbalSqlTracingLoggerTest.php | 65 --- .../DBAL/TracingDriverConnectionTest.php | 394 ++++++++++++++++++ .../DBAL/TracingDriverMiddlewareTest.php | 38 ++ .../Doctrine/DBAL/TracingDriverTest.php | 99 +++++ 17 files changed, 1057 insertions(+), 408 deletions(-) delete mode 100644 src/DependencyInjection/Compiler/DbalSqlTracingLoggerPass.php create mode 100644 src/DependencyInjection/Compiler/DbalTracingDriverPass.php delete mode 100644 src/Tracing/DbalSqlTracingLogger.php create mode 100644 src/Tracing/Doctrine/DBAL/TracingDriver.php create mode 100644 src/Tracing/Doctrine/DBAL/TracingDriverConnection.php create mode 100644 src/Tracing/Doctrine/DBAL/TracingDriverMiddleware.php delete mode 100644 tests/DependencyInjection/Compiler/DbalSqlTracingLoggerPassTest.php create mode 100644 tests/DependencyInjection/Compiler/DbalTracingDriverPassTest.php delete mode 100644 tests/Tracing/DbalSqlTracingLoggerTest.php create mode 100644 tests/Tracing/Doctrine/DBAL/TracingDriverConnectionTest.php create mode 100644 tests/Tracing/Doctrine/DBAL/TracingDriverMiddlewareTest.php create mode 100644 tests/Tracing/Doctrine/DBAL/TracingDriverTest.php diff --git a/composer.json b/composer.json index 1b4ade81..7b43aff7 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,6 @@ "symfony/security-core": "^3.4.43||^4.4.11||^5.0.11" }, "require-dev": { - "doctrine/dbal": "^2.10||^3.0", "doctrine/doctrine-bundle": "^2.2", "friendsofphp/php-cs-fixer": "^2.17", "jangregor/phpstan-prophecy": "^0.8", diff --git a/src/DependencyInjection/Compiler/DbalSqlTracingLoggerPass.php b/src/DependencyInjection/Compiler/DbalSqlTracingLoggerPass.php deleted file mode 100644 index 986a99ff..00000000 --- a/src/DependencyInjection/Compiler/DbalSqlTracingLoggerPass.php +++ /dev/null @@ -1,96 +0,0 @@ -hasParameter('doctrine.connections')) { - return; - } - - /** @var string[] $connections */ - $connections = $container->getParameter('doctrine.connections'); - - /** @var string[] $connectionsToTrace */ - $connectionsToTrace = $container->getParameter('sentry.tracing.dbal.connections'); - - foreach ($connectionsToTrace as $connectionName) { - if (!\in_array($connectionName, $connections, true)) { - continue; - } - - $configurationDefinition = $container->getDefinition(sprintf('doctrine.dbal.%s_connection.configuration', $connectionName)); - $setSQLLoggerMethodCall = $this->getSetSQLLoggerMethodCall($configurationDefinition); - - if (null === $setSQLLoggerMethodCall) { - $loggerDefinition = new Reference(DbalSqlTracingLogger::class); - } else { - $loggerDefinition = $this->addTracingLoggerToConnectionConfiguration($container, $setSQLLoggerMethodCall[1][0]); - } - - $configurationDefinition - ->removeMethodCall('setSQLLogger') - ->addMethodCall('setSQLLogger', [$loggerDefinition]); - } - } - - /** - * @return array{0: string, 1: mixed[]}|null - */ - private function getSetSQLLoggerMethodCall(Definition $definition): ?array - { - foreach ($definition->getMethodCalls() as $methodCall) { - if ('setSQLLogger' === $methodCall[0]) { - return $methodCall; - } - } - - return null; - } - - /** - * @param Reference|Definition $loggerDefinition The service definition - * - * @return Reference|Definition - */ - private function addTracingLoggerToConnectionConfiguration(ContainerBuilder $container, $loggerDefinition) - { - if ($loggerDefinition instanceof Definition) { - if (LoggerChain::class === $loggerDefinition->getClass()) { - $loggerDefinition->setArgument(0, array_merge($loggerDefinition->getArgument(0), [new Reference(DbalSqlTracingLogger::class)])); - } else { - $loggerDefinition = new Definition(LoggerChain::class, [[ - $loggerDefinition, - new Reference(DbalSqlTracingLogger::class), - ]]); - } - } else { - $realLoggerDefinition = $container->findDefinition((string) $loggerDefinition); - - if (LoggerChain::class === $realLoggerDefinition->getClass()) { - $realLoggerDefinition->setArgument(0, array_merge($realLoggerDefinition->getArgument(0), [new Reference(DbalSqlTracingLogger::class)])); - } else { - $loggerDefinition = new Definition(LoggerChain::class, [[ - $loggerDefinition, - new Reference(DbalSqlTracingLogger::class), - ]]); - } - } - - return $loggerDefinition; - } -} diff --git a/src/DependencyInjection/Compiler/DbalTracingDriverPass.php b/src/DependencyInjection/Compiler/DbalTracingDriverPass.php new file mode 100644 index 00000000..0f6fb52b --- /dev/null +++ b/src/DependencyInjection/Compiler/DbalTracingDriverPass.php @@ -0,0 +1,58 @@ +hasParameter('doctrine.connections')) { + return; + } + + /** @var string[] $connections */ + $connections = $container->getParameter('doctrine.connections'); + + /** @var string[] $connectionsToTrace */ + $connectionsToTrace = $container->getParameter('sentry.tracing.dbal.connections'); + + foreach ($connectionsToTrace as $connectionName) { + if (!\in_array(sprintf('doctrine.dbal.%s_connection', $connectionName), $connections, true)) { + continue; + } + + $configurationDefinition = $container->getDefinition(sprintf('doctrine.dbal.%s_connection.configuration', $connectionName)); + $setMiddlewaresMethodCallArguments = $this->getSetMiddlewaresMethodCallArguments($configurationDefinition); + $setMiddlewaresMethodCallArguments[0] = array_merge($setMiddlewaresMethodCallArguments[0] ?? [], [new Reference(TracingDriverMiddleware::class)]); + + $configurationDefinition + ->removeMethodCall('setMiddlewares') + ->addMethodCall('setMiddlewares', $setMiddlewaresMethodCallArguments); + } + } + + /** + * @return mixed[] + */ + private function getSetMiddlewaresMethodCallArguments(Definition $definition): array + { + foreach ($definition->getMethodCalls() as $methodCall) { + if ('setMiddlewares' === $methodCall[0]) { + return $methodCall[1]; + } + } + + return []; + } +} diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index f89e6ec0..a8073db0 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -5,7 +5,6 @@ namespace Sentry\SentryBundle\DependencyInjection; use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; -use InvalidArgumentException; use Jean85\PrettyVersions; use LogicException; use Sentry\Client; @@ -18,7 +17,7 @@ use Sentry\SentryBundle\EventListener\ErrorListener; use Sentry\SentryBundle\EventListener\MessengerListener; use Sentry\SentryBundle\SentryBundle; -use Sentry\SentryBundle\Tracing\DbalSqlTracingLogger; +use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware; use Sentry\Serializer\RepresentationSerializer; use Sentry\Serializer\Serializer; use Sentry\Transport\TransportFactoryInterface; @@ -145,7 +144,7 @@ private function registerErrorListenerConfiguration(ContainerBuilder $container, */ private function registerMessengerListenerConfiguration(ContainerBuilder $container, array $config): void { - if (!$config['enabled']) { + if (!$this->isConfigEnabled($container, $config)) { $container->removeDefinition(MessengerListener::class); return; @@ -159,14 +158,16 @@ private function registerMessengerListenerConfiguration(ContainerBuilder $contai */ private function registerTracingConfiguration(ContainerBuilder $container, array $config): void { - if ($this->isConfigEnabled($container, $config['dbal']) && !class_exists(DoctrineBundle::class)) { - throw new LogicException('DBAL tracing support cannot be enabled as the DoctrineBundle bundle is not installed. Try running "composer require doctrine/doctrine-bundle".'); + $isConfigEnabled = $this->isConfigEnabled($container, $config['dbal']); + + if ($isConfigEnabled && !class_exists(DoctrineBundle::class)) { + throw new LogicException('DBAL tracing support cannot be enabled as the DoctrineBundle bundle is not installed.'); } - $container->setParameter('sentry.tracing.dbal.connections', $config['dbal']['enabled'] ? $config['dbal']['connections'] : []); + $container->setParameter('sentry.tracing.dbal.connections', $isConfigEnabled ? $config['dbal']['connections'] : []); - if (!$config['dbal']['enabled']) { - $container->removeDefinition(DbalSqlTracingLogger::class); + if (!$isConfigEnabled) { + $container->removeDefinition(TracingDriverMiddleware::class); } } @@ -237,16 +238,4 @@ private function isIntegrationEnabled(string $integrationClass, array $integrati return false; } - - /** - * @param array $config - */ - protected function isConfigEnabled(ContainerBuilder $container, array $config): bool - { - if (!isset($config['enabled'])) { - throw new InvalidArgumentException('The $config argument has no key named "enabled".'); - } - - return (bool) $container->getParameterBag()->resolveValue($config['enabled']); - } } diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index 515b2fb3..390c31a5 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -65,7 +65,7 @@ - + diff --git a/src/SentryBundle.php b/src/SentryBundle.php index abfe2283..0b01bb5a 100644 --- a/src/SentryBundle.php +++ b/src/SentryBundle.php @@ -4,7 +4,7 @@ namespace Sentry\SentryBundle; -use Sentry\SentryBundle\DependencyInjection\Compiler\DbalSqlTracingLoggerPass; +use Sentry\SentryBundle\DependencyInjection\Compiler\DbalTracingDriverPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; @@ -16,6 +16,6 @@ public function build(ContainerBuilder $container): void { parent::build($container); - $container->addCompilerPass(new DbalSqlTracingLoggerPass()); + $container->addCompilerPass(new DbalTracingDriverPass()); } } diff --git a/src/Tracing/DbalSqlTracingLogger.php b/src/Tracing/DbalSqlTracingLogger.php deleted file mode 100644 index 2adc36c2..00000000 --- a/src/Tracing/DbalSqlTracingLogger.php +++ /dev/null @@ -1,65 +0,0 @@ -hub = $hub; - } - - /** - * {@inheritdoc} - */ - public function startQuery($sql, ?array $params = null, ?array $types = null) - { - $span = $this->hub->getSpan(); - - if (null === $span) { - return; - } - - $spanContext = new SpanContext(); - $spanContext->setOp('db.query'); - $spanContext->setDescription($sql); - - $this->span = $span->startChild($spanContext); - } - - /** - * {@inheritdoc} - */ - public function stopQuery() - { - if (null === $this->span) { - return; - } - - $this->span->finish(); - $this->span = null; - } -} diff --git a/src/Tracing/Doctrine/DBAL/TracingDriver.php b/src/Tracing/Doctrine/DBAL/TracingDriver.php new file mode 100644 index 00000000..4fdf6dd8 --- /dev/null +++ b/src/Tracing/Doctrine/DBAL/TracingDriver.php @@ -0,0 +1,77 @@ +hub = $hub; + $this->decoratedDriver = $decoratedDriver; + } + + /** + * {@inheritdoc} + */ + public function connect(array $params) + { + return new TracingDriverConnection( + $this->hub, + $this->decoratedDriver->connect($params), + $this->decoratedDriver->getDatabasePlatform()->getName(), + $params + ); + } + + /** + * {@inheritdoc} + */ + public function getDatabasePlatform() + { + return $this->decoratedDriver->getDatabasePlatform(); + } + + /** + * {@inheritdoc} + */ + public function getSchemaManager(Connection $conn, AbstractPlatform $platform) + { + return $this->decoratedDriver->getSchemaManager($conn, $platform); + } + + /** + * {@inheritdoc} + */ + public function getExceptionConverter(): ExceptionConverter + { + return $this->decoratedDriver->getExceptionConverter(); + } +} diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php new file mode 100644 index 00000000..e51bc463 --- /dev/null +++ b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php @@ -0,0 +1,232 @@ + The connection params + */ + private $params; + + /** + * Constructor. + * + * @param HubInterface $hub The current hub + * @param DriverConnectionInterface $decoratedConnection The connection to decorarte + * @param string $databasePlatform The name of the database platform + * @param array $params The connection params + */ + public function __construct( + HubInterface $hub, + DriverConnectionInterface $decoratedConnection, + string $databasePlatform, + array $params + ) { + $this->hub = $hub; + $this->decoratedConnection = $decoratedConnection; + $this->databasePlatform = $databasePlatform; + $this->params = $params; + } + + /** + * {@inheritdoc} + */ + public function prepare(string $sql): Statement + { + return $this->traceFunction(self::SPAN_OP_CONN_PREPARE, $sql, function () use ($sql): Statement { + return $this->decoratedConnection->prepare($sql); + }); + } + + /** + * {@inheritdoc} + */ + public function query(string $sql): Result + { + return $this->traceFunction(self::SPAN_OP_CONN_QUERY, $sql, function () use ($sql): Result { + return $this->decoratedConnection->query($sql); + }); + } + + /** + * {@inheritdoc} + */ + public function quote($value, $type = ParameterType::STRING) + { + return $this->decoratedConnection->quote($value, $type); + } + + /** + * {@inheritdoc} + */ + public function exec(string $sql): int + { + return $this->traceFunction(self::SPAN_OP_CONN_EXEC, $sql, function () use ($sql): int { + return $this->decoratedConnection->exec($sql); + }); + } + + /** + * {@inheritdoc} + */ + public function lastInsertId($name = null) + { + return $this->decoratedConnection->lastInsertId($name); + } + + /** + * {@inheritdoc} + */ + public function beginTransaction() + { + return $this->traceFunction(self::SPAN_OP_CONN_BEGIN_TRANSACTION, 'BEGIN TRANSACTION', function (): bool { + return $this->decoratedConnection->beginTransaction(); + }); + } + + /** + * {@inheritdoc} + */ + public function commit() + { + return $this->traceFunction(self::SPAN_OP_TRANSACTION_COMMIT, 'COMMIT', function (): bool { + return $this->decoratedConnection->commit(); + }); + } + + /** + * {@inheritdoc} + */ + public function rollBack() + { + return $this->traceFunction(self::SPAN_OP_TRANSACTION_ROLLBACK, 'ROLLBACK', function (): bool { + return $this->decoratedConnection->rollBack(); + }); + } + + /** + * @phpstan-template T + * + * @phpstan-param \Closure(): T $callback + * + * @phpstan-return T + */ + private function traceFunction(string $spanOperation, string $spanDescription, \Closure $callback) + { + $span = $this->hub->getSpan(); + + if (null !== $span) { + $spanContext = new SpanContext(); + $spanContext->setOp($spanOperation); + $spanContext->setDescription($spanDescription); + $spanContext->setTags($this->getSpanTags()); + + $span = $span->startChild($spanContext); + } + + try { + return $callback(); + } finally { + if (null !== $span) { + $span->finish(); + } + } + } + + /** + * @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md + * + * @return array + */ + private function getSpanTags(): array + { + $tags = ['db.system' => $this->databasePlatform]; + + if (isset($this->params['user'])) { + $tags['db.user'] = $this->params['user']; + } + + if (isset($this->params['dbname'])) { + $tags['db.name'] = $this->params['dbname']; + } + + if (isset($this->params['host']) && !empty($this->params['host']) && !isset($this->params['memory'])) { + if (false === filter_var($this->params['host'], \FILTER_VALIDATE_IP)) { + $tags['net.peer.name'] = $this->params['host']; + } else { + $tags['net.peer.ip'] = $this->params['host']; + } + } + + if (isset($this->params['port'])) { + $tags['net.peer.port'] = (string) $this->params['port']; + } + + if (isset($this->params['unix_socket'])) { + $tags['net.transport'] = 'Unix'; + } elseif (isset($this->params['memory'])) { + $tags['net.transport'] = 'inproc'; + } + + return $tags; + } +} diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverMiddleware.php b/src/Tracing/Doctrine/DBAL/TracingDriverMiddleware.php new file mode 100644 index 00000000..ab85dc7b --- /dev/null +++ b/src/Tracing/Doctrine/DBAL/TracingDriverMiddleware.php @@ -0,0 +1,39 @@ +hub = $hub; + } + + /** + * {@inheritdoc} + */ + public function wrap(DriverInterface $driver): DriverInterface + { + return new TracingDriver($this->hub, $driver); + } +} diff --git a/tests/DependencyInjection/Compiler/DbalSqlTracingLoggerPassTest.php b/tests/DependencyInjection/Compiler/DbalSqlTracingLoggerPassTest.php deleted file mode 100644 index bc4f3a84..00000000 --- a/tests/DependencyInjection/Compiler/DbalSqlTracingLoggerPassTest.php +++ /dev/null @@ -1,153 +0,0 @@ -createContainerBuilder(); - - $container->setParameter('doctrine.connections', ['connection_1', 'connection_2', 'connection_3', 'connection_4', 'connection_5']); - $container->setParameter('sentry.tracing.dbal.connections', ['connection_1', 'connection_2', 'connection_3', 'connection_4', 'connection_5']); - - $container - ->register('doctrine.dbal.logger', SQLLogger::class) - ->setPublic(true); - - $container - ->register('doctrine.dbal.logger.chain', LoggerChain::class) - ->setArgument(0, [new Reference('doctrine.dbal.logger')]) - ->setPublic(true); - - $container - ->register('doctrine.dbal.connection_1_connection.configuration', Configuration::class) - ->setPublic(true); - - $container - ->register('doctrine.dbal.connection_2_connection.configuration', Configuration::class) - ->addMethodCall('setSQLLogger', [new Reference('doctrine.dbal.logger')]) - ->setPublic(true); - - $container - ->register('doctrine.dbal.connection_3_connection.configuration', Configuration::class) - ->addMethodCall('setSQLLogger', [new Reference('doctrine.dbal.logger.chain')]) - ->setPublic(true); - - $container - ->register('doctrine.dbal.connection_4_connection.configuration', Configuration::class) - ->addMethodCall('setSQLLogger', [new Definition(LoggerChain::class, [[new Reference('doctrine.dbal.logger')]])]) - ->setPublic(true); - - $container - ->register('doctrine.dbal.connection_5_connection.configuration', Configuration::class) - ->addMethodCall('setSQLLogger', [new Definition(SQLLogger::class)]) - ->setPublic(true); - - $container->compile(); - - $this->assertEquals( - [['setSQLLogger', [new Reference(DbalSqlTracingLogger::class)]]], - $container->getDefinition('doctrine.dbal.connection_1_connection.configuration')->getMethodCalls() - ); - - $this->assertEquals( - [ - [ - 'setSQLLogger', - [ - new Definition(LoggerChain::class, [[ - new Reference('doctrine.dbal.logger'), - new Reference(DbalSqlTracingLogger::class), - ]]), - ], - ], - ], - $container->getDefinition('doctrine.dbal.connection_2_connection.configuration')->getMethodCalls() - ); - - $this->assertEquals( - [['setSQLLogger', [new Reference('doctrine.dbal.logger.chain')]]], - $container->getDefinition('doctrine.dbal.connection_3_connection.configuration')->getMethodCalls() - ); - - $this->assertEquals( - [ - [ - new Reference('doctrine.dbal.logger'), - new Reference(DbalSqlTracingLogger::class), - ], - ], - $container->getDefinition('doctrine.dbal.logger.chain')->getArguments() - ); - - $this->assertEquals( - [ - [ - 'setSQLLogger', - [ - new Definition(LoggerChain::class, [[ - new Reference('doctrine.dbal.logger'), - new Reference(DbalSqlTracingLogger::class), - ]]), - ], - ], - ], - $container->getDefinition('doctrine.dbal.connection_4_connection.configuration')->getMethodCalls() - ); - - $this->assertEquals( - [ - [ - 'setSQLLogger', - [ - new Definition(LoggerChain::class, [[ - new Definition(SQLLogger::class), - new Reference(DbalSqlTracingLogger::class), - ]]), - ], - ], - ], - $container->getDefinition('doctrine.dbal.connection_5_connection.configuration')->getMethodCalls() - ); - } - - private function createContainerBuilder(): ContainerBuilder - { - $container = new ContainerBuilder(); - $container->addCompilerPass(new DbalSqlTracingLoggerPass()); - - $container - ->register(DbalSqlTracingLogger::class, DbalSqlTracingLogger::class) - ->setPublic(true); - - return $container; - } -} diff --git a/tests/DependencyInjection/Compiler/DbalTracingDriverPassTest.php b/tests/DependencyInjection/Compiler/DbalTracingDriverPassTest.php new file mode 100644 index 00000000..bdc4e47c --- /dev/null +++ b/tests/DependencyInjection/Compiler/DbalTracingDriverPassTest.php @@ -0,0 +1,103 @@ +createContainerBuilder(); + $container->setParameter('doctrine.connections', ['doctrine.dbal.foo_connection', 'doctrine.dbal.bar_connection', 'doctrine.dbal.baz_connection']); + $container->setParameter('sentry.tracing.dbal.connections', ['foo', 'bar', 'baz', 'qux']); + + $container + ->register('foo.service', \stdClass::class) + ->setPublic(true); + + $container + ->register('doctrine.dbal.foo_connection.configuration', Configuration::class) + ->setPublic(true); + + $container + ->register('doctrine.dbal.bar_connection.configuration', Configuration::class) + ->addMethodCall('setMiddlewares', [[]]) + ->setPublic(true); + + $container + ->register('doctrine.dbal.baz_connection.configuration', Configuration::class) + ->addMethodCall('setMiddlewares', [[new Reference('foo.service')]]) + ->setPublic(true); + + $container->compile(); + + $this->assertEquals( + [ + [ + 'setMiddlewares', + [[new Reference(TracingDriverMiddleware::class)]], + ], + ], + $container->getDefinition('doctrine.dbal.foo_connection.configuration')->getMethodCalls() + ); + + $this->assertEquals( + [ + [ + 'setMiddlewares', + [[new Reference(TracingDriverMiddleware::class)]], + ], + ], + $container->getDefinition('doctrine.dbal.bar_connection.configuration')->getMethodCalls() + ); + + $this->assertEquals( + [ + [ + 'setMiddlewares', + [ + [ + new Reference('foo.service'), + new Reference(TracingDriverMiddleware::class), + ], + ], + ], + ], + $container->getDefinition('doctrine.dbal.baz_connection.configuration')->getMethodCalls() + ); + } + + public function testProcessDoesNothingIfDoctrineConnectionsParamIsMissing(): void + { + $container = $this->createContainerBuilder(); + $container->setParameter('sentry.tracing.dbal.connections', ['foo']); + + $container + ->register('doctrine.dbal.foo_connection.configuration', Configuration::class) + ->setPublic(true); + + $container->compile(); + + $this->assertEmpty($container->getDefinition('doctrine.dbal.foo_connection.configuration')->getMethodCalls()); + } + + private function createContainerBuilder(): ContainerBuilder + { + $container = new ContainerBuilder(); + $container->addCompilerPass(new DbalTracingDriverPass()); + + $container + ->register(TracingDriverMiddleware::class, TracingDriverMiddleware::class) + ->setPublic(true); + + return $container; + } +} diff --git a/tests/DependencyInjection/SentryExtensionTest.php b/tests/DependencyInjection/SentryExtensionTest.php index d504a5f2..afb6ca25 100644 --- a/tests/DependencyInjection/SentryExtensionTest.php +++ b/tests/DependencyInjection/SentryExtensionTest.php @@ -16,7 +16,7 @@ use Sentry\SentryBundle\EventListener\RequestListener; use Sentry\SentryBundle\EventListener\SubRequestListener; use Sentry\SentryBundle\SentryBundle; -use Sentry\SentryBundle\Tracing\DbalSqlTracingLogger; +use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware; use Sentry\Serializer\RepresentationSerializer; use Sentry\Serializer\Serializer; use Sentry\Transport\TransportFactoryInterface; @@ -262,18 +262,18 @@ public function testIgnoreErrorsIntegrationIsNotAddedTwiceIfAlreadyConfigured(): $this->assertSame(1, $ignoreErrorsIntegrationsCount); } - public function testDbalSqlLoggerIsConfiguredWhenDbalTracingIsEnable(): void + public function testTracingDriverMiddlewareIsConfiguredWhenDbalTracingIsEnable(): void { $container = $this->createContainerFromFixture('full'); - $this->assertTrue($container->hasDefinition(DbalSqlTracingLogger::class)); + $this->assertTrue($container->hasDefinition(TracingDriverMiddleware::class)); } - public function testDbalSqlLoggerIsRemovedWhenDbalTracingIsDisabled(): void + public function testTracingDriverMiddlewareIsRemovedWhenDbalTracingIsDisabled(): void { $container = $this->createContainerFromFixture('dbal_tracing_disabled'); - $this->assertFalse($container->hasDefinition(DbalSqlTracingLogger::class)); + $this->assertFalse($container->hasDefinition(TracingDriverMiddleware::class)); $this->assertEmpty($container->getParameter('sentry.tracing.dbal.connections')); } diff --git a/tests/Tracing/DbalSqlTracingLoggerTest.php b/tests/Tracing/DbalSqlTracingLoggerTest.php deleted file mode 100644 index 1aee8487..00000000 --- a/tests/Tracing/DbalSqlTracingLoggerTest.php +++ /dev/null @@ -1,65 +0,0 @@ -hub = $this->createMock(HubInterface::class); - $this->logger = new DbalSqlTracingLogger($this->hub); - } - - public function testStopQueryDoesNothingIfTransactionDidNotStartTheChildSpan(): void - { - $this->hub->expects($this->once()) - ->method('getSpan') - ->willReturn(null); - - $this->logger->startQuery('SELECT * FROM orders'); - $this->logger->stopQuery(); - } - - public function testStopQueryFinishesTheChildSpan(): void - { - $transaction = new Transaction(new TransactionContext()); - $transaction->initSpanRecorder(); - - $this->hub->expects($this->once()) - ->method('getSpan') - ->willReturn($transaction); - - $this->logger->startQuery('SELECT * FROM orders'); - - $spans = $transaction->getSpanRecorder()->getSpans(); - - $this->assertCount(2, $spans); - $this->assertNull($spans[1]->getEndTimestamp()); - - $this->logger->stopQuery(); - - $spans = $transaction->getSpanRecorder()->getSpans(); - - $this->assertCount(2, $spans); - $this->assertNotNull($spans[1]->getEndTimestamp()); - } -} diff --git a/tests/Tracing/Doctrine/DBAL/TracingDriverConnectionTest.php b/tests/Tracing/Doctrine/DBAL/TracingDriverConnectionTest.php new file mode 100644 index 00000000..51d28639 --- /dev/null +++ b/tests/Tracing/Doctrine/DBAL/TracingDriverConnectionTest.php @@ -0,0 +1,394 @@ +hub = $this->createMock(HubInterface::class); + $this->decoratedConnection = $this->createMock(DriverConnectionInterface::class); + $this->connection = new TracingDriverConnection($this->hub, $this->decoratedConnection, 'foo_platform', []); + } + + /** + * @dataProvider tagsDataProvider + * + * @param array $params + * @param array $expectedTags + */ + public function testPrepare(array $params, array $expectedTags): void + { + $statement = $this->createMock(DriverStatementInterface::class); + $connection = new TracingDriverConnection($this->hub, $this->decoratedConnection, 'foo_platform', $params); + $sql = 'SELECT 1 + 1'; + + $transaction = new Transaction(new TransactionContext(), $this->hub); + $transaction->initSpanRecorder(); + + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn($transaction); + + $this->decoratedConnection->expects($this->once()) + ->method('prepare') + ->with($sql) + ->willReturn($statement); + + $this->assertSame($statement, $connection->prepare($sql)); + + $spans = $transaction->getSpanRecorder()->getSpans(); + + $this->assertCount(2, $spans); + $this->assertSame(TracingDriverConnection::SPAN_OP_CONN_PREPARE, $spans[1]->getOp()); + $this->assertSame($sql, $spans[1]->getDescription()); + $this->assertSame($expectedTags, $spans[1]->getTags()); + $this->assertNotNull($spans[1]->getEndTimestamp()); + } + + public function testPrepareDoesNothingIfNoSpanIsSetOnHub(): void + { + $statement = $this->createMock(DriverStatementInterface::class); + $sql = 'SELECT 1 + 1'; + + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn(null); + + $this->decoratedConnection->expects($this->once()) + ->method('prepare') + ->with($sql) + ->willReturn($statement); + + $this->assertSame($statement, $this->connection->prepare($sql)); + } + + /** + * @dataProvider tagsDataProvider + * + * @param array $params + * @param array $expectedTags + */ + public function testQuery(array $params, array $expectedTags): void + { + $result = $this->createMock(DriverResultInterface::class); + $connection = new TracingDriverConnection($this->hub, $this->decoratedConnection, 'foo_platform', $params); + $sql = 'SELECT 1 + 1'; + + $transaction = new Transaction(new TransactionContext(), $this->hub); + $transaction->initSpanRecorder(); + + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn($transaction); + + $this->decoratedConnection->expects($this->once()) + ->method('query') + ->with($sql) + ->willReturn($result); + + $this->assertSame($result, $connection->query($sql)); + + $spans = $transaction->getSpanRecorder()->getSpans(); + + $this->assertCount(2, $spans); + $this->assertSame(TracingDriverConnection::SPAN_OP_CONN_QUERY, $spans[1]->getOp()); + $this->assertSame($sql, $spans[1]->getDescription()); + $this->assertSame($expectedTags, $spans[1]->getTags()); + $this->assertNotNull($spans[1]->getEndTimestamp()); + } + + public function testQueryDoesNothingIfNoSpanIsSetOnHub(): void + { + $result = $this->createMock(DriverResultInterface::class); + $sql = 'SELECT 1 + 1'; + + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn(null); + + $this->decoratedConnection->expects($this->once()) + ->method('query') + ->with($sql) + ->willReturn($result); + + $this->assertSame($result, $this->connection->query($sql)); + } + + public function testQuote(): void + { + $this->decoratedConnection->expects($this->once()) + ->method('quote') + ->with('foo', ParameterType::STRING) + ->willReturn('foo'); + + $this->assertSame('foo', $this->connection->quote('foo')); + } + + /** + * @dataProvider tagsDataProvider + * + * @param array $params + * @param array $expectedTags + */ + public function testExec(array $params, array $expectedTags): void + { + $connection = new TracingDriverConnection($this->hub, $this->decoratedConnection, 'foo_platform', $params); + $sql = 'SELECT 1 + 1'; + + $transaction = new Transaction(new TransactionContext(), $this->hub); + $transaction->initSpanRecorder(); + + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn($transaction); + + $this->decoratedConnection->expects($this->once()) + ->method('exec') + ->with($sql) + ->willReturn(10); + + $this->assertSame(10, $connection->exec($sql)); + + $spans = $transaction->getSpanRecorder()->getSpans(); + + $this->assertCount(2, $spans); + $this->assertSame(TracingDriverConnection::SPAN_OP_CONN_EXEC, $spans[1]->getOp()); + $this->assertSame($sql, $spans[1]->getDescription()); + $this->assertSame($expectedTags, $spans[1]->getTags()); + $this->assertNotNull($spans[1]->getEndTimestamp()); + } + + public function testLastInsertId(): void + { + $this->decoratedConnection->expects($this->once()) + ->method('lastInsertId') + ->with('foo') + ->willReturn('10'); + + $this->assertSame('10', $this->connection->lastInsertId('foo')); + } + + /** + * @dataProvider tagsDataProvider + * + * @param array $params + * @param array $expectedTags + */ + public function testBeginTransaction(array $params, array $expectedTags): void + { + $connection = new TracingDriverConnection($this->hub, $this->decoratedConnection, 'foo_platform', $params); + $transaction = new Transaction(new TransactionContext(), $this->hub); + $transaction->initSpanRecorder(); + + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn($transaction); + + $this->decoratedConnection->expects($this->once()) + ->method('beginTransaction') + ->willReturn(false); + + $this->assertFalse($connection->beginTransaction()); + + $spans = $transaction->getSpanRecorder()->getSpans(); + + $this->assertCount(2, $spans); + $this->assertSame(TracingDriverConnection::SPAN_OP_CONN_BEGIN_TRANSACTION, $spans[1]->getOp()); + $this->assertSame('BEGIN TRANSACTION', $spans[1]->getDescription()); + $this->assertSame($expectedTags, $spans[1]->getTags()); + $this->assertNotNull($spans[1]->getEndTimestamp()); + } + + public function testBeginTransactionDoesNothingIfNoSpanIsSetOnHub(): void + { + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn(null); + + $this->decoratedConnection->expects($this->once()) + ->method('beginTransaction') + ->willReturn(false); + + $this->assertFalse($this->connection->beginTransaction()); + } + + /** + * @dataProvider tagsDataProvider + * + * @param array $params + * @param array $expectedTags + */ + public function testCommit(array $params, array $expectedTags): void + { + $connection = new TracingDriverConnection($this->hub, $this->decoratedConnection, 'foo_platform', $params); + $transaction = new Transaction(new TransactionContext(), $this->hub); + $transaction->initSpanRecorder(); + + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn($transaction); + + $this->decoratedConnection->expects($this->once()) + ->method('commit') + ->willReturn(false); + + $this->assertFalse($connection->commit()); + + $spans = $transaction->getSpanRecorder()->getSpans(); + + $this->assertCount(2, $spans); + $this->assertSame(TracingDriverConnection::SPAN_OP_TRANSACTION_COMMIT, $spans[1]->getOp()); + $this->assertSame('COMMIT', $spans[1]->getDescription()); + $this->assertSame($expectedTags, $spans[1]->getTags()); + $this->assertNotNull($spans[1]->getEndTimestamp()); + } + + public function testCommitDoesNothingIfNoSpanIsSetOnHub(): void + { + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn(null); + + $this->decoratedConnection->expects($this->once()) + ->method('commit') + ->willReturn(false); + + $this->assertFalse($this->connection->commit()); + } + + /** + * @dataProvider tagsDataProvider + * + * @param array $params + * @param array $expectedTags + */ + public function testRollBack(array $params, array $expectedTags): void + { + $connection = new TracingDriverConnection($this->hub, $this->decoratedConnection, 'foo_platform', $params); + $transaction = new Transaction(new TransactionContext(), $this->hub); + $transaction->initSpanRecorder(); + + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn($transaction); + + $this->decoratedConnection->expects($this->once()) + ->method('rollBack') + ->willReturn(false); + + $this->assertFalse($connection->rollBack()); + + $spans = $transaction->getSpanRecorder()->getSpans(); + + $this->assertCount(2, $spans); + $this->assertSame(TracingDriverConnection::SPAN_OP_TRANSACTION_ROLLBACK, $spans[1]->getOp()); + $this->assertSame('ROLLBACK', $spans[1]->getDescription()); + $this->assertSame($expectedTags, $spans[1]->getTags()); + $this->assertNotNull($spans[1]->getEndTimestamp()); + } + + public function testRollBackDoesNothingIfNoSpanIsSetOnHub(): void + { + $this->hub->expects($this->once()) + ->method('getSpan') + ->willReturn(null); + + $this->decoratedConnection->expects($this->once()) + ->method('rollBack') + ->willReturn(false); + + $this->assertFalse($this->connection->rollBack()); + } + + /** + * @return \Generator + */ + public function tagsDataProvider(): \Generator + { + yield [ + [], + ['db.system' => 'foo_platform'], + ]; + + yield [ + [ + 'user' => 'root', + 'dbname' => 'INFORMATION_SCHEMA', + 'port' => 3306, + 'unix_socket' => '/var/run/mysqld/mysqld.sock', + ], + [ + 'db.system' => 'foo_platform', + 'db.user' => 'root', + 'db.name' => 'INFORMATION_SCHEMA', + 'net.peer.port' => '3306', + 'net.transport' => 'Unix', + ], + ]; + + yield [ + [ + 'user' => 'root', + 'dbname' => 'INFORMATION_SCHEMA', + 'port' => 3306, + 'memory' => true, + ], + [ + 'db.system' => 'foo_platform', + 'db.user' => 'root', + 'db.name' => 'INFORMATION_SCHEMA', + 'net.peer.port' => '3306', + 'net.transport' => 'inproc', + ], + ]; + + yield [ + [ + 'host' => 'localhost', + ], + [ + 'db.system' => 'foo_platform', + 'net.peer.name' => 'localhost', + ], + ]; + + yield [ + [ + 'host' => '127.0.0.1', + ], + [ + 'db.system' => 'foo_platform', + 'net.peer.ip' => '127.0.0.1', + ], + ]; + } +} diff --git a/tests/Tracing/Doctrine/DBAL/TracingDriverMiddlewareTest.php b/tests/Tracing/Doctrine/DBAL/TracingDriverMiddlewareTest.php new file mode 100644 index 00000000..6168ea17 --- /dev/null +++ b/tests/Tracing/Doctrine/DBAL/TracingDriverMiddlewareTest.php @@ -0,0 +1,38 @@ +hub = $this->createMock(HubInterface::class); + $this->middleware = new TracingDriverMiddleware($this->hub); + } + + public function testWrap(): void + { + $driver = $this->createMock(DriverInterface::class); + + $this->assertInstanceOf(TracingDriver::class, $this->middleware->wrap($driver)); + } +} diff --git a/tests/Tracing/Doctrine/DBAL/TracingDriverTest.php b/tests/Tracing/Doctrine/DBAL/TracingDriverTest.php new file mode 100644 index 00000000..fd0acd9f --- /dev/null +++ b/tests/Tracing/Doctrine/DBAL/TracingDriverTest.php @@ -0,0 +1,99 @@ +hub = $this->createMock(HubInterface::class); + $this->decoratedDriver = $this->createMock(DriverInterface::class); + $this->driver = new TracingDriver($this->hub, $this->decoratedDriver); + } + + public function testConnect(): void + { + $params = ['foo' => 'bar']; + + $databasePlatform = $this->createMock(AbstractPlatform::class); + $databasePlatform->expects($this->once()) + ->method('getName') + ->willReturn('foo'); + + $this->decoratedDriver->expects($this->once()) + ->method('connect') + ->with($params) + ->willReturn($this->createMock(DriverConnectionInterface::class)); + + $this->decoratedDriver->expects($this->once()) + ->method('getDatabasePlatform') + ->willReturn($databasePlatform); + + $this->assertInstanceOf(TracingDriverConnection::class, $this->driver->connect($params)); + } + + public function testGetDatabasePlatform(): void + { + $databasePlatform = $this->createMock(AbstractPlatform::class); + + $this->decoratedDriver->expects($this->once()) + ->method('getDatabasePlatform') + ->willReturn($databasePlatform); + + $this->assertSame($databasePlatform, $this->driver->getDatabasePlatform()); + } + + public function testGetSchemaManager(): void + { + $connection = $this->createMock(Connection::class); + $databasePlatform = $this->createMock(AbstractPlatform::class); + $schemaManager = $this->createMock(AbstractSchemaManager::class); + + $this->decoratedDriver->expects($this->once()) + ->method('getSchemaManager') + ->with($connection, $databasePlatform) + ->willReturn($schemaManager); + + $this->assertSame($schemaManager, $this->driver->getSchemaManager($connection, $databasePlatform)); + } + + public function testGetExceptionConverter(): void + { + $exceptionConverter = $this->createMock(ExceptionConverterInterface::class); + + $this->decoratedDriver->expects($this->once()) + ->method('getExceptionConverter') + ->willReturn($exceptionConverter); + + $this->assertSame($exceptionConverter, $this->driver->getExceptionConverter()); + } +} From f366a1fbfdbcccba735053fb6c9f391622205045 Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Thu, 11 Feb 2021 01:33:27 +0100 Subject: [PATCH 10/16] Support tracing for Doctrine DBAL 2.10+ Support tracing for Doctrine DBAL 2.10+ --- composer.json | 5 +- phpstan.neon | 1 + psalm-baseline.xml | 10 +- psalm.xml | 1 + .../Compiler/DbalTracingDriverPass.php | 58 ----- .../Compiler/DbalTracingPass.php | 87 ++++++++ src/DependencyInjection/Configuration.php | 3 +- src/Resources/config/services.xml | 4 + src/SentryBundle.php | 4 +- .../ExceptionConverterDriverInterface.php | 12 ++ .../Compatibility/MiddlewareInterface.php | 15 ++ .../Doctrine/DBAL/ConnectionConfigurator.php | 46 ++++ src/Tracing/Doctrine/DBAL/TracingDriver.php | 73 ++++++- .../Doctrine/DBAL/TracingDriverConnection.php | 42 +++- .../Doctrine/DBAL/TracingDriverMiddleware.php | 4 +- src/aliases.php | 29 +++ ...erPassTest.php => DbalTracingPassTest.php} | 40 +++- .../Doctrine/DBAL/TracingDriverTest.php | 201 +++++++++++++++--- 18 files changed, 525 insertions(+), 110 deletions(-) delete mode 100644 src/DependencyInjection/Compiler/DbalTracingDriverPass.php create mode 100644 src/DependencyInjection/Compiler/DbalTracingPass.php create mode 100644 src/Tracing/Doctrine/DBAL/Compatibility/ExceptionConverterDriverInterface.php create mode 100644 src/Tracing/Doctrine/DBAL/Compatibility/MiddlewareInterface.php create mode 100644 src/Tracing/Doctrine/DBAL/ConnectionConfigurator.php rename tests/DependencyInjection/Compiler/{DbalTracingDriverPassTest.php => DbalTracingPassTest.php} (64%) diff --git a/composer.json b/composer.json index 7b43aff7..e24ce085 100644 --- a/composer.json +++ b/composer.json @@ -32,8 +32,9 @@ "symfony/security-core": "^3.4.43||^4.4.11||^5.0.11" }, "require-dev": { - "doctrine/doctrine-bundle": "^2.2", - "friendsofphp/php-cs-fixer": "^2.17", + "doctrine/dbal": "^2.10||^3.0", + "doctrine/doctrine-bundle": "^1.12||^2.0", + "friendsofphp/php-cs-fixer": "^2.18", "jangregor/phpstan-prophecy": "^0.8", "monolog/monolog": "^1.3||^2.0", "phpspec/prophecy": "!=1.11.0", diff --git a/phpstan.neon b/phpstan.neon index 2fb21049..18215d3d 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -10,3 +10,4 @@ parameters: - tests/End2End/App dynamicConstantNames: - Symfony\Component\HttpKernel\Kernel::VERSION + - Doctrine\DBAL\Version::VERSION diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 65955abd..a38ae1f0 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,5 +1,5 @@ - + ConsoleListener @@ -8,4 +8,12 @@ public function __construct(HubInterface $hub) + + + getSchemaManager + + + ExceptionConverter + + diff --git a/psalm.xml b/psalm.xml index 3fc43a69..75118adf 100644 --- a/psalm.xml +++ b/psalm.xml @@ -6,6 +6,7 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="https://getpsalm.org/schema/config" xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" + errorBaseline="psalm-baseline.xml" > diff --git a/src/DependencyInjection/Compiler/DbalTracingDriverPass.php b/src/DependencyInjection/Compiler/DbalTracingDriverPass.php deleted file mode 100644 index 0f6fb52b..00000000 --- a/src/DependencyInjection/Compiler/DbalTracingDriverPass.php +++ /dev/null @@ -1,58 +0,0 @@ -hasParameter('doctrine.connections')) { - return; - } - - /** @var string[] $connections */ - $connections = $container->getParameter('doctrine.connections'); - - /** @var string[] $connectionsToTrace */ - $connectionsToTrace = $container->getParameter('sentry.tracing.dbal.connections'); - - foreach ($connectionsToTrace as $connectionName) { - if (!\in_array(sprintf('doctrine.dbal.%s_connection', $connectionName), $connections, true)) { - continue; - } - - $configurationDefinition = $container->getDefinition(sprintf('doctrine.dbal.%s_connection.configuration', $connectionName)); - $setMiddlewaresMethodCallArguments = $this->getSetMiddlewaresMethodCallArguments($configurationDefinition); - $setMiddlewaresMethodCallArguments[0] = array_merge($setMiddlewaresMethodCallArguments[0] ?? [], [new Reference(TracingDriverMiddleware::class)]); - - $configurationDefinition - ->removeMethodCall('setMiddlewares') - ->addMethodCall('setMiddlewares', $setMiddlewaresMethodCallArguments); - } - } - - /** - * @return mixed[] - */ - private function getSetMiddlewaresMethodCallArguments(Definition $definition): array - { - foreach ($definition->getMethodCalls() as $methodCall) { - if ('setMiddlewares' === $methodCall[0]) { - return $methodCall[1]; - } - } - - return []; - } -} diff --git a/src/DependencyInjection/Compiler/DbalTracingPass.php b/src/DependencyInjection/Compiler/DbalTracingPass.php new file mode 100644 index 00000000..07cbc3e6 --- /dev/null +++ b/src/DependencyInjection/Compiler/DbalTracingPass.php @@ -0,0 +1,87 @@ +hasParameter('doctrine.connections')) { + return; + } + + /** @var string[] $connections */ + $connections = $container->getParameter('doctrine.connections'); + + /** @var string[] $connectionsToTrace */ + $connectionsToTrace = $container->getParameter('sentry.tracing.dbal.connections'); + + foreach ($connectionsToTrace as $connectionName) { + if (!\in_array(sprintf(self::CONNECTION_SERVICE_NAME_FORMAT, $connectionName), $connections, true)) { + continue; + } + + if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + $this->configureConnectionForDoctrineDBALVersion3($container, $connectionName); + } else { + $this->configureConnectionForDoctrineDBALVersion2($container, $connectionName); + } + } + } + + private function configureConnectionForDoctrineDBALVersion3(ContainerBuilder $container, string $connectionName): void + { + $configurationDefinition = $container->getDefinition(sprintf(self::CONNECTION_CONFIG_SERVICE_NAME_FORMAT, $connectionName)); + $setMiddlewaresMethodCallArguments = $this->getSetMiddlewaresMethodCallArguments($configurationDefinition); + $setMiddlewaresMethodCallArguments[0] = array_merge($setMiddlewaresMethodCallArguments[0] ?? [], [new Reference(TracingDriverMiddleware::class)]); + + $configurationDefinition + ->removeMethodCall('setMiddlewares') + ->addMethodCall('setMiddlewares', $setMiddlewaresMethodCallArguments); + } + + private function configureConnectionForDoctrineDBALVersion2(ContainerBuilder $container, string $connectionName): void + { + $connectionDefinition = $container->getDefinition(sprintf(self::CONNECTION_SERVICE_NAME_FORMAT, $connectionName)); + $connectionDefinition->setConfigurator([new Reference(ConnectionConfigurator::class), 'configure']); + } + + /** + * @return mixed[] + */ + private function getSetMiddlewaresMethodCallArguments(Definition $definition): array + { + foreach ($definition->getMethodCalls() as $methodCall) { + if ('setMiddlewares' === $methodCall[0]) { + return $methodCall[1]; + } + } + + return []; + } +} diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 3e560c29..c3386855 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -154,9 +154,8 @@ private function addDistributedTracingSection(ArrayNodeDefinition $rootNode): vo ->addDefaultsIfNotSet() ->children() ->arrayNode('dbal') - ->addDefaultsIfNotSet() - ->fixXmlConfig('connection') ->canBeEnabled() + ->fixXmlConfig('connection') ->children() ->arrayNode('connections') ->defaultValue(class_exists(DoctrineBundle::class) ? ['%doctrine.default_connection%'] : []) diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index 390c31a5..903aa405 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -69,6 +69,10 @@ + + + + diff --git a/src/SentryBundle.php b/src/SentryBundle.php index 0b01bb5a..7dc981f6 100644 --- a/src/SentryBundle.php +++ b/src/SentryBundle.php @@ -4,7 +4,7 @@ namespace Sentry\SentryBundle; -use Sentry\SentryBundle\DependencyInjection\Compiler\DbalTracingDriverPass; +use Sentry\SentryBundle\DependencyInjection\Compiler\DbalTracingPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; @@ -16,6 +16,6 @@ public function build(ContainerBuilder $container): void { parent::build($container); - $container->addCompilerPass(new DbalTracingDriverPass()); + $container->addCompilerPass(new DbalTracingPass()); } } diff --git a/src/Tracing/Doctrine/DBAL/Compatibility/ExceptionConverterDriverInterface.php b/src/Tracing/Doctrine/DBAL/Compatibility/ExceptionConverterDriverInterface.php new file mode 100644 index 00000000..dde6b971 --- /dev/null +++ b/src/Tracing/Doctrine/DBAL/Compatibility/ExceptionConverterDriverInterface.php @@ -0,0 +1,12 @@ +tracingDriverMiddleware = $tracingDriverMiddleware; + } + + /** + * Configures the given connecton by wrapping its driver into an instance + * of the {@see TracingDriver} class. This is done using the reflection, + * and as such should be limited only to the versions of Doctrine DBAL that + * are lower than 3.0. Since 3.0 onwards, the concept of driver middlewares + * has been introduced which allows the same thing we're doing here, but in + * a more appropriate and "legal" way. + * + * @param Connection $connection The connection to configure + */ + public function configure(Connection $connection): void + { + $reflectionProperty = new \ReflectionProperty($connection, '_driver'); + $reflectionProperty->setAccessible(true); + $reflectionProperty->setValue($connection, $this->tracingDriverMiddleware->wrap($reflectionProperty->getValue($connection))); + $reflectionProperty->setAccessible(false); + } +} diff --git a/src/Tracing/Doctrine/DBAL/TracingDriver.php b/src/Tracing/Doctrine/DBAL/TracingDriver.php index 4fdf6dd8..eeeb3ad2 100644 --- a/src/Tracing/Doctrine/DBAL/TracingDriver.php +++ b/src/Tracing/Doctrine/DBAL/TracingDriver.php @@ -7,14 +7,23 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\Driver\API\ExceptionConverter; use Doctrine\DBAL\Driver as DriverInterface; +use Doctrine\DBAL\Driver\DriverException; +use Doctrine\DBAL\Driver\ExceptionConverterDriver as ExceptionConverterDriverInterface; +use Doctrine\DBAL\Exception\DriverException as DBALDriverException; use Doctrine\DBAL\Platforms\AbstractPlatform; +use Doctrine\DBAL\VersionAwarePlatformDriver as VersionAwarePlatformDriverInterface; +use Jean85\PrettyVersions; use Sentry\State\HubInterface; /** * This is a simple implementation of the {@see DriverInterface} interface that * decorates an existing driver to support distributed tracing capabilities. + * This implementation IS and MUST be compatible with all versions of Doctrine + * DBAL >= 2.11. + * + * @internal */ -final class TracingDriver implements DriverInterface +final class TracingDriver implements DriverInterface, VersionAwarePlatformDriverInterface, ExceptionConverterDriverInterface { /** * @var HubInterface The current hub @@ -41,11 +50,11 @@ public function __construct(HubInterface $hub, DriverInterface $decoratedDriver) /** * {@inheritdoc} */ - public function connect(array $params) + public function connect(array $params, $username = null, $password = null, array $driverOptions = []) { return new TracingDriverConnection( $this->hub, - $this->decoratedDriver->connect($params), + $this->decoratedDriver->connect($params, $username, $password, $driverOptions), $this->decoratedDriver->getDatabasePlatform()->getName(), $params ); @@ -62,7 +71,7 @@ public function getDatabasePlatform() /** * {@inheritdoc} */ - public function getSchemaManager(Connection $conn, AbstractPlatform $platform) + public function getSchemaManager(Connection $conn, ?AbstractPlatform $platform = null) { return $this->decoratedDriver->getSchemaManager($conn, $platform); } @@ -72,6 +81,62 @@ public function getSchemaManager(Connection $conn, AbstractPlatform $platform) */ public function getExceptionConverter(): ExceptionConverter { + if (!method_exists($this->decoratedDriver, 'getExceptionConverter')) { + throw new \BadMethodCallException(sprintf('The %s() method is not supported on Doctrine DBAL 2.x.', __METHOD__)); + } + return $this->decoratedDriver->getExceptionConverter(); } + + /** + * {@inheritdoc} + */ + public function getName() + { + if (method_exists($this->decoratedDriver, 'getName')) { + return $this->decoratedDriver->getName(); + } + + throw new \BadMethodCallException(sprintf('The %s() method is not supported on Doctrine DBAL 3.0.', __METHOD__)); + } + + /** + * {@inheritdoc} + */ + public function getDatabase(Connection $conn): string + { + if (method_exists($this->decoratedDriver, 'getDatabase')) { + return $this->decoratedDriver->getDatabase($conn); + } + + throw new \BadMethodCallException(sprintf('The %s() method is not supported on Doctrine DBAL 3.0.', __METHOD__)); + } + + /** + * {@inheritdoc} + */ + public function createDatabasePlatformForVersion($version): AbstractPlatform + { + if ($this->decoratedDriver instanceof VersionAwarePlatformDriver) { + return $this->decoratedDriver->createDatabasePlatformForVersion($version); + } + + return $this->getDatabasePlatform(); + } + + /** + * {@inheritdoc} + */ + public function convertException($message, DriverException $exception) + { + if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + throw new \BadMethodCallException(sprintf('The %s() method is not supported on Doctrine DBAL 3.0.', __METHOD__)); + } + + if ($this->decoratedDriver instanceof ExceptionConverterDriver) { + return $this->decoratedDriver->convertException($message, $exception); + } + + return new DBALDriverException($message, $exception); + } } diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php index e51bc463..5495f880 100644 --- a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php +++ b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php @@ -13,7 +13,7 @@ /** * This implementation wraps a driver connection and adds distributed tracing - * capabilities. + * capabilities to Doctrine DBAL. */ final class TracingDriverConnection implements DriverConnectionInterface { @@ -71,7 +71,7 @@ final class TracingDriverConnection implements DriverConnectionInterface * Constructor. * * @param HubInterface $hub The current hub - * @param DriverConnectionInterface $decoratedConnection The connection to decorarte + * @param DriverConnectionInterface $decoratedConnection The connection to decorate * @param string $databasePlatform The name of the database platform * @param array $params The connection params */ @@ -90,7 +90,7 @@ public function __construct( /** * {@inheritdoc} */ - public function prepare(string $sql): Statement + public function prepare($sql): Statement { return $this->traceFunction(self::SPAN_OP_CONN_PREPARE, $sql, function () use ($sql): Statement { return $this->decoratedConnection->prepare($sql); @@ -100,25 +100,25 @@ public function prepare(string $sql): Statement /** * {@inheritdoc} */ - public function query(string $sql): Result + public function query(?string $sql = null, ...$args): Result { - return $this->traceFunction(self::SPAN_OP_CONN_QUERY, $sql, function () use ($sql): Result { - return $this->decoratedConnection->query($sql); + return $this->traceFunction(self::SPAN_OP_CONN_QUERY, $sql, function () use ($sql, $args): Result { + return $this->decoratedConnection->query($sql, ...$args); }); } /** * {@inheritdoc} */ - public function quote($value, $type = ParameterType::STRING) + public function quote($input, $type = ParameterType::STRING) { - return $this->decoratedConnection->quote($value, $type); + return $this->decoratedConnection->quote($input, $type); } /** * {@inheritdoc} */ - public function exec(string $sql): int + public function exec($sql): int { return $this->traceFunction(self::SPAN_OP_CONN_EXEC, $sql, function () use ($sql): int { return $this->decoratedConnection->exec($sql); @@ -163,6 +163,30 @@ public function rollBack() }); } + /** + * {@inheritdoc} + */ + public function errorCode() + { + if (!method_exists($this->decoratedConnection, 'errorInfo')) { + throw new \BadMethodCallException(sprintf('The %s() method is not supported on Doctrine DBAL 3.0.', __METHOD__)); + } + + return $this->decoratedConnection->errorCode(); + } + + /** + * {@inheritdoc} + */ + public function errorInfo() + { + if (!method_exists($this->decoratedConnection, 'errorInfo')) { + throw new \BadMethodCallException(sprintf('The %s() method is not supported on Doctrine DBAL 3.0.', __METHOD__)); + } + + return $this->decoratedConnection->errorInfo(); + } + /** * @phpstan-template T * diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverMiddleware.php b/src/Tracing/Doctrine/DBAL/TracingDriverMiddleware.php index ab85dc7b..1a77e9d6 100644 --- a/src/Tracing/Doctrine/DBAL/TracingDriverMiddleware.php +++ b/src/Tracing/Doctrine/DBAL/TracingDriverMiddleware.php @@ -9,8 +9,8 @@ use Sentry\State\HubInterface; /** - * This middleware wraps a {@see Driver} instance into one that supports the - * distributed tracing feature of Sentry. + * This middleware wraps a {@see DriverInterface} instance into one that + * supports the distributed tracing feature of Sentry. */ final class TracingDriverMiddleware implements MiddlewareInterface { diff --git a/src/aliases.php b/src/aliases.php index 796ffda6..1b9e5621 100644 --- a/src/aliases.php +++ b/src/aliases.php @@ -2,10 +2,20 @@ declare(strict_types=1); +namespace Sentry\SentryBundle; + +use Doctrine\DBAL\Connection; +use Doctrine\DBAL\Driver\ExceptionConverterDriver as BaseExceptionConverterDriverInterface; +use Doctrine\DBAL\Driver\Middleware as DoctrineMiddlewareInterface; +use Doctrine\DBAL\Driver\Result; +use Doctrine\DBAL\Driver\Statement; +use Jean85\PrettyVersions; use Sentry\SentryBundle\EventListener\ErrorListenerExceptionEvent; use Sentry\SentryBundle\EventListener\RequestListenerControllerEvent; use Sentry\SentryBundle\EventListener\RequestListenerRequestEvent; use Sentry\SentryBundle\EventListener\SubRequestListenerRequestEvent; +use Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility\ExceptionConverterDriverInterface; +use Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility\MiddlewareInterface; use Symfony\Component\HttpKernel\Event\ControllerEvent; use Symfony\Component\HttpKernel\Event\ExceptionEvent; use Symfony\Component\HttpKernel\Event\FilterControllerEvent; @@ -55,3 +65,22 @@ class_alias(FilterControllerEvent::class, RequestListenerControllerEvent::class) class_alias(GetResponseEvent::class, SubRequestListenerRequestEvent::class); } } + +if (class_exists(Connection::class)) { + $doctrineVersion = PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(); + + if (version_compare($doctrineVersion, '3.0.0', '<') && !class_exists(Result::class, false)) { + /** @psalm-suppress UndefinedClass */ + class_alias(Statement::class, Result::class); + } + + if (!interface_exists(DoctrineMiddlewareInterface::class)) { + /** @psalm-suppress UndefinedClass */ + class_alias(MiddlewareInterface::class, DoctrineMiddlewareInterface::class); + } + + if (!interface_exists(BaseExceptionConverterDriverInterface::class, false)) { + /** @psalm-suppress UndefinedClass */ + class_alias(ExceptionConverterDriverInterface::class, BaseExceptionConverterDriverInterface::class); + } +} diff --git a/tests/DependencyInjection/Compiler/DbalTracingDriverPassTest.php b/tests/DependencyInjection/Compiler/DbalTracingPassTest.php similarity index 64% rename from tests/DependencyInjection/Compiler/DbalTracingDriverPassTest.php rename to tests/DependencyInjection/Compiler/DbalTracingPassTest.php index bdc4e47c..c570ef11 100644 --- a/tests/DependencyInjection/Compiler/DbalTracingDriverPassTest.php +++ b/tests/DependencyInjection/Compiler/DbalTracingPassTest.php @@ -5,16 +5,24 @@ namespace Sentry\SentryBundle\Tests\DependencyInjection\Compiler; use Doctrine\DBAL\Configuration; +use Doctrine\DBAL\Connection; +use Jean85\PrettyVersions; use PHPUnit\Framework\TestCase; -use Sentry\SentryBundle\DependencyInjection\Compiler\DbalTracingDriverPass; +use Sentry\SentryBundle\DependencyInjection\Compiler\DbalTracingPass; +use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Reference; -final class DbalTracingDriverPassTest extends TestCase +final class DbalTracingPassTest extends TestCase { - public function testProcess(): void + public function testProcessWithDoctrineDBALVersionAtLeast30(): void { + if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '<')) { + $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); + } + $container = $this->createContainerBuilder(); $container->setParameter('doctrine.connections', ['doctrine.dbal.foo_connection', 'doctrine.dbal.bar_connection', 'doctrine.dbal.baz_connection']); $container->setParameter('sentry.tracing.dbal.connections', ['foo', 'bar', 'baz', 'qux']); @@ -75,6 +83,26 @@ public function testProcess(): void ); } + public function testProcessWithDoctrineDBALVersionLowerThan30(): void + { + if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.'); + } + + $connection1 = (new Definition(Connection::class))->setPublic(true); + $connection2 = (new Definition(Connection::class))->setPublic(true); + + $container = $this->createContainerBuilder(); + $container->setParameter('doctrine.connections', ['doctrine.dbal.foo_connection', 'doctrine.dbal.bar_connection']); + $container->setParameter('sentry.tracing.dbal.connections', ['foo', 'baz']); + $container->setDefinition('doctrine.dbal.foo_connection', $connection1); + $container->setDefinition('doctrine.dbal.bar_connection', $connection2); + $container->compile(); + + $this->assertEquals([new Reference(ConnectionConfigurator::class), 'configure'], $connection1->getConfigurator()); + $this->assertNull($connection2->getConfigurator()); + } + public function testProcessDoesNothingIfDoctrineConnectionsParamIsMissing(): void { $container = $this->createContainerBuilder(); @@ -92,12 +120,16 @@ public function testProcessDoesNothingIfDoctrineConnectionsParamIsMissing(): voi private function createContainerBuilder(): ContainerBuilder { $container = new ContainerBuilder(); - $container->addCompilerPass(new DbalTracingDriverPass()); + $container->addCompilerPass(new DbalTracingPass()); $container ->register(TracingDriverMiddleware::class, TracingDriverMiddleware::class) ->setPublic(true); + $container + ->register(ConnectionConfigurator::class, ConnectionConfigurator::class) + ->setPublic(true); + return $container; } } diff --git a/tests/Tracing/Doctrine/DBAL/TracingDriverTest.php b/tests/Tracing/Doctrine/DBAL/TracingDriverTest.php index fd0acd9f..af7261ff 100644 --- a/tests/Tracing/Doctrine/DBAL/TracingDriverTest.php +++ b/tests/Tracing/Doctrine/DBAL/TracingDriverTest.php @@ -5,11 +5,16 @@ namespace Sentry\SentryBundle\Tests\Tracing\Doctrine\DBAL; use Doctrine\DBAL\Connection; -use Doctrine\DBAL\Driver\API\ExceptionConverter as ExceptionConverterInterface; +use Doctrine\DBAL\Driver\API\ExceptionConverter; use Doctrine\DBAL\Driver as DriverInterface; use Doctrine\DBAL\Driver\Connection as DriverConnectionInterface; +use Doctrine\DBAL\Driver\DriverException; +use Doctrine\DBAL\Driver\ExceptionConverterDriver; +use Doctrine\DBAL\Exception\DriverException as DBALDriverException; use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Schema\AbstractSchemaManager; +use Doctrine\DBAL\VersionAwarePlatformDriver; +use Jean85\PrettyVersions; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriver; @@ -23,53 +28,45 @@ final class TracingDriverTest extends TestCase */ private $hub; - /** - * @var DriverInterface&MockObject - */ - private $decoratedDriver; - - /** - * @var TracingDriver - */ - private $driver; - protected function setUp(): void { $this->hub = $this->createMock(HubInterface::class); - $this->decoratedDriver = $this->createMock(DriverInterface::class); - $this->driver = new TracingDriver($this->hub, $this->decoratedDriver); } public function testConnect(): void { - $params = ['foo' => 'bar']; - $databasePlatform = $this->createMock(AbstractPlatform::class); $databasePlatform->expects($this->once()) ->method('getName') ->willReturn('foo'); - $this->decoratedDriver->expects($this->once()) + $decoratedDriver = $this->createMock(DriverInterface::class); + $decoratedDriver->expects($this->once()) ->method('connect') - ->with($params) + ->with(['host' => 'localhost'], 'username', 'password', ['foo' => 'bar']) ->willReturn($this->createMock(DriverConnectionInterface::class)); - $this->decoratedDriver->expects($this->once()) + $decoratedDriver->expects($this->once()) ->method('getDatabasePlatform') ->willReturn($databasePlatform); - $this->assertInstanceOf(TracingDriverConnection::class, $this->driver->connect($params)); + $driver = new TracingDriver($this->hub, $decoratedDriver); + + $this->assertInstanceOf(TracingDriverConnection::class, $driver->connect(['host' => 'localhost'], 'username', 'password', ['foo' => 'bar'])); } public function testGetDatabasePlatform(): void { $databasePlatform = $this->createMock(AbstractPlatform::class); - $this->decoratedDriver->expects($this->once()) + $decoratedDriver = $this->createMock(DriverInterface::class); + $decoratedDriver->expects($this->once()) ->method('getDatabasePlatform') ->willReturn($databasePlatform); - $this->assertSame($databasePlatform, $this->driver->getDatabasePlatform()); + $driver = new TracingDriver($this->hub, $decoratedDriver); + + $this->assertSame($databasePlatform, $driver->getDatabasePlatform()); } public function testGetSchemaManager(): void @@ -78,22 +75,174 @@ public function testGetSchemaManager(): void $databasePlatform = $this->createMock(AbstractPlatform::class); $schemaManager = $this->createMock(AbstractSchemaManager::class); - $this->decoratedDriver->expects($this->once()) + $decoratedDriver = $this->createMock(DriverInterface::class); + $decoratedDriver->expects($this->once()) ->method('getSchemaManager') ->with($connection, $databasePlatform) ->willReturn($schemaManager); - $this->assertSame($schemaManager, $this->driver->getSchemaManager($connection, $databasePlatform)); + $driver = new TracingDriver($this->hub, $decoratedDriver); + + $this->assertSame($schemaManager, $driver->getSchemaManager($connection, $databasePlatform)); } public function testGetExceptionConverter(): void { - $exceptionConverter = $this->createMock(ExceptionConverterInterface::class); + if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '<')) { + $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); + } - $this->decoratedDriver->expects($this->once()) + $exceptionConverter = $this->createMock(ExceptionConverter::class); + + $decoratedDriver = $this->createMock(DriverInterface::class); + $decoratedDriver->expects($this->once()) ->method('getExceptionConverter') ->willReturn($exceptionConverter); - $this->assertSame($exceptionConverter, $this->driver->getExceptionConverter()); + $driver = new TracingDriver($this->hub, $decoratedDriver); + + $this->assertSame($exceptionConverter, $driver->getExceptionConverter()); + } + + public function testGetExceptionConverterThrowsIfDoctrineDBALVersionIsLowerThan30(): void + { + if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.'); + } + + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('The Sentry\\SentryBundle\\Tracing\\Doctrine\\DBAL\\TracingDriver::getExceptionConverter() method is not supported on Doctrine DBAL 2.x.'); + + $decoratedDriver = $this->createMock(DriverInterface::class); + $driver = new TracingDriver($this->hub, $decoratedDriver); + + $driver->getExceptionConverter(); + } + + public function testGetNameIfDoctrineDBALVersionIsLowerThan30(): void + { + if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.'); + } + + $decoratedDriver = $this->createMock(DriverInterface::class); + $decoratedDriver->expects($this->once()) + ->method('getName') + ->willReturn('foo'); + + $driver = new TracingDriver($this->hub, $decoratedDriver); + + $this->assertSame('foo', $driver->getName()); + } + + public function testGetNameThrowsIfDoctrineDBALVersionIsAtLeast30(): void + { + if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '<')) { + $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); + } + + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('The Sentry\\SentryBundle\\Tracing\\Doctrine\\DBAL\\TracingDriver::getName() method is not supported on Doctrine DBAL 3.0.'); } + + public function testGetDatabaseIfDoctrineDBALVersionIsLowerThan30(): void + { + if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.'); + } + + $connection = $this->createMock(Connection::class); + + $decoratedDriver = $this->createMock(DriverInterface::class); + $decoratedDriver->expects($this->once()) + ->method('getDatabase') + ->with($connection) + ->willReturn('foo'); + + $driver = new TracingDriver($this->hub, $decoratedDriver); + + $this->assertSame('foo', $driver->getDatabase($connection)); + } + + public function testGetDatabaseThrowsIfDoctrineDBALVersionIsAtLeast30(): void + { + if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '<')) { + $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); + } + + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('The Sentry\\SentryBundle\\Tracing\\Doctrine\\DBAL\\TracingDriver::getDatabase() method is not supported on Doctrine DBAL 3.0.'); + + $driver = new TracingDriver($this->hub, $this->createMock(DriverInterface::class)); + $driver->getDatabase($this->createMock(Connection::class)); + } + + public function testCreateDatabasePlatformForVersion(): void + { + $databasePlatform = $this->createMock(AbstractPlatform::class); + + $decoratedDriver = $this->createMock(VersionAwarePlatformDriverInterface::class); + $decoratedDriver->expects($this->once()) + ->method('createDatabasePlatformForVersion') + ->with('5.7') + ->willReturn($databasePlatform); + + $driver = new TracingDriver($this->hub, $decoratedDriver); + + $this->assertSame($databasePlatform, $driver->createDatabasePlatformForVersion('5.7')); + } + + public function testCreateDatabasePlatformForVersionWhenDriverDoesNotImplementInterface(): void + { + $databasePlatform = $this->createMock(AbstractPlatform::class); + + $decoratedDriver = $this->createMock(DriverInterface::class); + $decoratedDriver->expects($this->once()) + ->method('getDatabasePlatform') + ->willReturn($databasePlatform); + + $driver = new TracingDriver($this->hub, $decoratedDriver); + + $this->assertSame($databasePlatform, $driver->createDatabasePlatformForVersion('5.7')); + } + + public function testConvertException(): void + { + if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be <= 3.0.'); + } + + $exception = $this->createMock(DriverException::class); + $convertedException = new DBALDriverException('foo', $exception); + + $decoratedDriver = $this->createMock(ExceptionConverterDriverInterface::class); + $decoratedDriver->expects($this->once()) + ->method('convertException') + ->willReturn($convertedException); + + $driver = new TracingDriver($this->hub, $decoratedDriver); + + $this->assertSame($convertedException, $driver->convertException('foo', $this->createMock(DriverException::class))); + } + + public function testConvertExceptionThrowsIfDoctrineDBALVersionIsAtLeast30(): void + { + if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '<')) { + $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); + } + + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessage('The Sentry\\SentryBundle\\Tracing\\Doctrine\\DBAL\\TracingDriver::convertException() method is not supported on Doctrine DBAL 3.0.'); + + $driver = new TracingDriver($this->hub, $this->createMock(ExceptionConverterDriverInterface::class)); + $driver->convertException('foo', $this->createMock(DriverException::class)); + } +} + +interface ExceptionConverterDriverInterface extends DriverInterface, ExceptionConverterDriver +{ +} + +interface VersionAwarePlatformDriverInterface extends DriverInterface, VersionAwarePlatformDriver +{ } From e5f69b8627bbb7e6275a44fc1b20d267ca6e8ba7 Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Thu, 11 Feb 2021 01:33:27 +0100 Subject: [PATCH 11/16] Fix CR issues --- phpstan-baseline.neon | 95 +++++++++++++++++++ psalm.xml | 1 - .../Compiler/DbalTracingPass.php | 4 +- .../ExceptionConverterDriverInterface.php | 4 +- src/Tracing/Doctrine/DBAL/TracingDriver.php | 16 ++-- .../Doctrine/DBAL/TracingDriverConnection.php | 4 +- src/aliases.php | 18 ++-- .../Compiler/DbalTracingPassTest.php | 9 +- tests/DoctrineTestCase.php | 16 ++++ .../Doctrine/DBAL/TracingDriverTest.php | 33 ++++--- 10 files changed, 159 insertions(+), 41 deletions(-) create mode 100644 tests/DoctrineTestCase.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index f25218ff..808865e8 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -10,11 +10,81 @@ parameters: count: 1 path: src/DependencyInjection/Configuration.php + - + message: "#^Call to an undefined method Symfony\\\\Component\\\\Config\\\\Definition\\\\Builder\\\\NodeParentInterface\\:\\:end\\(\\)\\.$#" + count: 1 + path: src/DependencyInjection/Configuration.php + - message: "#^Else branch is unreachable because previous condition is always true\\.$#" count: 1 path: src/EventListener/ErrorListener.php + - + message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriver\\:\\:connect\\(\\) has parameter \\$driverOptions with no value type specified in iterable type array\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriver.php + + - + message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriver\\:\\:connect\\(\\) has parameter \\$password with no typehint specified\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriver.php + + - + message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriver\\:\\:connect\\(\\) has parameter \\$username with no typehint specified\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriver.php + + - + message: "#^Method Doctrine\\\\DBAL\\\\Driver\\:\\:connect\\(\\) invoked with 4 parameters, 1 required\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriver.php + + - + message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriver\\:\\:convertException\\(\\) has parameter \\$message with no typehint specified\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriver.php + + - + message: "#^Call to an undefined method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\Compatibility\\\\ExceptionConverterDriverInterface\\:\\:convertException\\(\\)\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriver.php + + - + message: "#^Parameter \\#2 \\$query of class Doctrine\\\\DBAL\\\\Exception\\\\DriverException constructor expects Doctrine\\\\DBAL\\\\Query\\|null, Doctrine\\\\DBAL\\\\Driver\\\\Exception given\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriver.php + + - + message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriverConnection\\:\\:prepare\\(\\) has parameter \\$sql with no typehint specified\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriverConnection.php + + - + message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriverConnection\\:\\:query\\(\\) has parameter \\$args with no typehint specified\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriverConnection.php + + - + message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriverConnection\\:\\:exec\\(\\) has parameter \\$sql with no typehint specified\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriverConnection.php + + - + message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriverConnection\\:\\:errorCode\\(\\) has no return typehint specified\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriverConnection.php + + - + message: "#^Call to an undefined method Doctrine\\\\DBAL\\\\Driver\\\\Connection\\:\\:errorCode\\(\\)\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriverConnection.php + + - + message: "#^Method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriverConnection\\:\\:errorInfo\\(\\) has no return typehint specified\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriverConnection.php + - message: "#^Class Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\GetResponseForExceptionEvent not found\\.$#" count: 1 @@ -115,3 +185,28 @@ parameters: count: 2 path: tests/EventListener/SubRequestListenerTest.php + - + message: "#^Trying to mock an undefined method getName\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\.$#" + count: 1 + path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php + + - + message: "#^Trying to mock an undefined method getDatabase\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\.$#" + count: 1 + path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php + + - + message: "#^Parameter \\#1 \\$driverException of class Doctrine\\\\DBAL\\\\Exception\\\\DriverException constructor expects Doctrine\\\\DBAL\\\\Driver\\\\Exception, string given\\.$#" + count: 1 + path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php + + - + message: "#^Parameter \\#2 \\$query of class Doctrine\\\\DBAL\\\\Exception\\\\DriverException constructor expects Doctrine\\\\DBAL\\\\Query\\|null, Doctrine\\\\DBAL\\\\Driver\\\\Exception&PHPUnit\\\\Framework\\\\MockObject\\\\MockObject given\\.$#" + count: 1 + path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php + + - + message: "#^Trying to mock an undefined method convertException\\(\\) on class Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Doctrine\\\\DBAL\\\\ExceptionConverterDriverInterface\\.$#" + count: 1 + path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php + diff --git a/psalm.xml b/psalm.xml index 75118adf..3fc43a69 100644 --- a/psalm.xml +++ b/psalm.xml @@ -6,7 +6,6 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="https://getpsalm.org/schema/config" xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" - errorBaseline="psalm-baseline.xml" > diff --git a/src/DependencyInjection/Compiler/DbalTracingPass.php b/src/DependencyInjection/Compiler/DbalTracingPass.php index 07cbc3e6..91d4c7fe 100644 --- a/src/DependencyInjection/Compiler/DbalTracingPass.php +++ b/src/DependencyInjection/Compiler/DbalTracingPass.php @@ -4,7 +4,7 @@ namespace Sentry\SentryBundle\DependencyInjection\Compiler; -use Jean85\PrettyVersions; +use Doctrine\DBAL\Driver\ResultStatement; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; @@ -46,7 +46,7 @@ public function process(ContainerBuilder $container): void continue; } - if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + if (!interface_exists(ResultStatement::class)) { $this->configureConnectionForDoctrineDBALVersion3($container, $connectionName); } else { $this->configureConnectionForDoctrineDBALVersion2($container, $connectionName); diff --git a/src/Tracing/Doctrine/DBAL/Compatibility/ExceptionConverterDriverInterface.php b/src/Tracing/Doctrine/DBAL/Compatibility/ExceptionConverterDriverInterface.php index dde6b971..1b5b1e3f 100644 --- a/src/Tracing/Doctrine/DBAL/Compatibility/ExceptionConverterDriverInterface.php +++ b/src/Tracing/Doctrine/DBAL/Compatibility/ExceptionConverterDriverInterface.php @@ -4,9 +4,11 @@ namespace Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility; +use Doctrine\DBAL\Driver as DriverInterface; + /** * @internal */ -interface ExceptionConverterDriverInterface +interface ExceptionConverterDriverInterface extends DriverInterface { } diff --git a/src/Tracing/Doctrine/DBAL/TracingDriver.php b/src/Tracing/Doctrine/DBAL/TracingDriver.php index eeeb3ad2..5491d843 100644 --- a/src/Tracing/Doctrine/DBAL/TracingDriver.php +++ b/src/Tracing/Doctrine/DBAL/TracingDriver.php @@ -7,12 +7,12 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\Driver\API\ExceptionConverter; use Doctrine\DBAL\Driver as DriverInterface; -use Doctrine\DBAL\Driver\DriverException; +use Doctrine\DBAL\Driver\DriverException as LegacyDriverExceptionInterface; use Doctrine\DBAL\Driver\ExceptionConverterDriver as ExceptionConverterDriverInterface; +use Doctrine\DBAL\Driver\ResultStatement; use Doctrine\DBAL\Exception\DriverException as DBALDriverException; use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\VersionAwarePlatformDriver as VersionAwarePlatformDriverInterface; -use Jean85\PrettyVersions; use Sentry\State\HubInterface; /** @@ -31,7 +31,7 @@ final class TracingDriver implements DriverInterface, VersionAwarePlatformDriver private $hub; /** - * @var DriverInterface The instance of the decorated driver + * @var DriverInterface|VersionAwarePlatformDriverInterface|ExceptionConverterDriverInterface The instance of the decorated driver */ private $decoratedDriver; @@ -91,7 +91,7 @@ public function getExceptionConverter(): ExceptionConverter /** * {@inheritdoc} */ - public function getName() + public function getName(): string { if (method_exists($this->decoratedDriver, 'getName')) { return $this->decoratedDriver->getName(); @@ -117,7 +117,7 @@ public function getDatabase(Connection $conn): string */ public function createDatabasePlatformForVersion($version): AbstractPlatform { - if ($this->decoratedDriver instanceof VersionAwarePlatformDriver) { + if ($this->decoratedDriver instanceof VersionAwarePlatformDriverInterface) { return $this->decoratedDriver->createDatabasePlatformForVersion($version); } @@ -127,13 +127,13 @@ public function createDatabasePlatformForVersion($version): AbstractPlatform /** * {@inheritdoc} */ - public function convertException($message, DriverException $exception) + public function convertException($message, LegacyDriverExceptionInterface $exception): DBALDriverException { - if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + if (!interface_exists(ResultStatement::class)) { throw new \BadMethodCallException(sprintf('The %s() method is not supported on Doctrine DBAL 3.0.', __METHOD__)); } - if ($this->decoratedDriver instanceof ExceptionConverterDriver) { + if ($this->decoratedDriver instanceof ExceptionConverterDriverInterface) { return $this->decoratedDriver->convertException($message, $exception); } diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php index 5495f880..551b6773 100644 --- a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php +++ b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php @@ -110,9 +110,9 @@ public function query(?string $sql = null, ...$args): Result /** * {@inheritdoc} */ - public function quote($input, $type = ParameterType::STRING) + public function quote($value, $type = ParameterType::STRING) { - return $this->decoratedConnection->quote($input, $type); + return $this->decoratedConnection->quote($value, $type); } /** diff --git a/src/aliases.php b/src/aliases.php index 1b9e5621..cc7675e7 100644 --- a/src/aliases.php +++ b/src/aliases.php @@ -5,11 +5,12 @@ namespace Sentry\SentryBundle; use Doctrine\DBAL\Connection; -use Doctrine\DBAL\Driver\ExceptionConverterDriver as BaseExceptionConverterDriverInterface; +use Doctrine\DBAL\Driver\DriverException as LegacyDriverExceptionInterface; +use Doctrine\DBAL\Driver\Exception as DriverExceptionInterface; +use Doctrine\DBAL\Driver\ExceptionConverterDriver as LegacyExceptionConverterDriverInterface; use Doctrine\DBAL\Driver\Middleware as DoctrineMiddlewareInterface; use Doctrine\DBAL\Driver\Result; use Doctrine\DBAL\Driver\Statement; -use Jean85\PrettyVersions; use Sentry\SentryBundle\EventListener\ErrorListenerExceptionEvent; use Sentry\SentryBundle\EventListener\RequestListenerControllerEvent; use Sentry\SentryBundle\EventListener\RequestListenerRequestEvent; @@ -67,9 +68,7 @@ class_alias(GetResponseEvent::class, SubRequestListenerRequestEvent::class); } if (class_exists(Connection::class)) { - $doctrineVersion = PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(); - - if (version_compare($doctrineVersion, '3.0.0', '<') && !class_exists(Result::class, false)) { + if (!interface_exists(Result::class)) { /** @psalm-suppress UndefinedClass */ class_alias(Statement::class, Result::class); } @@ -79,8 +78,13 @@ class_alias(Statement::class, Result::class); class_alias(MiddlewareInterface::class, DoctrineMiddlewareInterface::class); } - if (!interface_exists(BaseExceptionConverterDriverInterface::class, false)) { + if (!interface_exists(LegacyExceptionConverterDriverInterface::class)) { + /** @psalm-suppress UndefinedClass */ + class_alias(ExceptionConverterDriverInterface::class, LegacyExceptionConverterDriverInterface::class); + } + + if (!interface_exists(LegacyDriverExceptionInterface::class)) { /** @psalm-suppress UndefinedClass */ - class_alias(ExceptionConverterDriverInterface::class, BaseExceptionConverterDriverInterface::class); + class_alias(DriverExceptionInterface::class, LegacyDriverExceptionInterface::class); } } diff --git a/tests/DependencyInjection/Compiler/DbalTracingPassTest.php b/tests/DependencyInjection/Compiler/DbalTracingPassTest.php index c570ef11..4bbffc66 100644 --- a/tests/DependencyInjection/Compiler/DbalTracingPassTest.php +++ b/tests/DependencyInjection/Compiler/DbalTracingPassTest.php @@ -6,20 +6,19 @@ use Doctrine\DBAL\Configuration; use Doctrine\DBAL\Connection; -use Jean85\PrettyVersions; -use PHPUnit\Framework\TestCase; use Sentry\SentryBundle\DependencyInjection\Compiler\DbalTracingPass; +use Sentry\SentryBundle\Tests\DoctrineTestCase; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Reference; -final class DbalTracingPassTest extends TestCase +final class DbalTracingPassTest extends DoctrineTestCase { public function testProcessWithDoctrineDBALVersionAtLeast30(): void { - if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '<')) { + if (!$this->isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); } @@ -85,7 +84,7 @@ public function testProcessWithDoctrineDBALVersionAtLeast30(): void public function testProcessWithDoctrineDBALVersionLowerThan30(): void { - if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + if ($this->isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.'); } diff --git a/tests/DoctrineTestCase.php b/tests/DoctrineTestCase.php new file mode 100644 index 00000000..0f0e7c50 --- /dev/null +++ b/tests/DoctrineTestCase.php @@ -0,0 +1,16 @@ +getPrettyVersion(), '3.0.0', '<')) { + if (!$this->isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); } @@ -106,7 +105,7 @@ public function testGetExceptionConverter(): void public function testGetExceptionConverterThrowsIfDoctrineDBALVersionIsLowerThan30(): void { - if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + if ($this->isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.'); } @@ -121,7 +120,7 @@ public function testGetExceptionConverterThrowsIfDoctrineDBALVersionIsLowerThan3 public function testGetNameIfDoctrineDBALVersionIsLowerThan30(): void { - if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + if ($this->isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.'); } @@ -137,17 +136,20 @@ public function testGetNameIfDoctrineDBALVersionIsLowerThan30(): void public function testGetNameThrowsIfDoctrineDBALVersionIsAtLeast30(): void { - if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '<')) { + if (!$this->isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); } $this->expectException(\BadMethodCallException::class); $this->expectExceptionMessage('The Sentry\\SentryBundle\\Tracing\\Doctrine\\DBAL\\TracingDriver::getName() method is not supported on Doctrine DBAL 3.0.'); + + $driver = new TracingDriver($this->hub, $this->createMock(DriverInterface::class)); + $driver->getName(); } public function testGetDatabaseIfDoctrineDBALVersionIsLowerThan30(): void { - if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + if ($this->isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.'); } @@ -166,7 +168,7 @@ public function testGetDatabaseIfDoctrineDBALVersionIsLowerThan30(): void public function testGetDatabaseThrowsIfDoctrineDBALVersionIsAtLeast30(): void { - if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '<')) { + if (!$this->isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); } @@ -208,26 +210,27 @@ public function testCreateDatabasePlatformForVersionWhenDriverDoesNotImplementIn public function testConvertException(): void { - if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '>=')) { + if ($this->isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be <= 3.0.'); } - $exception = $this->createMock(DriverException::class); + $exception = $this->createMock(LegacyDriverExceptionInterface::class); $convertedException = new DBALDriverException('foo', $exception); $decoratedDriver = $this->createMock(ExceptionConverterDriverInterface::class); $decoratedDriver->expects($this->once()) ->method('convertException') + ->with('foo', $exception) ->willReturn($convertedException); $driver = new TracingDriver($this->hub, $decoratedDriver); - $this->assertSame($convertedException, $driver->convertException('foo', $this->createMock(DriverException::class))); + $this->assertSame($convertedException, $driver->convertException('foo', $exception)); } public function testConvertExceptionThrowsIfDoctrineDBALVersionIsAtLeast30(): void { - if (version_compare(PrettyVersions::getVersion('doctrine/dbal')->getPrettyVersion(), '3.0.0', '<')) { + if (!$this->isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); } @@ -235,7 +238,7 @@ public function testConvertExceptionThrowsIfDoctrineDBALVersionIsAtLeast30(): vo $this->expectExceptionMessage('The Sentry\\SentryBundle\\Tracing\\Doctrine\\DBAL\\TracingDriver::convertException() method is not supported on Doctrine DBAL 3.0.'); $driver = new TracingDriver($this->hub, $this->createMock(ExceptionConverterDriverInterface::class)); - $driver->convertException('foo', $this->createMock(DriverException::class)); + $driver->convertException('foo', $this->createMock(LegacyDriverExceptionInterface::class)); } } From b74f206eaa9abe008db9d1eed6ef836b5e2a3989 Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Thu, 11 Feb 2021 01:33:27 +0100 Subject: [PATCH 12/16] Minor changes --- src/Tracing/Doctrine/DBAL/TracingDriver.php | 6 +++--- .../Doctrine/DBAL/TracingDriverConnection.php | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Tracing/Doctrine/DBAL/TracingDriver.php b/src/Tracing/Doctrine/DBAL/TracingDriver.php index 5491d843..b7c7b898 100644 --- a/src/Tracing/Doctrine/DBAL/TracingDriver.php +++ b/src/Tracing/Doctrine/DBAL/TracingDriver.php @@ -81,11 +81,11 @@ public function getSchemaManager(Connection $conn, ?AbstractPlatform $platform = */ public function getExceptionConverter(): ExceptionConverter { - if (!method_exists($this->decoratedDriver, 'getExceptionConverter')) { - throw new \BadMethodCallException(sprintf('The %s() method is not supported on Doctrine DBAL 2.x.', __METHOD__)); + if (method_exists($this->decoratedDriver, 'getExceptionConverter')) { + return $this->decoratedDriver->getExceptionConverter(); } - return $this->decoratedDriver->getExceptionConverter(); + throw new \BadMethodCallException(sprintf('The %s() method is not supported on Doctrine DBAL 2.x.', __METHOD__)); } /** diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php index 551b6773..70a834ac 100644 --- a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php +++ b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php @@ -168,11 +168,11 @@ public function rollBack() */ public function errorCode() { - if (!method_exists($this->decoratedConnection, 'errorInfo')) { - throw new \BadMethodCallException(sprintf('The %s() method is not supported on Doctrine DBAL 3.0.', __METHOD__)); + if (method_exists($this->decoratedConnection, 'errorInfo')) { + return $this->decoratedConnection->errorCode(); } - return $this->decoratedConnection->errorCode(); + throw new \BadMethodCallException(sprintf('The %s() method is not supported on Doctrine DBAL 3.0.', __METHOD__)); } /** @@ -180,11 +180,11 @@ public function errorCode() */ public function errorInfo() { - if (!method_exists($this->decoratedConnection, 'errorInfo')) { - throw new \BadMethodCallException(sprintf('The %s() method is not supported on Doctrine DBAL 3.0.', __METHOD__)); + if (method_exists($this->decoratedConnection, 'errorInfo')) { + return $this->decoratedConnection->errorInfo(); } - return $this->decoratedConnection->errorInfo(); + throw new \BadMethodCallException(sprintf('The %s() method is not supported on Doctrine DBAL 3.0.', __METHOD__)); } /** From bcab5d95debcb0dc494c51c5707703a7ac0cea3f Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Fri, 12 Feb 2021 00:14:25 +0100 Subject: [PATCH 13/16] Fix span names for SQL transaction commit and rollback --- src/Tracing/Doctrine/DBAL/TracingDriverConnection.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php index 70a834ac..e4d1277d 100644 --- a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php +++ b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php @@ -40,12 +40,12 @@ final class TracingDriverConnection implements DriverConnectionInterface /** * @internal */ - public const SPAN_OP_TRANSACTION_COMMIT = 'sql.begin_transaction.commit'; + public const SPAN_OP_TRANSACTION_COMMIT = 'sql.transaction.commit'; /** * @internal */ - public const SPAN_OP_TRANSACTION_ROLLBACK = 'sql.begin_transaction.rollback'; + public const SPAN_OP_TRANSACTION_ROLLBACK = 'sql.transaciton.rollback'; /** * @var HubInterface The current hub From 6d505b397482461ad087806d74fabf950463bc62 Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Fri, 12 Feb 2021 20:45:02 +0100 Subject: [PATCH 14/16] Fix CR issues --- .github/workflows/static-analysis.yaml | 2 - .github/workflows/tests.yaml | 56 +++++++++++++++++++ composer.json | 2 +- phpstan-baseline.neon | 14 ++++- src/DependencyInjection/SentryExtension.php | 2 + .../DBAL/Compatibility/DriverInterface.php | 12 ++++ .../ExceptionConverterDriverInterface.php | 4 +- src/Tracing/Doctrine/DBAL/TracingDriver.php | 2 +- .../Doctrine/DBAL/TracingDriverConnection.php | 5 +- src/aliases.php | 40 +++++++------ .../Compiler/DbalTracingPassTest.php | 4 +- .../DependencyInjection/ConfigurationTest.php | 3 +- ..._disabled.php => dbal_tracing_enabled.php} | 2 +- .../DependencyInjection/Fixtures/php/full.php | 3 +- ..._disabled.xml => dbal_tracing_enabled.xml} | 2 +- .../DependencyInjection/Fixtures/xml/full.xml | 4 +- ..._disabled.yml => dbal_tracing_enabled.yml} | 2 +- .../DependencyInjection/Fixtures/yml/full.yml | 4 +- .../SentryExtensionTest.php | 13 ++++- tests/DoctrineTestCase.php | 8 ++- .../DBAL/TracingDriverConnectionTest.php | 11 +++- .../DBAL/TracingDriverMiddlewareTest.php | 11 +++- .../Doctrine/DBAL/TracingDriverTest.php | 55 +++++++++++------- 23 files changed, 196 insertions(+), 65 deletions(-) create mode 100644 src/Tracing/Doctrine/DBAL/Compatibility/DriverInterface.php rename tests/DependencyInjection/Fixtures/php/{dbal_tracing_disabled.php => dbal_tracing_enabled.php} (89%) rename tests/DependencyInjection/Fixtures/xml/{dbal_tracing_disabled.xml => dbal_tracing_enabled.xml} (94%) rename tests/DependencyInjection/Fixtures/yml/{dbal_tracing_disabled.yml => dbal_tracing_enabled.yml} (76%) diff --git a/.github/workflows/static-analysis.yaml b/.github/workflows/static-analysis.yaml index 423cfa70..cd1a1551 100644 --- a/.github/workflows/static-analysis.yaml +++ b/.github/workflows/static-analysis.yaml @@ -17,8 +17,6 @@ jobs: - name: Setup PHP uses: shivammathur/setup-php@v2 - with: - php-version: '7.4' - name: Install dependencies run: composer update --no-progress --no-interaction --prefer-dist diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 7b78eb5a..d8fa8d6a 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -65,3 +65,59 @@ jobs: with: file: './coverage.xml' fail_ci_if_error: true + + missing-optional-packages-tests: + name: Tests without optional packages + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + php: + - '7.2' + - '8.0' + dependencies: + - lowest + - highest + + steps: + - name: Checkout + uses: actions/checkout@v2 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + coverage: xdebug + + - name: Setup Problem Matchers for PHPUnit + run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" + + - name: Determine Composer cache directory + id: composer-cache + run: echo "::set-output name=directory::$(composer config cache-dir)" + + - name: Cache Composer dependencies + uses: actions/cache@v2 + with: + path: ${{ steps.composer-cache.outputs.directory }} + key: ${{ runner.os }}-${{ matrix.php }}-composer-${{ matrix.dependencies }}-${{ hashFiles('**/composer.lock') }} + restore-keys: ${{ runner.os }}-${{ matrix.php }}-composer-${{ matrix.dependencies }}- + + - name: Remove optional packages + run: composer remove doctrine/dbal doctrine/doctrine-bundle symfony/messenger --dev --no-update + + - name: Install highest dependencies + run: composer update --no-progress --no-interaction --prefer-dist + if: ${{ matrix.dependencies == 'highest' }} + + - name: Install lowest dependencies + run: composer update --no-progress --no-interaction --prefer-dist --prefer-lowest + if: ${{ matrix.dependencies == 'lowest' }} + + - name: Run tests + run: vendor/bin/phpunit --coverage-clover=build/coverage-report.xml + + - name: Upload code coverage + uses: codecov/codecov-action@v1 + with: + file: build/coverage-report.xml diff --git a/composer.json b/composer.json index e24ce085..4550059e 100644 --- a/composer.json +++ b/composer.json @@ -54,7 +54,7 @@ }, "suggest": { "monolog/monolog": "Allow sending log messages to Sentry by using the included Monolog handler.", - "doctrine/dbal": "Allow distributed tracing of SQL queries using Sentry." + "doctrine/doctrine-bundle": "Allow distributed tracing of database queries using Sentry." }, "autoload": { "files": [ diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 808865e8..2552a4ad 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -36,7 +36,17 @@ parameters: path: src/Tracing/Doctrine/DBAL/TracingDriver.php - - message: "#^Method Doctrine\\\\DBAL\\\\Driver\\:\\:connect\\(\\) invoked with 4 parameters, 1 required\\.$#" + message: "#^Call to an undefined method Doctrine\\\\DBAL\\\\Driver\\|Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\Compatibility\\\\ExceptionConverterDriverInterface\\:\\:connect\\(\\)\\.$#" + count: 1 + path: src/Tracing/Doctrine/DBAL/TracingDriver.php + + - + message: "#^Call to an undefined method Doctrine\\\\DBAL\\\\Driver\\|Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\Compatibility\\\\ExceptionConverterDriverInterface\\:\\:getDatabasePlatform\\(\\)\\.$#" + count: 2 + path: src/Tracing/Doctrine/DBAL/TracingDriver.php + + - + message: "#^Call to an undefined method Doctrine\\\\DBAL\\\\Driver\\|Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\Compatibility\\\\ExceptionConverterDriverInterface\\:\\:getSchemaManager\\(\\)\\.$#" count: 1 path: src/Tracing/Doctrine/DBAL/TracingDriver.php @@ -206,7 +216,7 @@ parameters: path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php - - message: "#^Trying to mock an undefined method convertException\\(\\) on class Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Doctrine\\\\DBAL\\\\ExceptionConverterDriverInterface\\.$#" + message: "#^Trying to mock an undefined method convertException\\(\\) on class Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Doctrine\\\\DBAL\\\\StubExceptionConverterDriverInterface\\.$#" count: 1 path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php diff --git a/src/DependencyInjection/SentryExtension.php b/src/DependencyInjection/SentryExtension.php index a8073db0..03b99de3 100644 --- a/src/DependencyInjection/SentryExtension.php +++ b/src/DependencyInjection/SentryExtension.php @@ -17,6 +17,7 @@ use Sentry\SentryBundle\EventListener\ErrorListener; use Sentry\SentryBundle\EventListener\MessengerListener; use Sentry\SentryBundle\SentryBundle; +use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware; use Sentry\Serializer\RepresentationSerializer; use Sentry\Serializer\Serializer; @@ -167,6 +168,7 @@ private function registerTracingConfiguration(ContainerBuilder $container, array $container->setParameter('sentry.tracing.dbal.connections', $isConfigEnabled ? $config['dbal']['connections'] : []); if (!$isConfigEnabled) { + $container->removeDefinition(ConnectionConfigurator::class); $container->removeDefinition(TracingDriverMiddleware::class); } } diff --git a/src/Tracing/Doctrine/DBAL/Compatibility/DriverInterface.php b/src/Tracing/Doctrine/DBAL/Compatibility/DriverInterface.php new file mode 100644 index 00000000..8d320f5e --- /dev/null +++ b/src/Tracing/Doctrine/DBAL/Compatibility/DriverInterface.php @@ -0,0 +1,12 @@ += 2.11. + * DBAL >= 2.10. * * @internal */ diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php index e4d1277d..de7ecab8 100644 --- a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php +++ b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php @@ -13,7 +13,8 @@ /** * This implementation wraps a driver connection and adds distributed tracing - * capabilities to Doctrine DBAL. + * capabilities to Doctrine DBAL. This implementation IS and MUST be compatible + * with all versions of Doctrine DBAL >= 2.10. */ final class TracingDriverConnection implements DriverConnectionInterface { @@ -45,7 +46,7 @@ final class TracingDriverConnection implements DriverConnectionInterface /** * @internal */ - public const SPAN_OP_TRANSACTION_ROLLBACK = 'sql.transaciton.rollback'; + public const SPAN_OP_TRANSACTION_ROLLBACK = 'sql.transaction.rollback'; /** * @var HubInterface The current hub diff --git a/src/aliases.php b/src/aliases.php index cc7675e7..4d5a40c0 100644 --- a/src/aliases.php +++ b/src/aliases.php @@ -4,7 +4,7 @@ namespace Sentry\SentryBundle; -use Doctrine\DBAL\Connection; +use Doctrine\DBAL\Driver as DoctrineDriverInterface; use Doctrine\DBAL\Driver\DriverException as LegacyDriverExceptionInterface; use Doctrine\DBAL\Driver\Exception as DriverExceptionInterface; use Doctrine\DBAL\Driver\ExceptionConverterDriver as LegacyExceptionConverterDriverInterface; @@ -15,6 +15,7 @@ use Sentry\SentryBundle\EventListener\RequestListenerControllerEvent; use Sentry\SentryBundle\EventListener\RequestListenerRequestEvent; use Sentry\SentryBundle\EventListener\SubRequestListenerRequestEvent; +use Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility\DriverInterface; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility\ExceptionConverterDriverInterface; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility\MiddlewareInterface; use Symfony\Component\HttpKernel\Event\ControllerEvent; @@ -67,24 +68,27 @@ class_alias(GetResponseEvent::class, SubRequestListenerRequestEvent::class); } } -if (class_exists(Connection::class)) { - if (!interface_exists(Result::class)) { - /** @psalm-suppress UndefinedClass */ - class_alias(Statement::class, Result::class); - } +if (interface_exists(Statement::class) && !interface_exists(Result::class)) { + /** @psalm-suppress UndefinedClass */ + class_alias(Statement::class, Result::class); +} - if (!interface_exists(DoctrineMiddlewareInterface::class)) { - /** @psalm-suppress UndefinedClass */ - class_alias(MiddlewareInterface::class, DoctrineMiddlewareInterface::class); - } +if (interface_exists(DriverExceptionInterface::class) && !interface_exists(LegacyDriverExceptionInterface::class)) { + /** @psalm-suppress UndefinedClass */ + class_alias(DriverExceptionInterface::class, LegacyDriverExceptionInterface::class); +} - if (!interface_exists(LegacyExceptionConverterDriverInterface::class)) { - /** @psalm-suppress UndefinedClass */ - class_alias(ExceptionConverterDriverInterface::class, LegacyExceptionConverterDriverInterface::class); - } +if (!interface_exists(DoctrineMiddlewareInterface::class)) { + /** @psalm-suppress UndefinedClass */ + class_alias(MiddlewareInterface::class, DoctrineMiddlewareInterface::class); +} - if (!interface_exists(LegacyDriverExceptionInterface::class)) { - /** @psalm-suppress UndefinedClass */ - class_alias(DriverExceptionInterface::class, LegacyDriverExceptionInterface::class); - } +if (!interface_exists(DoctrineDriverInterface::class)) { + /** @psalm-suppress UndefinedClass */ + class_alias(DriverInterface::class, DoctrineDriverInterface::class); +} + +if (!interface_exists(LegacyExceptionConverterDriverInterface::class)) { + /** @psalm-suppress UndefinedClass */ + class_alias(ExceptionConverterDriverInterface::class, LegacyExceptionConverterDriverInterface::class); } diff --git a/tests/DependencyInjection/Compiler/DbalTracingPassTest.php b/tests/DependencyInjection/Compiler/DbalTracingPassTest.php index 4bbffc66..55d72891 100644 --- a/tests/DependencyInjection/Compiler/DbalTracingPassTest.php +++ b/tests/DependencyInjection/Compiler/DbalTracingPassTest.php @@ -18,7 +18,7 @@ final class DbalTracingPassTest extends DoctrineTestCase { public function testProcessWithDoctrineDBALVersionAtLeast30(): void { - if (!$this->isDoctrineDBALVersion3Installed()) { + if (!self::isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); } @@ -84,7 +84,7 @@ public function testProcessWithDoctrineDBALVersionAtLeast30(): void public function testProcessWithDoctrineDBALVersionLowerThan30(): void { - if ($this->isDoctrineDBALVersion3Installed()) { + if (self::isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.'); } diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index 7d945214..c377c242 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -4,6 +4,7 @@ namespace Sentry\SentryBundle\Tests\DependencyInjection; +use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; use Jean85\PrettyVersions; use PHPUnit\Framework\TestCase; use Sentry\SentryBundle\DependencyInjection\Configuration; @@ -38,7 +39,7 @@ public function testProcessConfigurationWithDefaultConfiguration(): void 'tracing' => [ 'dbal' => [ 'enabled' => false, - 'connections' => ['%doctrine.default_connection%'], + 'connections' => class_exists(DoctrineBundle::class) ? ['%doctrine.default_connection%'] : [], ], ], ]; diff --git a/tests/DependencyInjection/Fixtures/php/dbal_tracing_disabled.php b/tests/DependencyInjection/Fixtures/php/dbal_tracing_enabled.php similarity index 89% rename from tests/DependencyInjection/Fixtures/php/dbal_tracing_disabled.php rename to tests/DependencyInjection/Fixtures/php/dbal_tracing_enabled.php index ed57d764..85794aa1 100644 --- a/tests/DependencyInjection/Fixtures/php/dbal_tracing_disabled.php +++ b/tests/DependencyInjection/Fixtures/php/dbal_tracing_enabled.php @@ -8,7 +8,7 @@ $container->loadFromExtension('sentry', [ 'tracing' => [ 'dbal' => [ - 'enabled' => false, + 'enabled' => true, 'connections' => ['default'], ], ], diff --git a/tests/DependencyInjection/Fixtures/php/full.php b/tests/DependencyInjection/Fixtures/php/full.php index 3b7f8699..1e3b848f 100644 --- a/tests/DependencyInjection/Fixtures/php/full.php +++ b/tests/DependencyInjection/Fixtures/php/full.php @@ -44,7 +44,8 @@ ], 'tracing' => [ 'dbal' => [ - 'enabled' => true, + 'enabled' => false, + 'connections' => ['default'], ], ], ]); diff --git a/tests/DependencyInjection/Fixtures/xml/dbal_tracing_disabled.xml b/tests/DependencyInjection/Fixtures/xml/dbal_tracing_enabled.xml similarity index 94% rename from tests/DependencyInjection/Fixtures/xml/dbal_tracing_disabled.xml rename to tests/DependencyInjection/Fixtures/xml/dbal_tracing_enabled.xml index 08ed0e94..9c4bc03f 100644 --- a/tests/DependencyInjection/Fixtures/xml/dbal_tracing_disabled.xml +++ b/tests/DependencyInjection/Fixtures/xml/dbal_tracing_enabled.xml @@ -8,7 +8,7 @@ - + default diff --git a/tests/DependencyInjection/Fixtures/xml/full.xml b/tests/DependencyInjection/Fixtures/xml/full.xml index 418c8328..b1eda4d7 100644 --- a/tests/DependencyInjection/Fixtures/xml/full.xml +++ b/tests/DependencyInjection/Fixtures/xml/full.xml @@ -38,7 +38,9 @@ - + + default + diff --git a/tests/DependencyInjection/Fixtures/yml/dbal_tracing_disabled.yml b/tests/DependencyInjection/Fixtures/yml/dbal_tracing_enabled.yml similarity index 76% rename from tests/DependencyInjection/Fixtures/yml/dbal_tracing_disabled.yml rename to tests/DependencyInjection/Fixtures/yml/dbal_tracing_enabled.yml index 3c819c2a..d5708431 100644 --- a/tests/DependencyInjection/Fixtures/yml/dbal_tracing_disabled.yml +++ b/tests/DependencyInjection/Fixtures/yml/dbal_tracing_enabled.yml @@ -1,6 +1,6 @@ sentry: tracing: dbal: - enabled: false + enabled: true connections: - default diff --git a/tests/DependencyInjection/Fixtures/yml/full.yml b/tests/DependencyInjection/Fixtures/yml/full.yml index b123c173..f0ce0a3b 100644 --- a/tests/DependencyInjection/Fixtures/yml/full.yml +++ b/tests/DependencyInjection/Fixtures/yml/full.yml @@ -39,4 +39,6 @@ sentry: capture_soft_fails: false tracing: dbal: - enabled: true + enabled: false + connections: + - enabled diff --git a/tests/DependencyInjection/SentryExtensionTest.php b/tests/DependencyInjection/SentryExtensionTest.php index afb6ca25..af040d43 100644 --- a/tests/DependencyInjection/SentryExtensionTest.php +++ b/tests/DependencyInjection/SentryExtensionTest.php @@ -4,6 +4,7 @@ namespace Sentry\SentryBundle\Tests\DependencyInjection; +use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; use Jean85\PrettyVersions; use PHPUnit\Framework\TestCase; use Sentry\ClientInterface; @@ -16,6 +17,7 @@ use Sentry\SentryBundle\EventListener\RequestListener; use Sentry\SentryBundle\EventListener\SubRequestListener; use Sentry\SentryBundle\SentryBundle; +use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware; use Sentry\Serializer\RepresentationSerializer; use Sentry\Serializer\Serializer; @@ -264,16 +266,23 @@ public function testIgnoreErrorsIntegrationIsNotAddedTwiceIfAlreadyConfigured(): public function testTracingDriverMiddlewareIsConfiguredWhenDbalTracingIsEnable(): void { - $container = $this->createContainerFromFixture('full'); + if (!class_exists(DoctrineBundle::class)) { + $this->markTestSkipped('This test requires the "doctrine/doctrine-bundle" Composer package to be installed.'); + } + + $container = $this->createContainerFromFixture('dbal_tracing_enabled'); $this->assertTrue($container->hasDefinition(TracingDriverMiddleware::class)); + $this->assertTrue($container->hasDefinition(ConnectionConfigurator::class)); + $this->assertNotEmpty($container->getParameter('sentry.tracing.dbal.connections')); } public function testTracingDriverMiddlewareIsRemovedWhenDbalTracingIsDisabled(): void { - $container = $this->createContainerFromFixture('dbal_tracing_disabled'); + $container = $this->createContainerFromFixture('full'); $this->assertFalse($container->hasDefinition(TracingDriverMiddleware::class)); + $this->assertFalse($container->hasDefinition(ConnectionConfigurator::class)); $this->assertEmpty($container->getParameter('sentry.tracing.dbal.connections')); } diff --git a/tests/DoctrineTestCase.php b/tests/DoctrineTestCase.php index 0f0e7c50..f5d32981 100644 --- a/tests/DoctrineTestCase.php +++ b/tests/DoctrineTestCase.php @@ -4,13 +4,19 @@ namespace Sentry\SentryBundle\Tests; +use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; use Doctrine\DBAL\Driver\ResultStatement; use PHPUnit\Framework\TestCase; abstract class DoctrineTestCase extends TestCase { - protected function isDoctrineDBALVersion3Installed(): bool + protected static function isDoctrineDBALVersion3Installed(): bool { return !interface_exists(ResultStatement::class); } + + protected static function isDoctrineBundlePackageInstalled(): bool + { + return class_exists(DoctrineBundle::class); + } } diff --git a/tests/Tracing/Doctrine/DBAL/TracingDriverConnectionTest.php b/tests/Tracing/Doctrine/DBAL/TracingDriverConnectionTest.php index 51d28639..6d114ce9 100644 --- a/tests/Tracing/Doctrine/DBAL/TracingDriverConnectionTest.php +++ b/tests/Tracing/Doctrine/DBAL/TracingDriverConnectionTest.php @@ -9,13 +9,13 @@ use Doctrine\DBAL\Driver\Statement as DriverStatementInterface; use Doctrine\DBAL\ParameterType; use PHPUnit\Framework\MockObject\MockObject; -use PHPUnit\Framework\TestCase; +use Sentry\SentryBundle\Tests\DoctrineTestCase; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverConnection; use Sentry\State\HubInterface; use Sentry\Tracing\Transaction; use Sentry\Tracing\TransactionContext; -final class TracingDriverConnectionTest extends TestCase +final class TracingDriverConnectionTest extends DoctrineTestCase { /** * @var MockObject&HubInterface @@ -32,6 +32,13 @@ final class TracingDriverConnectionTest extends TestCase */ private $connection; + public static function setUpBeforeClass(): void + { + if (!self::isDoctrineBundlePackageInstalled()) { + self::markTestSkipped(); + } + } + protected function setUp(): void { $this->hub = $this->createMock(HubInterface::class); diff --git a/tests/Tracing/Doctrine/DBAL/TracingDriverMiddlewareTest.php b/tests/Tracing/Doctrine/DBAL/TracingDriverMiddlewareTest.php index 6168ea17..69dd5683 100644 --- a/tests/Tracing/Doctrine/DBAL/TracingDriverMiddlewareTest.php +++ b/tests/Tracing/Doctrine/DBAL/TracingDriverMiddlewareTest.php @@ -6,12 +6,12 @@ use Doctrine\DBAL\Driver as DriverInterface; use PHPUnit\Framework\MockObject\MockObject; -use PHPUnit\Framework\TestCase; +use Sentry\SentryBundle\Tests\DoctrineTestCase; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriver; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware; use Sentry\State\HubInterface; -final class TracingDriverMiddlewareTest extends TestCase +final class TracingDriverMiddlewareTest extends DoctrineTestCase { /** * @var MockObject&HubInterface @@ -23,6 +23,13 @@ final class TracingDriverMiddlewareTest extends TestCase */ private $middleware; + public static function setUpBeforeClass(): void + { + if (!self::isDoctrineBundlePackageInstalled()) { + self::markTestSkipped(); + } + } + protected function setUp(): void { $this->hub = $this->createMock(HubInterface::class); diff --git a/tests/Tracing/Doctrine/DBAL/TracingDriverTest.php b/tests/Tracing/Doctrine/DBAL/TracingDriverTest.php index e326e2dc..6fa50e9e 100644 --- a/tests/Tracing/Doctrine/DBAL/TracingDriverTest.php +++ b/tests/Tracing/Doctrine/DBAL/TracingDriverTest.php @@ -8,12 +8,12 @@ use Doctrine\DBAL\Driver\API\ExceptionConverter; use Doctrine\DBAL\Driver as DriverInterface; use Doctrine\DBAL\Driver\Connection as DriverConnectionInterface; -use Doctrine\DBAL\Driver\DriverException as LegacyDriverExceptionInterface; -use Doctrine\DBAL\Driver\ExceptionConverterDriver; +use Doctrine\DBAL\Driver\DriverException as DriverExceptionInterface; +use Doctrine\DBAL\Driver\ExceptionConverterDriver as ExceptionConverterDriverInterface; use Doctrine\DBAL\Exception\DriverException as DBALDriverException; use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Schema\AbstractSchemaManager; -use Doctrine\DBAL\VersionAwarePlatformDriver; +use Doctrine\DBAL\VersionAwarePlatformDriver as VersionAwarePlatformDriverInterface; use PHPUnit\Framework\MockObject\MockObject; use Sentry\SentryBundle\Tests\DoctrineTestCase; use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriver; @@ -32,6 +32,13 @@ protected function setUp(): void $this->hub = $this->createMock(HubInterface::class); } + public static function setUpBeforeClass(): void + { + if (!self::isDoctrineBundlePackageInstalled()) { + self::markTestSkipped(); + } + } + public function testConnect(): void { $databasePlatform = $this->createMock(AbstractPlatform::class); @@ -87,7 +94,7 @@ public function testGetSchemaManager(): void public function testGetExceptionConverter(): void { - if (!$this->isDoctrineDBALVersion3Installed()) { + if (!self::isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); } @@ -105,7 +112,7 @@ public function testGetExceptionConverter(): void public function testGetExceptionConverterThrowsIfDoctrineDBALVersionIsLowerThan30(): void { - if ($this->isDoctrineDBALVersion3Installed()) { + if (self::isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.'); } @@ -120,7 +127,7 @@ public function testGetExceptionConverterThrowsIfDoctrineDBALVersionIsLowerThan3 public function testGetNameIfDoctrineDBALVersionIsLowerThan30(): void { - if ($this->isDoctrineDBALVersion3Installed()) { + if (self::isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.'); } @@ -136,7 +143,7 @@ public function testGetNameIfDoctrineDBALVersionIsLowerThan30(): void public function testGetNameThrowsIfDoctrineDBALVersionIsAtLeast30(): void { - if (!$this->isDoctrineDBALVersion3Installed()) { + if (!self::isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); } @@ -149,7 +156,7 @@ public function testGetNameThrowsIfDoctrineDBALVersionIsAtLeast30(): void public function testGetDatabaseIfDoctrineDBALVersionIsLowerThan30(): void { - if ($this->isDoctrineDBALVersion3Installed()) { + if (self::isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.'); } @@ -168,7 +175,7 @@ public function testGetDatabaseIfDoctrineDBALVersionIsLowerThan30(): void public function testGetDatabaseThrowsIfDoctrineDBALVersionIsAtLeast30(): void { - if (!$this->isDoctrineDBALVersion3Installed()) { + if (!self::isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); } @@ -183,7 +190,7 @@ public function testCreateDatabasePlatformForVersion(): void { $databasePlatform = $this->createMock(AbstractPlatform::class); - $decoratedDriver = $this->createMock(VersionAwarePlatformDriverInterface::class); + $decoratedDriver = $this->createMock(StubVersionAwarePlatformDriverInterface::class); $decoratedDriver->expects($this->once()) ->method('createDatabasePlatformForVersion') ->with('5.7') @@ -210,14 +217,14 @@ public function testCreateDatabasePlatformForVersionWhenDriverDoesNotImplementIn public function testConvertException(): void { - if ($this->isDoctrineDBALVersion3Installed()) { + if (self::isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be <= 3.0.'); } - $exception = $this->createMock(LegacyDriverExceptionInterface::class); + $exception = $this->createMock(DriverExceptionInterface::class); $convertedException = new DBALDriverException('foo', $exception); - $decoratedDriver = $this->createMock(ExceptionConverterDriverInterface::class); + $decoratedDriver = $this->createMock(StubExceptionConverterDriverInterface::class); $decoratedDriver->expects($this->once()) ->method('convertException') ->with('foo', $exception) @@ -230,22 +237,30 @@ public function testConvertException(): void public function testConvertExceptionThrowsIfDoctrineDBALVersionIsAtLeast30(): void { - if (!$this->isDoctrineDBALVersion3Installed()) { + if (!self::isDoctrineDBALVersion3Installed()) { $this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.'); } $this->expectException(\BadMethodCallException::class); $this->expectExceptionMessage('The Sentry\\SentryBundle\\Tracing\\Doctrine\\DBAL\\TracingDriver::convertException() method is not supported on Doctrine DBAL 3.0.'); - $driver = new TracingDriver($this->hub, $this->createMock(ExceptionConverterDriverInterface::class)); - $driver->convertException('foo', $this->createMock(LegacyDriverExceptionInterface::class)); + $driver = new TracingDriver($this->hub, $this->createMock(StubExceptionConverterDriverInterface::class)); + $driver->convertException('foo', $this->createMock(DriverExceptionInterface::class)); } } -interface ExceptionConverterDriverInterface extends DriverInterface, ExceptionConverterDriver -{ +if (interface_exists(VersionAwarePlatformDriverInterface::class)) { + interface StubVersionAwarePlatformDriverInterface extends DriverInterface, VersionAwarePlatformDriverInterface + { + } } -interface VersionAwarePlatformDriverInterface extends DriverInterface, VersionAwarePlatformDriver -{ +if (interface_exists(ExceptionConverterDriverInterface::class)) { + interface StubExceptionConverterDriverInterface extends ExceptionConverterDriverInterface, DriverInterface + { + } +} else { + interface StubExceptionConverterDriverInterface extends DriverInterface + { + } } From 368aa38327b62d39bfe3426f7f65aecc00afeff9 Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Sat, 13 Feb 2021 02:24:52 +0100 Subject: [PATCH 15/16] Bump minimum required Symfony version to avoid libxml deprecation --- composer.json | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/composer.json b/composer.json index 4550059e..cc2220d0 100644 --- a/composer.json +++ b/composer.json @@ -23,13 +23,13 @@ "jean85/pretty-package-versions": "^1.5 || ^2.0", "php-http/discovery": "^1.11", "sentry/sdk": "^3.1", - "symfony/config": "^3.4.43||^4.4.11||^5.0.11", - "symfony/console": "^3.4.43||^4.4.11||^5.0.11", - "symfony/dependency-injection": "^3.4.43||^4.4.11||^5.0.11", - "symfony/event-dispatcher": "^3.4.43||^4.4.11||^5.0.11", - "symfony/http-kernel": "^3.4.43||^4.4.11||^5.0.11", + "symfony/config": "^3.4.44||^4.4.11||^5.0.11", + "symfony/console": "^3.4.44||^4.4.11||^5.0.11", + "symfony/dependency-injection": "^3.4.44||^4.4.11||^5.0.11", + "symfony/event-dispatcher": "^3.4.44||^4.4.11||^5.0.11", + "symfony/http-kernel": "^3.4.44||^4.4.11||^5.0.11", "symfony/psr-http-message-bridge": "^2.0", - "symfony/security-core": "^3.4.43||^4.4.11||^5.0.11" + "symfony/security-core": "^3.4.44||^4.4.11||^5.0.11" }, "require-dev": { "doctrine/dbal": "^2.10||^3.0", @@ -43,13 +43,14 @@ "phpstan/phpstan": "^0.12", "phpstan/phpstan-phpunit": "^0.12", "phpunit/phpunit": "^8.5||^9.0", - "symfony/browser-kit": "^3.4.43||^4.4.11||^5.0.11", - "symfony/framework-bundle": "^3.4.43||^4.4.11||^5.0.11", + "symfony/browser-kit": "^3.4.44||^4.4.12||^5.0.11", + "symfony/dom-crawler": "^3.4.44||^4.4.12||^5.0.11", + "symfony/framework-bundle": "^3.4.44||^4.4.12||^5.0.11", "symfony/messenger": "^4.4.11||^5.0.11", "symfony/monolog-bundle": "^3.4", "symfony/phpunit-bridge": "^5.0", "symfony/polyfill-php80": "^1.22", - "symfony/yaml": "^3.4.43||^4.4.11||^5.0.11", + "symfony/yaml": "^3.4.44||^4.4.11||^5.0.11", "vimeo/psalm": "^4.3" }, "suggest": { From 679830007d85d76c2cc4955b6918469a0fdaf6e7 Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Sat, 13 Feb 2021 18:06:42 +0100 Subject: [PATCH 16/16] Avoid recomputing every time the tags of the trace span --- .../Doctrine/DBAL/TracingDriverConnection.php | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php index de7ecab8..5d319d47 100644 --- a/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php +++ b/src/Tracing/Doctrine/DBAL/TracingDriverConnection.php @@ -64,9 +64,9 @@ final class TracingDriverConnection implements DriverConnectionInterface private $databasePlatform; /** - * @var array The connection params + * @var array The tags to attach to the span */ - private $params; + private $spanTags; /** * Constructor. @@ -85,7 +85,7 @@ public function __construct( $this->hub = $hub; $this->decoratedConnection = $decoratedConnection; $this->databasePlatform = $databasePlatform; - $this->params = $params; + $this->spanTags = $this->getSpanTags($params); } /** @@ -203,7 +203,7 @@ private function traceFunction(string $spanOperation, string $spanDescription, \ $spanContext = new SpanContext(); $spanContext->setOp($spanOperation); $spanContext->setDescription($spanDescription); - $spanContext->setTags($this->getSpanTags()); + $spanContext->setTags($this->spanTags); $span = $span->startChild($spanContext); } @@ -218,37 +218,41 @@ private function traceFunction(string $spanOperation, string $spanDescription, \ } /** - * @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md + * Gets a map of key-value pairs that will be set as tags of the span. + * + * @param array $params The connection params * * @return array + * + * @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md */ - private function getSpanTags(): array + private function getSpanTags(array $params): array { $tags = ['db.system' => $this->databasePlatform]; - if (isset($this->params['user'])) { - $tags['db.user'] = $this->params['user']; + if (isset($params['user'])) { + $tags['db.user'] = $params['user']; } - if (isset($this->params['dbname'])) { - $tags['db.name'] = $this->params['dbname']; + if (isset($params['dbname'])) { + $tags['db.name'] = $params['dbname']; } - if (isset($this->params['host']) && !empty($this->params['host']) && !isset($this->params['memory'])) { - if (false === filter_var($this->params['host'], \FILTER_VALIDATE_IP)) { - $tags['net.peer.name'] = $this->params['host']; + if (isset($params['host']) && !empty($params['host']) && !isset($params['memory'])) { + if (false === filter_var($params['host'], \FILTER_VALIDATE_IP)) { + $tags['net.peer.name'] = $params['host']; } else { - $tags['net.peer.ip'] = $this->params['host']; + $tags['net.peer.ip'] = $params['host']; } } - if (isset($this->params['port'])) { - $tags['net.peer.port'] = (string) $this->params['port']; + if (isset($params['port'])) { + $tags['net.peer.port'] = (string) $params['port']; } - if (isset($this->params['unix_socket'])) { + if (isset($params['unix_socket'])) { $tags['net.transport'] = 'Unix'; - } elseif (isset($this->params['memory'])) { + } elseif (isset($params['memory'])) { $tags['net.transport'] = 'inproc'; }