Move tracing to its own main directory and apply comments review

Author: rjd22

.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

.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

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": [

src/DependencyInjection/Compiler/ConnectDbalQueryListenerPass.php

@@ -4,29 +4,31 @@
 
 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
 {
     /**
+     * {@inheritdoc}
+     *
      * @return void
      */
     public function process(ContainerBuilder $container)
     {
         $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)])
             );
         }
     }

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')

src/EventListener/Tracing/DbalListener.php renamed to src/Tracing/DbalSqlTracingLogger.php

@@ -2,29 +2,25 @@
 
 declare(strict_types=1);
 
-namespace Sentry\SentryBundle\EventListener\Tracing;
+namespace Sentry\SentryBundle\Tracing;
 
 use Doctrine\DBAL\Logging\SQLLogger;
 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
- * debug stack logger.
- */
-final class DbalListener implements SQLLogger
+final class DbalSqlTracingLogger implements SQLLogger
 {
     /**
      * @var HubInterface The current hub
      */
     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();
     }
 }

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' => [],

tests/EventListener/Tracing/DbalListenerTest.php renamed to tests/Tracing/DbalSqlTracingLoggerTest.php

@@ -2,31 +2,31 @@
 
 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
      */
     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