refactor: cleanup Doctrine Tracing Middleware

FriendsOfOpenTelemetry · Jan 4, 2024 · 1e39175 · 1e39175
1 parent f608fbc
commit 1e39175
Show file tree

Hide file tree

Showing 8 changed files with 32 additions and 135 deletions.
diff --git a/src/Middleware/Doctrine/Metric/Connection.php b/src/Middleware/Doctrine/Metric/Connection.php
diff --git a/src/Middleware/Doctrine/Metric/Driver.php b/src/Middleware/Doctrine/Metric/Driver.php
diff --git a/src/Middleware/Doctrine/Metric/Middleware.php b/src/Middleware/Doctrine/Metric/Middleware.php
diff --git a/src/Middleware/Doctrine/Metric/Statement.php b/src/Middleware/Doctrine/Metric/Statement.php
diff --git a/src/Middleware/Doctrine/Trace/Connection.php b/src/Middleware/Doctrine/Trace/Connection.php
@@ -6,41 +6,18 @@
 use Doctrine\DBAL\Driver\Middleware\AbstractConnectionMiddleware;
 use Doctrine\DBAL\Driver\Result;
 use Doctrine\DBAL\Driver\Statement as DriverStatement;
-use OpenTelemetry\API\Trace\SpanInterface;
-use OpenTelemetry\API\Trace\TracerInterface;
-use OpenTelemetry\Context\ScopeInterface;
 
 class Connection extends AbstractConnectionMiddleware
 {
     public function __construct(
-        private ConnectionInterface $connection,
-        private TracerInterface $tracer,
-        private ?SpanInterface $span = null,
-        private ?ScopeInterface $scope = null,
+        ConnectionInterface $connection,
     ) {
         parent::__construct($connection);
     }
 
-    public function __destruct()
-    {
-        if (null === $this->scope) {
-            return;
-        }
-        $this->scope->detach();
-
-        if (null === $this->span) {
-            return;
-        }
-        $this->span->end();
-    }
-
     public function prepare(string $sql): DriverStatement
     {
-        return new Statement(
-            parent::prepare($sql),
-            $this->tracer,
-            $sql,
-        );
+        return new Statement(parent::prepare($sql));
     }
 
     public function query(string $sql): Result

diff --git a/src/Middleware/Doctrine/Trace/Driver.php b/src/Middleware/Doctrine/Trace/Driver.php
@@ -2,93 +2,62 @@
 
 namespace FriendsOfOpenTelemetry\OpenTelemetryBundle\Middleware\Doctrine\Trace;
 
-use Doctrine\DBAL\Connection as ConnectionInterface;
 use Doctrine\DBAL\Driver as DriverInterface;
 use Doctrine\DBAL\Driver\Middleware\AbstractDriverMiddleware;
 use Doctrine\DBAL\DriverManager;
 use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
-use Doctrine\DBAL\Platforms\AbstractPlatform;
 use Doctrine\DBAL\Platforms\DB2Platform;
 use Doctrine\DBAL\Platforms\OraclePlatform;
 use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
 use Doctrine\DBAL\Platforms\SqlitePlatform;
 use Doctrine\DBAL\Platforms\SQLServerPlatform;
+use OpenTelemetry\API\Trace\SpanInterface;
 use OpenTelemetry\API\Trace\SpanKind;
 use OpenTelemetry\API\Trace\TracerInterface;
 use OpenTelemetry\Context\Context;
+use OpenTelemetry\Context\ScopeInterface;
 use OpenTelemetry\SemConv\TraceAttributes;
 
 /**
- * Types extracted from \Doctrine\DBAL\DriverManager.
- *
- * @phpstan-type OverrideParams array{
- *     application_name?: string,
- *     charset?: string,
- *     dbname?: string,
- *     default_dbname?: string,
- *     driver?: key-of<DriverManager::DRIVER_MAP>,
- *     driverClass?: class-string<DriverInterface>,
- *     driverOptions?: array<int, mixed>,
- *     host?: string,
- *     password?: string,
- *     path?: string,
- *     persistent?: bool,
- *     platform?: AbstractPlatform,
- *     port?: int,
- *     serverVersion?: string,
- *     url?: string,
- *     user?: string,
- *     unix_socket?: string,
- * }
- * @phpstan-type Params array{
- *     application_name?: string,
- *     charset?: string,
- *     dbname?: string,
- *     defaultTableOptions?: array<string, mixed>,
- *     default_dbname?: string,
- *     driver?: key-of<DriverManager::DRIVER_MAP>,
- *     driverClass?: class-string<DriverInterface>,
- *     driverOptions?: array<int, mixed>,
- *     host?: string,
- *     keepSlave?: bool,
- *     keepReplica?: bool,
- *     master?: OverrideParams,
- *     memory?: bool,
- *     password?: string,
- *     path?: string,
- *     persistent?: bool,
- *     platform?: AbstractPlatform,
- *     port?: int,
- *     primary?: OverrideParams,
- *     replica?: array<OverrideParams>,
- *     serverVersion?: string,
- *     sharding?: array<string,mixed>,
- *     slaves?: array<OverrideParams>,
- *     url?: string,
- *     user?: string,
- *     wrapperClass?: class-string<ConnectionInterface>,
- *     unix_socket?: string,
- * }
+ * @phpstan-import-type OverrideParams from DriverManager
+ * @phpstan-import-type Params from DriverManager
  */
 final class Driver extends AbstractDriverMiddleware
 {
+    private ?ScopeInterface $scope = null;
+    private ?SpanInterface $span = null;
+
     public function __construct(
+        private readonly TracerInterface $tracer,
         DriverInterface $driver,
-        private TracerInterface $tracer
     ) {
         parent::__construct($driver);
     }
 
+    public function __destruct()
+    {
+        if (null === $this->scope) {
+            return;
+        }
+
+        $this->scope->detach();
+
+        if (null === $this->span) {
+            return;
+        }
+
+        $this->span->end();
+    }
+
     /**
      * @param Params $params
      */
     public function connect(
         #[\SensitiveParameter]
         array $params
     ) {
-        $databasePlatform = $this->getDatabasePlatform();
         $spanBuilder = $this->tracer
-            ->spanBuilder('doctrine.dbal.middleware.driver')
+            ->spanBuilder('doctrine.dbal.driver')
             ->setAttribute(TraceAttributes::DB_SYSTEM, $this->getSemanticDbSystem())
             ->setAttribute(TraceAttributes::DB_CONNECTION_STRING, $params['url'])
             ->setAttribute(TraceAttributes::DB_NAME, $params['dbname'])
@@ -97,17 +66,12 @@ public function connect(
 
         $spanBuilder->setSpanKind(SpanKind::KIND_SERVER);
 
-        $parent = Context::getCurrent();
+        $context = Context::getCurrent();
 
-        $span = $spanBuilder->setParent($parent)->startSpan();
-        $scope = $span->storeInContext($parent)->activate();
+        $span = $spanBuilder->setParent($context)->startSpan();
+        $span->storeInContext($context)->activate();
 
-        return new Connection(
-            parent::connect($params),
-            $this->tracer,
-            $span,
-            $scope,
-        );
+        return new Connection(parent::connect($params));
     }
 
     private function getSemanticDbSystem(): string

diff --git a/src/Middleware/Doctrine/Trace/Middleware.php b/src/Middleware/Doctrine/Trace/Middleware.php
@@ -14,6 +14,6 @@ public function __construct(private readonly TracerInterface $tracer)
 
     public function wrap(DriverInterface $driver): DriverInterface
     {
-        return new Driver($driver, $this->tracer);
+        return new Driver($this->tracer, $driver);
     }
 }
diff --git a/src/Middleware/Doctrine/Trace/Statement.php b/src/Middleware/Doctrine/Trace/Statement.php
@@ -5,14 +5,11 @@
 use Doctrine\DBAL\Driver\Middleware\AbstractStatementMiddleware;
 use Doctrine\DBAL\Driver\Result;
 use Doctrine\DBAL\Driver\Statement as StatementInterface;
-use OpenTelemetry\API\Trace\TracerInterface;
 
 final class Statement extends AbstractStatementMiddleware
 {
     public function __construct(
-        private StatementInterface $statement,
-        private TracerInterface $tracer,
-        private string $sql,
+        StatementInterface $statement,
     ) {
         parent::__construct($statement);
     }