Fix missing instrumentation of the Statement::execute() method of D…

…octrine DBAL (#548)

Co-authored-by: Alessandro Lai <[email protected]>

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Fix missing instrumentation of the `Statement::execute()` method of Doctrine DBAL (#548)
+
 ## 4.2.1 (2021-08-24)
 
 - Fix return type for `TracingDriver::getDatabase()` method (#541)

phpstan-baseline.neon

@@ -95,31 +95,6 @@ parameters:
  count: 1
  path: src/Tracing/Doctrine/DBAL/TracingDriverConnection.php
 
- -
- message: "#^Class Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\FilterControllerEvent not found\\.$#"
- count: 1
- path: src/aliases.php
-
- -
- message: "#^Class Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\FilterResponseEvent not found\\.$#"
- count: 1
- path: src/aliases.php
-
- -
- message: "#^Class Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\GetResponseEvent not found\\.$#"
- count: 2
- path: src/aliases.php
-
- -
- message: "#^Class Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\GetResponseForExceptionEvent not found\\.$#"
- count: 1
- path: src/aliases.php
-
- -
- message: "#^Class Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\PostResponseEvent not found\\.$#"
- count: 1
- path: src/aliases.php
-
  -
  message: "#^Class Symfony\\\\Bundle\\\\FrameworkBundle\\\\Client not found\\.$#"
  count: 1
@@ -280,3 +255,48 @@ parameters:
  count: 1
  path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php
 
+ -
+ message: "#^Trying to mock an undefined method closeCursor\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
+ count: 1
+ path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php
+
+ -
+ message: "#^Trying to mock an undefined method columnCount\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
+ count: 1
+ path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php
+
+ -
+ message: "#^Trying to mock an undefined method errorCode\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
+ count: 1
+ path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php
+
+ -
+ message: "#^Trying to mock an undefined method errorInfo\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
+ count: 1
+ path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php
+
+ -
+ message: "#^Trying to mock an undefined method fetch\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
+ count: 1
+ path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php
+
+ -
+ message: "#^Trying to mock an undefined method fetchAll\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
+ count: 1
+ path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php
+
+ -
+ message: "#^Trying to mock an undefined method fetchColumn\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
+ count: 1
+ path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php
+
+ -
+ message: "#^Trying to mock an undefined method rowCount\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
+ count: 1
+ path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php
+
+ -
+ message: "#^Trying to mock an undefined method setFetchMode\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
+ count: 1
+ path: tests/Tracing/Doctrine/DBAL/TracingStatementForV2Test.php
+

phpstan.neon

@@ -6,11 +6,16 @@ parameters:
  paths:
  - src
  - tests
- excludes_analyse:
+ bootstrapFiles:
+ - src/aliases.php
+ excludePaths:
+ - src/aliases.php
+ - src/Tracing/Doctrine/DBAL/TracingStatementForV2.php
  - tests/End2End/App
  dynamicConstantNames:
  - Symfony\Component\HttpKernel\Kernel::VERSION
  - Symfony\Component\HttpKernel\Kernel::VERSION_ID
- - Doctrine\DBAL\Version::VERSION
  stubFiles:
  - tests/Stubs/Profile.phpstub
+ featureToggles:
+ disableRuntimeReflectionProvider: true

psalm-baseline.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="4.8.1@f73f2299dbc59a3e6c4d66cff4605176e728ee69">
+<files psalm-version="4.9.3@4c262932602b9bbab5020863d1eb22d49de0dbf4">
  <file src="src/EventListener/ConsoleCommandListener.php">
  <InvalidExtendClass occurrences="1">
  <code>ConsoleListener</code>
@@ -8,4 +8,38 @@
  <code>public function __construct(HubInterface $hub, bool $captureErrors = true)</code>
  </MethodSignatureMismatch>
  </file>
+ <file src="src/Tracing/Doctrine/DBAL/TracingStatementForV2.php">
+ <InvalidReturnStatement occurrences="2">
+ <code>$this-&gt;decoratedStatement</code>
+ <code>$this-&gt;traceFunction($spanContext, [$this-&gt;decoratedStatement, 'execute'], $params)</code>
+ </InvalidReturnStatement>
+ <InvalidReturnType occurrences="2">
+ <code>\Traversable</code>
+ <code>bool</code>
+ </InvalidReturnType>
+ <MethodSignatureMismatch occurrences="1">
+ <code>TracingStatementForV2</code>
+ </MethodSignatureMismatch>
+ <UndefinedInterfaceMethod occurrences="9">
+ <code>closeCursor</code>
+ <code>columnCount</code>
+ <code>errorCode</code>
+ <code>errorInfo</code>
+ <code>fetch</code>
+ <code>fetchAll</code>
+ <code>fetchColumn</code>
+ <code>rowCount</code>
+ <code>setFetchMode</code>
+ </UndefinedInterfaceMethod>
+ </file>
+ <file src="src/aliases.php">
+ <UndefinedClass occurrences="6">
+ <code>FilterControllerEvent</code>
+ <code>FilterResponseEvent</code>
+ <code>GetResponseEvent</code>
+ <code>GetResponseEvent</code>
+ <code>GetResponseForExceptionEvent</code>
+ <code>PostResponseEvent</code>
+ </UndefinedClass>
+ </file>
 </files>

src/DependencyInjection/Compiler/DbalTracingPass.php

@@ -4,7 +4,6 @@
 
 namespace Sentry\SentryBundle\DependencyInjection\Compiler;
 
-use Doctrine\DBAL\Driver\ResultStatement;
 use Doctrine\DBAL\Result;
 use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator;
 use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware;
@@ -57,7 +56,7 @@ public function process(ContainerBuilder $container): void
  throw new \InvalidArgumentException(sprintf('The Doctrine connection "%s" does not exists and cannot be instrumented.', $connectionName));
  }
 
- if (!interface_exists(ResultStatement::class)) {
+ if (class_exists(Result::class)) {
  $this->configureConnectionForDoctrineDBALVersion3($container, $connectionName);
  } else {
  $this->configureConnectionForDoctrineDBALVersion2($container, $connectionName);

src/Tracing/Doctrine/DBAL/AbstractTracingStatement.php

@@ -0,0 +1,84 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Sentry\SentryBundle\Tracing\Doctrine\DBAL;
+
+use Doctrine\DBAL\Driver\Statement;
+use Sentry\State\HubInterface;
+use Sentry\Tracing\Span;
+use Sentry\Tracing\SpanContext;
+
+abstract class AbstractTracingStatement
+{
+ /**
+ * @internal
+ */
+ public const SPAN_OP_STMT_EXECUTE = 'sql.stmt.execute';
+
+ /**
+ * @var HubInterface The current hub
+ */
+ protected $hub;
+
+ /**
+ * @var Statement The decorated statement
+ */
+ protected $decoratedStatement;
+
+ /**
+ * @var string The SQL query executed by the decorated statement
+ */
+ protected $sqlQuery;
+
+ /**
+ * @var array<string, string> The span tags
+ */
+ protected $spanTags;
+
+ /**
+ * Constructor.
+ *
+ * @param HubInterface $hub The current hub
+ * @param Statement $decoratedStatement The decorated statement
+ * @param string $sqlQuery The SQL query executed by the decorated statement
+ * @param array<string, string> $spanTags The span tags
+ */
+ public function __construct(HubInterface $hub, Statement $decoratedStatement, string $sqlQuery, array $spanTags)
+ {
+ $this->hub = $hub;
+ $this->decoratedStatement = $decoratedStatement;
+ $this->sqlQuery = $sqlQuery;
+ $this->spanTags = $spanTags;
+ }
+
+ /**
+ * Calls the given callback by passing to it the specified arguments and
+ * wrapping its execution into a child {@see Span} of the current one.
+ *
+ * @param callable $callback The function to call
+ * @param mixed ...$args The arguments to pass to the callback
+ *
+ * @phpstan-template T
+ *
+ * @phpstan-param callable(mixed...): T $callback
+ *
+ * @phpstan-return T
+ */
+ protected function traceFunction(SpanContext $spanContext, callable $callback, ...$args)
+ {
+ $span = $this->hub->getSpan();
+
+ if (null !== $span) {
+ $span = $span->startChild($spanContext);
+ }
+
+ try {
+ return $callback(...$args);
+ } finally {
+ if (null !== $span) {
+ $span->finish();
+ }
+ }
+ }
+}

src/Tracing/Doctrine/DBAL/TracingDriverConnection.php

@@ -58,11 +58,6 @@ final class TracingDriverConnection implements DriverConnectionInterface
  */
  private $decoratedConnection;
 
- /**
- * @var string The name of the database platform
- */
- private $databasePlatform;
-
  /**
  * @var array<string, string> The tags to attach to the span
  */
@@ -84,18 +79,19 @@ public function __construct(
  ) {
  $this->hub = $hub;
  $this->decoratedConnection = $decoratedConnection;
- $this->databasePlatform = $databasePlatform;
- $this->spanTags = $this->getSpanTags($params);
+ $this->spanTags = $this->getSpanTags($databasePlatform, $params);
  }
 
  /**
  * {@inheritdoc}
  */
  public function prepare($sql): Statement
  {
- return $this->traceFunction(self::SPAN_OP_CONN_PREPARE, $sql, function () use ($sql): Statement {
+ $statement = $this->traceFunction(self::SPAN_OP_CONN_PREPARE, $sql, function () use ($sql): Statement {
  return $this->decoratedConnection->prepare($sql);
  });
+
+ return new TracingStatement($this->hub, $statement, $sql, $this->spanTags);
  }
 
  /**
@@ -225,15 +221,16 @@ private function traceFunction(string $spanOperation, string $spanDescription, \
  /**
  * Gets a map of key-value pairs that will be set as tags of the span.
  *
- * @param array<string, mixed> $params The connection params
+ * @param string $databasePlatform The database platform
+ * @param array<string, mixed> $params The connection params
  *
  * @return array<string, string>
  *
  * @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md
  */
- private function getSpanTags(array $params): array
+ private function getSpanTags(string $databasePlatform, array $params): array
  {
- $tags = ['db.system' => $this->databasePlatform];
+ $tags = ['db.system' => $databasePlatform];
 
  if (isset($params['user'])) {
  $tags['db.user'] = $params['user'];