Skip to content

Commit 753738a

Browse files
committed
feat(PDO): correct a bug on prepared statement regarding DBM correlation
Signed-off-by: Alexandre Rulleau <alexandre.rulleau@datadoghq.com>
1 parent 575faf4 commit 753738a

File tree

2 files changed

+137
-8
lines changed

2 files changed

+137
-8
lines changed

src/DDTrace/Integrations/PDO/PDOIntegration.php

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ class PDOIntegration extends Integration
1515
const NAME = 'pdo';
1616

1717
const CONNECTION_TAGS_KEY = 'connection_tags';
18+
const PREPARE_SPAN_KEY = 'prepare_span';
1819

1920
const DB_DRIVER_TO_SYSTEM = [
2021
'cubrid' => 'other_sql',
@@ -118,17 +119,37 @@ public static function init(): int
118119
\DDTrace\install_hook('PDO::prepare', static function (HookData $hook) {
119120
list($query) = $hook->args;
120121

121-
$span = $hook->span();
122-
Integration::handleOrphan($span);
123-
$span->name = 'PDO.prepare';
124-
$span->resource = Integration::toString($query);
122+
// Create the prepare span
123+
$prepareSpan = $hook->span();
124+
Integration::handleOrphan($prepareSpan);
125+
$prepareSpan->name = 'PDO.prepare';
126+
$prepareSpan->resource = Integration::toString($query);
125127
$instance = $hook->instance;
126-
PDOIntegration::setCommonSpanInfo($instance, $span);
128+
PDOIntegration::setCommonSpanInfo($instance, $prepareSpan);
129+
130+
// Create the execute span as a child of prepare for DBM injection
131+
// This ensures traceparent points to the execute span
132+
$executeSpan = \DDTrace\start_span();
133+
$executeSpan->name = 'PDOStatement.execute';
134+
$executeSpan->resource = Integration::toString($query);
135+
PDOIntegration::setCommonSpanInfo($instance, $executeSpan);
127136

137+
// Inject DBM with the execute span's ID
128138
PDOIntegration::injectDBIntegration($instance, $hook);
129-
PDOIntegration::handleRasp($instance, $span);
139+
PDOIntegration::handleRasp($instance, $executeSpan);
140+
141+
// Close the execute span (removes from stack) but don't finish it yet
142+
// It will be resumed during execute
143+
\DDTrace\close_span();
144+
145+
// Store the execute span to be resumed by execute
146+
$hook->data = ['execute_span' => $executeSpan];
130147
}, static function (HookData $hook) {
131148
ObjectKVStore::propagate($hook->instance, $hook->returned, PDOIntegration::CONNECTION_TAGS_KEY);
149+
// Store the execute span in the PDOStatement for later use
150+
if ($hook->returned instanceof \PDOStatement && !$hook->exception && isset($hook->data['execute_span'])) {
151+
ObjectKVStore::put($hook->returned, PDOIntegration::PREPARE_SPAN_KEY, $hook->data['execute_span']);
152+
}
132153
});
133154

134155
// public bool PDO::commit ( void )
@@ -143,12 +164,29 @@ public static function init(): int
143164
\DDTrace\install_hook(
144165
'PDOStatement::execute',
145166
static function (HookData $hook) {
146-
$hook->span();
167+
// Check if we have a stored execute span from prepare
168+
$storedSpan = ObjectKVStore::get($hook->instance, PDOIntegration::PREPARE_SPAN_KEY);
169+
if (!$storedSpan) {
170+
// No stored span (e.g., from PDO::query()), create normally
171+
$hook->span();
172+
}
173+
// If we have a stored span, don't create a new one - we'll use the stored one
174+
// The stored span was already created during prepare with the correct traceparent
147175
},
148176
static function (HookData $hook) {
149-
$span = $hook->span();
150177
$instance = $hook->instance;
178+
$storedSpan = ObjectKVStore::get($instance, PDOIntegration::PREPARE_SPAN_KEY);
179+
180+
if ($storedSpan) {
181+
// Use the execute span that was created during prepare
182+
$span = $storedSpan;
183+
} else {
184+
// No stored span (e.g., from PDO::query()), use the hook's span
185+
$span = $hook->span();
186+
}
187+
151188
Integration::handleOrphan($span);
189+
// Update span properties now that execute has completed
152190
$span->name = 'PDOStatement.execute';
153191
Integration::handleInternalSpanServiceName($span, PDOIntegration::NAME);
154192
$span->type = Type::SQL;

tests/Integrations/PDO/PDOTest.php

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,6 +680,97 @@ public function testPDOStatementExceptionPeerServiceEnabled()
680680
]);
681681
}
682682

683+
public function testPreparedStatementExecuteIsChildOfPrepare()
684+
{
685+
$query = "SELECT * FROM tests WHERE id = ?";
686+
$traces = $this->isolateTracer(function () use ($query) {
687+
$pdo = $this->pdoInstance();
688+
$stmt = $pdo->prepare($query);
689+
$stmt->execute([1]);
690+
$results = $stmt->fetchAll();
691+
$this->assertEquals('Tom', $results[0]['name']);
692+
$stmt->closeCursor();
693+
$stmt = null;
694+
$pdo = null;
695+
});
696+
697+
// Get the raw spans to check parent-child relationship
698+
$spans = $traces[0];
699+
700+
// Find prepare and execute spans
701+
$constructSpan = null;
702+
$prepareSpan = null;
703+
$executeSpan = null;
704+
705+
foreach ($spans as $span) {
706+
if ($span['name'] === 'PDO.__construct') {
707+
$constructSpan = $span;
708+
} elseif ($span['name'] === 'PDO.prepare') {
709+
$prepareSpan = $span;
710+
} elseif ($span['name'] === 'PDOStatement.execute') {
711+
$executeSpan = $span;
712+
}
713+
}
714+
715+
$this->assertNotNull($constructSpan, 'PDO.__construct span should exist');
716+
$this->assertNotNull($prepareSpan, 'PDO.prepare span should exist');
717+
$this->assertNotNull($executeSpan, 'PDOStatement.execute span should exist');
718+
719+
// Verify that execute span's parent is the prepare span
720+
$this->assertEquals(
721+
$prepareSpan['span_id'],
722+
$executeSpan['parent_id'],
723+
'PDOStatement.execute should be a child of PDO.prepare'
724+
);
725+
}
726+
727+
public function testDirectQueryHasNoParentIssues()
728+
{
729+
$query = "SELECT * FROM tests WHERE id=1";
730+
$traces = $this->isolateTracer(function () use ($query) {
731+
$pdo = $this->pdoInstance();
732+
$pdo->query($query);
733+
$pdo = null;
734+
});
735+
736+
// Get the raw spans
737+
$spans = $traces[0];
738+
739+
// Find construct and query spans
740+
$constructSpan = null;
741+
$querySpan = null;
742+
743+
foreach ($spans as $span) {
744+
if ($span['name'] === 'PDO.__construct') {
745+
$constructSpan = $span;
746+
} elseif ($span['name'] === 'PDO.query') {
747+
$querySpan = $span;
748+
}
749+
}
750+
751+
$this->assertNotNull($constructSpan, 'PDO.__construct span should exist');
752+
$this->assertNotNull($querySpan, 'PDO.query span should exist');
753+
754+
// Verify that query span has a parent (should be root or construct)
755+
$this->assertTrue(
756+
isset($querySpan['parent_id']),
757+
'PDO.query should have a parent_id'
758+
);
759+
760+
// Verify spans are created correctly with proper structure
761+
$this->assertSpans($traces, [
762+
SpanAssertion::exists('PDO.__construct'),
763+
SpanAssertion::build('PDO.query', 'pdo', 'sql', $query)
764+
->withExactTags($this->baseTags())
765+
->withExactMetrics([
766+
Tag::DB_ROW_COUNT => 1.0,
767+
Tag::ANALYTICS_KEY => 1.0,
768+
'_dd.agent_psr' => 1.0,
769+
'_sampling_priority_v1' => 1.0,
770+
]),
771+
]);
772+
}
773+
683774
public function testLimitedTracerPDO()
684775
{
685776
$query = "SELECT * FROM tests WHERE id = ?";

0 commit comments

Comments
 (0)