Skip to content

Commit d9fb860

Browse files
ste93cryste93cry
committed
Fix missing instrumentation of the Statement::execute() method of Doctrine DBAL
1 parent dc611e5 commit d9fb860

13 files changed

+721
-34
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## Unreleased
44

55
- Fix return type for `TracingDriver::getDatabase()` method (#541)
6+
- Fix missing instrumentation of the `Statement::execute()` method of Doctrine DBAL (#548)
67

78
## 4.2.0 (2021-08-12)
89

‎phpstan-baseline.neon

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,3 +280,103 @@ parameters:
280280
count: 1
281281
path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php
282282

283+
-
284+
message: "#^Call to an undefined method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatement\\:\\:closeCursor\\(\\)\\.$#"
285+
count: 1
286+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
287+
288+
-
289+
message: "#^Call to an undefined method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatement\\:\\:columnCount\\(\\)\\.$#"
290+
count: 1
291+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
292+
293+
-
294+
message: "#^Call to an undefined method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatement\\:\\:errorCode\\(\\)\\.$#"
295+
count: 1
296+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
297+
298+
-
299+
message: "#^Call to an undefined method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatement\\:\\:errorInfo\\(\\)\\.$#"
300+
count: 1
301+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
302+
303+
-
304+
message: "#^Call to an undefined method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatement\\:\\:fetch\\(\\)\\.$#"
305+
count: 1
306+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
307+
308+
-
309+
message: "#^Call to an undefined method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatement\\:\\:fetchAll\\(\\)\\.$#"
310+
count: 1
311+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
312+
313+
-
314+
message: "#^Call to an undefined method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatement\\:\\:fetchColumn\\(\\)\\.$#"
315+
count: 1
316+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
317+
318+
-
319+
message: "#^Call to an undefined method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatement\\:\\:getIterator\\(\\)\\.$#"
320+
count: 1
321+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
322+
323+
-
324+
message: "#^Call to an undefined method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatement\\:\\:rowCount\\(\\)\\.$#"
325+
count: 1
326+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
327+
328+
-
329+
message: "#^Call to an undefined method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingStatement\\:\\:setFetchMode\\(\\)\\.$#"
330+
count: 1
331+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
332+
333+
-
334+
message: "#^Call to method PHPUnit\\\\Framework\\\\Assert\\:\\:assertTrue\\(\\) with Doctrine\\\\DBAL\\\\Driver\\\\Result will always evaluate to false\\.$#"
335+
count: 2
336+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
337+
338+
-
339+
message: "#^Trying to mock an undefined method closeCursor\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
340+
count: 1
341+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
342+
343+
-
344+
message: "#^Trying to mock an undefined method columnCount\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
345+
count: 1
346+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
347+
348+
-
349+
message: "#^Trying to mock an undefined method errorCode\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
350+
count: 1
351+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
352+
353+
-
354+
message: "#^Trying to mock an undefined method errorInfo\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
355+
count: 1
356+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
357+
358+
-
359+
message: "#^Trying to mock an undefined method fetch\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
360+
count: 1
361+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
362+
363+
-
364+
message: "#^Trying to mock an undefined method fetchAll\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
365+
count: 1
366+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
367+
368+
-
369+
message: "#^Trying to mock an undefined method fetchColumn\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
370+
count: 1
371+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
372+
373+
-
374+
message: "#^Trying to mock an undefined method rowCount\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
375+
count: 1
376+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
377+
378+
-
379+
message: "#^Trying to mock an undefined method setFetchMode\\(\\) on class Doctrine\\\\DBAL\\\\Driver\\\\Statement\\.$#"
380+
count: 1
381+
path: tests/Tracing/Doctrine/DBAL/TracingStatementTest.php
382+

‎phpstan.neon

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ parameters:
66
paths:
77
- src
88
- tests
9-
excludes_analyse:
9+
excludePaths:
1010
- tests/End2End/App
11+
- src/Tracing/Doctrine/DBAL/TracingStatement.php
1112
dynamicConstantNames:
1213
- Symfony\Component\HttpKernel\Kernel::VERSION
1314
- Symfony\Component\HttpKernel\Kernel::VERSION_ID
14-
- Doctrine\DBAL\Version::VERSION
1515
stubFiles:
1616
- tests/Stubs/Profile.phpstub

‎psalm-baseline.xml

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<?xml version="1.0" encoding="UTF-8"?>
2-
<files psalm-version="4.8.1@f73f2299dbc59a3e6c4d66cff4605176e728ee69">
2+
<files psalm-version="4.9.3@4c262932602b9bbab5020863d1eb22d49de0dbf4">
33
<file src="src/EventListener/ConsoleCommandListener.php">
44
<InvalidExtendClass occurrences="1">
55
<code>ConsoleListener</code>
@@ -8,4 +8,38 @@
88
<code>public function __construct(HubInterface $hub, bool $captureErrors = true)</code>
99
</MethodSignatureMismatch>
1010
</file>
11+
<file src="src/Tracing/Doctrine/DBAL/TracingStatementForV2.php">
12+
<InvalidReturnStatement occurrences="2">
13+
<code>$this-&gt;decoratedStatement</code>
14+
<code>$this-&gt;traceFunction($spanContext, [$this-&gt;decoratedStatement, 'execute'], $params)</code>
15+
</InvalidReturnStatement>
16+
<InvalidReturnType occurrences="2">
17+
<code>\Traversable</code>
18+
<code>bool</code>
19+
</InvalidReturnType>
20+
<MethodSignatureMismatch occurrences="1">
21+
<code>TracingStatementForV2</code>
22+
</MethodSignatureMismatch>
23+
<UndefinedInterfaceMethod occurrences="9">
24+
<code>closeCursor</code>
25+
<code>columnCount</code>
26+
<code>errorCode</code>
27+
<code>errorInfo</code>
28+
<code>fetch</code>
29+
<code>fetchAll</code>
30+
<code>fetchColumn</code>
31+
<code>rowCount</code>
32+
<code>setFetchMode</code>
33+
</UndefinedInterfaceMethod>
34+
</file>
35+
<file src="src/aliases.php">
36+
<UndefinedClass occurrences="6">
37+
<code>FilterControllerEvent</code>
38+
<code>FilterResponseEvent</code>
39+
<code>GetResponseEvent</code>
40+
<code>GetResponseEvent</code>
41+
<code>GetResponseForExceptionEvent</code>
42+
<code>PostResponseEvent</code>
43+
</UndefinedClass>
44+
</file>
1145
</files>

‎src/DependencyInjection/Compiler/DbalTracingPass.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
namespace Sentry\SentryBundle\DependencyInjection\Compiler;
66

7-
use Doctrine\DBAL\Driver\ResultStatement;
87
use Doctrine\DBAL\Result;
98
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator;
109
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware;
@@ -57,7 +56,7 @@ public function process(ContainerBuilder $container): void
5756
throw new \InvalidArgumentException(sprintf('The Doctrine connection "%s" does not exists and cannot be instrumented.', $connectionName));
5857
}
5958

60-
if (!interface_exists(ResultStatement::class)) {
59+
if (class_exists(Result::class)) {
6160
$this->configureConnectionForDoctrineDBALVersion3($container, $connectionName);
6261
} else {
6362
$this->configureConnectionForDoctrineDBALVersion2($container, $connectionName);

‎src/Tracing/Doctrine/DBAL/AbstractTracingStatement.php

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Sentry\SentryBundle\Tracing\Doctrine\DBAL;
6+
7+
use Doctrine\DBAL\Driver\Statement;
8+
use Sentry\State\HubInterface;
9+
use Sentry\Tracing\Span;
10+
use Sentry\Tracing\SpanContext;
11+
12+
abstract class AbstractTracingStatement
13+
{
14+
/**
15+
* @internal
16+
*/
17+
public const SPAN_OP_STMT_EXECUTE = 'sql.stmt.execute';
18+
19+
/**
20+
* @var HubInterface The current hub
21+
*/
22+
protected $hub;
23+
24+
/**
25+
* @var Statement The decorated statement
26+
*/
27+
protected $decoratedStatement;
28+
29+
/**
30+
* @var string The SQL query executed by the decorated statement
31+
*/
32+
protected $sqlQuery;
33+
34+
/**
35+
* @var array<string, string> The span tags
36+
*/
37+
protected $spanTags;
38+
39+
/**
40+
* Constructor.
41+
*
42+
* @param HubInterface $hub The current hub
43+
* @param Statement $decoratedStatement The decorated statement
44+
* @param string $sqlQuery The SQL query executed by the decorated statement
45+
* @param array<string, string> $spanTags The span tags
46+
*/
47+
public function __construct(HubInterface $hub, Statement $decoratedStatement, string $sqlQuery, array $spanTags)
48+
{
49+
$this->hub = $hub;
50+
$this->decoratedStatement = $decoratedStatement;
51+
$this->sqlQuery = $sqlQuery;
52+
$this->spanTags = $spanTags;
53+
}
54+
55+
/**
56+
* Calls the given callback by passing to it the specified arguments and
57+
* wrapping its execution into a child {@see Span} of the current one.
58+
*
59+
* @param callable $callback The function to call
60+
* @param mixed ...$args The arguments to pass to the callback
61+
*
62+
* @phpstan-template T
63+
*
64+
* @phpstan-param callable(mixed...): T $callback
65+
*
66+
* @phpstan-return T
67+
*/
68+
protected function traceFunction(SpanContext $spanContext, callable $callback, ...$args)
69+
{
70+
$span = $this->hub->getSpan();
71+
72+
if (null !== $span) {
73+
$span = $span->startChild($spanContext);
74+
}
75+
76+
try {
77+
return $callback(...$args);
78+
} finally {
79+
if (null !== $span) {
80+
$span->finish();
81+
}
82+
}
83+
}
84+
}

‎src/Tracing/Doctrine/DBAL/TracingDriverConnection.php

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,6 @@ final class TracingDriverConnection implements DriverConnectionInterface
5858
*/
5959
private $decoratedConnection;
6060

61-
/**
62-
* @var string The name of the database platform
63-
*/
64-
private $databasePlatform;
65-
6661
/**
6762
* @var array<string, string> The tags to attach to the span
6863
*/
@@ -84,18 +79,19 @@ public function __construct(
8479
) {
8580
$this->hub = $hub;
8681
$this->decoratedConnection = $decoratedConnection;
87-
$this->databasePlatform = $databasePlatform;
88-
$this->spanTags = $this->getSpanTags($params);
82+
$this->spanTags = $this->getSpanTags($databasePlatform, $params);
8983
}
9084

9185
/**
9286
* {@inheritdoc}
9387
*/
9488
public function prepare($sql): Statement
9589
{
96-
return $this->traceFunction(self::SPAN_OP_CONN_PREPARE, $sql, function () use ($sql): Statement {
90+
$statement = $this->traceFunction(self::SPAN_OP_CONN_PREPARE, $sql, function () use ($sql): Statement {
9791
return $this->decoratedConnection->prepare($sql);
9892
});
93+
94+
return new TracingStatement($this->hub, $statement, $sql, $this->spanTags);
9995
}
10096

10197
/**
@@ -225,15 +221,16 @@ private function traceFunction(string $spanOperation, string $spanDescription, \
225221
/**
226222
* Gets a map of key-value pairs that will be set as tags of the span.
227223
*
228-
* @param array<string, mixed> $params The connection params
224+
* @param string $databasePlatform The database platform
225+
* @param array<string, mixed> $params The connection params
229226
*
230227
* @return array<string, string>
231228
*
232229
* @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md
233230
*/
234-
private function getSpanTags(array $params): array
231+
private function getSpanTags(string $databasePlatform, array $params): array
235232
{
236-
$tags = ['db.system' => $this->databasePlatform];
233+
$tags = ['db.system' => $databasePlatform];
237234

238235
if (isset($params['user'])) {
239236
$tags['db.user'] = $params['user'];

0 commit comments

Comments
 (0)