Skip to content

Commit 7f6cc09

Browse files
bwoebibwoebi
committed
Fix force-activating service mode and leaking the injected comment
Signed-off-by: Bob Weinand <[email protected]>
1 parent c47fb09 commit 7f6cc09

9 files changed

Lines changed: 72 additions & 20 deletions

File tree

‎ext/ddtrace.c‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2852,6 +2852,20 @@ PHP_FUNCTION(dd_trace_internal_fn) {
28522852
DDTRACE_G(sidecar_queue_id) = queueId; // usually we want to stop using it, except here
28532853
ddtrace_telemetry_lifecycle_end();
28542854
RETVAL_TRUE;
2855+
} else if (params_count == 3 && FUNCTION_NAME_MATCHES("force_overwrite_property")) {
2856+
zval *obj = ZVAL_VARARG_PARAM(params, 0);
2857+
zval *name = ZVAL_VARARG_PARAM(params, 1);
2858+
zval *value = ZVAL_VARARG_PARAM(params, 2);
2859+
if (Z_TYPE_P(obj) == IS_OBJECT && Z_TYPE_P(name) == IS_STRING) {
2860+
#if PHP_VERSION_ID < 80000
2861+
zend_std_write_property(obj, name, value, NULL);
2862+
RETVAL_TRUE;
2863+
#else
2864+
if (&EG(error_zval) != zend_std_write_property(Z_OBJ_P(obj), Z_STR_P(name), value, NULL)) {
2865+
RETVAL_TRUE;
2866+
}
2867+
#endif
2868+
}
28552869
} else if (params_count == 1 && FUNCTION_NAME_MATCHES("detect_composer_installed_json")) {
28562870
ddog_CharSlice path = dd_zend_string_to_CharSlice(Z_STR_P(ZVAL_VARARG_PARAM(params, 0)));
28572871
ddtrace_detect_composer_installed_json(&ddtrace_sidecar, ddtrace_sidecar_instance_id, &DDTRACE_G(sidecar_queue_id), path);

‎src/DDTrace/Integrations/DatabaseIntegrationHelper.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class DatabaseIntegrationHelper
2121
Tag::TARGET_HOST,
2222
];
2323

24-
public static function injectDatabaseIntegrationData(HookData $hook, $backend, $argNum = 0, $forcedMode = null)
24+
public static function injectDatabaseIntegrationData(HookData $hook, $backend, $argNum = 0, $preventFullMode = false)
2525
{
2626
$allowedBackends = [
2727
"sqlsrv" => true,
@@ -32,14 +32,14 @@ public static function injectDatabaseIntegrationData(HookData $hook, $backend, $
3232
"odbc" => true,
3333
];
3434

35-
$propagationMode = $forcedMode ?? dd_trace_env_config("DD_DBM_PROPAGATION_MODE");
35+
$propagationMode = dd_trace_env_config("DD_DBM_PROPAGATION_MODE");
3636
if ($propagationMode != \DDTrace\DBM_PROPAGATION_DISABLED && isset($allowedBackends[$backend])) {
3737
$fullPropagationBackends = [
3838
"mysql" => true,
3939
"pgsql" => true,
4040
];
4141

42-
if ($propagationMode == \DDTrace\DBM_PROPAGATION_FULL && !isset($fullPropagationBackends[$backend])) {
42+
if ($propagationMode == \DDTrace\DBM_PROPAGATION_FULL && (!isset($fullPropagationBackends[$backend]) || $preventFullMode)) {
4343
$propagationMode = \DDTrace\DBM_PROPAGATION_SERVICE;
4444
}
4545

‎src/DDTrace/Integrations/Mysqli/MysqliIntegration.php‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,15 +148,17 @@ function (SpanData $span, $args) {
148148

149149
\DDTrace\install_hook('mysqli_prepare', static function (HookData $hook) {
150150
list(, $query) = $hook->args;
151+
$hook->data = $query;
151152

152153
$span = $hook->span();
153154
self::setDefaultAttributes($span, 'mysqli_prepare', $query);
154155

155156
// For prepared statements, downgrade to service mode
156-
DatabaseIntegrationHelper::injectDatabaseIntegrationData($hook, 'mysql', 1, \DDTrace\DBM_PROPAGATION_SERVICE);
157+
DatabaseIntegrationHelper::injectDatabaseIntegrationData($hook, 'mysql', 1, true);
157158
self::handleRasp($span);
158159
}, static function (HookData $hook) {
159-
list($mysqli, $query) = $hook->args;
160+
list($mysqli) = $hook->args;
161+
$query = $hook->data; // unmodified query
160162
$span = $hook->span();
161163
self::setConnectionInfo($span, $mysqli);
162164

@@ -219,15 +221,16 @@ function (SpanData $span, $args) {
219221

220222
\DDTrace\install_hook('mysqli::prepare', static function (HookData $hook) {
221223
list($query) = $hook->args;
224+
$hook->data = $query;
222225

223226
$span = $hook->span();
224227
self::setDefaultAttributes($span, 'mysqli.prepare', $query);
225228

226229
// For prepared statements, downgrade to service mode
227-
DatabaseIntegrationHelper::injectDatabaseIntegrationData($hook, 'mysql', 0, \DDTrace\DBM_PROPAGATION_SERVICE);
230+
DatabaseIntegrationHelper::injectDatabaseIntegrationData($hook, 'mysql', 0, true);
228231
self::handleRasp($span);
229232
}, static function (HookData $hook) {
230-
list($query) = $hook->args;
233+
$query = $hook->data; // unmodified query
231234
$span = $hook->span();
232235
$instance = $hook->instance;
233236
MysqliIntegration::setConnectionInfo($span, $instance);

‎src/DDTrace/Integrations/PDO/PDOIntegration.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ public static function init(): int
117117
// public PDOStatement PDO::prepare ( string $statement [, array $driver_options = array() ] )
118118
\DDTrace\install_hook('PDO::prepare', static function (HookData $hook) {
119119
list($query) = $hook->args;
120+
$hook->data = $query;
120121

121122
$span = $hook->span();
122123
Integration::handleOrphan($span);
@@ -125,10 +126,14 @@ public static function init(): int
125126
$instance = $hook->instance;
126127
PDOIntegration::setCommonSpanInfo($instance, $span);
127128

128-
PDOIntegration::injectDBIntegration($instance, $hook, \DDTrace\DBM_PROPAGATION_SERVICE);
129+
PDOIntegration::injectDBIntegration($instance, $hook, true);
129130
PDOIntegration::handleRasp($instance, $span);
130131
}, static function (HookData $hook) {
131-
ObjectKVStore::propagate($hook->instance, $hook->returned, PDOIntegration::CONNECTION_TAGS_KEY);
132+
$pdo = $hook->returned;
133+
ObjectKVStore::propagate($hook->instance, $pdo, PDOIntegration::CONNECTION_TAGS_KEY);
134+
if ($pdo instanceof \PDOStatement) {
135+
\dd_trace_internal_fn("force_overwrite_property", $pdo, "queryString", $hook->data); // Restore the query string minus the DBM injected stuff
136+
}
132137
});
133138

134139
// public bool PDO::commit ( void )

‎src/DDTrace/Integrations/SQLSRV/SQLSRVIntegration.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,16 @@ public static function init(): int
6565
// sqlsrv_prepare ( resource $conn , string $query [, array $params [, array $options ]] ) : resource
6666
\DDTrace\install_hook('sqlsrv_prepare', static function (HookData $hook) {
6767
list($conn, $query) = $hook->args;
68+
$hook->data = $query;
6869

6970
$span = $hook->span();
7071
self::setDefaultAttributes($conn, $span, 'sqlsrv_prepare', $query);
7172

7273
// For prepared statements, downgrade to service mode
73-
DatabaseIntegrationHelper::injectDatabaseIntegrationData($hook, 'sqlsrv', 1, \DDTrace\DBM_PROPAGATION_SERVICE);
74+
DatabaseIntegrationHelper::injectDatabaseIntegrationData($hook, 'sqlsrv', 1, true);
7475
}, static function (HookData $hook) {
75-
list($conn, $query) = $hook->args;
76+
list($conn) = $hook->args;
77+
$query = $hook->data; // store actual query without DBM injected comment
7678
$span = $hook->span();
7779
if (\is_resource($hook->returned)) {
7880
resource_weak_store($hook->returned, self::CONNECTION_TAGS_KEY, resource_weak_get($conn, self::CONNECTION_TAGS_KEY));

‎tests/Integrations/Guzzle/Latest/response_body_stream_closed.phpt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,8 @@ for ($i = 1; $i <= 3; $i++) {
3333
--EXPECT--
3434
Request 1
3535
stream destructed
36-
stream destructed
3736
Request 2
3837
stream destructed
39-
stream destructed
4038
Request 3
4139
stream destructed
4240
stream destructed

‎tests/Integrations/Mysqli/MysqliTest.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
use DDTrace\Tag;
66
use DDTrace\Tests\Common\IntegrationTestCase;
77
use DDTrace\Tests\Common\SpanAssertion;
8+
use function DDTrace\close_span;
9+
use function DDTrace\start_trace_span;
810

911
class MysqliTest extends IntegrationTestCase
1012
{
@@ -33,6 +35,7 @@ protected function envsToCleanUpAtTearDown()
3335
'DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED',
3436
'DD_SERVICE',
3537
'DD_SERVICE_MAPPING',
38+
'DD_DBM_PROPAGATION_MODE',
3639
];
3740
}
3841

@@ -579,8 +582,12 @@ public function testProceduralPreparedStatementPeerServiceEnabled()
579582

580583
public function testPreparedStatementUsesServiceModeForDBM()
581584
{
585+
$this->putEnvAndReloadConfig(['DD_DBM_PROPAGATION_MODE=full']);
586+
582587
$query = "SELECT * FROM tests WHERE id = ?";
583588
$traces = $this->isolateTracer(function () use ($query) {
589+
start_trace_span();
590+
584591
$mysqli = new \mysqli(self::$host, self::$user, self::$password, self::$database);
585592
$stmt = $mysqli->prepare($query);
586593
$id = 1;
@@ -590,6 +597,8 @@ public function testPreparedStatementUsesServiceModeForDBM()
590597
$results = $result->fetch_all();
591598
$this->assertNotEmpty($results);
592599
$mysqli->close();
600+
601+
close_span();
593602
});
594603

595604
// Get the raw spans
@@ -621,6 +630,9 @@ public function testPreparedStatementUsesServiceModeForDBM()
621630
'mysqli_stmt.execute should be a sibling of mysqli.prepare'
622631
);
623632

633+
$this->assertEquals($query, $prepareSpan['resource']);
634+
$this->assertEquals($query, $executeSpan['resource']);
635+
624636
// Verify that SERVICE mode is used for the prepare span
625637
$this->assertArrayNotHasKey(
626638
'_dd.dbm_trace_injected',

‎tests/Integrations/PDO/PDOTest.php‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
use DDTrace\Tag;
66
use DDTrace\Tests\Common\IntegrationTestCase;
77
use DDTrace\Tests\Common\SpanAssertion;
8+
use function DDTrace\close_span;
9+
use function DDTrace\start_trace_span;
810

911
final class PDOTest extends IntegrationTestCase
1012
{
@@ -52,6 +54,7 @@ protected function envsToCleanUpAtTearDown()
5254
'DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED',
5355
'DD_SERVICE_MAPPING',
5456
'DD_SERVICE',
57+
'DD_DBM_PROPAGATION_MODE',
5558
];
5659
}
5760

@@ -682,8 +685,12 @@ public function testPDOStatementExceptionPeerServiceEnabled()
682685

683686
public function testPreparedStatementUsesServiceModeForDBM()
684687
{
688+
$this->putEnvAndReloadConfig(['DD_DBM_PROPAGATION_MODE=full']);
689+
685690
$query = "SELECT * FROM tests WHERE id = ?";
686691
$traces = $this->isolateTracer(function () use ($query) {
692+
start_trace_span();
693+
687694
$pdo = $this->pdoInstance();
688695
$stmt = $pdo->prepare($query);
689696
$stmt->execute([1]);
@@ -692,6 +699,8 @@ public function testPreparedStatementUsesServiceModeForDBM()
692699
$stmt->closeCursor();
693700
$stmt = null;
694701
$pdo = null;
702+
703+
close_span();
695704
});
696705

697706
// Get the raw spans
@@ -723,6 +732,9 @@ public function testPreparedStatementUsesServiceModeForDBM()
723732
'PDOStatement.execute should be a sibling of PDO.prepare'
724733
);
725734

735+
$this->assertEquals($query, $prepareSpan['resource']);
736+
$this->assertEquals($query, $executeSpan['resource']);
737+
726738
// Verify that SERVICE mode is used for the prepare span
727739
$this->assertArrayNotHasKey(
728740
'_dd.dbm_trace_injected',
@@ -758,12 +770,6 @@ public function testDirectQueryHasNoParentIssues()
758770
$this->assertNotNull($constructSpan, 'PDO.__construct span should exist');
759771
$this->assertNotNull($querySpan, 'PDO.query span should exist');
760772

761-
// Verify that query span has a parent (should be root or construct)
762-
$this->assertTrue(
763-
isset($querySpan['parent_id']),
764-
'PDO.query should have a parent_id'
765-
);
766-
767773
// Verify spans are created correctly with proper structure
768774
$this->assertSpans($traces, [
769775
SpanAssertion::exists('PDO.__construct'),
@@ -833,7 +839,7 @@ private function ensureActiveQueriesErrorCanHappen()
833839
{
834840
$opts = array(
835841
\PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION,
836-
\PDO::MYSQL_ATTR_USE_BUFFERED_QUERY => false
842+
PHP_VERSION_ID >= 80400 ? \Pdo\Mysql::ATTR_USE_BUFFERED_QUERY : \PDO::MYSQL_ATTR_USE_BUFFERED_QUERY => false
837843
);
838844

839845
$pdo = $this->pdoInstance($opts);

‎tests/Integrations/SQLSRV/SQLSRVTest.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
use DDTrace\Tag;
77
use DDTrace\Tests\Common\IntegrationTestCase;
88
use DDTrace\Tests\Common\SpanAssertion;
9+
use function DDTrace\close_span;
10+
use function DDTrace\start_trace_span;
911

1012
class SQLSRVTest extends IntegrationTestCase
1113
{
@@ -58,6 +60,7 @@ protected function envsToCleanUpAtTearDown()
5860
'DD_TRACE_PEER_SERVICE_DEFAULTS_ENABLED',
5961
'DD_TRACE_REMOVE_INTEGRATION_SERVICE_NAMES_ENABLED',
6062
'DD_SERVICE',
63+
'DD_DBM_PROPAGATION_MODE',
6164
];
6265
}
6366

@@ -325,12 +328,18 @@ public function testPrepareErrorPeerServiceEnabled()
325328

326329
public function testPreparedStatementUsesServiceModeForDBM()
327330
{
331+
$this->putEnvAndReloadConfig(['DD_DBM_PROPAGATION_MODE=full']);
332+
328333
$query = "SELECT * FROM tests WHERE id = ?";
329334
$traces = $this->isolateTracer(function () use ($query) {
335+
start_trace_span();
336+
330337
$conn = $this->createConnection();
331338
$stmt = sqlsrv_prepare($conn, $query, [1], ['Scrollable' => 'buffered']);
332339
sqlsrv_execute($stmt);
333340
sqlsrv_close($conn);
341+
342+
close_span();
334343
});
335344

336345
// Get the raw spans
@@ -362,6 +371,9 @@ public function testPreparedStatementUsesServiceModeForDBM()
362371
'sqlsrv_execute should be a sibling of sqlsrv_prepare'
363372
);
364373

374+
$this->assertEquals($query, $prepareSpan['resource']);
375+
$this->assertEquals($query, $executeSpan['resource']);
376+
365377
// Verify that SERVICE mode is used for the prepare span
366378
$this->assertArrayNotHasKey(
367379
'_dd.dbm_trace_injected',

0 commit comments

Comments
 (0)