Skip to content

Commit 26ef1c2

Browse files
cedriczielcedricziel
authored
fix: Track attributes for new PDO subclasses & span hierarchy (open-telemetry#338)
* fix: Track attributes for new PDO subclasses * fix: guard against corner case where return value is a string * fix: maintain correct hierarchy and add a test * chore: combine implementations for attribute tracking
1 parent ea137c0 commit 26ef1c2

File tree

6 files changed

+396
-10
lines changed

6 files changed

+396
-10
lines changed

‎src/Instrumentation/PDO/ignore-by-php-version.neon.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
$ignoreErrors = [];
6+
7+
if (version_compare(PHP_VERSION, '8.4', '<')) {
8+
$ignoreErrors = [
9+
'#Call to an undefined static method PDO::connect\(\)#',
10+
'#PHPDoc tag @var for variable \$db contains unknown class PDO\\\\Sqlite#',
11+
'#Call to method createFunction\(\) on an unknown class PDO\\\\Sqlite#',
12+
'#Call to method exec\(\) on an unknown class PDO\\\\Sqlite#',
13+
'#Call to method query\(\) on an unknown class PDO\\\\Sqlite#',
14+
];
15+
} elseif (version_compare(PHP_VERSION, '8.4', '>=')) {
16+
$ignoreErrors = [
17+
'#Call to an undefined method Pdo\\\\Sqlite::createFunction\(\)#',
18+
];
19+
}
20+
21+
return [
22+
'parameters' => [
23+
'ignoreErrors' => $ignoreErrors,
24+
],
25+
];

‎src/Instrumentation/PDO/phpstan.neon.dist‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
includes:
22
- vendor/phpstan/phpstan-phpunit/extension.neon
3+
- ignore-by-php-version.neon.php
34

45
parameters:
56
tmpDir: var/cache/phpstan

‎src/Instrumentation/PDO/src/PDOInstrumentation.php‎

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,59 @@ public static function register(): void
3131
);
3232
$pdoTracker = new PDOTracker();
3333

34+
// Hook for the new PDO::connect static method
35+
if (method_exists(PDO::class, 'connect')) {
36+
hook(
37+
PDO::class,
38+
'connect',
39+
pre: static function (
40+
$object,
41+
array $params,
42+
string $class,
43+
string $function,
44+
?string $filename,
45+
?int $lineno,
46+
) use ($instrumentation) {
47+
/** @psalm-suppress ArgumentTypeCoercion */
48+
$builder = self::makeBuilder($instrumentation, 'PDO::connect', $function, $class, $filename, $lineno)
49+
->setSpanKind(SpanKind::KIND_CLIENT);
50+
51+
$parent = Context::getCurrent();
52+
$span = $builder->startSpan();
53+
Context::storage()->attach($span->storeInContext($parent));
54+
},
55+
post: static function (
56+
$object,
57+
array $params,
58+
$result,
59+
?Throwable $exception,
60+
) use ($pdoTracker) {
61+
$scope = Context::storage()->scope();
62+
if (!$scope) {
63+
return;
64+
}
65+
$span = Span::fromContext($scope->context());
66+
67+
$dsn = $params[0] ?? '';
68+
69+
// guard against PDO::connect returning a string
70+
if ($result instanceof PDO) {
71+
$attributes = $pdoTracker->trackPdoAttributes($result, $dsn);
72+
$span->setAttributes($attributes);
73+
}
74+
75+
self::end($exception);
76+
}
77+
);
78+
}
79+
3480
hook(
3581
PDO::class,
3682
'__construct',
3783
pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation) {
3884
/** @psalm-suppress ArgumentTypeCoercion */
3985
$builder = self::makeBuilder($instrumentation, 'PDO::__construct', $function, $class, $filename, $lineno)
4086
->setSpanKind(SpanKind::KIND_CLIENT);
41-
if ($class === PDO::class) {
42-
//@todo split params[0] into host + port, replace deprecated trace attribute
43-
$builder
44-
->setAttribute(TraceAttributes::SERVER_ADDRESS, $params[0] ?? 'unknown')
45-
->setAttribute(TraceAttributes::SERVER_PORT, $params[0] ?? null);
46-
}
4787
$parent = Context::getCurrent();
4888
$span = $builder->startSpan();
4989
Context::storage()->attach($span->storeInContext($parent));
@@ -213,9 +253,10 @@ public static function register(): void
213253
if ($spanContext = $pdoTracker->getSpanForPreparedStatement($statement)) {
214254
$builder->addLink($spanContext);
215255
}
256+
$parent = Context::getCurrent();
216257
$span = $builder->startSpan();
217258

218-
Context::storage()->attach($span->storeInContext(Context::getCurrent()));
259+
Context::storage()->attach($span->storeInContext($parent));
219260
},
220261
post: static function (PDOStatement $statement, array $params, mixed $retval, ?Throwable $exception) {
221262
self::end($exception);
@@ -240,9 +281,9 @@ public static function register(): void
240281
if ($spanContext = $pdoTracker->getSpanForPreparedStatement($statement)) {
241282
$builder->addLink($spanContext);
242283
}
284+
$parent = Context::getCurrent();
243285
$span = $builder->startSpan();
244-
245-
Context::storage()->attach($span->storeInContext(Context::getCurrent()));
286+
Context::storage()->attach($span->storeInContext($parent));
246287
},
247288
post: static function (PDOStatement $statement, array $params, mixed $retval, ?Throwable $exception) {
248289
self::end($exception);

‎src/Instrumentation/PDO/src/PDOTracker.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,19 +151,64 @@ private static function extractAttributesFromDSN(string $dsn): array
151151
return $attributes;
152152
}
153153

154+
// SQL Server format handling
155+
if (str_starts_with($dsn, 'sqlsrv:')) {
156+
if (preg_match('/Server=([^,;]+)(?:,([0-9]+))?/', $dsn, $serverMatches)) {
157+
$server = $serverMatches[1];
158+
if ($server !== '') {
159+
$attributes[TraceAttributes::SERVER_ADDRESS] = $server;
160+
}
161+
162+
if (isset($serverMatches[2]) && $serverMatches[2] !== '') {
163+
$attributes[TraceAttributes::SERVER_PORT] = (int) $serverMatches[2];
164+
}
165+
}
166+
167+
if (preg_match('/Database=([^;]*)/', $dsn, $dbMatches)) {
168+
$dbname = $dbMatches[1];
169+
if ($dbname !== '') {
170+
$attributes[TraceAttributes::DB_NAMESPACE] = $dbname;
171+
}
172+
}
173+
174+
return $attributes;
175+
}
176+
154177
//deprecated, no replacement at this time
155178
/*if (preg_match('/user=([^;]*)/', $dsn, $matches)) {
156179
$user = $matches[1];
157180
if ($user !== '') {
158181
$attributes[TraceAttributes::DB_USER] = $user;
159182
}
160183
}*/
184+
185+
// Extract host information
161186
if (preg_match('/host=([^;]*)/', $dsn, $matches)) {
162187
$host = $matches[1];
163188
if ($host !== '') {
164189
$attributes[TraceAttributes::SERVER_ADDRESS] = $host;
165190
}
191+
} elseif (preg_match('/mysql:([^;:]+)/', $dsn, $hostMatches)) {
192+
$host = $hostMatches[1];
193+
if ($host !== '' && $host !== 'dbname') {
194+
$attributes[TraceAttributes::SERVER_ADDRESS] = $host;
195+
}
166196
}
197+
198+
// Extract port information
199+
if (preg_match('/port=([0-9]+)/', $dsn, $portMatches)) {
200+
$port = (int) $portMatches[1];
201+
$attributes[TraceAttributes::SERVER_PORT] = $port;
202+
} elseif (preg_match('/[.0-9]+:([0-9]+)/', $dsn, $portMatches)) {
203+
// This pattern matches IP:PORT format like 127.0.0.1:3308
204+
$port = (int) $portMatches[1];
205+
$attributes[TraceAttributes::SERVER_PORT] = $port;
206+
} elseif (preg_match('/:([0-9]+)/', $dsn, $portMatches)) {
207+
$port = (int) $portMatches[1];
208+
$attributes[TraceAttributes::SERVER_PORT] = $port;
209+
}
210+
211+
// Extract database name
167212
if (preg_match('/dbname=([^;]*)/', $dsn, $matches)) {
168213
$dbname = $matches[1];
169214
if ($dbname !== '') {

‎src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php‎

Lines changed: 129 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66

77
use ArrayObject;
88
use OpenTelemetry\API\Instrumentation\Configurator;
9+
use OpenTelemetry\API\Trace\SpanInterface;
10+
use OpenTelemetry\API\Trace\SpanKind;
11+
use OpenTelemetry\Context\Context;
912
use OpenTelemetry\Context\ScopeInterface;
1013
use OpenTelemetry\SDK\Trace\ImmutableSpan;
1114
use OpenTelemetry\SDK\Trace\SpanExporter\InMemoryExporter;
@@ -18,14 +21,24 @@
1821
class PDOInstrumentationTest extends TestCase
1922
{
2023
private ScopeInterface $scope;
21-
/** @var ArrayObject<int, ImmutableSpan> */
24+
/** @var ArrayObject<array-key, mixed> */
2225
private ArrayObject $storage;
2326

2427
private function createDB(): PDO
2528
{
2629
return new PDO('sqlite::memory:');
2730
}
2831

32+
private function createDBWithNewSubclass(): PDO
33+
{
34+
if (!class_exists('Pdo\Sqlite')) {
35+
$this->markTestSkipped('Pdo\Sqlite class is not available in this PHP version');
36+
}
37+
38+
/** @psalm-suppress UndefinedMethod */
39+
return PDO::connect('sqlite::memory:');
40+
}
41+
2942
private function fillDB():string
3043
{
3144
return <<<SQL
@@ -72,6 +85,51 @@ public function test_pdo_construct(): void
7285
$this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME));
7386
}
7487

88+
/**
89+
* @psalm-suppress UndefinedClass
90+
* @psalm-suppress InvalidClass
91+
*/
92+
public function test_pdo_sqlite_subclass(): void
93+
{
94+
// skip if php version is less than 8.4
95+
if (version_compare(PHP_VERSION, '8.4', '<')) {
96+
$this->markTestSkipped('Pdo\Sqlite class is not available in this PHP version');
97+
}
98+
99+
$this->assertCount(0, $this->storage);
100+
101+
/**
102+
* Need to suppress because of different casing of the class name
103+
*
104+
* @psalm-suppress UndefinedClass
105+
* @psalm-suppress InvalidClass
106+
* @var Pdo\Sqlite $db
107+
*/
108+
$db = self::createDBWithNewSubclass();
109+
$this->assertCount(1, $this->storage);
110+
$span = $this->storage->offsetGet(0);
111+
$this->assertSame('PDO::connect', $span->getName());
112+
$this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME));
113+
114+
// Test that the subclass-specific methods work
115+
$db->createFunction('test_function', static fn ($value) => strtoupper($value));
116+
117+
// Test that standard PDO operations still work
118+
$db->exec($this->fillDB());
119+
$span = $this->storage->offsetGet(1);
120+
$this->assertSame('PDO::exec', $span->getName());
121+
$this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME));
122+
$this->assertCount(2, $this->storage);
123+
124+
// Test that the custom function works
125+
$result = $db->query("SELECT test_function('hello')")->fetchColumn();
126+
$this->assertEquals('HELLO', $result);
127+
$span = $this->storage->offsetGet(2);
128+
$this->assertSame('PDO::query', $span->getName());
129+
$this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME));
130+
$this->assertCount(3, $this->storage);
131+
}
132+
75133
public function test_constructor_exception(): void
76134
{
77135
$this->expectException(\PDOException::class);
@@ -214,4 +272,74 @@ public function test_encode_db_statement_as_utf8(): void
214272
$this->assertTrue(mb_check_encoding($span_db_exec->getAttributes()->get(TraceAttributes::DB_QUERY_TEXT), 'UTF-8'));
215273
$this->assertCount(5, $this->storage);
216274
}
275+
276+
public function test_span_hierarchy_with_pdo_operations(): void
277+
{
278+
$this->assertCount(0, $this->storage);
279+
280+
// Create a server span
281+
$tracerProvider = new TracerProvider(
282+
new SimpleSpanProcessor(
283+
new InMemoryExporter($this->storage)
284+
)
285+
);
286+
$tracer = $tracerProvider->getTracer('test');
287+
/** @var SpanInterface $serverSpan */
288+
$serverSpan = $tracer->spanBuilder('HTTP GET /api/users')
289+
->setSpanKind(SpanKind::KIND_SERVER)
290+
->startSpan();
291+
292+
// Create scope for server span
293+
$serverScope = Context::storage()->attach($serverSpan->storeInContext(Context::getCurrent()));
294+
295+
// Create an internal span (simulating business logic)
296+
/** @var SpanInterface $internalSpan */
297+
$internalSpan = $tracer->spanBuilder('processUserData')
298+
->setSpanKind(SpanKind::KIND_INTERNAL)
299+
->startSpan();
300+
301+
// Create scope for internal span
302+
$internalScope = Context::storage()->attach($internalSpan->storeInContext(Context::getCurrent()));
303+
304+
// Perform PDO operations within the internal span context
305+
$db = self::createDB();
306+
$this->assertCount(1, $this->storage); // PDO constructor span
307+
308+
// Create and populate test table
309+
$db->exec($this->fillDB());
310+
$this->assertCount(2, $this->storage); // PDO exec span
311+
312+
// Query data
313+
$stmt = $db->prepare('SELECT * FROM technology WHERE name = ?');
314+
$this->assertCount(3, $this->storage); // PDO prepare span
315+
316+
$stmt->execute(['PHP']);
317+
$this->assertCount(4, $this->storage); // PDOStatement execute span
318+
319+
$result = $stmt->fetchAll();
320+
$this->assertCount(5, $this->storage); // PDOStatement fetchAll span
321+
322+
// Verify span hierarchy
323+
/** @var ImmutableSpan $pdoSpan */
324+
$pdoSpan = $this->storage->offsetGet(0);
325+
/** @var ImmutableSpan $execSpan */
326+
$execSpan = $this->storage->offsetGet(1);
327+
/** @var ImmutableSpan $prepareSpan */
328+
$prepareSpan = $this->storage->offsetGet(2);
329+
/** @var ImmutableSpan $executeSpan */
330+
$executeSpan = $this->storage->offsetGet(3);
331+
/** @var ImmutableSpan $fetchAllSpan */
332+
$fetchAllSpan = $this->storage->offsetGet(4);
333+
334+
// All PDO spans should be children of the internal span
335+
$this->assertEquals($internalSpan->getContext()->getSpanId(), $pdoSpan->getParentSpanId());
336+
$this->assertEquals($internalSpan->getContext()->getSpanId(), $execSpan->getParentSpanId());
337+
$this->assertEquals($internalSpan->getContext()->getSpanId(), $prepareSpan->getParentSpanId());
338+
$this->assertEquals($internalSpan->getContext()->getSpanId(), $executeSpan->getParentSpanId());
339+
$this->assertEquals($internalSpan->getContext()->getSpanId(), $fetchAllSpan->getParentSpanId());
340+
341+
// Detach scopes
342+
$internalScope->detach();
343+
$serverScope->detach();
344+
}
217345
}

0 commit comments

Comments
 (0)