diff --git a/src/Instrumentation/PDO/ignore-by-php-version.neon.php b/src/Instrumentation/PDO/ignore-by-php-version.neon.php new file mode 100644 index 000000000..56e3bedb2 --- /dev/null +++ b/src/Instrumentation/PDO/ignore-by-php-version.neon.php @@ -0,0 +1,25 @@ +=')) { + $ignoreErrors = [ + '#Call to an undefined method Pdo\\\\Sqlite::createFunction\(\)#', + ]; +} + +return [ + 'parameters' => [ + 'ignoreErrors' => $ignoreErrors, + ], +]; diff --git a/src/Instrumentation/PDO/phpstan.neon.dist b/src/Instrumentation/PDO/phpstan.neon.dist index ed94c13da..0a1ee924d 100644 --- a/src/Instrumentation/PDO/phpstan.neon.dist +++ b/src/Instrumentation/PDO/phpstan.neon.dist @@ -1,5 +1,6 @@ includes: - vendor/phpstan/phpstan-phpunit/extension.neon + - ignore-by-php-version.neon.php parameters: tmpDir: var/cache/phpstan diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index c27a338a5..d4d9574cb 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -31,6 +31,52 @@ public static function register(): void ); $pdoTracker = new PDOTracker(); + // Hook for the new PDO::connect static method + if (method_exists(PDO::class, 'connect')) { + hook( + PDO::class, + 'connect', + pre: static function ( + $object, + array $params, + string $class, + string $function, + ?string $filename, + ?int $lineno, + ) use ($instrumentation) { + /** @psalm-suppress ArgumentTypeCoercion */ + $builder = self::makeBuilder($instrumentation, 'PDO::connect', $function, $class, $filename, $lineno) + ->setSpanKind(SpanKind::KIND_CLIENT); + + $parent = Context::getCurrent(); + $span = $builder->startSpan(); + Context::storage()->attach($span->storeInContext($parent)); + }, + post: static function ( + $object, + array $params, + $result, + ?Throwable $exception, + ) use ($pdoTracker) { + $scope = Context::storage()->scope(); + if (!$scope) { + return; + } + $span = Span::fromContext($scope->context()); + + $dsn = $params[0] ?? ''; + + // guard against PDO::connect returning a string + if ($result instanceof PDO) { + $attributes = $pdoTracker->trackPdoAttributes($result, $dsn); + $span->setAttributes($attributes); + } + + self::end($exception); + } + ); + } + hook( PDO::class, '__construct', @@ -38,12 +84,6 @@ public static function register(): void /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::__construct', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); - if ($class === PDO::class) { - //@todo split params[0] into host + port, replace deprecated trace attribute - $builder - ->setAttribute(TraceAttributes::SERVER_ADDRESS, $params[0] ?? 'unknown') - ->setAttribute(TraceAttributes::SERVER_PORT, $params[0] ?? null); - } $parent = Context::getCurrent(); $span = $builder->startSpan(); Context::storage()->attach($span->storeInContext($parent)); @@ -213,9 +253,10 @@ public static function register(): void if ($spanContext = $pdoTracker->getSpanForPreparedStatement($statement)) { $builder->addLink($spanContext); } + $parent = Context::getCurrent(); $span = $builder->startSpan(); - Context::storage()->attach($span->storeInContext(Context::getCurrent())); + Context::storage()->attach($span->storeInContext($parent)); }, post: static function (PDOStatement $statement, array $params, mixed $retval, ?Throwable $exception) { self::end($exception); @@ -240,9 +281,9 @@ public static function register(): void if ($spanContext = $pdoTracker->getSpanForPreparedStatement($statement)) { $builder->addLink($spanContext); } + $parent = Context::getCurrent(); $span = $builder->startSpan(); - - Context::storage()->attach($span->storeInContext(Context::getCurrent())); + Context::storage()->attach($span->storeInContext($parent)); }, post: static function (PDOStatement $statement, array $params, mixed $retval, ?Throwable $exception) { self::end($exception); diff --git a/src/Instrumentation/PDO/src/PDOTracker.php b/src/Instrumentation/PDO/src/PDOTracker.php index acfa9cb8f..4753a0baa 100644 --- a/src/Instrumentation/PDO/src/PDOTracker.php +++ b/src/Instrumentation/PDO/src/PDOTracker.php @@ -151,6 +151,29 @@ private static function extractAttributesFromDSN(string $dsn): array return $attributes; } + // SQL Server format handling + if (str_starts_with($dsn, 'sqlsrv:')) { + if (preg_match('/Server=([^,;]+)(?:,([0-9]+))?/', $dsn, $serverMatches)) { + $server = $serverMatches[1]; + if ($server !== '') { + $attributes[TraceAttributes::SERVER_ADDRESS] = $server; + } + + if (isset($serverMatches[2]) && $serverMatches[2] !== '') { + $attributes[TraceAttributes::SERVER_PORT] = (int) $serverMatches[2]; + } + } + + if (preg_match('/Database=([^;]*)/', $dsn, $dbMatches)) { + $dbname = $dbMatches[1]; + if ($dbname !== '') { + $attributes[TraceAttributes::DB_NAMESPACE] = $dbname; + } + } + + return $attributes; + } + //deprecated, no replacement at this time /*if (preg_match('/user=([^;]*)/', $dsn, $matches)) { $user = $matches[1]; @@ -158,12 +181,34 @@ private static function extractAttributesFromDSN(string $dsn): array $attributes[TraceAttributes::DB_USER] = $user; } }*/ + + // Extract host information if (preg_match('/host=([^;]*)/', $dsn, $matches)) { $host = $matches[1]; if ($host !== '') { $attributes[TraceAttributes::SERVER_ADDRESS] = $host; } + } elseif (preg_match('/mysql:([^;:]+)/', $dsn, $hostMatches)) { + $host = $hostMatches[1]; + if ($host !== '' && $host !== 'dbname') { + $attributes[TraceAttributes::SERVER_ADDRESS] = $host; + } } + + // Extract port information + if (preg_match('/port=([0-9]+)/', $dsn, $portMatches)) { + $port = (int) $portMatches[1]; + $attributes[TraceAttributes::SERVER_PORT] = $port; + } elseif (preg_match('/[.0-9]+:([0-9]+)/', $dsn, $portMatches)) { + // This pattern matches IP:PORT format like 127.0.0.1:3308 + $port = (int) $portMatches[1]; + $attributes[TraceAttributes::SERVER_PORT] = $port; + } elseif (preg_match('/:([0-9]+)/', $dsn, $portMatches)) { + $port = (int) $portMatches[1]; + $attributes[TraceAttributes::SERVER_PORT] = $port; + } + + // Extract database name if (preg_match('/dbname=([^;]*)/', $dsn, $matches)) { $dbname = $matches[1]; if ($dbname !== '') { diff --git a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php index ba43e304a..7248cec69 100644 --- a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php +++ b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php @@ -6,6 +6,9 @@ use ArrayObject; use OpenTelemetry\API\Instrumentation\Configurator; +use OpenTelemetry\API\Trace\SpanInterface; +use OpenTelemetry\API\Trace\SpanKind; +use OpenTelemetry\Context\Context; use OpenTelemetry\Context\ScopeInterface; use OpenTelemetry\SDK\Trace\ImmutableSpan; use OpenTelemetry\SDK\Trace\SpanExporter\InMemoryExporter; @@ -18,7 +21,7 @@ class PDOInstrumentationTest extends TestCase { private ScopeInterface $scope; - /** @var ArrayObject */ + /** @var ArrayObject */ private ArrayObject $storage; private function createDB(): PDO @@ -26,6 +29,16 @@ private function createDB(): PDO return new PDO('sqlite::memory:'); } + private function createDBWithNewSubclass(): PDO + { + if (!class_exists('Pdo\Sqlite')) { + $this->markTestSkipped('Pdo\Sqlite class is not available in this PHP version'); + } + + /** @psalm-suppress UndefinedMethod */ + return PDO::connect('sqlite::memory:'); + } + private function fillDB():string { return <<assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); } + /** + * @psalm-suppress UndefinedClass + * @psalm-suppress InvalidClass + */ + public function test_pdo_sqlite_subclass(): void + { + // skip if php version is less than 8.4 + if (version_compare(PHP_VERSION, '8.4', '<')) { + $this->markTestSkipped('Pdo\Sqlite class is not available in this PHP version'); + } + + $this->assertCount(0, $this->storage); + + /** + * Need to suppress because of different casing of the class name + * + * @psalm-suppress UndefinedClass + * @psalm-suppress InvalidClass + * @var Pdo\Sqlite $db + */ + $db = self::createDBWithNewSubclass(); + $this->assertCount(1, $this->storage); + $span = $this->storage->offsetGet(0); + $this->assertSame('PDO::connect', $span->getName()); + $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + + // Test that the subclass-specific methods work + $db->createFunction('test_function', static fn ($value) => strtoupper($value)); + + // Test that standard PDO operations still work + $db->exec($this->fillDB()); + $span = $this->storage->offsetGet(1); + $this->assertSame('PDO::exec', $span->getName()); + $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertCount(2, $this->storage); + + // Test that the custom function works + $result = $db->query("SELECT test_function('hello')")->fetchColumn(); + $this->assertEquals('HELLO', $result); + $span = $this->storage->offsetGet(2); + $this->assertSame('PDO::query', $span->getName()); + $this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM_NAME)); + $this->assertCount(3, $this->storage); + } + public function test_constructor_exception(): void { $this->expectException(\PDOException::class); @@ -214,4 +272,74 @@ public function test_encode_db_statement_as_utf8(): void $this->assertTrue(mb_check_encoding($span_db_exec->getAttributes()->get(TraceAttributes::DB_QUERY_TEXT), 'UTF-8')); $this->assertCount(5, $this->storage); } + + public function test_span_hierarchy_with_pdo_operations(): void + { + $this->assertCount(0, $this->storage); + + // Create a server span + $tracerProvider = new TracerProvider( + new SimpleSpanProcessor( + new InMemoryExporter($this->storage) + ) + ); + $tracer = $tracerProvider->getTracer('test'); + /** @var SpanInterface $serverSpan */ + $serverSpan = $tracer->spanBuilder('HTTP GET /api/users') + ->setSpanKind(SpanKind::KIND_SERVER) + ->startSpan(); + + // Create scope for server span + $serverScope = Context::storage()->attach($serverSpan->storeInContext(Context::getCurrent())); + + // Create an internal span (simulating business logic) + /** @var SpanInterface $internalSpan */ + $internalSpan = $tracer->spanBuilder('processUserData') + ->setSpanKind(SpanKind::KIND_INTERNAL) + ->startSpan(); + + // Create scope for internal span + $internalScope = Context::storage()->attach($internalSpan->storeInContext(Context::getCurrent())); + + // Perform PDO operations within the internal span context + $db = self::createDB(); + $this->assertCount(1, $this->storage); // PDO constructor span + + // Create and populate test table + $db->exec($this->fillDB()); + $this->assertCount(2, $this->storage); // PDO exec span + + // Query data + $stmt = $db->prepare('SELECT * FROM technology WHERE name = ?'); + $this->assertCount(3, $this->storage); // PDO prepare span + + $stmt->execute(['PHP']); + $this->assertCount(4, $this->storage); // PDOStatement execute span + + $result = $stmt->fetchAll(); + $this->assertCount(5, $this->storage); // PDOStatement fetchAll span + + // Verify span hierarchy + /** @var ImmutableSpan $pdoSpan */ + $pdoSpan = $this->storage->offsetGet(0); + /** @var ImmutableSpan $execSpan */ + $execSpan = $this->storage->offsetGet(1); + /** @var ImmutableSpan $prepareSpan */ + $prepareSpan = $this->storage->offsetGet(2); + /** @var ImmutableSpan $executeSpan */ + $executeSpan = $this->storage->offsetGet(3); + /** @var ImmutableSpan $fetchAllSpan */ + $fetchAllSpan = $this->storage->offsetGet(4); + + // All PDO spans should be children of the internal span + $this->assertEquals($internalSpan->getContext()->getSpanId(), $pdoSpan->getParentSpanId()); + $this->assertEquals($internalSpan->getContext()->getSpanId(), $execSpan->getParentSpanId()); + $this->assertEquals($internalSpan->getContext()->getSpanId(), $prepareSpan->getParentSpanId()); + $this->assertEquals($internalSpan->getContext()->getSpanId(), $executeSpan->getParentSpanId()); + $this->assertEquals($internalSpan->getContext()->getSpanId(), $fetchAllSpan->getParentSpanId()); + + // Detach scopes + $internalScope->detach(); + $serverScope->detach(); + } } diff --git a/src/Instrumentation/PDO/tests/Unit/PDOTrackerTest.php b/src/Instrumentation/PDO/tests/Unit/PDOTrackerTest.php new file mode 100644 index 000000000..5049f2039 --- /dev/null +++ b/src/Instrumentation/PDO/tests/Unit/PDOTrackerTest.php @@ -0,0 +1,146 @@ +setAccessible(true); + + $result = $reflectionMethod->invoke(null, $dsn); + + foreach ($expectedAttributes as $key => $value) { + $this->assertArrayHasKey($key, $result); + $this->assertSame($value, $result[$key]); + } + } + + /** + * Data provider for testExtractAttributesFromDSN + * + * @return array}> + */ + public function dsnProvider(): array + { + return [ + 'standard format with host and port' => [ + 'mysql:host=localhost;port=3306;dbname=test', + [ + TraceAttributes::SERVER_ADDRESS => 'localhost', + TraceAttributes::SERVER_PORT => 3306, + TraceAttributes::DB_NAMESPACE => 'test', + ], + ], + 'format with host but no port' => [ + 'mysql:host=localhost;dbname=test', + [ + TraceAttributes::SERVER_ADDRESS => 'localhost', + TraceAttributes::DB_NAMESPACE => 'test', + ], + ], + 'format with port but using alternative format' => [ + 'mysql:localhost:3306;dbname=test', + [ + TraceAttributes::SERVER_ADDRESS => 'localhost', + TraceAttributes::SERVER_PORT => 3306, + TraceAttributes::DB_NAMESPACE => 'test', + ], + ], + 'format with neither host parameter nor port parameter' => [ + 'mysql:localhost;dbname=test', + [ + TraceAttributes::SERVER_ADDRESS => 'localhost', + TraceAttributes::DB_NAMESPACE => 'test', + ], + ], + 'format with IP address as host' => [ + 'mysql:host=127.0.0.1;port=3306;dbname=test', + [ + TraceAttributes::SERVER_ADDRESS => '127.0.0.1', + TraceAttributes::SERVER_PORT => 3306, + TraceAttributes::DB_NAMESPACE => 'test', + ], + ], + 'format with domain name as host' => [ + 'mysql:host=example.com;port=3306;dbname=test', + [ + TraceAttributes::SERVER_ADDRESS => 'example.com', + TraceAttributes::SERVER_PORT => 3306, + TraceAttributes::DB_NAMESPACE => 'test', + ], + ], + 'PostgreSQL format' => [ + 'pgsql:host=localhost;port=5432;dbname=test', + [ + TraceAttributes::SERVER_ADDRESS => 'localhost', + TraceAttributes::SERVER_PORT => 5432, + TraceAttributes::DB_NAMESPACE => 'test', + ], + ], + 'SQLite format' => [ + 'sqlite:/path/to/database.sqlite', + [ + TraceAttributes::DB_SYSTEM_NAME => 'sqlite', + TraceAttributes::DB_NAMESPACE => '/path/to/database.sqlite', + ], + ], + 'SQLite in-memory format' => [ + 'sqlite::memory:', + [ + TraceAttributes::DB_SYSTEM_NAME => 'sqlite', + TraceAttributes::DB_NAMESPACE => 'memory', + ], + ], + 'Oracle format' => [ + 'oci:host=localhost;port=1521;dbname=test', + [ + TraceAttributes::SERVER_ADDRESS => 'localhost', + TraceAttributes::SERVER_PORT => 1521, + TraceAttributes::DB_NAMESPACE => 'test', + ], + ], + 'SQL Server format' => [ + 'sqlsrv:Server=localhost,1433;Database=test', + [ + TraceAttributes::SERVER_ADDRESS => 'localhost', + TraceAttributes::SERVER_PORT => 1433, + TraceAttributes::DB_NAMESPACE => 'test', + ], + ], + 'MySQL format with host in DSN prefix' => [ + 'mysql:dbname=test;charset=utf8', + [ + TraceAttributes::DB_NAMESPACE => 'test', + ], + ], + 'MySQL format with host in DSN prefix and port' => [ + 'mysql:dbname=test;port=3307;charset=utf8', + [ + TraceAttributes::SERVER_PORT => 3307, + TraceAttributes::DB_NAMESPACE => 'test', + ], + ], + 'MySQL format with host in DSN prefix and colon port' => [ + 'mysql:127.0.0.1:3308;dbname=test', + [ + TraceAttributes::SERVER_ADDRESS => '127.0.0.1', + TraceAttributes::SERVER_PORT => 3308, + TraceAttributes::DB_NAMESPACE => 'test', + ], + ], + ]; + } +}