Skip to content

Commit 0d49f74

Browse files
committed
apply review feedback
- tidy constructor - do not use a WeakMap to track prepared statements, as the SpanContext is lost when prepare span flushed/exported
1 parent 7c3a020 commit 0d49f74

File tree

3 files changed

+21
-9
lines changed

3 files changed

+21
-9
lines changed

src/Instrumentation/Doctrine/src/DoctrineInstrumentation.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ public static function register(): void
185185
}
186186
);
187187
}
188+
188189
private static function makeBuilder(
189190
CachedInstrumentation $instrumentation,
190191
string $name,

src/Instrumentation/Doctrine/src/DoctrineTracker.php

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,28 +6,24 @@
66

77
use Doctrine\DBAL\Driver\Statement;
88
use OpenTelemetry\API\Trace\SpanContextInterface;
9-
use WeakMap;
10-
use WeakReference;
119

1210
/**
1311
* @internal
1412
*/
1513
class DoctrineTracker
1614
{
17-
private WeakMap $statementToSpanContextMap;
18-
19-
public function __construct()
20-
{
21-
$this->statementToSpanContextMap = new WeakMap();
15+
public function __construct(
16+
private \SplObjectStorage $statementToSpanContextMap = new \SplObjectStorage(),
17+
) {
2218
}
2319

2420
public function trackStatement(Statement $statement, SpanContextInterface $context): void
2521
{
26-
$this->statementToSpanContextMap[$statement] = WeakReference::create($context);
22+
$this->statementToSpanContextMap[$statement] = $context;
2723
}
2824

2925
public function getSpanContextForStatement(Statement $statement): ?SpanContextInterface
3026
{
31-
return $this->statementToSpanContextMap[$statement]?->get();
27+
return $this->statementToSpanContextMap[$statement] ?? null;
3228
}
3329
}

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,21 @@ public function test_prepare_then_execute_statement(): void
138138
$this->assertEquals($prepare->getContext(), $execute->getLinks()[0]->getSpanContext(), 'execute span is linked to prepare span');
139139
}
140140

141+
public function test_tracked_span_context_when_prepare_span_flushed(): void
142+
{
143+
$connection = self::createConnection();
144+
$statement = self::fillDB();
145+
$connection->executeStatement($statement);
146+
147+
$stmt = $connection->prepare('SELECT * FROM `technology` WHERE name = :name');
148+
$this->storage->exchangeArray([]); //removes the reference to prepared span, including the tracked SpanContext
149+
$stmt->bindValue('name', 'PHP');
150+
$stmt->executeQuery();
151+
152+
$execute = $this->storage->offsetGet(0);
153+
$this->assertCount(1, $execute->getLinks());
154+
}
155+
141156
public function test_query_with_bind_variables(): void
142157
{
143158
$connection = self::createConnection();

0 commit comments

Comments
 (0)