Conversation
Add DD_DBM_TRACE_PREPARED_STATEMENTS (datadog.dbm_trace_prepared_statements, default false). When enabled together with DD_DBM_PROPAGATION_MODE=full, the prepare hooks (PDO::prepare, mysqli::prepare, mysqli_prepare) no longer downgrade to service mode on the full-propagation backends, so a prepared statement carries a traceparent like any other statement and the prepare span carries _dd.dbm_trace_injected. Since DataDog#3545 an application whose framework prepares every statement gets service-mode DBM from a full-mode configuration: the comment is written while the prepare span is active and the statement executes later in a sibling span, so the tracer stopped propagating. That was the right default and stays the default. This option lets an application accept the prepare span as the linked span (same trace, same statement, one sibling from the execution) rather than have no span-level link at all, which is what every release before DataDog#3545 did. The change is one condition in DatabaseIntegrationHelper::injectDatabaseIntegrationData(): the option is read as the right operand behind $preventFullMode, so it is only consulted on a prepare that was already about to downgrade in full mode on mysql or pgsql. Non-prepared statements, other modes, and backends outside $fullPropagationBackends never reach it. No public API change. Tests: the DataDog#3545 tests for PDO, mysqli and sqlsrv are unchanged; each gains a sibling with the option on (PDO and mysqli: prepare span tagged, execute still a sibling, resources clean; sqlsrv: still service mode). Two helper-level cases pin that the comment carries the injecting span's own id and that the option is a no-op outside full mode. Addresses DataDog#2993. 🤖 Generated with Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds
DD_DBM_TRACE_PREPARED_STATEMENTS(INIdatadog.dbm_trace_prepared_statements, boolean, defaultfalse).When it is enabled and
DD_DBM_PROPAGATION_MODE=full, the prepare hooks (PDO::prepare,mysqli::prepare,mysqli_prepare) stop downgrading toservicemode on the full-propagation backends (mysql,pgsql). Theprepared statement then carries a
traceparentexactly as a non-prepared statement does, and the prepare spancarries
_dd.dbm_trace_injected. Nothing else changes: the default is off, backends outside$fullPropagationBackendsstill downgrade, and no non-prepared code path reads the option.The whole behaviour change is one condition in
DatabaseIntegrationHelper::injectDatabaseIntegrationData():Addresses #2993.
What this unlocks
Since #3545, an application whose framework prepares every statement gets
service-mode Database Monitoringout of a
full-mode configuration. Laravel's query builder is the common case (PDO::preparefollowed byPDOStatement::executefor every query); Doctrine DBAL and most PDO wrappers behave the same way. For thoseapplications, with this option:
fulltodayfull+ this optiondddbs,dde,ddps,ddpvtraceparenttrace.modeservicefullPDO.prepare/mysqli.preparespan shows the linked sample and its explain planThe backend joins on the
(trace_id, span_id)in the comment and copies the resolved span'soperation,service,duration,peer.db.nameandpeer.hostnameonto the sample. So a sample from a prepare-everythingapplication goes from "came from service X" to "came from this request, this span, this statement". That is the
piece #3545 removed for these applications, and it has had no replacement since.
Background
#3545 made the prepare hooks use
servicemode, and 7f6cc09 turned that into the$preventFullModeparameterthis option gates. The reasoning was correct: the comment is written while the prepare span is active, the
statement runs later in a sibling span, so the propagated span id was not the span that executed the query.
The side effect is that a framework which prepares unconditionally loses span-level linking for all of its
queries, not just for a few. #2993 shows the symptom without a diagnosis ("I have set env
DD_DBM_PROPAGATION_MODE=fullon my app but i don't have trace") and went stale. On one Laravel service we runagainst MySQL 8 with
fullconfigured, every one of the roughly 12,000 prepared-statement executions DBMsampled in a day records
"mode":"service".Every release before #3545 propagated the prepare span's context for prepared statements. This PR does not
revert #3545; it puts that earlier behaviour behind an explicit opt-in, for applications that would rather have
the prepare span than nothing. The default and the three #3545 tests are untouched.
Design: why the prepare span's own context
There are three candidates for the span id to put in the comment at prepare time.
The execute span. It does not exist yet, and PHP has no writable span id (
SpanDataexposes onlyhexId()), so it cannot be pre-allocated at prepare time the way dd-trace-java does for SQL Server. Thatwould need extension-level work and is out of scope here.
The parent span, as the common ancestor of prepare and execute. I built this first and it is wrong, for a
reason visible in what the backend does with the link. A full-mode sample resolves to this shape (ids and
names replaced):
The ids decode from the comment's
traceparent, and the fields copied onto the sample are database-shaped.A
web.requestor controller span has nopeer.*tags and itsdurationis the whole request, so thesample would attach to a non-database span showing a plausible but wrong duration. It would also break an
invariant every tracer keeps today: the span tagged
_dd.dbm_trace_injectedis the span whose id is in thecomment, and it is always the SQL span (java
StatementInstrumentation.java:128-135, py_database_monitoring.py:143-145, gocontrib/database/sql/conn.go:124, dotnetDbScopeFactory.cs:115-121,rb
sql_comment.rb:26, jsplugins/database.js:133-136).The prepare span. It is
span.type=sql, its service is the integration's, it carries the connection'speer.*tags, and Obfuscate the :name placeholders in PDO away for DBM correlation #3801 already keeps its resource equal to the statement DBM sees. It is the same trace andthe same statement, one sibling from the span that executed it. This is what the PR does, and it is what
every release before feat(PDO): correct a bug on prepared statement regarding DBM correlation #3545 did.
Trade-offs accepted
These are the costs of enabling the option. Each one is why it is opt-in rather than a change to the default.
The linked span is
prepare, notexecute. DBM displays the prepare span'sduration. With client-side(emulated) prepares that is microseconds; with server-side prepares it is one round trip. The execution time
is on the sibling
PDOStatement.execute/mysqli_stmt.executespan in the same trace, one click away, butit is not what the sample shows. This is exactly the imprecision feat(PDO): correct a bug on prepared statement regarding DBM correlation #3545 removed. The option makes accepting it
the application's decision instead of the tracer's.
Statement reuse links every execution to the prepare-time trace. Prepare once, execute N times: N
executions carry one
traceparent. For per-request prepare-and-execute (the frameworks above) this is anon-issue. For prepared-statement caches in long-running workers (Octane, Swoole, RoadRunner, FrankenPHP
worker mode) or persistent connections, later executions can link to a trace that has already finished. The
proposed docs text says so.
The sampling decision is forced at the first prepare.
generate_distributed_tracing_headers()forcesthe sampling decision on the active span (
tracer/handlers_http.h), as it already does for everynon-prepared statement in
fullmode. With the option on, that now also happens on the first prepare in atrace. For an application that already runs
fullwith any non-prepared statement, nothing new. For a pureprepare-everything application,
fullmode starts costing whatfullmode costs everywhere else.One config lookup per prepare in
fullmode.dd_trace_env_config()is the right operand of&&,reached only on a prepare that was already about to downgrade, on a path that already runs a regex and
builds the comment string. Off, or outside
full, or on a non-prepared statement, the lookup does not run.Same name as dd-trace-java, different mechanism. The name follows dd-trace-java 1.44+, the precedent
Allow appending SQL comments via DD_DBM_ALWAYS_APPEND_SQL_COMMENT #3954 used ("Modeled after dd-trace-java#9798"). The mechanism is not the same and the PR should not be read
as parity:
connection.setClientInfo("ApplicationName", "_DD_" + traceparent)mysql,pgsqlJava's channel is not available to PHP without a different feature (a
SET application_nameper executeis a round trip and Postgres-only). The docs text below states the difference up front so the confusion that
appeared on dd-trace-java#7940 does not repeat.
Limitations that remain between DBM and PDO
These are not introduced by the PR and it does not address them. They determine where a user will see a
result after enabling the option, so reviewers should have them in front of them.
prepare as
statement/com/Prepareand its execution asstatement/com/Execute, both with NULLSQL_TEXTand NULL
DIGEST_TEXT; they never enterevents_statements_summary_by_digest, and the Agent's statementsampler filters exactly those rows (
integrations-core,mysql/statement_samples.py:WHERE sql_text IS NOT NULL ... AND digest_text IS NOT NULL). Two consequences. On MySQL over PDO, thisoption has a DBM-visible effect only with
PDO::ATTR_EMULATE_PREPARES => true(PDO's default, whichLaravel and Doctrine keep).
mysqliprepared statements are always server-side, so formysqlithe commentand the tag are correct but the sample never exists for them to link to. Confirmed on MySQL 8.0 by reading
the general log and
performance_schemaside by side. This is a MySQL and Agent property; the tracer cannotchange it.
span's id, or making a span id writable, is extension work and a separate proposal. This PR takes the
sibling span rather than no span.
dynamic_serviceas the fallback target. Add dynamic_service DBM propagation mode #3940 left open whether "when full is not possible" shouldfall back to
dynamic_servicerather thanservice. This option gates whether the prepare downgradehappens, not what it downgrades to, so if that is later changed this condition needs no change.
Plan cache and digest cardinality (#1983)
#1983 kept full mode to
mysqlandpgsqlbecause "full context propagation messes with the query plancaching" on mssql. A
traceparentmakes every prepare textually unique, so the question applies here.Measured on MySQL 8.0: 50 traces each preparing and executing the same statement, so 50 distinct
traceparentvalues, counting
performance_schema.events_statements_summary_by_digest:full, option offfull, option onIdentical. MySQL normalises comments out of the digest, and MySQL 8.0 has no text-keyed plan cache; the
behaviour #1983 describes is a SQL Server property, and
sqlsrvstays outside$fullPropagationBackendsandoutside this option (asserted by a test). Server-side statement reuse is unaffected either way: each
PDO::prepare()creates its own handle regardless of the comment.Postgres was not measured.
pg_stat_statementscomputes its query id from the parse tree, so commentsshould not affect it, but I have not verified that and do not claim it.
Why this cannot regress an existing user
With the option off, the code takes exactly the path it takes today. The added
dd_trace_env_configread isthe right operand of
&&, reached only on a prepare that was already about to downgrade infullmode on afull-propagation backend. Every other call site (
exec,query,execute_query,mysqli_query,real_query,sqlsrv_query) passes$preventFullMode = falseand never reaches it.Verified, not asserted:
PDO::prepareand capturing the rewritten query.disabled,serviceanddynamic_serviceare byte-identical either way. Infull, the only delta is theadded
traceparent.DD_DBM_ALWAYS_APPEND_SQL_COMMENTkeeps the comment appended;DD_DBM_INJECT_SQL_BASEHASHkeeps tag order alphabetical.
behaviour) fails exactly the three option-on tests (helper, PDO, mysqli) and nothing else. Removing the
prepared-statement downgrade fails exactly the PDO and mysqli feat(PDO): correct a bug on prepared statement regarding DBM correlation #3545 tests; the sqlsrv feat(PDO): correct a bug on prepared statement regarding DBM correlation #3545 test keeps
passing because sqlsrv is downgraded by the backend check, which this option does not touch.
PDOStatement::queryStringstays clean. Theforce_overwrite_propertyrestore from 7f6cc09 ismode-independent; the option-on tests assert the prepare and execute resources equal the original statement.
$hook->datafor their post-hooks and have nouser-facing
queryStringto restore, so the PDO-specific concern in 7f6cc09 does not arise there.Tests
Each new test is the #3545 test for that backend with the option on, or the existing helper-level test with
the prepare flag set.
tests/Integration/DatabaseMonitoringTest.php(no database): with the option, a prepare-flagged injectionproduces the same output as
testInjection(the injecting span's own seeded id, the marker on that span); theoption is a no-op in
service,dynamic_serviceanddisabled.tests/Integrations/PDO/PDOTest.php,tests/Integrations/Mysqli/MysqliTest.php: the prepare span carries_dd.dbm_trace_injected, execute remains a sibling, both resources remain the clean statement.tests/Integrations/SQLSRV/SQLSRVTest.php: with the option on,sqlsrv_preparestill has no marker.PHP 8.3 (debug) and PHP 8.5 (NTS), aarch64, in the repo's docker environment:
make test_integration FILTER=DatabaseMonitoringTestmake test_integrations_pdomake test_integrations_mysqlimake test_integrations_sqlsrvThe skip is
testPDOConnectOk(PDO::connect()is PHP 8.4+); the risky tests are thetestParseDsnDbNameQuoteHandlingdata sets printing output on 8.5. Neither is touched by this PR.Also run:
composer ci-lint,tests/ext/telemetry/config.phpt,tooling/generate-supported-configurations.sh(regenerated; only the new entry differs).
phpcs --standard=phpcs.xmlreports the same error count on thechanged files as on
master.PDOBenchis unaffected: it measuresexecute()on a statement prepared once in@BeforeMethodsand does not set the option. Not run locally: other PHP versions, Windows, ZTS, musl,ASAN/valgrind, the full
.phptsuite.Docs
This repository hosts no documentation snippets. Proposed text for the PHP tab of
database_monitoring/connect_dbm_and_apmand the PHP library configuration reference:Reviewer checklist