Skip to content

Commit e125ff7

Browse files
authored
Prevent crash in SQL Server's JDBC when tracing execute methods with generated keys (#9321)
* Prevent crash in SQL Server JDBC when tracing execute methods with generated keys When tracing the JDBC statement execute methods, we prepend a comment to the SQL query used for DBM trace propagation. However, 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. I decided to only append the comment in this specific case to avoid the comment from being truncated. @see microsoft/mssql-jdbc#2729 * remove dynamic typing when packing all the JDBC arguments * move JDBC test on injection with generated keys to SQL Server file * remove useless constant in StatementAdvice
1 parent db0df01 commit e125ff7

File tree

2 files changed

+32
-3
lines changed

2 files changed

+32
-3
lines changed

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,6 @@ public String[] helperClassNames() {
6161
return new String[] {packageName + ".JDBCDecorator", packageName + ".SQLCommenter"};
6262
}
6363

64-
// prepend mode will prepend the SQL comment to the raw sql query
65-
private static final boolean appendComment = false;
66-
6764
@Override
6865
public void methodAdvice(MethodTransformer transformer) {
6966
transformer.applyAdvice(
@@ -75,6 +72,7 @@ public static class StatementAdvice {
7572
@Advice.OnMethodEnter(suppress = Throwable.class)
7673
public static AgentScope onEnter(
7774
@Advice.Argument(value = 0, readOnly = false) String sql,
75+
@Advice.AllArguments() Object[] args,
7876
@Advice.This final Statement statement) {
7977
// TODO consider matching known non-wrapper implementations to avoid this check
8078
final int callDepth = CallDepthThreadLocalMap.incrementCallDepth(Statement.class);
@@ -127,6 +125,22 @@ public static AgentScope onEnter(
127125
// context_info and v$session.action respectively.
128126
// we should not also inject it into SQL comments to avoid duplication
129127
final boolean injectTraceInComment = injectTraceContext && !isSqlServer && !isOracle;
128+
129+
// prepend mode will prepend the SQL comment to the raw sql query
130+
boolean appendComment = false;
131+
132+
// There is a bug in the SQL Server JDBC driver that prevents
133+
// the generated keys from being returned when the
134+
// SQL comment is prepended to the SQL query.
135+
// We only append in this case to avoid the comment from being truncated.
136+
// @see https://github.com/microsoft/mssql-jdbc/issues/2729
137+
if (isSqlServer
138+
&& args.length == 2
139+
&& args[1] instanceof Integer
140+
&& (Integer) args[1] == Statement.RETURN_GENERATED_KEYS) {
141+
appendComment = true;
142+
}
143+
130144
sql =
131145
SQLCommenter.inject(
132146
sql,

dd-java-agent/instrumentation/jdbc/src/test/groovy/SQLServerInjectionForkedTest.groovy

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,19 @@ class SQLServerInjectionForkedTest extends AgentTestRunner {
3636
// Verify that the SQL does NOT contain traceparent
3737
assert !statement.sql.contains("traceparent")
3838
}
39+
40+
def "SQL Server apend comment when getting generated keys"() {
41+
setup:
42+
def connection = new TestConnection(false)
43+
def metadata = new TestDatabaseMetaData()
44+
metadata.setURL("jdbc:microsoft:sqlserver://localhost:1433;DatabaseName=testdb;")
45+
connection.setMetaData(metadata)
46+
47+
when:
48+
def statement = connection.createStatement() as TestStatement
49+
statement.executeUpdate(query, 1)
50+
51+
then:
52+
assert statement.sql == "${query} /*${serviceInjection}*/"
53+
}
3954
}

0 commit comments

Comments
 (0)