Skip to content

Add option to JDBC instrumentation to always append DBM comment #9798

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add option to JDBC instrumentation to always append DBM comment #9798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9f6ef81
Add option to JDBC instrumentation to always append DBM comment
na-ji Oct 17, 2025
5cb578a
document the newly added DBM environment variable
na-ji Oct 17, 2025
e620029
flip order of comment append condition in JDBC instrumentation
na-ji Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .../main/java/datadog/trace/instrumentation/jdbc/DBMCompatibleConnectionInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ public static String onEnter(
dbService =
traceConfig(activeSpan()).getServiceMapping().getOrDefault(dbService, dbService);
}
boolean append = "sqlserver".equals(dbInfo.getType());

boolean append = DECORATE.shouldAppendSqlComment() || "sqlserver".equals(dbInfo.getType());
sql =
SQLCommenter.inject(
sql, dbService, dbInfo.getType(), dbInfo.getHost(), dbInfo.getDb(), null, append);
Expand Down
4 changes: 4 additions & 0 deletions .../instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,4 +423,8 @@ public boolean shouldInjectSQLComment() {
return Config.get().getDbmPropagationMode().equals(DBM_PROPAGATION_MODE_FULL)
|| Config.get().getDbmPropagationMode().equals(DBM_PROPAGATION_MODE_STATIC);
}

public boolean shouldAppendSqlComment() {
return Config.get().isDbmAlwaysAppendSqlComment();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this Config.get().isDbm... is stored in a static final, then the JIT will be able to optimize this better. In that case, the JVM will be able constant propagate static final vars. And this method will also get inlined, so it will be equivalent to just using a constant inline in the code.

Copy link
Member Author

@na-ji na-ji Oct 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I wanted to do at first. But I haven't found why, it doesn't work on one specific test:

dd-trace-java/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DBMCompatibleConnectionInstrumentation.java

Lines 113 to 114 in 9f6ef81

// Using INJECT_COMMENT fails to update when a test calls injectSysConfig
if (!DECORATE.shouldInjectSQLComment()) {

The other test, it works without issue. The issue happening is that when calling injectSysConfig during setup, the value isn't set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify what you mean by the value isn't set? A static final is set once and only once during class initialization.

Do you mean that the values aren't available to Config? The values used by Config are external and should be available. Unless you are also wanting to support remote configuration here

}
}
3 changes: 2 additions & 1 deletion ...ation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,15 @@ public static AgentScope onEnter(
final boolean injectTraceInComment = injectTraceContext && !isSqlServer && !isOracle;

// prepend mode will prepend the SQL comment to the raw sql query
boolean appendComment = false;
boolean appendComment = DECORATE.shouldAppendSqlComment();

// There is a bug in the SQL Server JDBC driver that prevents
// the generated keys from being returned when the
// SQL comment is prepended to the SQL query.
// We only append in this case to avoid the comment from being truncated.
// @see https://github.com/microsoft/mssql-jdbc/issues/2729
if (isSqlServer
&& !appendComment
&& args.length == 2
&& args[1] instanceof Integer
&& (Integer) args[1] == Statement.RETURN_GENERATED_KEYS) {
Expand Down
30 changes: 28 additions & 2 deletions dd-java-agent/instrumentation/jdbc/src/test/groovy/DBMInjectionForkedTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ class DBMInjectionForkedTest extends InstrumentationSpecification {

static query = "SELECT 1"
static serviceInjection = "ddps='my_service_name',dddbs='remapped_testdb'"
static fullInjection = serviceInjection + ",traceparent='00-00000000000000000000000000000004-0000000000000003-01'"

def "prepared stmt"() {
setup:
Expand All @@ -36,6 +35,20 @@ class DBMInjectionForkedTest extends InstrumentationSpecification {
assert statement.sql == "/*${serviceInjection}*/ ${query}"
}

def "append comment on prepared stmt"() {
setup:
injectSysConfig(TraceInstrumentationConfig.DB_DBM_ALWAYS_APPEND_SQL_COMMENT, "true")
def connection = new TestConnection(false)

when:
def statement = connection.prepareStatement(query) as TestPreparedStatement
statement.execute()

then:
// even in full propagation mode, we cannot inject trace info in prepared statements
assert statement.sql == "${query} /*${serviceInjection}*/"
}

def "single query"() {
setup:
def connection = new TestConnection(false)
Expand All @@ -45,6 +58,19 @@ class DBMInjectionForkedTest extends InstrumentationSpecification {
statement.executeQuery(query)

then:
assert statement.sql == "/*${fullInjection}*/ ${query}"
assert statement.sql == "/*${serviceInjection},traceparent='00-00000000000000000000000000000006-0000000000000005-01'*/ ${query}"
}

def "append comment on single query"() {
setup:
injectSysConfig(TraceInstrumentationConfig.DB_DBM_ALWAYS_APPEND_SQL_COMMENT, "true")
def connection = new TestConnection(false)

when:
def statement = connection.createStatement() as TestStatement
statement.executeQuery(query)

then:
assert statement.sql == "${query} /*${serviceInjection},traceparent='00-00000000000000000000000000000008-0000000000000007-01'*/"
}
}
18 changes: 17 additions & 1 deletion dd-java-agent/instrumentation/jdbc/src/test/groovy/SQLServerInjectionForkedTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class SQLServerInjectionForkedTest extends InstrumentationSpecification {
assert !statement.sql.contains("traceparent")
}

def "SQL Server apend comment when getting generated keys"() {
def "SQL Server append comment when getting generated keys"() {
setup:
def connection = new TestConnection(false)
def metadata = new TestDatabaseMetaData()
Expand All @@ -51,4 +51,20 @@ class SQLServerInjectionForkedTest extends InstrumentationSpecification {
then:
assert statement.sql == "${query} /*${serviceInjection}*/"
}

def "SQL Server append comment when configured to do so"() {
setup:
injectSysConfig(TraceInstrumentationConfig.DB_DBM_ALWAYS_APPEND_SQL_COMMENT, "true")
def connection = new TestConnection(false)
def metadata = new TestDatabaseMetaData()
metadata.setURL("jdbc:microsoft:sqlserver://localhost:1433;DatabaseName=testdb;")
connection.setMetaData(metadata)

when:
def statement = connection.createStatement() as TestStatement
statement.executeUpdate(query)

then:
assert statement.sql == "${query} /*${serviceInjection}*/"
}
}
1 change: 1 addition & 0 deletions dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public final class ConfigDefaults {
static final boolean DEFAULT_DB_CLIENT_HOST_SPLIT_BY_HOST = false;
static final String DEFAULT_DB_DBM_PROPAGATION_MODE_MODE = "disabled";
static final boolean DEFAULT_DB_DBM_TRACE_PREPARED_STATEMENTS = false;
static final boolean DEFAULT_DB_DBM_ALWAYS_APPEND_SQL_COMMENT = false;
// Default value is set to 0, it disables the latency trace interceptor
static final int DEFAULT_TRACE_KEEP_LATENCY_THRESHOLD_MS = 0;
static final int DEFAULT_SCOPE_DEPTH_LIMIT = 100;
Expand Down
1 change: 1 addition & 0 deletions dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public final class TraceInstrumentationConfig {
public static final String DB_DBM_INJECT_SQL_BASEHASH = "dbm.inject.sql.basehash";
public static final String DB_DBM_PROPAGATION_MODE_MODE = "dbm.propagation.mode";
public static final String DB_DBM_TRACE_PREPARED_STATEMENTS = "dbm.trace_prepared_statements";
public static final String DB_DBM_ALWAYS_APPEND_SQL_COMMENT = "dbm.always_append_sql_comment";

public static final String JDBC_CONNECTION_CLASS_NAME = "trace.jdbc.connection.class.name";

Expand Down
11 changes: 11 additions & 0 deletions internal-api/src/main/java/datadog/trace/api/Config.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import static datadog.trace.api.ConfigDefaults.DEFAULT_DB_CLIENT_HOST_SPLIT_BY_INSTANCE_TYPE_SUFFIX;
import static datadog.trace.api.ConfigDefaults.DEFAULT_DB_DBM_PROPAGATION_MODE_MODE;
import static datadog.trace.api.ConfigDefaults.DEFAULT_DB_DBM_TRACE_PREPARED_STATEMENTS;
import static datadog.trace.api.ConfigDefaults.DEFAULT_DB_DBM_ALWAYS_APPEND_SQL_COMMENT;
import static datadog.trace.api.ConfigDefaults.DEFAULT_DEBUGGER_EXCEPTION_CAPTURE_INTERMEDIATE_SPANS_ENABLED;
import static datadog.trace.api.ConfigDefaults.DEFAULT_DEBUGGER_EXCEPTION_CAPTURE_INTERVAL_SECONDS;
import static datadog.trace.api.ConfigDefaults.DEFAULT_DEBUGGER_EXCEPTION_ENABLED;
Expand Down Expand Up @@ -511,6 +512,7 @@
import static datadog.trace.api.config.TraceInstrumentationConfig.DB_DBM_INJECT_SQL_BASEHASH;
import static datadog.trace.api.config.TraceInstrumentationConfig.DB_DBM_PROPAGATION_MODE_MODE;
import static datadog.trace.api.config.TraceInstrumentationConfig.DB_DBM_TRACE_PREPARED_STATEMENTS;
import static datadog.trace.api.config.TraceInstrumentationConfig.DB_DBM_ALWAYS_APPEND_SQL_COMMENT;
import static datadog.trace.api.config.TraceInstrumentationConfig.ELASTICSEARCH_BODY_AND_PARAMS_ENABLED;
import static datadog.trace.api.config.TraceInstrumentationConfig.ELASTICSEARCH_BODY_ENABLED;
import static datadog.trace.api.config.TraceInstrumentationConfig.ELASTICSEARCH_PARAMS_ENABLED;
Expand Down Expand Up @@ -1076,6 +1078,7 @@ public static String getHostName() {
private final boolean dbmInjectSqlBaseHash;
private final String dbmPropagationMode;
private final boolean dbmTracePreparedStatements;
private final boolean dbmAlwaysAppendSqlComment;

private final boolean dynamicInstrumentationEnabled;
private final String dynamicInstrumentationSnapshotUrl;
Expand Down Expand Up @@ -1638,6 +1641,10 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins
configProvider.getBoolean(
DB_DBM_TRACE_PREPARED_STATEMENTS, DEFAULT_DB_DBM_TRACE_PREPARED_STATEMENTS);

dbmAlwaysAppendSqlComment =
configProvider.getBoolean(
DB_DBM_ALWAYS_APPEND_SQL_COMMENT, DEFAULT_DB_DBM_ALWAYS_APPEND_SQL_COMMENT);

dbmInjectSqlBaseHash = configProvider.getBoolean(DB_DBM_INJECT_SQL_BASEHASH, false);

splitByTags = tryMakeImmutableSet(configProvider.getList(SPLIT_BY_TAGS));
Expand Down Expand Up @@ -5035,6 +5042,10 @@ public boolean isDbmTracePreparedStatements() {
return dbmTracePreparedStatements;
}

public boolean isDbmAlwaysAppendSqlComment() {
return dbmAlwaysAppendSqlComment;
}

public String getDbmPropagationMode() {
return dbmPropagationMode;
}
Expand Down
1 change: 1 addition & 0 deletions metadata/supported-configurations.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@
"DD_DATA_STREAMS_ENABLED": ["A"],
"DD_DBM_PROPAGATION_MODE": ["A"],
"DD_DBM_TRACE_PREPARED_STATEMENTS": ["A"],
"DB_DBM_ALWAYS_APPEND_SQL_COMMENT": ["A"],
"DD_DISTRIBUTED_DEBUGGER_ENABLED": ["A"],
"DD_DOGSTATSD_ARGS": ["A"],
"DD_DOGSTATSD_HOST": ["A"],
Expand Down