From 264e5b564491eb1a0a76f7e8483cbc308a9430c2 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Fri, 23 May 2025 00:06:00 +0000 Subject: [PATCH 1/6] chore: update dependabot configuration [skip ci] --- .github/dependabot.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 4515fadea..5e0e46da0 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -89,6 +89,7 @@ updates: - "/src/Instrumentation/Yii" - "/src/Logs/Monolog" - "/src/MetaPackages/opentelemetry" + - "/src/Propagation/Instana" - "/src/Propagation/ServerTiming" - "/src/Propagation/TraceResponse" - "/src/ResourceDetectors/Azure" From b0b79df44a6d9e607c21030c85f8b38c194c44e1 Mon Sep 17 00:00:00 2001 From: Grzegorz Drozd Date: Fri, 30 May 2025 13:08:58 +0200 Subject: [PATCH 2/6] Add metrics to pdo --- src/Instrumentation/PDO/composer.json | 7 +- .../PDO/src/PDOInstrumentation.php | 189 +++++++++++------- .../Integration/PDOInstrumentationTest.php | 52 +++++ 3 files changed, 180 insertions(+), 68 deletions(-) diff --git a/src/Instrumentation/PDO/composer.json b/src/Instrumentation/PDO/composer.json index 7f6c9a682..cdcb167d7 100644 --- a/src/Instrumentation/PDO/composer.json +++ b/src/Instrumentation/PDO/composer.json @@ -37,6 +37,10 @@ }, "files": [ "_register.php" + ], + "classmap": [ + "/home/grzegorz/work/opentelemetry-php/src/SDK/Metrics/Util", + "/home/grzegorz/work/opentelemetry-php/src/SDK/Util" ] }, "autoload-dev": { @@ -46,7 +50,8 @@ }, "config": { "allow-plugins": { - "php-http/discovery": false + "php-http/discovery": false, + "tbachert/spi": true } } } diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index ddf81c91e..209a5a302 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -4,14 +4,19 @@ namespace OpenTelemetry\Contrib\Instrumentation\PDO; +require_once '/home/grzegorz/work/opentelemetry-php/src/SDK/Util/AttributeTrackerById.php'; +require_once '/home/grzegorz/work/opentelemetry-php/src/SDK/Util/AttributeTrackerByObject.php'; +require_once '/home/grzegorz/work/opentelemetry-php/src/SDK/Metrics/Util/TimerTrackerById.php'; +require_once '/home/grzegorz/work/opentelemetry-php/src/SDK/Metrics/Util/TimerTrackerByObject.php'; + 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\SDK\Common\Configuration\Configuration; +use function OpenTelemetry\Instrumentation\hook; use OpenTelemetry\SemConv\TraceAttributes; use OpenTelemetry\SemConv\Version; use PDO; @@ -27,66 +32,35 @@ public static function register(): void $instrumentation = new CachedInstrumentation( 'io.opentelemetry.contrib.php.pdo', null, - Version::VERSION_1_32_0->url(), + Version::VERSION_1_30_0->url(), ); $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); - } - ); - } - + $timersTracker = new \OpenTelemetry\SDK\Metrics\Util\TimerTrackerByObject(); + $attributesTracker = new \OpenTelemetry\SDK\Util\AttributeTrackerByObject(); hook( PDO::class, '__construct', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $attributesTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::__construct', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); + $connectionAttributes = [ + TraceAttributes::SERVER_ADDRESS => $params[0] ?? 'unknown', + TraceAttributes::SERVER_PORT => $params[0] ?? null + ]; + if ($class === PDO::class) { + //@todo split params[0] into host + port, replace deprecated trace attribute + $builder->setAttributes($connectionAttributes); + + $attributesTracker->set($pdo, $connectionAttributes); + } $parent = Context::getCurrent(); $span = $builder->startSpan(); Context::storage()->attach($span->storeInContext($parent)); + + $instrumentation->meter() + ->createUpDownCounter('db.client.connection.count', '1') + ->add(1, $connectionAttributes, $parent); }, post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($pdoTracker) { $scope = Context::storage()->scope(); @@ -104,15 +78,27 @@ public static function register(): void } ); + hook(PDO::class, '__destruct', post:function ($pdo) use ($instrumentation, $attributesTracker) { + $parent = Context::getCurrent(); + + $connectionAttributes = $attributesTracker->get($pdo); + $instrumentation->meter() + ->createUpDownCounter('db.client.connection.count', '1') + ->add(-1, $connectionAttributes, $parent); + }); + hook( PDO::class, 'query', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation, $timersTracker, $attributesTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::query', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); + if ($class === PDO::class) { - $builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8')); + $encodedQuery = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); + $builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, $encodedQuery); + $attributesTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); } $parent = Context::getCurrent(); $span = $builder->startSpan(); @@ -121,21 +107,35 @@ public static function register(): void $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); + + $metricAttributes = $attributesTracker->get($pdo); + self::createPendingOperationMetric($instrumentation, $metricAttributes, 1); + $timersTracker->start($pdo); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) { + post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($instrumentation, $attributesTracker, $timersTracker) { + $duration = $timersTracker->durationMs($pdo); self::end($exception); + + $metricAttributes = $attributesTracker->get($pdo); + self::createDurationMetric($instrumentation, $metricAttributes, $duration); + self::createPendingOperationMetric($instrumentation, $metricAttributes, -1); + if ($statement instanceof PDOStatement && $statement->rowCount()) { + self::createReturnedRowsMetric($instrumentation, $metricAttributes, $statement->rowCount()); + } } ); hook( PDO::class, 'exec', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation, $attributesTracker, $timersTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::exec', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); if ($class === PDO::class) { - $builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8')); + $encodedQuery = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); + $builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, $encodedQuery); + $attributesTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); } $parent = Context::getCurrent(); $span = $builder->startSpan(); @@ -144,9 +144,22 @@ public static function register(): void $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); + + $metricAttributes = $attributesTracker->get($pdo); + self::createPendingOperationMetric($instrumentation, $metricAttributes, 1); + + $timersTracker->start($pdo); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) { + post: static function (PDO $pdo, array $params, mixed $affectedRows, ?Throwable $exception) use ($instrumentation, $attributesTracker, $timersTracker) { + $duration = $timersTracker->durationMs($pdo); self::end($exception); + + $metricAttributes = $attributesTracker->get($pdo); + self::createDurationMetric($instrumentation, $metricAttributes, $duration); + self::createPendingOperationMetric($instrumentation, $metricAttributes, -1); + if (!empty($affectedRows)) { + self::createReturnedRowsMetric($instrumentation, $metricAttributes, $affectedRows); + } } ); @@ -243,7 +256,6 @@ public static function register(): void pre: static function (PDOStatement $statement, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { $attributes = $pdoTracker->trackedAttributesForStatement($statement); if (self::isDistributeStatementToLinkedSpansEnabled()) { - /** @psalm-suppress InvalidArrayAssignment */ $attributes[TraceAttributes::DB_QUERY_TEXT] = $statement->queryString; } /** @psalm-suppress ArgumentTypeCoercion */ @@ -253,10 +265,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($parent)); + Context::storage()->attach($span->storeInContext(Context::getCurrent())); }, post: static function (PDOStatement $statement, array $params, mixed $retval, ?Throwable $exception) { self::end($exception); @@ -266,14 +277,15 @@ public static function register(): void hook( PDOStatement::class, 'execute', - pre: static function (PDOStatement $statement, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDOStatement $statement, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation, $timersTracker, $attributesTracker) { $attributes = $pdoTracker->trackedAttributesForStatement($statement); if (self::isDistributeStatementToLinkedSpansEnabled()) { - /** @psalm-suppress InvalidArrayAssignment */ $attributes[TraceAttributes::DB_QUERY_TEXT] = $statement->queryString; } + $attributesTracker->append($statement, TraceAttributes::DB_QUERY_TEXT, $statement->queryString); + /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDOStatement::execute', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT) @@ -281,12 +293,21 @@ public static function register(): void if ($spanContext = $pdoTracker->getSpanForPreparedStatement($statement)) { $builder->addLink($spanContext); } - $parent = Context::getCurrent(); $span = $builder->startSpan(); - Context::storage()->attach($span->storeInContext($parent)); + + Context::storage()->attach($span->storeInContext(Context::getCurrent())); + + $metricAttributes = $attributesTracker->get($statement); + self::createPendingOperationMetric($instrumentation, $metricAttributes, 1); + $timersTracker->start($statement); }, - post: static function (PDOStatement $statement, array $params, mixed $retval, ?Throwable $exception) { + post: static function (PDOStatement $statement, array $params, mixed $retval, ?Throwable $exception) use ($instrumentation, $attributesTracker, $timersTracker) { + $duration = $timersTracker->durationMs($statement); self::end($exception); + + $metricAttributes = $attributesTracker->get($statement); + self::createDurationMetric($instrumentation, $metricAttributes, $duration); + self::createPendingOperationMetric($instrumentation, $metricAttributes, -1); } ); } @@ -301,8 +322,9 @@ private static function makeBuilder( /** @psalm-suppress ArgumentTypeCoercion */ return $instrumentation->tracer() ->spanBuilder($name) - ->setAttribute(TraceAttributes::CODE_FUNCTION_NAME, sprintf('%s::%s', $class, $function)) - ->setAttribute(TraceAttributes::CODE_FILE_PATH, $filename) + ->setAttribute(TraceAttributes::CODE_FUNCTION_NAME, $function) + ->setAttribute(TraceAttributes::CODE_NAMESPACE, $class) + ->setAttribute(TraceAttributes::CODE_FILEPATH, $filename) ->setAttribute(TraceAttributes::CODE_LINE_NUMBER, $lineno); } private static function end(?Throwable $exception): void @@ -327,6 +349,39 @@ private static function isDistributeStatementToLinkedSpansEnabled(): bool return Configuration::getBoolean('OTEL_PHP_INSTRUMENTATION_PDO_DISTRIBUTE_STATEMENT_TO_LINKED_SPANS', false); } - return filter_var(get_cfg_var('otel.instrumentation.pdo.distribute_statement_to_linked_spans'), FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) ?? false; + return get_cfg_var('otel.instrumentation.pdo.distribute_statement_to_linked_spans'); + } + + protected static function createPendingOperationMetric( + CachedInstrumentation $instrumentation, + array $attributes, + int $value, + ): void { + $parent = Context::getCurrent(); + $instrumentation->meter() + ->createUpDownCounter('db.client.connection.pending_requests', '1') + ->add($value, $attributes, $parent); + } + + protected static function createReturnedRowsMetric( + CachedInstrumentation $instrumentation, + array $attributes, + int $value, + ): void { + $parent = Context::getCurrent(); + $instrumentation->meter() + ->createHistogram('db.client.response.returned_rows', '1') + ->record($value, $attributes, $parent); + } + + protected static function createDurationMetric( + CachedInstrumentation $instrumentation, + array $attributes, + float $value, + ): void { + $parent = Context::getCurrent(); + $instrumentation->meter() + ->createHistogram('db.client.operation.duration', 'ms') + ->record($value, $attributes, $parent); } } diff --git a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php index 9398c8701..922fe30e0 100644 --- a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php +++ b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php @@ -5,11 +5,22 @@ namespace OpenTelemetry\Tests\Instrumentation\PDO\Integration; use ArrayObject; +use OpenTelemetry\API\Common\Time\TestClock; 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\Common\Attribute\Attributes; +use OpenTelemetry\SDK\Common\Export\InMemoryStorageManager; +use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeFactory; +use OpenTelemetry\SDK\Metrics\Exemplar\ExemplarFilter\AllExemplarFilter; +use OpenTelemetry\SDK\Metrics\Exemplar\ExemplarFilter\WithSampledTraceExemplarFilter; +use OpenTelemetry\SDK\Metrics\MeterProvider; +use OpenTelemetry\SDK\Metrics\MetricReader\ExportingReader; +use OpenTelemetry\SDK\Metrics\StalenessHandler\ImmediateStalenessHandlerFactory; +use OpenTelemetry\SDK\Metrics\View\CriteriaViewRegistry; +use OpenTelemetry\SDK\Resource\ResourceInfoFactory; use OpenTelemetry\SDK\Trace\ImmutableSpan; use OpenTelemetry\SDK\Trace\SpanExporter\InMemoryExporter; use OpenTelemetry\SDK\Trace\SpanProcessor\SimpleSpanProcessor; @@ -26,6 +37,8 @@ class PDOInstrumentationTest extends TestCase private ScopeInterface $scope; /** @var ArrayObject */ private ArrayObject $storage; + private $metricReader; + private $meterProvider; private function createDB(): PDO { @@ -67,9 +80,24 @@ public function setUp(): void new InMemoryExporter($this->storage) ) ); + $clock = new TestClock(); + $exporter = new \OpenTelemetry\SDK\Metrics\MetricExporter\InMemoryExporter(InMemoryStorageManager::metrics()); + $this->metricReader = new ExportingReader($exporter); + $this->meterProvider = new MeterProvider( + null, + ResourceInfoFactory::emptyResource(), + $clock, + Attributes::factory(), + new InstrumentationScopeFactory(Attributes::factory()), + [$this->metricReader], + new CriteriaViewRegistry(), + new AllExemplarFilter(), + new ImmediateStalenessHandlerFactory(), + ); $this->scope = Configurator::create() ->withTracerProvider($tracerProvider) + ->withMeterProvider($this->meterProvider) ->activate(); } @@ -402,4 +430,28 @@ public function test_span_hierarchy_with_pdo_operations(): void ] ); } + + + public function test_connection_metrics(): void + { + $db = self::createDB(); + $db->exec($this->fillDB()); + + $non_utf8_id = mb_convert_encoding('rückwärts', 'ISO-8859-1', 'UTF-8'); + + $db->prepare("SELECT id FROM technology WHERE id = '{$non_utf8_id}'"); + $span_db_prepare = $this->storage->offsetGet(2); + $this->assertTrue(mb_check_encoding($span_db_prepare->getAttributes()->get(TraceAttributes::DB_QUERY_TEXT), 'UTF-8')); + $this->assertCount(3, $this->storage); + + + + $meterProvider = \OpenTelemetry\API\Globals::meterProvider(); + if ($meterProvider instanceof MeterProvider) { + $meterProvider->forceFlush(); + $meterProvider->shutdown(); + } + $metrics = InMemoryStorageManager::metrics(); + var_dump($metrics); + } } From 814dd59120dfc9c7d328f43e52bb9c5e724d4ed5 Mon Sep 17 00:00:00 2001 From: Grzegorz Drozd <1885137+GrzegorzDrozd@users.noreply.github.com> Date: Mon, 16 Jun 2025 20:24:51 +0200 Subject: [PATCH 3/6] Add metrics to PDO - add duration tracking - add returned rows tracking - add pending operations metric - add active connections metric - minor code fixes --- .../PDO/src/PDOInstrumentation.php | 274 ++++++++++++------ src/Instrumentation/PDO/src/PDOTracker.php | 40 ++- .../Integration/PDOInstrumentationTest.php | 84 +++++- 3 files changed, 282 insertions(+), 116 deletions(-) diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 209a5a302..1a3aa3f1a 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -4,22 +4,19 @@ namespace OpenTelemetry\Contrib\Instrumentation\PDO; -require_once '/home/grzegorz/work/opentelemetry-php/src/SDK/Util/AttributeTrackerById.php'; -require_once '/home/grzegorz/work/opentelemetry-php/src/SDK/Util/AttributeTrackerByObject.php'; -require_once '/home/grzegorz/work/opentelemetry-php/src/SDK/Metrics/Util/TimerTrackerById.php'; -require_once '/home/grzegorz/work/opentelemetry-php/src/SDK/Metrics/Util/TimerTrackerByObject.php'; - 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 OpenTelemetry\SDK\Common\Configuration\Configuration; use function OpenTelemetry\Instrumentation\hook; +use OpenTelemetry\SDK\Common\Configuration\Configuration; +use OpenTelemetry\SDK\Metrics\Util\TimerTrackerByObject; use OpenTelemetry\SemConv\TraceAttributes; use OpenTelemetry\SemConv\Version; use PDO; +use PDOException; use PDOStatement; use Throwable; @@ -32,37 +29,88 @@ public static function register(): void $instrumentation = new CachedInstrumentation( 'io.opentelemetry.contrib.php.pdo', null, - Version::VERSION_1_30_0->url(), + Version::VERSION_1_32_0->url(), ); $pdoTracker = new PDOTracker(); - $timersTracker = new \OpenTelemetry\SDK\Metrics\Util\TimerTrackerByObject(); - $attributesTracker = new \OpenTelemetry\SDK\Util\AttributeTrackerByObject(); + $timersTracker = new TimerTrackerByObject(); + + // 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 ($instrumentation, $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); + + $attributes = $pdoTracker->get($result); + $parent = Context::getCurrent(); + + $instrumentation->meter() + ->createUpDownCounter('db.client.connection.count', '1') + ->add(1, $attributes, $parent); + + $pdoTracker->trackPdoInstancesDestruction( + $object, + function ($pdoInstance) use ($instrumentation, $pdoTracker) { + $parent = Context::getCurrent(); + + $attributes = $pdoTracker->get($pdoInstance); + $instrumentation->meter() + ->createUpDownCounter('db.client.connection.count', '1') + ->add(-1, $attributes, $parent); + } + ); + } + ); + } + hook( PDO::class, '__construct', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $attributesTracker) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::__construct', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); - $connectionAttributes = [ - TraceAttributes::SERVER_ADDRESS => $params[0] ?? 'unknown', - TraceAttributes::SERVER_PORT => $params[0] ?? null - ]; - if ($class === PDO::class) { - //@todo split params[0] into host + port, replace deprecated trace attribute - $builder->setAttributes($connectionAttributes); - - $attributesTracker->set($pdo, $connectionAttributes); - } $parent = Context::getCurrent(); $span = $builder->startSpan(); Context::storage()->attach($span->storeInContext($parent)); - - $instrumentation->meter() - ->createUpDownCounter('db.client.connection.count', '1') - ->add(1, $connectionAttributes, $parent); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($pdoTracker) { + post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($instrumentation, $pdoTracker) { $scope = Context::storage()->scope(); if (!$scope) { return; @@ -75,52 +123,71 @@ public static function register(): void $span->setAttributes($attributes); self::end($exception); - } - ); - hook(PDO::class, '__destruct', post:function ($pdo) use ($instrumentation, $attributesTracker) { - $parent = Context::getCurrent(); + $attributes = $pdoTracker->get($pdo); + $parent = Context::getCurrent(); - $connectionAttributes = $attributesTracker->get($pdo); - $instrumentation->meter() - ->createUpDownCounter('db.client.connection.count', '1') - ->add(-1, $connectionAttributes, $parent); - }); + $instrumentation->meter() + ->createUpDownCounter('db.client.connection.count', '1') + ->add(1, $attributes, $parent); + + $pdoTracker->trackPdoInstancesDestruction( + $pdo, + function ($pdoInstance) use ($instrumentation, $pdoTracker) { + $parent = Context::getCurrent(); + + $attributes = $pdoTracker->get($pdoInstance); + $instrumentation->meter() + ->createUpDownCounter('db.client.connection.count', '1') + ->add(-1, $attributes, $parent); + } + ); + } + ); hook( PDO::class, 'query', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation, $timersTracker, $attributesTracker) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker, $timersTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::query', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); - + $encodedQuery = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); if ($class === PDO::class) { - $encodedQuery = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); $builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, $encodedQuery); - $attributesTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); } $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->trackedAttributesForPdo($pdo); + $attributes = $pdoTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); - $metricAttributes = $attributesTracker->get($pdo); - self::createPendingOperationMetric($instrumentation, $metricAttributes, 1); + self::createPendingOperationMetric($instrumentation, $attributes, 1); $timersTracker->start($pdo); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($instrumentation, $attributesTracker, $timersTracker) { + post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { $duration = $timersTracker->durationMs($pdo); - self::end($exception); - - $metricAttributes = $attributesTracker->get($pdo); - self::createDurationMetric($instrumentation, $metricAttributes, $duration); - self::createPendingOperationMetric($instrumentation, $metricAttributes, -1); + // this happens ONLY when error mode is set to silent + // this is an alternative to changes in the ::end method + // if ($statement === false && $exception === null) { + // $exception = new class($pdo->errorInfo()) extends \PDOException { + // // to workaround setting code that is not INT + // public function __construct(array $errorInfo) { + // $this->message = $errorInfo[2] ?? 'PDO error'; + // $this->code = $errorInfo[0] ?? 0; + // } + // }; + // } + + self::end($exception, $statement === false ? $pdo->errorInfo() : []); + + $attributes = $pdoTracker->get($pdo); + self::createDurationMetric($instrumentation, $attributes, $duration); + self::createPendingOperationMetric($instrumentation, $attributes, -1); if ($statement instanceof PDOStatement && $statement->rowCount()) { - self::createReturnedRowsMetric($instrumentation, $metricAttributes, $statement->rowCount()); + self::createReturnedRowsMetric($instrumentation, $attributes, $statement->rowCount()); } } ); @@ -128,37 +195,35 @@ public static function register(): void hook( PDO::class, 'exec', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation, $attributesTracker, $timersTracker) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker, $timersTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::exec', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); + $encodedQuery = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); if ($class === PDO::class) { - $encodedQuery = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); $builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, $encodedQuery); - $attributesTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); } $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->trackedAttributesForPdo($pdo); + $attributes = $pdoTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); - $metricAttributes = $attributesTracker->get($pdo); - self::createPendingOperationMetric($instrumentation, $metricAttributes, 1); + self::createPendingOperationMetric($instrumentation, $attributes, 1); $timersTracker->start($pdo); }, - post: static function (PDO $pdo, array $params, mixed $affectedRows, ?Throwable $exception) use ($instrumentation, $attributesTracker, $timersTracker) { + post: static function (PDO $pdo, array $params, mixed $affectedRows, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { $duration = $timersTracker->durationMs($pdo); - self::end($exception); + self::end($exception, $affectedRows === false ? $pdo->errorInfo() : []); - $metricAttributes = $attributesTracker->get($pdo); - self::createDurationMetric($instrumentation, $metricAttributes, $duration); - self::createPendingOperationMetric($instrumentation, $metricAttributes, -1); + $attributes = $pdoTracker->get($pdo); + self::createDurationMetric($instrumentation, $attributes, $duration); + self::createPendingOperationMetric($instrumentation, $attributes, -1); if (!empty($affectedRows)) { - self::createReturnedRowsMetric($instrumentation, $metricAttributes, $affectedRows); + self::createReturnedRowsMetric($instrumentation, $attributes, $affectedRows); } } ); @@ -166,96 +231,106 @@ public static function register(): void hook( PDO::class, 'prepare', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker, $timersTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::prepare', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); + $encodedQuery = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); if ($class === PDO::class) { - $builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8')); + $builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, $encodedQuery); } $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->trackedAttributesForPdo($pdo); + $attributes = $pdoTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); + + self::createPendingOperationMetric($instrumentation, $attributes, 1); + + $timersTracker->start($pdo); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($pdoTracker) { + post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { + $duration = $timersTracker->durationMs($pdo); if ($statement instanceof PDOStatement) { $pdoTracker->trackStatement($statement, $pdo, Span::getCurrent()->getContext()); } - self::end($exception); + self::end($exception, $statement === false ? $pdo->errorInfo() : []); + $attributes = $pdoTracker->get($pdo); + self::createDurationMetric($instrumentation, $attributes, $duration); + self::createPendingOperationMetric($instrumentation, $attributes, -1); } ); hook( PDO::class, 'beginTransaction', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::beginTransaction', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->trackedAttributesForPdo($pdo); + $attributes = $pdoTracker->get($pdo); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) { - self::end($exception); + post: static function (PDO $pdo, array $params, mixed $retval, ?Throwable $exception) { + self::end($exception, $retval === false ? $pdo->errorInfo() : []); } ); hook( PDO::class, 'commit', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::commit', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->trackedAttributesForPdo($pdo); + $attributes = $pdoTracker->get($pdo); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) { - self::end($exception); + post: static function (PDO $pdo, array $params, mixed $retval, ?Throwable $exception) { + self::end($exception, $retval === false ? $pdo->errorInfo() : []); } ); hook( PDO::class, 'rollBack', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::rollBack', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->trackedAttributesForPdo($pdo); + $attributes = $pdoTracker->get($pdo); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) { - self::end($exception); + post: static function (PDO $pdo, array $params, mixed $retval, ?Throwable $exception) { + self::end($exception, $retval === false ? $pdo->errorInfo() : []); } ); hook( PDOStatement::class, 'fetchAll', - pre: static function (PDOStatement $statement, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDOStatement $statement, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker) { $attributes = $pdoTracker->trackedAttributesForStatement($statement); if (self::isDistributeStatementToLinkedSpansEnabled()) { + /** @psalm-suppress InvalidArrayAssignment */ $attributes[TraceAttributes::DB_QUERY_TEXT] = $statement->queryString; } /** @psalm-suppress ArgumentTypeCoercion */ @@ -265,9 +340,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); @@ -277,15 +353,14 @@ public static function register(): void hook( PDOStatement::class, 'execute', - pre: static function (PDOStatement $statement, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation, $timersTracker, $attributesTracker) { + pre: static function (PDOStatement $statement, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker, $timersTracker) { $attributes = $pdoTracker->trackedAttributesForStatement($statement); if (self::isDistributeStatementToLinkedSpansEnabled()) { + /** @psalm-suppress InvalidArrayAssignment */ $attributes[TraceAttributes::DB_QUERY_TEXT] = $statement->queryString; } - $attributesTracker->append($statement, TraceAttributes::DB_QUERY_TEXT, $statement->queryString); - /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDOStatement::execute', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT) @@ -293,24 +368,24 @@ public static function register(): void if ($spanContext = $pdoTracker->getSpanForPreparedStatement($statement)) { $builder->addLink($spanContext); } + $parent = Context::getCurrent(); $span = $builder->startSpan(); + Context::storage()->attach($span->storeInContext($parent)); - Context::storage()->attach($span->storeInContext(Context::getCurrent())); - - $metricAttributes = $attributesTracker->get($statement); - self::createPendingOperationMetric($instrumentation, $metricAttributes, 1); + self::createPendingOperationMetric($instrumentation, $attributes, 1); $timersTracker->start($statement); }, - post: static function (PDOStatement $statement, array $params, mixed $retval, ?Throwable $exception) use ($instrumentation, $attributesTracker, $timersTracker) { + post: static function (PDOStatement $statement, array $params, mixed $retval, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { $duration = $timersTracker->durationMs($statement); - self::end($exception); + self::end($exception, $retval === false ? $statement->errorInfo() : []); - $metricAttributes = $attributesTracker->get($statement); - self::createDurationMetric($instrumentation, $metricAttributes, $duration); - self::createPendingOperationMetric($instrumentation, $metricAttributes, -1); + $attributes = $pdoTracker->trackedAttributesForStatement($statement); + self::createDurationMetric($instrumentation, $attributes, $duration); + self::createPendingOperationMetric($instrumentation, $attributes, -1); } ); } + private static function makeBuilder( CachedInstrumentation $instrumentation, string $name, @@ -322,12 +397,12 @@ private static function makeBuilder( /** @psalm-suppress ArgumentTypeCoercion */ return $instrumentation->tracer() ->spanBuilder($name) - ->setAttribute(TraceAttributes::CODE_FUNCTION_NAME, $function) - ->setAttribute(TraceAttributes::CODE_NAMESPACE, $class) - ->setAttribute(TraceAttributes::CODE_FILEPATH, $filename) + ->setAttribute(TraceAttributes::CODE_FUNCTION_NAME, sprintf('%s::%s', $class, $function)) + ->setAttribute(TraceAttributes::CODE_FILE_PATH, $filename) ->setAttribute(TraceAttributes::CODE_LINE_NUMBER, $lineno); } - private static function end(?Throwable $exception): void + + private static function end(?Throwable $exception, array $errorInfo = []): void { $scope = Context::storage()->scope(); if (!$scope) { @@ -338,6 +413,15 @@ private static function end(?Throwable $exception): void if ($exception) { $span->recordException($exception); $span->setStatus(StatusCode::STATUS_ERROR, $exception->getMessage()); + + } elseif (!empty($errorInfo[2]) && $errorMessage = $errorInfo[2]) { + $span->addEvent('exception', [ + 'exception.type' => PDOException::class, + 'exception.message' => $errorMessage, + // @todo try to add stacktrace? + ]); + + $span->setStatus(StatusCode::STATUS_ERROR, $errorMessage); } $span->end(); diff --git a/src/Instrumentation/PDO/src/PDOTracker.php b/src/Instrumentation/PDO/src/PDOTracker.php index 4753a0baa..d9bec5a86 100644 --- a/src/Instrumentation/PDO/src/PDOTracker.php +++ b/src/Instrumentation/PDO/src/PDOTracker.php @@ -4,7 +4,9 @@ namespace OpenTelemetry\Contrib\Instrumentation\PDO; +use Error; use OpenTelemetry\API\Trace\SpanContextInterface; +use OpenTelemetry\SDK\Util\AttributeTrackerByObject; use OpenTelemetry\SemConv\TraceAttributes; use PDO; use PDOStatement; @@ -14,7 +16,7 @@ /** * @phan-file-suppress PhanNonClassMethodCall,PhanTypeArraySuspicious */ -final class PDOTracker +final class PDOTracker extends AttributeTrackerByObject { /** * @var WeakMap> @@ -26,13 +28,15 @@ final class PDOTracker private WeakMap $statementMapToPdoMap; private WeakMap $preparedStatementToSpanMap; + private WeakMap $pdoInstances; + public function __construct() { - /** @psalm-suppress PropertyTypeCoercion */ - $this->pdoToAttributesMap = new WeakMap(); /** @psalm-suppress PropertyTypeCoercion */ $this->statementMapToPdoMap = new WeakMap(); $this->preparedStatementToSpanMap = new WeakMap(); + $this->pdoInstances = new WeakMap(); + parent::__construct(); } /** @@ -58,7 +62,7 @@ public function trackedAttributesForStatement(PDOStatement $statement): array } /** @psalm-var array */ - return $this->pdoToAttributesMap[$pdo] ?? []; + return $this->get($pdo); } /** @@ -75,15 +79,14 @@ public function trackPdoAttributes(PDO $pdo, string $dsn): array $dbSystem = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); /** @psalm-suppress InvalidArrayAssignment */ $attributes[TraceAttributes::DB_SYSTEM_NAME] = self::mapDriverNameToAttribute($dbSystem); - } catch (\Error) { - // if we caught an exception, the driver is likely not supporting the operation, default to "other" + } catch (Error) { + // if we caught an exception, and it is not set from extractAttributesFromDSN, + // the driver is likely not supporting the operation, default to "other" /** @psalm-suppress PossiblyInvalidArrayAssignment */ - $attributes[TraceAttributes::DB_SYSTEM_NAME] = 'other_sql'; + $attributes[TraceAttributes::DB_SYSTEM_NAME] ??= 'other_sql'; } - $this->pdoToAttributesMap[$pdo] = $attributes; - - return $attributes; + return $this->add($pdo, $attributes); } /** @@ -93,7 +96,22 @@ public function trackPdoAttributes(PDO $pdo, string $dsn): array public function trackedAttributesForPdo(PDO $pdo): array { /** @psalm-var array */ - return $this->pdoToAttributesMap[$pdo] ?? []; + return $this->get($pdo); + } + + public function trackPdoInstancesDestruction(PDO $pdo, callable $callbackFunction): void + { + $this->pdoInstances[$pdo] = new class($pdo, $callbackFunction) { + public function __construct( + private PDO $instance, + private $callback + ) { + } + public function __destruct() + { + ($this->callback)($this->instance); + } + }; } public function getSpanForPreparedStatement(PDOStatement $statement): ?SpanContextInterface diff --git a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php index 922fe30e0..66c9fa0a6 100644 --- a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php +++ b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php @@ -6,6 +6,7 @@ use ArrayObject; use OpenTelemetry\API\Common\Time\TestClock; +use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\Trace\SpanInterface; use OpenTelemetry\API\Trace\SpanKind; @@ -14,11 +15,13 @@ use OpenTelemetry\SDK\Common\Attribute\Attributes; use OpenTelemetry\SDK\Common\Export\InMemoryStorageManager; use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeFactory; +use OpenTelemetry\SDK\Metrics\Data\Metric; +use OpenTelemetry\SDK\Metrics\Data\Sum; +use OpenTelemetry\SDK\Metrics\Data\Temporality; use OpenTelemetry\SDK\Metrics\Exemplar\ExemplarFilter\AllExemplarFilter; -use OpenTelemetry\SDK\Metrics\Exemplar\ExemplarFilter\WithSampledTraceExemplarFilter; use OpenTelemetry\SDK\Metrics\MeterProvider; use OpenTelemetry\SDK\Metrics\MetricReader\ExportingReader; -use OpenTelemetry\SDK\Metrics\StalenessHandler\ImmediateStalenessHandlerFactory; +use OpenTelemetry\SDK\Metrics\StalenessHandler\DelayedStalenessHandlerFactory; use OpenTelemetry\SDK\Metrics\View\CriteriaViewRegistry; use OpenTelemetry\SDK\Resource\ResourceInfoFactory; use OpenTelemetry\SDK\Trace\ImmutableSpan; @@ -28,6 +31,7 @@ use OpenTelemetry\SemConv\TraceAttributes; use OpenTelemetry\TestUtils\TraceStructureAssertionTrait; use PDO; +use PDOException; use PHPUnit\Framework\TestCase; class PDOInstrumentationTest extends TestCase @@ -39,6 +43,7 @@ class PDOInstrumentationTest extends TestCase private ArrayObject $storage; private $metricReader; private $meterProvider; + private $metricExporter; private function createDB(): PDO { @@ -81,8 +86,11 @@ public function setUp(): void ) ); $clock = new TestClock(); - $exporter = new \OpenTelemetry\SDK\Metrics\MetricExporter\InMemoryExporter(InMemoryStorageManager::metrics()); - $this->metricReader = new ExportingReader($exporter); + $this->metricExporter = new \OpenTelemetry\SDK\Metrics\MetricExporter\InMemoryExporter( + InMemoryStorageManager::metrics(), + Temporality::CUMULATIVE + ); + $this->metricReader = new ExportingReader($this->metricExporter); $this->meterProvider = new MeterProvider( null, ResourceInfoFactory::emptyResource(), @@ -92,7 +100,7 @@ public function setUp(): void [$this->metricReader], new CriteriaViewRegistry(), new AllExemplarFilter(), - new ImmediateStalenessHandlerFactory(), + new DelayedStalenessHandlerFactory($clock, 20), ); $this->scope = Configurator::create() @@ -163,7 +171,7 @@ public function test_pdo_sqlite_subclass(): void public function test_constructor_exception(): void { - $this->expectException(\PDOException::class); + $this->expectException(PDOException::class); $this->expectExceptionMessage('could not find driver'); new PDO('unknown:foo'); } @@ -431,7 +439,6 @@ public function test_span_hierarchy_with_pdo_operations(): void ); } - public function test_connection_metrics(): void { $db = self::createDB(); @@ -439,19 +446,76 @@ public function test_connection_metrics(): void $non_utf8_id = mb_convert_encoding('rückwärts', 'ISO-8859-1', 'UTF-8'); - $db->prepare("SELECT id FROM technology WHERE id = '{$non_utf8_id}'"); + $stmt = $db->prepare("SELECT id FROM technology WHERE id = '{$non_utf8_id}'"); $span_db_prepare = $this->storage->offsetGet(2); $this->assertTrue(mb_check_encoding($span_db_prepare->getAttributes()->get(TraceAttributes::DB_QUERY_TEXT), 'UTF-8')); $this->assertCount(3, $this->storage); + $stmt->execute(); + $stmt->fetchAll(); + unset($db); + $meterProvider = Globals::meterProvider(); + if ($meterProvider instanceof MeterProvider) { + $meterProvider->forceFlush(); + $meterProvider->shutdown(); + } + + $metricStats = []; + /** @var Metric $metric */ + foreach (InMemoryStorageManager::metrics() as $metric) { + $metricStats[$metric->name] ??= []; + $metricStats[$metric->name][] = $metric->data; + } + self::assertCount(2, $metricStats['db.client.connection.count']); + self::assertCount(2, $metricStats['db.client.connection.pending_requests']); + self::assertCount(2, $metricStats['db.client.operation.duration']); + self::assertCount(2, $metricStats['db.client.response.returned_rows']); + + // check count metrics: + $this->checkMetricSum($metricStats['db.client.connection.count'], 2, 2); + $this->checkMetricSum($metricStats['db.client.connection.pending_requests'], 0, 2); + $this->checkMetricDataPointsCount($metricStats['db.client.operation.duration'], 2); + $this->checkMetricDataPointsCount($metricStats['db.client.response.returned_rows'], 2); + } - $meterProvider = \OpenTelemetry\API\Globals::meterProvider(); + public function test_error_info_in_silent_mode(): void + { + $db = self::createDB(); + $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT); + $db->exec($this->fillDB()); + + $db->query('SELECT id FROM non_existing_table'); + + $meterProvider = Globals::meterProvider(); if ($meterProvider instanceof MeterProvider) { $meterProvider->forceFlush(); $meterProvider->shutdown(); } $metrics = InMemoryStorageManager::metrics(); - var_dump($metrics); + self::assertTrue(true); + } + + public function checkMetricDataPointsCount($metrics, $expectedCount): void + { + $sum = 0; + /** @var Sum $point */ + foreach ($metrics as $point) { + $sum += $point->dataPoints[0]->count; + } + self::assertEquals($expectedCount, $sum); + } + + public function checkMetricSum($metrics, $expectedSum, ?int $count = null): void + { + $sum = 0; + /** @var Sum $point */ + foreach ($metrics as $point) { + $sum += $point->dataPoints[0]->value; + } + self::assertEquals($expectedSum, $sum); + if ($count) { + self::assertCount($count, $metrics); + } } } From 07ac3b48d85c4a324a35269eddf42fb9f85baeda Mon Sep 17 00:00:00 2001 From: Grzegorz Drozd <1885137+GrzegorzDrozd@users.noreply.github.com> Date: Mon, 16 Jun 2025 20:24:51 +0200 Subject: [PATCH 4/6] Add metrics to PDO - remove not needed debug code --- src/Instrumentation/PDO/composer.json | 7 +- .../PDO/src/PDOInstrumentation.php | 274 ++++++++++++------ src/Instrumentation/PDO/src/PDOTracker.php | 40 ++- .../Integration/PDOInstrumentationTest.php | 84 +++++- 4 files changed, 283 insertions(+), 122 deletions(-) diff --git a/src/Instrumentation/PDO/composer.json b/src/Instrumentation/PDO/composer.json index cdcb167d7..7f6c9a682 100644 --- a/src/Instrumentation/PDO/composer.json +++ b/src/Instrumentation/PDO/composer.json @@ -37,10 +37,6 @@ }, "files": [ "_register.php" - ], - "classmap": [ - "/home/grzegorz/work/opentelemetry-php/src/SDK/Metrics/Util", - "/home/grzegorz/work/opentelemetry-php/src/SDK/Util" ] }, "autoload-dev": { @@ -50,8 +46,7 @@ }, "config": { "allow-plugins": { - "php-http/discovery": false, - "tbachert/spi": true + "php-http/discovery": false } } } diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 209a5a302..1a3aa3f1a 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -4,22 +4,19 @@ namespace OpenTelemetry\Contrib\Instrumentation\PDO; -require_once '/home/grzegorz/work/opentelemetry-php/src/SDK/Util/AttributeTrackerById.php'; -require_once '/home/grzegorz/work/opentelemetry-php/src/SDK/Util/AttributeTrackerByObject.php'; -require_once '/home/grzegorz/work/opentelemetry-php/src/SDK/Metrics/Util/TimerTrackerById.php'; -require_once '/home/grzegorz/work/opentelemetry-php/src/SDK/Metrics/Util/TimerTrackerByObject.php'; - 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 OpenTelemetry\SDK\Common\Configuration\Configuration; use function OpenTelemetry\Instrumentation\hook; +use OpenTelemetry\SDK\Common\Configuration\Configuration; +use OpenTelemetry\SDK\Metrics\Util\TimerTrackerByObject; use OpenTelemetry\SemConv\TraceAttributes; use OpenTelemetry\SemConv\Version; use PDO; +use PDOException; use PDOStatement; use Throwable; @@ -32,37 +29,88 @@ public static function register(): void $instrumentation = new CachedInstrumentation( 'io.opentelemetry.contrib.php.pdo', null, - Version::VERSION_1_30_0->url(), + Version::VERSION_1_32_0->url(), ); $pdoTracker = new PDOTracker(); - $timersTracker = new \OpenTelemetry\SDK\Metrics\Util\TimerTrackerByObject(); - $attributesTracker = new \OpenTelemetry\SDK\Util\AttributeTrackerByObject(); + $timersTracker = new TimerTrackerByObject(); + + // 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 ($instrumentation, $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); + + $attributes = $pdoTracker->get($result); + $parent = Context::getCurrent(); + + $instrumentation->meter() + ->createUpDownCounter('db.client.connection.count', '1') + ->add(1, $attributes, $parent); + + $pdoTracker->trackPdoInstancesDestruction( + $object, + function ($pdoInstance) use ($instrumentation, $pdoTracker) { + $parent = Context::getCurrent(); + + $attributes = $pdoTracker->get($pdoInstance); + $instrumentation->meter() + ->createUpDownCounter('db.client.connection.count', '1') + ->add(-1, $attributes, $parent); + } + ); + } + ); + } + hook( PDO::class, '__construct', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $attributesTracker) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::__construct', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); - $connectionAttributes = [ - TraceAttributes::SERVER_ADDRESS => $params[0] ?? 'unknown', - TraceAttributes::SERVER_PORT => $params[0] ?? null - ]; - if ($class === PDO::class) { - //@todo split params[0] into host + port, replace deprecated trace attribute - $builder->setAttributes($connectionAttributes); - - $attributesTracker->set($pdo, $connectionAttributes); - } $parent = Context::getCurrent(); $span = $builder->startSpan(); Context::storage()->attach($span->storeInContext($parent)); - - $instrumentation->meter() - ->createUpDownCounter('db.client.connection.count', '1') - ->add(1, $connectionAttributes, $parent); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($pdoTracker) { + post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($instrumentation, $pdoTracker) { $scope = Context::storage()->scope(); if (!$scope) { return; @@ -75,52 +123,71 @@ public static function register(): void $span->setAttributes($attributes); self::end($exception); - } - ); - hook(PDO::class, '__destruct', post:function ($pdo) use ($instrumentation, $attributesTracker) { - $parent = Context::getCurrent(); + $attributes = $pdoTracker->get($pdo); + $parent = Context::getCurrent(); - $connectionAttributes = $attributesTracker->get($pdo); - $instrumentation->meter() - ->createUpDownCounter('db.client.connection.count', '1') - ->add(-1, $connectionAttributes, $parent); - }); + $instrumentation->meter() + ->createUpDownCounter('db.client.connection.count', '1') + ->add(1, $attributes, $parent); + + $pdoTracker->trackPdoInstancesDestruction( + $pdo, + function ($pdoInstance) use ($instrumentation, $pdoTracker) { + $parent = Context::getCurrent(); + + $attributes = $pdoTracker->get($pdoInstance); + $instrumentation->meter() + ->createUpDownCounter('db.client.connection.count', '1') + ->add(-1, $attributes, $parent); + } + ); + } + ); hook( PDO::class, 'query', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation, $timersTracker, $attributesTracker) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker, $timersTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::query', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); - + $encodedQuery = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); if ($class === PDO::class) { - $encodedQuery = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); $builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, $encodedQuery); - $attributesTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); } $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->trackedAttributesForPdo($pdo); + $attributes = $pdoTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); - $metricAttributes = $attributesTracker->get($pdo); - self::createPendingOperationMetric($instrumentation, $metricAttributes, 1); + self::createPendingOperationMetric($instrumentation, $attributes, 1); $timersTracker->start($pdo); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($instrumentation, $attributesTracker, $timersTracker) { + post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { $duration = $timersTracker->durationMs($pdo); - self::end($exception); - - $metricAttributes = $attributesTracker->get($pdo); - self::createDurationMetric($instrumentation, $metricAttributes, $duration); - self::createPendingOperationMetric($instrumentation, $metricAttributes, -1); + // this happens ONLY when error mode is set to silent + // this is an alternative to changes in the ::end method + // if ($statement === false && $exception === null) { + // $exception = new class($pdo->errorInfo()) extends \PDOException { + // // to workaround setting code that is not INT + // public function __construct(array $errorInfo) { + // $this->message = $errorInfo[2] ?? 'PDO error'; + // $this->code = $errorInfo[0] ?? 0; + // } + // }; + // } + + self::end($exception, $statement === false ? $pdo->errorInfo() : []); + + $attributes = $pdoTracker->get($pdo); + self::createDurationMetric($instrumentation, $attributes, $duration); + self::createPendingOperationMetric($instrumentation, $attributes, -1); if ($statement instanceof PDOStatement && $statement->rowCount()) { - self::createReturnedRowsMetric($instrumentation, $metricAttributes, $statement->rowCount()); + self::createReturnedRowsMetric($instrumentation, $attributes, $statement->rowCount()); } } ); @@ -128,37 +195,35 @@ public static function register(): void hook( PDO::class, 'exec', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation, $attributesTracker, $timersTracker) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker, $timersTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::exec', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); + $encodedQuery = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); if ($class === PDO::class) { - $encodedQuery = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); $builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, $encodedQuery); - $attributesTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); } $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->trackedAttributesForPdo($pdo); + $attributes = $pdoTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); - $metricAttributes = $attributesTracker->get($pdo); - self::createPendingOperationMetric($instrumentation, $metricAttributes, 1); + self::createPendingOperationMetric($instrumentation, $attributes, 1); $timersTracker->start($pdo); }, - post: static function (PDO $pdo, array $params, mixed $affectedRows, ?Throwable $exception) use ($instrumentation, $attributesTracker, $timersTracker) { + post: static function (PDO $pdo, array $params, mixed $affectedRows, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { $duration = $timersTracker->durationMs($pdo); - self::end($exception); + self::end($exception, $affectedRows === false ? $pdo->errorInfo() : []); - $metricAttributes = $attributesTracker->get($pdo); - self::createDurationMetric($instrumentation, $metricAttributes, $duration); - self::createPendingOperationMetric($instrumentation, $metricAttributes, -1); + $attributes = $pdoTracker->get($pdo); + self::createDurationMetric($instrumentation, $attributes, $duration); + self::createPendingOperationMetric($instrumentation, $attributes, -1); if (!empty($affectedRows)) { - self::createReturnedRowsMetric($instrumentation, $metricAttributes, $affectedRows); + self::createReturnedRowsMetric($instrumentation, $attributes, $affectedRows); } } ); @@ -166,96 +231,106 @@ public static function register(): void hook( PDO::class, 'prepare', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker, $timersTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::prepare', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); + $encodedQuery = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); if ($class === PDO::class) { - $builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8')); + $builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, $encodedQuery); } $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->trackedAttributesForPdo($pdo); + $attributes = $pdoTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); + + self::createPendingOperationMetric($instrumentation, $attributes, 1); + + $timersTracker->start($pdo); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($pdoTracker) { + post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { + $duration = $timersTracker->durationMs($pdo); if ($statement instanceof PDOStatement) { $pdoTracker->trackStatement($statement, $pdo, Span::getCurrent()->getContext()); } - self::end($exception); + self::end($exception, $statement === false ? $pdo->errorInfo() : []); + $attributes = $pdoTracker->get($pdo); + self::createDurationMetric($instrumentation, $attributes, $duration); + self::createPendingOperationMetric($instrumentation, $attributes, -1); } ); hook( PDO::class, 'beginTransaction', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::beginTransaction', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->trackedAttributesForPdo($pdo); + $attributes = $pdoTracker->get($pdo); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) { - self::end($exception); + post: static function (PDO $pdo, array $params, mixed $retval, ?Throwable $exception) { + self::end($exception, $retval === false ? $pdo->errorInfo() : []); } ); hook( PDO::class, 'commit', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::commit', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->trackedAttributesForPdo($pdo); + $attributes = $pdoTracker->get($pdo); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) { - self::end($exception); + post: static function (PDO $pdo, array $params, mixed $retval, ?Throwable $exception) { + self::end($exception, $retval === false ? $pdo->errorInfo() : []); } ); hook( PDO::class, 'rollBack', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::rollBack', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->trackedAttributesForPdo($pdo); + $attributes = $pdoTracker->get($pdo); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); }, - post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) { - self::end($exception); + post: static function (PDO $pdo, array $params, mixed $retval, ?Throwable $exception) { + self::end($exception, $retval === false ? $pdo->errorInfo() : []); } ); hook( PDOStatement::class, 'fetchAll', - pre: static function (PDOStatement $statement, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation) { + pre: static function (PDOStatement $statement, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker) { $attributes = $pdoTracker->trackedAttributesForStatement($statement); if (self::isDistributeStatementToLinkedSpansEnabled()) { + /** @psalm-suppress InvalidArrayAssignment */ $attributes[TraceAttributes::DB_QUERY_TEXT] = $statement->queryString; } /** @psalm-suppress ArgumentTypeCoercion */ @@ -265,9 +340,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); @@ -277,15 +353,14 @@ public static function register(): void hook( PDOStatement::class, 'execute', - pre: static function (PDOStatement $statement, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($pdoTracker, $instrumentation, $timersTracker, $attributesTracker) { + pre: static function (PDOStatement $statement, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker, $timersTracker) { $attributes = $pdoTracker->trackedAttributesForStatement($statement); if (self::isDistributeStatementToLinkedSpansEnabled()) { + /** @psalm-suppress InvalidArrayAssignment */ $attributes[TraceAttributes::DB_QUERY_TEXT] = $statement->queryString; } - $attributesTracker->append($statement, TraceAttributes::DB_QUERY_TEXT, $statement->queryString); - /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDOStatement::execute', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT) @@ -293,24 +368,24 @@ public static function register(): void if ($spanContext = $pdoTracker->getSpanForPreparedStatement($statement)) { $builder->addLink($spanContext); } + $parent = Context::getCurrent(); $span = $builder->startSpan(); + Context::storage()->attach($span->storeInContext($parent)); - Context::storage()->attach($span->storeInContext(Context::getCurrent())); - - $metricAttributes = $attributesTracker->get($statement); - self::createPendingOperationMetric($instrumentation, $metricAttributes, 1); + self::createPendingOperationMetric($instrumentation, $attributes, 1); $timersTracker->start($statement); }, - post: static function (PDOStatement $statement, array $params, mixed $retval, ?Throwable $exception) use ($instrumentation, $attributesTracker, $timersTracker) { + post: static function (PDOStatement $statement, array $params, mixed $retval, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { $duration = $timersTracker->durationMs($statement); - self::end($exception); + self::end($exception, $retval === false ? $statement->errorInfo() : []); - $metricAttributes = $attributesTracker->get($statement); - self::createDurationMetric($instrumentation, $metricAttributes, $duration); - self::createPendingOperationMetric($instrumentation, $metricAttributes, -1); + $attributes = $pdoTracker->trackedAttributesForStatement($statement); + self::createDurationMetric($instrumentation, $attributes, $duration); + self::createPendingOperationMetric($instrumentation, $attributes, -1); } ); } + private static function makeBuilder( CachedInstrumentation $instrumentation, string $name, @@ -322,12 +397,12 @@ private static function makeBuilder( /** @psalm-suppress ArgumentTypeCoercion */ return $instrumentation->tracer() ->spanBuilder($name) - ->setAttribute(TraceAttributes::CODE_FUNCTION_NAME, $function) - ->setAttribute(TraceAttributes::CODE_NAMESPACE, $class) - ->setAttribute(TraceAttributes::CODE_FILEPATH, $filename) + ->setAttribute(TraceAttributes::CODE_FUNCTION_NAME, sprintf('%s::%s', $class, $function)) + ->setAttribute(TraceAttributes::CODE_FILE_PATH, $filename) ->setAttribute(TraceAttributes::CODE_LINE_NUMBER, $lineno); } - private static function end(?Throwable $exception): void + + private static function end(?Throwable $exception, array $errorInfo = []): void { $scope = Context::storage()->scope(); if (!$scope) { @@ -338,6 +413,15 @@ private static function end(?Throwable $exception): void if ($exception) { $span->recordException($exception); $span->setStatus(StatusCode::STATUS_ERROR, $exception->getMessage()); + + } elseif (!empty($errorInfo[2]) && $errorMessage = $errorInfo[2]) { + $span->addEvent('exception', [ + 'exception.type' => PDOException::class, + 'exception.message' => $errorMessage, + // @todo try to add stacktrace? + ]); + + $span->setStatus(StatusCode::STATUS_ERROR, $errorMessage); } $span->end(); diff --git a/src/Instrumentation/PDO/src/PDOTracker.php b/src/Instrumentation/PDO/src/PDOTracker.php index 4753a0baa..d9bec5a86 100644 --- a/src/Instrumentation/PDO/src/PDOTracker.php +++ b/src/Instrumentation/PDO/src/PDOTracker.php @@ -4,7 +4,9 @@ namespace OpenTelemetry\Contrib\Instrumentation\PDO; +use Error; use OpenTelemetry\API\Trace\SpanContextInterface; +use OpenTelemetry\SDK\Util\AttributeTrackerByObject; use OpenTelemetry\SemConv\TraceAttributes; use PDO; use PDOStatement; @@ -14,7 +16,7 @@ /** * @phan-file-suppress PhanNonClassMethodCall,PhanTypeArraySuspicious */ -final class PDOTracker +final class PDOTracker extends AttributeTrackerByObject { /** * @var WeakMap> @@ -26,13 +28,15 @@ final class PDOTracker private WeakMap $statementMapToPdoMap; private WeakMap $preparedStatementToSpanMap; + private WeakMap $pdoInstances; + public function __construct() { - /** @psalm-suppress PropertyTypeCoercion */ - $this->pdoToAttributesMap = new WeakMap(); /** @psalm-suppress PropertyTypeCoercion */ $this->statementMapToPdoMap = new WeakMap(); $this->preparedStatementToSpanMap = new WeakMap(); + $this->pdoInstances = new WeakMap(); + parent::__construct(); } /** @@ -58,7 +62,7 @@ public function trackedAttributesForStatement(PDOStatement $statement): array } /** @psalm-var array */ - return $this->pdoToAttributesMap[$pdo] ?? []; + return $this->get($pdo); } /** @@ -75,15 +79,14 @@ public function trackPdoAttributes(PDO $pdo, string $dsn): array $dbSystem = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); /** @psalm-suppress InvalidArrayAssignment */ $attributes[TraceAttributes::DB_SYSTEM_NAME] = self::mapDriverNameToAttribute($dbSystem); - } catch (\Error) { - // if we caught an exception, the driver is likely not supporting the operation, default to "other" + } catch (Error) { + // if we caught an exception, and it is not set from extractAttributesFromDSN, + // the driver is likely not supporting the operation, default to "other" /** @psalm-suppress PossiblyInvalidArrayAssignment */ - $attributes[TraceAttributes::DB_SYSTEM_NAME] = 'other_sql'; + $attributes[TraceAttributes::DB_SYSTEM_NAME] ??= 'other_sql'; } - $this->pdoToAttributesMap[$pdo] = $attributes; - - return $attributes; + return $this->add($pdo, $attributes); } /** @@ -93,7 +96,22 @@ public function trackPdoAttributes(PDO $pdo, string $dsn): array public function trackedAttributesForPdo(PDO $pdo): array { /** @psalm-var array */ - return $this->pdoToAttributesMap[$pdo] ?? []; + return $this->get($pdo); + } + + public function trackPdoInstancesDestruction(PDO $pdo, callable $callbackFunction): void + { + $this->pdoInstances[$pdo] = new class($pdo, $callbackFunction) { + public function __construct( + private PDO $instance, + private $callback + ) { + } + public function __destruct() + { + ($this->callback)($this->instance); + } + }; } public function getSpanForPreparedStatement(PDOStatement $statement): ?SpanContextInterface diff --git a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php index 922fe30e0..66c9fa0a6 100644 --- a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php +++ b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php @@ -6,6 +6,7 @@ use ArrayObject; use OpenTelemetry\API\Common\Time\TestClock; +use OpenTelemetry\API\Globals; use OpenTelemetry\API\Instrumentation\Configurator; use OpenTelemetry\API\Trace\SpanInterface; use OpenTelemetry\API\Trace\SpanKind; @@ -14,11 +15,13 @@ use OpenTelemetry\SDK\Common\Attribute\Attributes; use OpenTelemetry\SDK\Common\Export\InMemoryStorageManager; use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeFactory; +use OpenTelemetry\SDK\Metrics\Data\Metric; +use OpenTelemetry\SDK\Metrics\Data\Sum; +use OpenTelemetry\SDK\Metrics\Data\Temporality; use OpenTelemetry\SDK\Metrics\Exemplar\ExemplarFilter\AllExemplarFilter; -use OpenTelemetry\SDK\Metrics\Exemplar\ExemplarFilter\WithSampledTraceExemplarFilter; use OpenTelemetry\SDK\Metrics\MeterProvider; use OpenTelemetry\SDK\Metrics\MetricReader\ExportingReader; -use OpenTelemetry\SDK\Metrics\StalenessHandler\ImmediateStalenessHandlerFactory; +use OpenTelemetry\SDK\Metrics\StalenessHandler\DelayedStalenessHandlerFactory; use OpenTelemetry\SDK\Metrics\View\CriteriaViewRegistry; use OpenTelemetry\SDK\Resource\ResourceInfoFactory; use OpenTelemetry\SDK\Trace\ImmutableSpan; @@ -28,6 +31,7 @@ use OpenTelemetry\SemConv\TraceAttributes; use OpenTelemetry\TestUtils\TraceStructureAssertionTrait; use PDO; +use PDOException; use PHPUnit\Framework\TestCase; class PDOInstrumentationTest extends TestCase @@ -39,6 +43,7 @@ class PDOInstrumentationTest extends TestCase private ArrayObject $storage; private $metricReader; private $meterProvider; + private $metricExporter; private function createDB(): PDO { @@ -81,8 +86,11 @@ public function setUp(): void ) ); $clock = new TestClock(); - $exporter = new \OpenTelemetry\SDK\Metrics\MetricExporter\InMemoryExporter(InMemoryStorageManager::metrics()); - $this->metricReader = new ExportingReader($exporter); + $this->metricExporter = new \OpenTelemetry\SDK\Metrics\MetricExporter\InMemoryExporter( + InMemoryStorageManager::metrics(), + Temporality::CUMULATIVE + ); + $this->metricReader = new ExportingReader($this->metricExporter); $this->meterProvider = new MeterProvider( null, ResourceInfoFactory::emptyResource(), @@ -92,7 +100,7 @@ public function setUp(): void [$this->metricReader], new CriteriaViewRegistry(), new AllExemplarFilter(), - new ImmediateStalenessHandlerFactory(), + new DelayedStalenessHandlerFactory($clock, 20), ); $this->scope = Configurator::create() @@ -163,7 +171,7 @@ public function test_pdo_sqlite_subclass(): void public function test_constructor_exception(): void { - $this->expectException(\PDOException::class); + $this->expectException(PDOException::class); $this->expectExceptionMessage('could not find driver'); new PDO('unknown:foo'); } @@ -431,7 +439,6 @@ public function test_span_hierarchy_with_pdo_operations(): void ); } - public function test_connection_metrics(): void { $db = self::createDB(); @@ -439,19 +446,76 @@ public function test_connection_metrics(): void $non_utf8_id = mb_convert_encoding('rückwärts', 'ISO-8859-1', 'UTF-8'); - $db->prepare("SELECT id FROM technology WHERE id = '{$non_utf8_id}'"); + $stmt = $db->prepare("SELECT id FROM technology WHERE id = '{$non_utf8_id}'"); $span_db_prepare = $this->storage->offsetGet(2); $this->assertTrue(mb_check_encoding($span_db_prepare->getAttributes()->get(TraceAttributes::DB_QUERY_TEXT), 'UTF-8')); $this->assertCount(3, $this->storage); + $stmt->execute(); + $stmt->fetchAll(); + unset($db); + $meterProvider = Globals::meterProvider(); + if ($meterProvider instanceof MeterProvider) { + $meterProvider->forceFlush(); + $meterProvider->shutdown(); + } + + $metricStats = []; + /** @var Metric $metric */ + foreach (InMemoryStorageManager::metrics() as $metric) { + $metricStats[$metric->name] ??= []; + $metricStats[$metric->name][] = $metric->data; + } + self::assertCount(2, $metricStats['db.client.connection.count']); + self::assertCount(2, $metricStats['db.client.connection.pending_requests']); + self::assertCount(2, $metricStats['db.client.operation.duration']); + self::assertCount(2, $metricStats['db.client.response.returned_rows']); + + // check count metrics: + $this->checkMetricSum($metricStats['db.client.connection.count'], 2, 2); + $this->checkMetricSum($metricStats['db.client.connection.pending_requests'], 0, 2); + $this->checkMetricDataPointsCount($metricStats['db.client.operation.duration'], 2); + $this->checkMetricDataPointsCount($metricStats['db.client.response.returned_rows'], 2); + } - $meterProvider = \OpenTelemetry\API\Globals::meterProvider(); + public function test_error_info_in_silent_mode(): void + { + $db = self::createDB(); + $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT); + $db->exec($this->fillDB()); + + $db->query('SELECT id FROM non_existing_table'); + + $meterProvider = Globals::meterProvider(); if ($meterProvider instanceof MeterProvider) { $meterProvider->forceFlush(); $meterProvider->shutdown(); } $metrics = InMemoryStorageManager::metrics(); - var_dump($metrics); + self::assertTrue(true); + } + + public function checkMetricDataPointsCount($metrics, $expectedCount): void + { + $sum = 0; + /** @var Sum $point */ + foreach ($metrics as $point) { + $sum += $point->dataPoints[0]->count; + } + self::assertEquals($expectedCount, $sum); + } + + public function checkMetricSum($metrics, $expectedSum, ?int $count = null): void + { + $sum = 0; + /** @var Sum $point */ + foreach ($metrics as $point) { + $sum += $point->dataPoints[0]->value; + } + self::assertEquals($expectedSum, $sum); + if ($count) { + self::assertCount($count, $metrics); + } } } From f917443d817c465b493161001a5230f8a4c3e8be Mon Sep 17 00:00:00 2001 From: Grzegorz Drozd <1885137+GrzegorzDrozd@users.noreply.github.com> Date: Mon, 3 Nov 2025 17:43:52 +0100 Subject: [PATCH 5/6] Add metrics to PDO - code update, dependency update --- .../PDO/src/PDOInstrumentation.php | 328 +++++++++--------- src/Instrumentation/PDO/src/PDOTracker.php | 6 +- .../Integration/PDOInstrumentationTest.php | 2 +- 3 files changed, 175 insertions(+), 161 deletions(-) diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index e48ada172..8aef5de38 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -5,16 +5,17 @@ namespace OpenTelemetry\Contrib\Instrumentation\PDO; use OpenTelemetry\API\Instrumentation\CachedInstrumentation; +use OpenTelemetry\API\Metrics\TimerTrackerByObject; 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 OpenTelemetry\SemConv\TraceAttributes; use function OpenTelemetry\Instrumentation\hook; use OpenTelemetry\SDK\Common\Configuration\Configuration; use OpenTelemetry\SemConv\Attributes\CodeAttributes; use OpenTelemetry\SemConv\Attributes\DbAttributes; -use OpenTelemetry\SDK\Metrics\Util\TimerTrackerByObject; use OpenTelemetry\SemConv\Version; use PDO; use PDOException; @@ -71,7 +72,7 @@ public static function register(): void $span = Span::fromContext($scope->context()); $dsn = $params[0] ?? ''; - + $attributes = []; // guard against PDO::connect returning a string if ($result instanceof PDO) { $attributes = $pdoTracker->trackPdoAttributes($result, $dsn); @@ -79,25 +80,7 @@ public static function register(): void } self::end($exception); - - $attributes = $pdoTracker->get($result); - $parent = Context::getCurrent(); - - $instrumentation->meter() - ->createUpDownCounter('db.client.connection.count', '1') - ->add(1, $attributes, $parent); - - $pdoTracker->trackPdoInstancesDestruction( - $object, - function ($pdoInstance) use ($instrumentation, $pdoTracker) { - $parent = Context::getCurrent(); - - $attributes = $pdoTracker->get($pdoInstance); - $instrumentation->meter() - ->createUpDownCounter('db.client.connection.count', '1') - ->add(-1, $attributes, $parent); - } - ); + self::createConnectionMetrics($instrumentation, $attributes, $pdoTracker, $object); } ); } @@ -105,7 +88,7 @@ function ($pdoInstance) use ($instrumentation, $pdoTracker) { hook( PDO::class, '__construct', - pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation, $pdoTracker) { + pre: static function (PDO $pdo, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::__construct', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); @@ -126,25 +109,7 @@ function ($pdoInstance) use ($instrumentation, $pdoTracker) { $span->setAttributes($attributes); self::end($exception); - - $attributes = $pdoTracker->get($pdo); - $parent = Context::getCurrent(); - - $instrumentation->meter() - ->createUpDownCounter('db.client.connection.count', '1') - ->add(1, $attributes, $parent); - - $pdoTracker->trackPdoInstancesDestruction( - $pdo, - function ($pdoInstance) use ($instrumentation, $pdoTracker) { - $parent = Context::getCurrent(); - - $attributes = $pdoTracker->get($pdoInstance); - $instrumentation->meter() - ->createUpDownCounter('db.client.connection.count', '1') - ->add(-1, $attributes, $parent); - } - ); + self::createConnectionMetrics($instrumentation, $attributes, $pdoTracker, $pdo); } ); @@ -155,52 +120,14 @@ function ($pdoInstance) use ($instrumentation, $pdoTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::query', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); - $query = mb_convert_encoding($params[0] ?? self::UNDEFINED, 'UTF-8'); - if (!is_string($query)) { - $query = self::UNDEFINED; - } - if ($class === PDO::class) { - $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, $query); - } - $parent = Context::getCurrent(); - $span = $builder->startSpan(); - - $attributes = $pdoTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); - $span->setAttributes($attributes); - - Context::storage()->attach($span->storeInContext($parent)); - - self::createPendingOperationMetric($instrumentation, $attributes, 1); - $timersTracker->start($pdo); - - if (class_exists('OpenTelemetry\Contrib\SqlCommenter\SqlCommenter') && $query !== self::UNDEFINED) { - if (array_key_exists(DbAttributes::DB_SYSTEM_NAME, $attributes)) { - /** @psalm-suppress PossiblyInvalidCast */ - switch ((string) $attributes[DbAttributes::DB_SYSTEM_NAME]) { - case 'postgresql': - case 'mysql': - /** - * @psalm-suppress UndefinedClass - */ - $commenter = \OpenTelemetry\Contrib\SqlCommenter\SqlCommenter::getInstance(); - $query = $commenter->inject($query); - if ($commenter->isAttributeEnabled()) { - $span->setAttributes([ - DbAttributes::DB_QUERY_TEXT => (string) $query, - ]); - } - - return [ - 0 => $query, - ]; - default: - // Do nothing, not a database we want to propagate - break; - } - } - } - - return []; + return self::preSendingQuery( + $instrumentation, + $builder, + $params[0], + $pdoTracker, + $pdo, + $timersTracker + ); }, post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { $duration = $timersTracker->durationMs($pdo); @@ -220,7 +147,12 @@ function ($pdoInstance) use ($instrumentation, $pdoTracker) { $attributes = $pdoTracker->get($pdo); self::createDurationMetric($instrumentation, $attributes, $duration); - self::createPendingOperationMetric($instrumentation, $attributes, -1); + self::createPendingOperationMetric( + $instrumentation, + 'db.client.connection.pending_requests', + 0, + $attributes + ); if ($statement instanceof PDOStatement && $statement->rowCount()) { self::createReturnedRowsMetric($instrumentation, $attributes, $statement->rowCount()); } @@ -234,52 +166,14 @@ function ($pdoInstance) use ($instrumentation, $pdoTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::exec', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); - $query = mb_convert_encoding($params[0] ?? self::UNDEFINED, 'UTF-8'); - if (!is_string($query)) { - $query = self::UNDEFINED; - } - if ($class === PDO::class) { - $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, $query); - } - $parent = Context::getCurrent(); - $span = $builder->startSpan(); - - $attributes = $pdoTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); - $span->setAttributes($attributes); - - Context::storage()->attach($span->storeInContext($parent)); - self::createPendingOperationMetric($instrumentation, $attributes, 1); - - $timersTracker->start($pdo); - - if (class_exists('OpenTelemetry\Contrib\SqlCommenter\SqlCommenter') && $query !== self::UNDEFINED) { - if (array_key_exists(DbAttributes::DB_SYSTEM_NAME, $attributes)) { - /** @psalm-suppress PossiblyInvalidCast */ - switch ((string) $attributes[DbAttributes::DB_SYSTEM_NAME]) { - case 'postgresql': - case 'mysql': - /** - * @psalm-suppress UndefinedClass - */ - $commenter = \OpenTelemetry\Contrib\SqlCommenter\SqlCommenter::getInstance(); - $query = $commenter->inject($query); - if ($commenter->isAttributeEnabled()) { - $span->setAttributes([ - DbAttributes::DB_QUERY_TEXT => (string) $query, - ]); - } - - return [ - 0 => $query, - ]; - default: - // Do nothing, not a database we want to propagate - break; - } - } - } - - return []; + return self::preSendingQuery( + $instrumentation, + $builder, + $params[0], + $pdoTracker, + $pdo, + $timersTracker + ); }, post: static function (PDO $pdo, array $params, mixed $affectedRows, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { $duration = $timersTracker->durationMs($pdo); @@ -287,7 +181,12 @@ function ($pdoInstance) use ($instrumentation, $pdoTracker) { $attributes = $pdoTracker->get($pdo); self::createDurationMetric($instrumentation, $attributes, $duration); - self::createPendingOperationMetric($instrumentation, $attributes, -1); + self::createPendingOperationMetric( + $instrumentation, + 'db.client.connection.pending_requests', + 0, + $attributes + ); if (!empty($affectedRows)) { self::createReturnedRowsMetric($instrumentation, $attributes, $affectedRows); } @@ -301,19 +200,24 @@ function ($pdoInstance) use ($instrumentation, $pdoTracker) { /** @psalm-suppress ArgumentTypeCoercion */ $builder = self::makeBuilder($instrumentation, 'PDO::prepare', $function, $class, $filename, $lineno) ->setSpanKind(SpanKind::KIND_CLIENT); - $encodedQuery = mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8'); + $query = mb_convert_encoding($params[0] ?? self::UNDEFINED, 'UTF-8'); if ($class === PDO::class) { $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, mb_convert_encoding($params[0] ?? 'undefined', 'UTF-8')); } $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $encodedQuery); + $attributes = $pdoTracker->append($pdo, TraceAttributes::DB_QUERY_TEXT, $query); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); - self::createPendingOperationMetric($instrumentation, $attributes, 1); + self::createPendingOperationMetric( + $instrumentation, + 'db.client.connection.pending_requests', + 1, + $attributes + ); $timersTracker->start($pdo); }, @@ -326,7 +230,12 @@ function ($pdoInstance) use ($instrumentation, $pdoTracker) { self::end($exception, $statement === false ? $pdo->errorInfo() : []); $attributes = $pdoTracker->get($pdo); self::createDurationMetric($instrumentation, $attributes, $duration); - self::createPendingOperationMetric($instrumentation, $attributes, -1); + self::createPendingOperationMetric( + $instrumentation, + 'db.client.connection.pending_requests', + 0, + $attributes + ); } ); @@ -380,7 +289,7 @@ function ($pdoInstance) use ($instrumentation, $pdoTracker) { $parent = Context::getCurrent(); $span = $builder->startSpan(); - $attributes = $pdoTracker->get($pdo); + $attributes = $pdoTracker->trackedAttributesForPdo($pdo); $span->setAttributes($attributes); Context::storage()->attach($span->storeInContext($parent)); @@ -412,7 +321,7 @@ function ($pdoInstance) use ($instrumentation, $pdoTracker) { Context::storage()->attach($span->storeInContext($parent)); }, post: static function (PDOStatement $statement, array $params, mixed $retval, ?Throwable $exception) { - self::end($exception); + self::end($exception, $retval === false ? $statement->errorInfo() : []); } ); @@ -438,7 +347,12 @@ function ($pdoInstance) use ($instrumentation, $pdoTracker) { $span = $builder->startSpan(); Context::storage()->attach($span->storeInContext($parent)); - self::createPendingOperationMetric($instrumentation, $attributes, 1); + self::createPendingOperationMetric( + $instrumentation, + 'db.client.connection.pending_requests', + 1, + $attributes + ); $timersTracker->start($statement); }, post: static function (PDOStatement $statement, array $params, mixed $retval, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { @@ -447,7 +361,12 @@ function ($pdoInstance) use ($instrumentation, $pdoTracker) { $attributes = $pdoTracker->trackedAttributesForStatement($statement); self::createDurationMetric($instrumentation, $attributes, $duration); - self::createPendingOperationMetric($instrumentation, $attributes, -1); + self::createPendingOperationMetric( + $instrumentation, + 'db.client.connection.pending_requests', + 0, + $attributes + ); } ); } @@ -468,7 +387,7 @@ private static function makeBuilder( ->setAttribute(CodeAttributes::CODE_LINE_NUMBER, $lineno); } - private static function end(?Throwable $exception, array $errorInfo = []): void + private static function end(Throwable|PDOException|null $exception, array $errorInfo = []): void { $scope = Context::storage()->scope(); if (!$scope) { @@ -476,18 +395,28 @@ private static function end(?Throwable $exception, array $errorInfo = []): void } $scope->detach(); $span = Span::fromContext($scope->context()); + + // when silent mode is set to true, there are no exceptions, so we need to create one to record it using + // a common way of creating exceptions. + // The only problem is that PHP Exception code is int and PDOException code is string + if ($exception === null && !empty($errorInfo[2])) { + /** @noinspection CallableParameterUseCaseInTypeContextInspection */ + $exception = new class($errorInfo) extends \PDOException { + // to workaround setting code that is not INT + /** + * @noinspection MagicMethodsValidityInspection + * @noinspection PhpMissingParentConstructorInspection + */ + public function __construct(array $errorInfo) { + $this->message = $errorInfo[2] ?? 'PDO error'; + $this->code = $errorInfo[0] ?? 0; + } + }; + } + if ($exception) { $span->recordException($exception); $span->setStatus(StatusCode::STATUS_ERROR, $exception->getMessage()); - - } elseif (!empty($errorInfo[2]) && $errorMessage = $errorInfo[2]) { - $span->addEvent('exception', [ - 'exception.type' => PDOException::class, - 'exception.message' => $errorMessage, - // @todo try to add stacktrace? - ]); - - $span->setStatus(StatusCode::STATUS_ERROR, $errorMessage); } $span->end(); @@ -504,12 +433,13 @@ private static function isDistributeStatementToLinkedSpansEnabled(): bool protected static function createPendingOperationMetric( CachedInstrumentation $instrumentation, - array $attributes, + string $metricName, int $value, + array $attributes, ): void { $parent = Context::getCurrent(); $instrumentation->meter() - ->createUpDownCounter('db.client.connection.pending_requests', '1') + ->createUpDownCounter($metricName, '1') ->add($value, $attributes, $parent); } @@ -534,4 +464,92 @@ protected static function createDurationMetric( ->createHistogram('db.client.operation.duration', 'ms') ->record($value, $attributes, $parent); } + + protected static function createConnectionMetrics( + CachedInstrumentation $instrumentation, + array $attributes, + PDOTracker $pdoTracker, + PDO $pdo + ): void { + self::createPendingOperationMetric( + $instrumentation, + 'db.client.connection.count', + 1, + $attributes + ); + + $pdoTracker->trackPdoInstancesDestruction( + $pdo, + function ($pdoInstance) use ($instrumentation, $pdoTracker) { + $attributes = $pdoTracker->get($pdoInstance); + + self::createPendingOperationMetric( + $instrumentation, + 'db.client.connection.count', + 0, + $attributes + ); + } + ); + } + + public static function preSendingQuery( + CachedInstrumentation $instrumentation, + SpanBuilderInterface $builder, + $query, + PDOTracker $pdoTracker, + PDO $pdo, + TimerTrackerByObject $timersTracker + ): array { + $query = mb_convert_encoding($query ?? self::UNDEFINED, 'UTF-8'); + if (!is_string($query)) { + $query = self::UNDEFINED; + } else { + $builder->setAttribute(DbAttributes::DB_QUERY_TEXT, $query); + } + $parent = Context::getCurrent(); + $span = $builder->startSpan(); + + $attributes = $pdoTracker->append($pdo, DbAttributes::DB_QUERY_TEXT, $query); + $span->setAttributes($attributes); + + Context::storage()->attach($span->storeInContext($parent)); + self::createPendingOperationMetric( + $instrumentation, + 'db.client.connection.pending_requests', + 1, + $attributes + ); + + $timersTracker->start($pdo); + + if (class_exists('OpenTelemetry\Contrib\SqlCommenter\SqlCommenter') && $query !== self::UNDEFINED) { + if (array_key_exists(DbAttributes::DB_SYSTEM_NAME, $attributes)) { + /** @psalm-suppress PossiblyInvalidCast */ + switch ((string)$attributes[DbAttributes::DB_SYSTEM_NAME]) { + case 'postgresql': + case 'mysql': + /** + * @psalm-suppress UndefinedClass + */ + $commenter = \OpenTelemetry\Contrib\SqlCommenter\SqlCommenter::getInstance(); + $query = $commenter->inject($query); + if ($commenter->isAttributeEnabled()) { + $span->setAttributes([ + DbAttributes::DB_QUERY_TEXT => (string)$query, + ]); + } + + return [ + 0 => $query, + ]; + default: + // Do nothing, not a database we want to propagate + break; + } + } + } + + return []; + } } diff --git a/src/Instrumentation/PDO/src/PDOTracker.php b/src/Instrumentation/PDO/src/PDOTracker.php index d9bec5a86..b9ea614d2 100644 --- a/src/Instrumentation/PDO/src/PDOTracker.php +++ b/src/Instrumentation/PDO/src/PDOTracker.php @@ -5,8 +5,8 @@ namespace OpenTelemetry\Contrib\Instrumentation\PDO; use Error; +use OpenTelemetry\API\Instrumentation\AutoInstrumentation\AttributeTrackerByObject; use OpenTelemetry\API\Trace\SpanContextInterface; -use OpenTelemetry\SDK\Util\AttributeTrackerByObject; use OpenTelemetry\SemConv\TraceAttributes; use PDO; use PDOStatement; @@ -18,10 +18,6 @@ */ final class PDOTracker extends AttributeTrackerByObject { - /** - * @var WeakMap> - */ - private WeakMap $pdoToAttributesMap; /** * @var WeakMap> */ diff --git a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php index 66c9fa0a6..de06542cc 100644 --- a/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php +++ b/src/Instrumentation/PDO/tests/Integration/PDOInstrumentationTest.php @@ -474,7 +474,7 @@ public function test_connection_metrics(): void // check count metrics: $this->checkMetricSum($metricStats['db.client.connection.count'], 2, 2); - $this->checkMetricSum($metricStats['db.client.connection.pending_requests'], 0, 2); + $this->checkMetricSum($metricStats['db.client.connection.pending_requests'], 2, 2); $this->checkMetricDataPointsCount($metricStats['db.client.operation.duration'], 2); $this->checkMetricDataPointsCount($metricStats['db.client.response.returned_rows'], 2); } From d835a3550738eab8488106c82f146e3e129752f9 Mon Sep 17 00:00:00 2001 From: Grzegorz Drozd <1885137+GrzegorzDrozd@users.noreply.github.com> Date: Mon, 3 Nov 2025 17:53:31 +0100 Subject: [PATCH 6/6] Add metrics to PDO - fix duration metric --- src/Instrumentation/PDO/src/PDOInstrumentation.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Instrumentation/PDO/src/PDOInstrumentation.php b/src/Instrumentation/PDO/src/PDOInstrumentation.php index 8aef5de38..e874f3315 100644 --- a/src/Instrumentation/PDO/src/PDOInstrumentation.php +++ b/src/Instrumentation/PDO/src/PDOInstrumentation.php @@ -130,7 +130,7 @@ public static function register(): void ); }, post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { - $duration = $timersTracker->durationMs($pdo); + $duration = $timersTracker->durationS($pdo); // this happens ONLY when error mode is set to silent // this is an alternative to changes in the ::end method // if ($statement === false && $exception === null) { @@ -176,7 +176,7 @@ public static function register(): void ); }, post: static function (PDO $pdo, array $params, mixed $affectedRows, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { - $duration = $timersTracker->durationMs($pdo); + $duration = $timersTracker->durationS($pdo); self::end($exception, $affectedRows === false ? $pdo->errorInfo() : []); $attributes = $pdoTracker->get($pdo); @@ -222,7 +222,7 @@ public static function register(): void $timersTracker->start($pdo); }, post: static function (PDO $pdo, array $params, mixed $statement, ?Throwable $exception) use ($instrumentation, $pdoTracker, $timersTracker) { - $duration = $timersTracker->durationMs($pdo); + $duration = $timersTracker->durationS($pdo); if ($statement instanceof PDOStatement) { $pdoTracker->trackStatement($statement, $pdo, Span::getCurrent()->getContext()); }