Skip to content

Commit de8cac6

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

File tree

11 files changed

+662
-34
lines changed

11 files changed

+662
-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: 11 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,14 @@
88
<code>public function __construct(HubInterface $hub, bool $captureErrors = true)</code>
99
</MethodSignatureMismatch>
1010
</file>
11+
<file src="src/aliases.php">
12+
<UndefinedClass occurrences="6">
13+
<code>FilterControllerEvent</code>
14+
<code>FilterResponseEvent</code>
15+
<code>GetResponseEvent</code>
16+
<code>GetResponseEvent</code>
17+
<code>GetResponseForExceptionEvent</code>
18+
<code>PostResponseEvent</code>
19+
</UndefinedClass>
20+
</file>
1121
</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: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
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 Doctrine\DBAL\ParameterType;
9+
use Sentry\State\HubInterface;
10+
use Sentry\Tracing\Span;
11+
use Sentry\Tracing\SpanContext;
12+
13+
abstract class AbstractTracingStatement implements Statement
14+
{
15+
/**
16+
* @internal
17+
*/
18+
public const SPAN_OP_STMT_EXECUTE = 'sql.stmt.execute';
19+
20+
/**
21+
* @var HubInterface The current hub
22+
*/
23+
protected $hub;
24+
25+
/**
26+
* @var Statement The decorated statement
27+
*/
28+
protected $decoratedStatement;
29+
30+
/**
31+
* @var string The SQL query executed by the decorated statement
32+
*/
33+
protected $sqlQuery;
34+
35+
/**
36+
* @var array<string, string> The span tags
37+
*/
38+
protected $spanTags;
39+
40+
/**
41+
* Constructor.
42+
*
43+
* @param HubInterface $hub The current hub
44+
* @param Statement $decoratedStatement The decorated statement
45+
* @param string $sqlQuery The SQL query executed by the decorated statement
46+
* @param array<string, string> $spanTags The span tags
47+
*/
48+
public function __construct(HubInterface $hub, Statement $decoratedStatement, string $sqlQuery, array $spanTags)
49+
{
50+
$this->hub = $hub;
51+
$this->decoratedStatement = $decoratedStatement;
52+
$this->sqlQuery = $sqlQuery;
53+
$this->spanTags = $spanTags;
54+
}
55+
56+
/**
57+
* {@inheritdoc}
58+
*/
59+
public function bindValue($param, $value, $type = ParameterType::STRING): bool
60+
{
61+
return $this->decoratedStatement->bindValue($param, $value, $type);
62+
}
63+
64+
/**
65+
* {@inheritdoc}
66+
*/
67+
public function bindParam($param, &$variable, $type = ParameterType::STRING, $length = null): bool
68+
{
69+
return $this->decoratedStatement->bindParam($param, $variable, $type, $length);
70+
}
71+
72+
/**
73+
* Calls the given callback by passing to it the specified arguments and
74+
* wrapping its execution into a child {@see Span} of the current one.
75+
*
76+
* @param string $spanOperation The operation to set on the span
77+
* @param string $spanDescription The description to set on the span
78+
* @param array<string, string> $spanTags The tags to set on the span
79+
* @param callable $callback The function to call
80+
* @param mixed ...$args The arguments to pass to the callback
81+
*
82+
* @phpstan-template T
83+
*
84+
* @phpstan-param callable(mixed...): T $callback
85+
*
86+
* @phpstan-return T
87+
*/
88+
protected function traceFunction(string $spanOperation, string $spanDescription, array $spanTags, callable $callback, ...$args)
89+
{
90+
$span = $this->hub->getSpan();
91+
92+
if (null !== $span) {
93+
$spanContext = new SpanContext();
94+
$spanContext->setOp($spanOperation);
95+
$spanContext->setDescription($spanDescription);
96+
$spanContext->setTags($spanTags);
97+
98+
$span = $span->startChild($spanContext);
99+
}
100+
101+
try {
102+
return $callback(...$args);
103+
} finally {
104+
if (null !== $span) {
105+
$span->finish();
106+
}
107+
}
108+
}
109+
}

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