Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .github/workflows/php.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ jobs:

- project: 'Instrumentation/Curl'
php-version: 8.1
- project: 'Instrumentation/Doctrine'
php-version: 8.1
- project: 'Instrumentation/ExtAmqp'
php-version: 8.1
- project: 'Instrumentation/ExtRdKafka'
Expand Down
2 changes: 1 addition & 1 deletion src/Instrumentation/Doctrine/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"minimum-stability": "dev",
"prefer-stable": true,
"require": {
"php": "^8.2",
"php": "^8.1",
"ext-opentelemetry": "*",
"doctrine/dbal": "^3 || ^4 || ^5",
"open-telemetry/api": "^1.0",
Expand Down
58 changes: 39 additions & 19 deletions src/Instrumentation/Doctrine/src/AttributesResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ public static function get(string $attributeName, array $params): string|int|nul
/**
* Resolve attribute `server.address`
*/
private static function getServerAddress(array $params): string
private static function getServerAddress(array $params): ?string
{
return $params[1][0]['host'] ?? 'unknown';
return $params[1][0]['host'] ?? null;
}

/**
Expand Down Expand Up @@ -107,24 +107,57 @@ private static function getDbCollectionName(array $params): string

/**
* Resolve attribute `db.query.text`
* No sanitization is implemented because implicitly the query is expected to be expressed as a preparated statement
* No sanitization is implemented because implicitly the query is expected to be expressed as a prepared statement
* which happen automatically in Doctrine if parameters are bound to the query.
*/
private static function getDbQueryText(array $params): string
{
return $params[1][0] ?? 'undefined';
}

private static function getDbNamespace(array $params): string
private static function getDbNamespace(array $params): ?string
{
return $params[1][0]['dbname'] ?? 'unknown';
return $params[1][0]['dbname'] ?? null;
}

public static function getTarget(array $params): ?string
{
$query = $params[0] ?? null;

if (!$query) {
return null;
}

// Fetch target name
$matches = [];
preg_match_all('/( from| into| update| join)\s*([a-zA-Z0-9`"[\]_]+)/i', $query, $matches);

$targetName = null;
if ($matches !== []) {
$targetName = $matches[2][0] ?? null;
}
if ($targetName === null) {
return null;
}
//strip quotes and backticks from the target name
$targetName = str_replace(['`', '"', '[', ']'], '', $targetName);

return $targetName;
}

/**
* Resolve attribute `db.query.summary`
* See https://opentelemetry.io/docs/specs/semconv/database/database-spans/#generating-a-summary-of-the-query-text
*/
public static function getDbQuerySummary(array $params): string
{
$operationName = self::getDbOperationName($params);
$targetName = self::getTarget($params);

return $operationName . ($targetName ? ' ' . $targetName : '');
}

public static function getDbOperationName(array $params): string
{
$query = $params[0] ?? null;

Expand All @@ -136,19 +169,6 @@ public static function getDbQuerySummary(array $params): string
$operationName = explode(' ', $query);
$operationName = $operationName[0];

// Fetch target name
$matches = [];
preg_match_all('/( from| into| update| join)\s*([a-zA-Z0-9`"[\]_]+)/i', $query, $matches);

$targetName = null;
if (strtolower($operationName) == 'select') {
if ($matches[2]) {
$targetName = implode(' ', $matches[2]);
}
} elseif ($matches) {
$targetName = $matches[2][0] ?? '';
}

return $operationName . ($targetName ? ' ' . $targetName : '');
return $operationName;
}
}
70 changes: 54 additions & 16 deletions src/Instrumentation/Doctrine/src/DoctrineInstrumentation.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@

namespace OpenTelemetry\Contrib\Instrumentation\Doctrine;

use Doctrine\DBAL\Driver\Result as ResultInterface;
use Doctrine\DBAL\Driver\Statement;
use OpenTelemetry\API\Instrumentation\CachedInstrumentation;
use OpenTelemetry\API\Trace\Span;
use OpenTelemetry\API\Trace\SpanBuilderInterface;
use OpenTelemetry\API\Trace\SpanKind;
use OpenTelemetry\API\Trace\StatusCode;
use OpenTelemetry\Context\Context;

use function OpenTelemetry\Instrumentation\hook;

use OpenTelemetry\SemConv\TraceAttributes;
use Throwable;

Expand All @@ -23,18 +23,19 @@ class DoctrineInstrumentation
public static function register(): void
{
$instrumentation = new CachedInstrumentation('io.opentelemetry.contrib.php.doctrine');
$tracker = new DoctrineTracker();

hook(
\Doctrine\DBAL\Driver::class,
'connect',
pre: static function (\Doctrine\DBAL\Driver $driver, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation) {
/** @psalm-suppress ArgumentTypeCoercion */
$builder = self::makeBuilder($instrumentation, 'Doctrine\DBAL\Driver::connect', $function, $class, $filename, $lineno)
->setSpanKind(SpanKind::KIND_CLIENT)
->setAttribute(TraceAttributes::SERVER_ADDRESS, AttributesResolver::get(TraceAttributes::SERVER_ADDRESS, func_get_args()))
->setAttribute(TraceAttributes::SERVER_PORT, AttributesResolver::get(TraceAttributes::SERVER_PORT, func_get_args()))
->setAttribute(TraceAttributes::DB_SYSTEM_NAME, AttributesResolver::get(TraceAttributes::DB_SYSTEM_NAME, func_get_args()))
->setAttribute(TraceAttributes::DB_NAMESPACE, AttributesResolver::get(TraceAttributes::DB_NAMESPACE, func_get_args()));
->setSpanKind(SpanKind::KIND_CLIENT)
->setAttribute(TraceAttributes::SERVER_ADDRESS, AttributesResolver::get(TraceAttributes::SERVER_ADDRESS, func_get_args()))
->setAttribute(TraceAttributes::SERVER_PORT, AttributesResolver::get(TraceAttributes::SERVER_PORT, func_get_args()))
->setAttribute(TraceAttributes::DB_SYSTEM_NAME, AttributesResolver::get(TraceAttributes::DB_SYSTEM_NAME, func_get_args()))
->setAttribute(TraceAttributes::DB_NAMESPACE, AttributesResolver::get(TraceAttributes::DB_NAMESPACE, func_get_args()));
$parent = Context::getCurrent();
$span = $builder->startSpan();
Context::storage()->attach($span->storeInContext($parent));
Expand All @@ -52,6 +53,8 @@ public static function register(): void
$builder = self::makeBuilder($instrumentation, AttributesResolver::getDbQuerySummary($params), $function, $class, $filename, $lineno)
->setSpanKind(SpanKind::KIND_CLIENT);
$builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, AttributesResolver::get(TraceAttributes::DB_QUERY_TEXT, func_get_args()));
$builder->setAttribute(TraceAttributes::DB_OPERATION_NAME, AttributesResolver::getDbOperationName($params));
$builder->setAttribute(TraceAttributes::DB_COLLECTION_NAME, AttributesResolver::getTarget($params));
$parent = Context::getCurrent();
$span = $builder->startSpan();
Context::storage()->attach($span->storeInContext($parent));
Expand All @@ -68,7 +71,9 @@ public static function register(): void
/** @psalm-suppress ArgumentTypeCoercion */
$builder = self::makeBuilder($instrumentation, AttributesResolver::getDbQuerySummary($params), $function, $class, $filename, $lineno)
->setSpanKind(SpanKind::KIND_CLIENT)
->setAttribute(TraceAttributes::DB_QUERY_TEXT, AttributesResolver::get(TraceAttributes::DB_QUERY_TEXT, func_get_args()));
->setAttribute(TraceAttributes::DB_QUERY_TEXT, AttributesResolver::get(TraceAttributes::DB_QUERY_TEXT, func_get_args()))
->setAttribute(TraceAttributes::DB_OPERATION_NAME, AttributesResolver::getDbOperationName($params))
->setAttribute(TraceAttributes::DB_COLLECTION_NAME, AttributesResolver::getTarget($params));
$parent = Context::getCurrent();
$span = $builder->startSpan();

Expand All @@ -86,13 +91,22 @@ public static function register(): void
/** @psalm-suppress ArgumentTypeCoercion */
$builder = self::makeBuilder($instrumentation, AttributesResolver::getDbQuerySummary($params), $function, $class, $filename, $lineno)
->setSpanKind(SpanKind::KIND_CLIENT)
->setAttribute(TraceAttributes::DB_QUERY_TEXT, AttributesResolver::get(TraceAttributes::DB_QUERY_TEXT, func_get_args()));
->setAttribute(TraceAttributes::DB_QUERY_TEXT, AttributesResolver::get(TraceAttributes::DB_QUERY_TEXT, func_get_args()))
->setAttribute(TraceAttributes::DB_OPERATION_NAME, 'prepare');
$parent = Context::getCurrent();
$span = $builder->startSpan();

Context::storage()->attach($span->storeInContext($parent));
},
post: static function (\Doctrine\DBAL\Driver\Connection $connection, array $params, mixed $statement, ?Throwable $exception) {
post: static function (\Doctrine\DBAL\Driver\Connection $connection, array $params, ?Statement $statement, ?Throwable $exception) use ($tracker) {
if ($statement) {
$scope = Context::storage()->scope();
$context = $scope?->context();
if ($context) {
$span = Span::fromContext($context);
$tracker->trackStatement($statement, $span->getContext());
}
}
self::end($exception);
}
);
Expand All @@ -102,8 +116,9 @@ public static function register(): void
'beginTransaction',
pre: static function (\Doctrine\DBAL\Driver\Connection $connection, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation) {
/** @psalm-suppress ArgumentTypeCoercion */
$builder = self::makeBuilder($instrumentation, 'Doctrine\DBAL\Driver\Connection::beginTransaction', $function, $class, $filename, $lineno)
->setSpanKind(SpanKind::KIND_CLIENT);
$builder = self::makeBuilder($instrumentation, 'Doctrine::beginTransaction', $function, $class, $filename, $lineno)
->setSpanKind(SpanKind::KIND_CLIENT)
->setAttribute(TraceAttributes::DB_OPERATION_NAME, 'begin');
$parent = Context::getCurrent();
$span = $builder->startSpan();

Expand All @@ -119,8 +134,9 @@ public static function register(): void
'commit',
pre: static function (\Doctrine\DBAL\Driver\Connection $connection, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation) {
/** @psalm-suppress ArgumentTypeCoercion */
$builder = self::makeBuilder($instrumentation, 'Doctrine\DBAL\Driver\Connection::commit', $function, $class, $filename, $lineno)
->setSpanKind(SpanKind::KIND_CLIENT);
$builder = self::makeBuilder($instrumentation, 'Doctrine::commit', $function, $class, $filename, $lineno)
->setSpanKind(SpanKind::KIND_CLIENT)
->setAttribute(TraceAttributes::DB_OPERATION_NAME, 'commit');
$parent = Context::getCurrent();
$span = $builder->startSpan();

Expand All @@ -136,8 +152,9 @@ public static function register(): void
'rollBack',
pre: static function (\Doctrine\DBAL\Driver\Connection $connection, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation) {
/** @psalm-suppress ArgumentTypeCoercion */
$builder = self::makeBuilder($instrumentation, 'Doctrine\DBAL\Driver\Connection::rollBack', $function, $class, $filename, $lineno)
->setSpanKind(SpanKind::KIND_CLIENT);
$builder = self::makeBuilder($instrumentation, 'Doctrine::rollBack', $function, $class, $filename, $lineno)
->setSpanKind(SpanKind::KIND_CLIENT)
->setAttribute(TraceAttributes::DB_OPERATION_NAME, 'rollback');
$parent = Context::getCurrent();
$span = $builder->startSpan();
Context::storage()->attach($span->storeInContext($parent));
Expand All @@ -146,6 +163,27 @@ public static function register(): void
self::end($exception);
}
);

hook(
\Doctrine\DBAL\Driver\Statement::class,
'execute',
pre: static function (\Doctrine\DBAL\Driver\Statement $statement, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $tracker) {
/** @psalm-suppress ArgumentTypeCoercion */
$builder = self::makeBuilder($instrumentation, 'Doctrine::execute', $function, $class, $filename, $lineno)
->setSpanKind(SpanKind::KIND_CLIENT)
->setAttribute(TraceAttributes::DB_OPERATION_NAME, 'execute');
if ($ctx = $tracker->getSpanContextForStatement($statement)) {
$builder->addLink($ctx);
}
$parent = Context::getCurrent();
$span = $builder->startSpan();

Context::storage()->attach($span->storeInContext($parent));
},
post: static function (\Doctrine\DBAL\Driver\Statement $statement, array $params, ResultInterface $result, ?Throwable $exception) {
self::end($exception);
}
);
}
private static function makeBuilder(
CachedInstrumentation $instrumentation,
Expand Down
33 changes: 33 additions & 0 deletions src/Instrumentation/Doctrine/src/DoctrineTracker.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

declare(strict_types=1);

namespace OpenTelemetry\Contrib\Instrumentation\Doctrine;

use Doctrine\DBAL\Driver\Statement;
use OpenTelemetry\API\Trace\SpanContextInterface;
use WeakMap;
use WeakReference;

/**
* @internal
*/
class DoctrineTracker
{
private WeakMap $statementToSpanContextMap;

public function __construct()
{
$this->statementToSpanContextMap = new WeakMap();
}

public function trackStatement(Statement $statement, SpanContextInterface $context): void
{
$this->statementToSpanContextMap[$statement] = WeakReference::create($context);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it will return null after the ::prepare() span is flushed and exported? Should be testable by something like:

$stmt = $connection->prepare('SELECT * FROM `technology` WHERE name = :name');
$this->storage->exchangeArray([]);

$stmt->bindValue('name', 'PHP');
$stmt->executeQuery();
$execute = $this->storage->offsetGet(0);
$this->assertCount(1, $execute->getLinks());
Suggested change
$this->statementToSpanContextMap[$statement] = WeakReference::create($context);
$this->statementToSpanContextMap[$statement] = $context;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right. I've used SplObjectStorage now, and added a test. It doesn't look like prepared statements can be destroyed easily, so I guess holding on to span context forever is okay...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should revert the WeakMap->SplObjectStorage change to avoid leaking memory; only the usage of WeakReference was problematic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted to WeakMap

}

public function getSpanContextForStatement(Statement $statement): ?SpanContextInterface
{
return $this->statementToSpanContextMap[$statement]?->get();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,49 @@ public function test_statement_execution(): void

$connection->prepare('SELECT * FROM `technology`');
$span = $this->storage->offsetGet(2);
$this->assertSame('SELECT `technology`', $span->getName());
$this->assertCount(3, $this->storage);
$this->assertSame('SELECT technology', $span->getName());
$this->assertSame('prepare', $span->getAttributes()->get(TraceAttributes::DB_OPERATION_NAME));

$connection->executeQuery('SELECT * FROM `technology`');
$span = $this->storage->offsetGet(3);
$this->assertSame('SELECT `technology`', $span->getName());
$this->assertSame('SELECT technology', $span->getName());
$this->assertSame('SELECT', $span->getAttributes()->get(TraceAttributes::DB_OPERATION_NAME));
$this->assertCount(4, $this->storage);
}

public function test_prepare_then_execute_statement(): void
{
$connection = self::createConnection();
$statement = self::fillDB();
$connection->executeStatement($statement);

$stmt = $connection->prepare('SELECT * FROM `technology` WHERE name = :name');
$prepare = $this->storage->offsetGet(2);
$this->assertSame('prepare', $prepare->getAttributes()->get(TraceAttributes::DB_OPERATION_NAME));
$this->assertSame('SELECT technology', $prepare->getName());

$stmt->bindValue('name', 'PHP');
$stmt->executeQuery();
$execute = $this->storage->offsetGet(3);
$this->assertSame('Doctrine::execute', $execute->getName());
$this->assertSame('execute', $execute->getAttributes()->get(TraceAttributes::DB_OPERATION_NAME));
$this->assertCount(1, $execute->getLinks());
$this->assertEquals($prepare->getContext(), $execute->getLinks()[0]->getSpanContext(), 'execute span is linked to prepare span');
}

public function test_query_with_bind_variables(): void
{
$connection = self::createConnection();
$statement = self::fillDB();
$connection->executeStatement($statement);

$connection->executeQuery('SELECT * FROM `technology` WHERE name = :name', ['name' => 'PHP']);
$prepare = $this->storage->offsetGet(2);
$this->assertSame('SELECT technology', $prepare->getName());
$execute = $this->storage->offsetGet(3);
$this->assertSame('Doctrine::execute', $execute->getName());
$this->assertCount(1, $execute->getLinks());
$this->assertEquals($prepare->getContext(), $execute->getLinks()[0]->getSpanContext(), 'execute span is linked to prepare span');
$this->assertCount(4, $this->storage);
}

Expand All @@ -122,7 +159,8 @@ public function test_transaction(): void
$connection = self::createConnection();
$connection->beginTransaction();
$span = $this->storage->offsetGet(1);
$this->assertSame('Doctrine\DBAL\Driver\Connection::beginTransaction', $span->getName());
$this->assertSame('Doctrine::beginTransaction', $span->getName());
$this->assertSame('begin', $span->getAttributes()->get(TraceAttributes::DB_OPERATION_NAME));
$this->assertCount(2, $this->storage);

$statement = self::fillDB();
Expand All @@ -131,7 +169,8 @@ public function test_transaction(): void
$this->assertSame('CREATE technology', $span->getName());
$connection->commit();
$span = $this->storage->offsetGet(3);
$this->assertSame('Doctrine\DBAL\Driver\Connection::commit', $span->getName());
$this->assertSame('Doctrine::commit', $span->getName());
$this->assertSame('commit', $span->getAttributes()->get(TraceAttributes::DB_OPERATION_NAME));
$this->assertCount(4, $this->storage);

$connection->beginTransaction();
Expand All @@ -140,7 +179,8 @@ public function test_transaction(): void
$connection->executeStatement("INSERT INTO technology(`name`, `date`) VALUES('Java', '1995-05-23');");
$connection->rollback();
$span = $this->storage->offsetGet(6);
$this->assertSame('Doctrine\DBAL\Driver\Connection::rollBack', $span->getName());
$this->assertSame('Doctrine::rollBack', $span->getName());
$this->assertSame('rollback', $span->getAttributes()->get(TraceAttributes::DB_OPERATION_NAME));
$this->assertCount(7, $this->storage);
$this->assertFalse($connection->isTransactionActive());

Expand Down
Loading