Skip to content

Commit 8229200

Browse files
DominicDettaDominicDetta
committed
fix: apply semantic conventions on db attributes
1 parent 32d028b commit 8229200

File tree

4 files changed

+165
-17
lines changed

4 files changed

+165
-17
lines changed

‎src/Instrumentation/Doctrine/composer.json‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@
4848
},
4949
"config": {
5050
"allow-plugins": {
51-
"php-http/discovery": false
51+
"php-http/discovery": false,
52+
"tbachert/spi": true
5253
}
5354
}
54-
}
55+
}

‎src/Instrumentation/Doctrine/src/AttributesResolver.php‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace OpenTelemetry\Contrib\Instrumentation\Doctrine;
6+
7+
use Doctrine\DBAL\SQL\Parser;
8+
use Exception;
9+
10+
final class AttributesResolver
11+
{
12+
/**
13+
*
14+
* Values:
15+
* See list of well-known values at https://opentelemetry.io/docs/specs/semconv/database/sql/
16+
*
17+
* Keys:
18+
* optional driver names used in Doctrine to indicate the well-known values to use in opentelemetry
19+
* See list of available doctrine drivers: https://www.doctrine-project.org/projects/doctrine-dbal/en/4.2/reference/configuration.html#driver
20+
*
21+
* Multiple entries can be created for the same well-known value
22+
*/
23+
private const DB_SYSTEMS_KNOWN = [
24+
'cockroachdb',
25+
'ibm_db2' => 'db2',
26+
'derby',
27+
'edb',
28+
'firebird',
29+
'h2',
30+
'hsqldb',
31+
'ingres',
32+
'interbase',
33+
'mariadb',
34+
'maxdb',
35+
'sqlsrv' => 'mssql',
36+
'mssqlcompact',
37+
'mysqli' => 'mysql',
38+
'oci8' => 'oracle',
39+
'pervasive',
40+
'pgsql' => 'postgresql',
41+
'sqlite3' => 'sqlite',
42+
'trino',
43+
];
44+
45+
public static function get(string $attributeName, array $params): string
46+
{
47+
$method = 'get' . str_replace('.', '', ucwords($attributeName, '.'));
48+
49+
if (!method_exists(AttributesResolver::class, $method)) {
50+
throw new Exception(sprintf('Attribute %s not supported by Doctrine', $attributeName));
51+
}
52+
53+
return self::{$method}($params);
54+
}
55+
56+
/**
57+
* Resolve attribute `server.address`
58+
*/
59+
private static function getServerAddress(array $params): string
60+
{
61+
return $params[1][0]['host'] ?? 'unknown';
62+
}
63+
64+
/**
65+
* Resolve attribute `server.port`
66+
*/
67+
private static function getServerPort(array $params): string
68+
{
69+
return $params[1][0]['port'] ?? 'unknown';
70+
}
71+
72+
/**
73+
* Resolve attribute `db.system`
74+
*/
75+
private static function getDbSystem(array $params): string
76+
{
77+
$dbSystem = $params[1][0]['driver'] ?? null;
78+
79+
if (strpos($dbSystem, 'pdo_') !== false) {
80+
// Remove pdo_ word to ignore it while searching well-known db.system
81+
$dbSystem = ltrim($dbSystem, 'pdo_');
82+
}
83+
84+
if (in_array($dbSystem, self::DB_SYSTEMS_KNOWN)) {
85+
return $dbSystem;
86+
}
87+
88+
// Fetch the db system using the alias if exists
89+
if (isset(self::DB_SYSTEMS_KNOWN[$dbSystem])) {
90+
return self::DB_SYSTEMS_KNOWN[$dbSystem];
91+
}
92+
return 'other_sql';
93+
}
94+
95+
/**
96+
* Resolve attribute `db.collection.name`
97+
*/
98+
private static function getDbCollectionName(array $params): string
99+
{
100+
return $params[1][0]['dbname'] ?? 'unknown';
101+
}
102+
103+
/**
104+
* Resolve attribute `db.query.text`
105+
* No sanitization is implemented because implicitly the query is expected to be expressed as a preparated statement
106+
* which happen automatically in Doctrine if parameters are bound to the query.
107+
*/
108+
private static function getDbQueryText(array $params): string
109+
{
110+
return $params[1][0] ?? 'undefined';
111+
}
112+
113+
private static function getDbNamespace(array $params): string
114+
{
115+
return $params[1][0]['dbname'] ?? 'unknown';
116+
}
117+
118+
/**
119+
* Resolve attribute `db.query.summary`
120+
* See https://opentelemetry.io/docs/specs/semconv/database/database-spans/#generating-a-summary-of-the-query-text
121+
*/
122+
public static function getDbQuerySummary(array $params): string
123+
{
124+
$query = $params[0] ?? null;
125+
126+
if (!$query) {
127+
return '';
128+
}
129+
130+
// Fetch operation name
131+
$operationName = explode(' ', $query);
132+
$operationName = $operationName[0];
133+
134+
// Fetch target name
135+
$matches = [];
136+
preg_match_all('/(from|into|update|join)\s*([a-zA-Z0-9[\]_]+)/i', $query, $matches);
137+
138+
$targetName = null;
139+
if (strtolower($operationName) == 'select') {
140+
if ($matches && isset($matches[2]) && $matches[2]) {
141+
$targetName = implode(' ', $matches[2]);
142+
}
143+
} elseif ($matches) {
144+
$targetName = $matches[2][0] ?? '';
145+
}
146+
return $operationName . ($targetName ? ' ' . $targetName : '');
147+
}
148+
}

‎src/Instrumentation/Doctrine/src/DoctrineInstrumentation.php‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,11 @@ public static function register(): void
3030
pre: static function (\Doctrine\DBAL\Driver $driver, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation) {
3131
/** @psalm-suppress ArgumentTypeCoercion */
3232
$builder = self::makeBuilder($instrumentation, 'Doctrine\DBAL\Driver::connect', $function, $class, $filename, $lineno)
33-
->setSpanKind(SpanKind::KIND_CLIENT);
34-
$builder
35-
->setAttribute(TraceAttributes::SERVER_ADDRESS, $params[0]['host'] ?? 'unknown')
36-
->setAttribute(TraceAttributes::SERVER_PORT, $params[0]['port'] ?? 'unknown')
37-
->setAttribute(TraceAttributes::DB_SYSTEM, $params[0]['driver'] ?? 'unknown')
38-
->setAttribute(TraceAttributes::DB_NAMESPACE, $params[0]['dbname'] ?? 'unknown');
33+
->setSpanKind(SpanKind::KIND_CLIENT)
34+
->setAttribute(TraceAttributes::SERVER_ADDRESS, AttributesResolver::get(TraceAttributes::SERVER_ADDRESS, func_get_args()))
35+
->setAttribute(TraceAttributes::SERVER_PORT, AttributesResolver::get(TraceAttributes::SERVER_PORT, func_get_args()))
36+
->setAttribute(TraceAttributes::DB_SYSTEM, AttributesResolver::get(TraceAttributes::DB_SYSTEM, func_get_args()))
37+
->setAttribute(TraceAttributes::DB_NAMESPACE, AttributesResolver::get(TraceAttributes::DB_NAMESPACE, func_get_args()));
3938
$parent = Context::getCurrent();
4039
$span = $builder->startSpan();
4140
Context::storage()->attach($span->storeInContext($parent));
@@ -50,9 +49,9 @@ public static function register(): void
5049
'query',
5150
pre: static function (\Doctrine\DBAL\Driver\Connection $connection, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation) {
5251
/** @psalm-suppress ArgumentTypeCoercion */
53-
$builder = self::makeBuilder($instrumentation, 'Doctrine\DBAL\Driver\Connection::query', $function, $class, $filename, $lineno)
52+
$builder = self::makeBuilder($instrumentation, AttributesResolver::getDbQuerySummary($params), $function, $class, $filename, $lineno)
5453
->setSpanKind(SpanKind::KIND_CLIENT);
55-
$builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, $params[0] ?? 'undefined');
54+
$builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, AttributesResolver::get(TraceAttributes::DB_QUERY_TEXT, func_get_args()));
5655
$parent = Context::getCurrent();
5756
$span = $builder->startSpan();
5857
Context::storage()->attach($span->storeInContext($parent));
@@ -67,9 +66,9 @@ public static function register(): void
6766
'exec',
6867
pre: static function (\Doctrine\DBAL\Driver\Connection $connection, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation) {
6968
/** @psalm-suppress ArgumentTypeCoercion */
70-
$builder = self::makeBuilder($instrumentation, 'Doctrine\DBAL\Driver\Connection::exec', $function, $class, $filename, $lineno)
71-
->setSpanKind(SpanKind::KIND_CLIENT);
72-
$builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, $params[0] ?? 'undefined');
69+
$builder = self::makeBuilder($instrumentation, AttributesResolver::getDbQuerySummary($params), $function, $class, $filename, $lineno)
70+
->setSpanKind(SpanKind::KIND_CLIENT)
71+
->setAttribute(TraceAttributes::DB_QUERY_TEXT, AttributesResolver::get(TraceAttributes::DB_QUERY_TEXT, func_get_args()));
7372
$parent = Context::getCurrent();
7473
$span = $builder->startSpan();
7574

@@ -85,9 +84,9 @@ public static function register(): void
8584
'prepare',
8685
pre: static function (\Doctrine\DBAL\Driver\Connection $connection, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation) {
8786
/** @psalm-suppress ArgumentTypeCoercion */
88-
$builder = self::makeBuilder($instrumentation, 'Doctrine\DBAL\Driver\Connection::prepare', $function, $class, $filename, $lineno)
89-
->setSpanKind(SpanKind::KIND_CLIENT);
90-
$builder->setAttribute(TraceAttributes::DB_QUERY_TEXT, $params[0] ?? 'undefined');
87+
$builder = self::makeBuilder($instrumentation, AttributesResolver::getDbQuerySummary($params), $function, $class, $filename, $lineno)
88+
->setSpanKind(SpanKind::KIND_CLIENT)
89+
->setAttribute(TraceAttributes::DB_QUERY_TEXT, AttributesResolver::get(TraceAttributes::DB_QUERY_TEXT, func_get_args()));
9190
$parent = Context::getCurrent();
9291
$span = $builder->startSpan();
9392

‎src/Instrumentation/Doctrine/tests/Integration/DoctrineInstrumentationTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public function test_connection(): void
7979
$this->assertTrue($conn->isConnected());
8080
$span = $this->storage->offsetGet(0);
8181
$this->assertSame('Doctrine\DBAL\Driver::connect', $span->getName());
82-
$this->assertEquals('sqlite3', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM));
82+
$this->assertEquals('sqlite', $span->getAttributes()->get(TraceAttributes::DB_SYSTEM));
8383
}
8484

8585
public function test_connection_exception(): void

0 commit comments

Comments
 (0)