From bba254daf26c109f4d453add318950668c1b15fe Mon Sep 17 00:00:00 2001 From: Zhengda Lu Date: Tue, 22 Jul 2025 15:08:49 -0400 Subject: [PATCH 1/3] Fix duplicate trace injection for SQL Server and Oracle --- .../jdbc/StatementInstrumentation.java | 5 ++- .../groovy/OracleInjectionForkedTest.groovy | 37 +++++++++++++++++++ .../src/test/groovy/SQLCommenterTest.groovy | 2 + .../SQLServerInjectionForkedTest.groovy | 5 ++- 4 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 dd-java-agent/instrumentation/jdbc/src/test/groovy/OracleInjectionForkedTest.groovy diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java index 33b4ff4af31..9aecd0c701a 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java @@ -123,6 +123,9 @@ public static AgentScope onEnter( span.setTag(DBM_TRACE_INJECTED, true); } } + // For SQL Server and Oracle, trace context is propagated via context_info and v$session.action + // respectively, so we should not also inject it into SQL comments to avoid duplication + final boolean injectTraceInComment = injectTraceContext && !isSqlServer && !isOracle; sql = SQLCommenter.inject( sql, @@ -131,7 +134,7 @@ public static AgentScope onEnter( dbInfo.getHost(), dbInfo.getDb(), traceParent, - injectTraceContext, + injectTraceInComment, appendComment); } DECORATE.onStatement(span, copy); diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/OracleInjectionForkedTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/OracleInjectionForkedTest.groovy new file mode 100644 index 00000000000..6d52ad1a3e0 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/OracleInjectionForkedTest.groovy @@ -0,0 +1,37 @@ +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.config.TraceInstrumentationConfig +import test.TestConnection +import test.TestDatabaseMetaData +import test.TestStatement + +class OracleInjectionForkedTest extends AgentTestRunner { + + @Override + void configurePreAgent() { + super.configurePreAgent() + + injectSysConfig(TraceInstrumentationConfig.DB_DBM_PROPAGATION_MODE_MODE, "full") + injectSysConfig("service.name", "my_service_name") + } + + static query = "SELECT 1 FROM DUAL" + static serviceInjection = "ddps='my_service_name',dddbs='oracle',ddh='localhost',dddb='testdb'" + + def "Oracle no trace injection with full propagation mode"() { + setup: + def connection = new TestConnection(false) + def metadata = new TestDatabaseMetaData() + metadata.setURL("jdbc:oracle:thin:@localhost:1521:testdb") + connection.setMetaData(metadata) + + when: + def statement = connection.createStatement() as TestStatement + statement.executeQuery(query) + + then: + // Should only have service metadata, not traceparent, because Oracle uses V$SESSION.ACTION + assert statement.sql == "/*${serviceInjection}*/ ${query}" + // Verify that the SQL does NOT contain traceparent + assert !statement.sql.contains("traceparent") + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/SQLCommenterTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/SQLCommenterTest.groovy index 79afb02602c..bb1119db1f1 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/SQLCommenterTest.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/SQLCommenterTest.groovy @@ -70,6 +70,8 @@ class SQLCommenterTest extends AgentTestRunner { "SELECT * from FOO -- test query" | "SqlCommenter" | "Test" | "my-service" | "mysql" | "h" | "n" | "TestVersion" | true | true | "00-00000000000000007fffffffffffffff-000000024cb016ea-01" | "SELECT * from FOO -- test query /*ddps='SqlCommenter',dddbs='my-service',ddh='h',dddb='n',dde='Test',ddpv='TestVersion',traceparent='00-00000000000000007fffffffffffffff-000000024cb016ea-01'*/" "SELECT /* customer-comment */ * FROM foo" | "SqlCommenter" | "Test" | "my-service" | "mysql" | "h" | "n" | "TestVersion" | true | true | "00-00000000000000007fffffffffffffff-000000024cb016ea-01" | "SELECT /* customer-comment */ * FROM foo /*ddps='SqlCommenter',dddbs='my-service',ddh='h',dddb='n',dde='Test',ddpv='TestVersion',traceparent='00-00000000000000007fffffffffffffff-000000024cb016ea-01'*/" "SELECT * FROM foo" | "SqlCommenter" | "Test" | "my-service" | "mysql" | "h" | "n" | "TestVersion" | false | true | null | "SELECT * FROM foo /*ddps='SqlCommenter',dddbs='my-service',ddh='h',dddb='n',dde='Test',ddpv='TestVersion'*/" + "SELECT * FROM DUAL" | "SqlCommenter" | "Test" | "my-service" | "oracle" | "h" | "n" | "TestVersion" | false | true | "00-00000000000000007fffffffffffffff-000000024cb016ea-00" | "SELECT * FROM DUAL /*ddps='SqlCommenter',dddbs='my-service',ddh='h',dddb='n',dde='Test',ddpv='TestVersion'*/" + "SELECT * FROM sys.tables" | "SqlCommenter" | "Test" | "my-service" | "sqlserver"| "h" | "n" | "TestVersion" | false | true | "00-00000000000000007fffffffffffffff-000000024cb016ea-00" | "SELECT * FROM sys.tables /*ddps='SqlCommenter',dddbs='my-service',ddh='h',dddb='n',dde='Test',ddpv='TestVersion'*/" "SELECT /* customer-comment */ * FROM foo" | "SqlCommenter" | "Test" | "my-service" | "mysql" | "h" | "n" | "TestVersion" | false | true | null | "SELECT /* customer-comment */ * FROM foo /*ddps='SqlCommenter',dddbs='my-service',ddh='h',dddb='n',dde='Test',ddpv='TestVersion'*/" "SELECT * from FOO -- test query" | "SqlCommenter" | "Test" | "my-service" | "mysql" | "h" | "n" | "TestVersion" | false | true | null | "SELECT * from FOO -- test query /*ddps='SqlCommenter',dddbs='my-service',ddh='h',dddb='n',dde='Test',ddpv='TestVersion'*/" "" | "SqlCommenter" | "Test" | "my-service" | "mysql" | "h" | "n" | "TestVersion" | true | true | "00-00000000000000007fffffffffffffff-000000024cb016ea-00" | "" diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/SQLServerInjectionForkedTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/SQLServerInjectionForkedTest.groovy index 2e958be9a3b..74829719b87 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/SQLServerInjectionForkedTest.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/SQLServerInjectionForkedTest.groovy @@ -19,7 +19,7 @@ class SQLServerInjectionForkedTest extends AgentTestRunner { static query = "SELECT 1" static serviceInjection = "ddps='my_service_name',dddbs='sqlserver',ddh='localhost',dddb='testdb'" - def "SQL Server no trace injection with full"() { + def "SQL Server no trace injection with full propagation mode"() { setup: def connection = new TestConnection(false) def metadata = new TestDatabaseMetaData() @@ -31,6 +31,9 @@ class SQLServerInjectionForkedTest extends AgentTestRunner { statement.executeQuery(query) then: + // Should only have service metadata, not traceparent, because SQL Server uses CONTEXT_INFO assert statement.sql == "/*${serviceInjection}*/ ${query}" + // Verify that the SQL does NOT contain traceparent + assert !statement.sql.contains("traceparent") } } From 92e52e9fd845a308c3de05455b4774726e7a3b2f Mon Sep 17 00:00:00 2001 From: Zhengda Lu Date: Tue, 22 Jul 2025 15:56:59 -0400 Subject: [PATCH 2/3] fix format --- .../trace/instrumentation/jdbc/StatementInstrumentation.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java index 9aecd0c701a..654d98dbed4 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/StatementInstrumentation.java @@ -123,8 +123,9 @@ public static AgentScope onEnter( span.setTag(DBM_TRACE_INJECTED, true); } } - // For SQL Server and Oracle, trace context is propagated via context_info and v$session.action - // respectively, so we should not also inject it into SQL comments to avoid duplication + // For SQL Server and Oracle, trace context is propagated via + // context_info and v$session.action respectively. + // we should not also inject it into SQL comments to avoid duplication final boolean injectTraceInComment = injectTraceContext && !isSqlServer && !isOracle; sql = SQLCommenter.inject( From 49207b33eeb832aa6e41e4a9a1a820a3f71dbdd1 Mon Sep 17 00:00:00 2001 From: Zhengda Lu Date: Wed, 23 Jul 2025 11:15:02 -0400 Subject: [PATCH 3/3] remove unused tests --- .../groovy/OracleInjectionForkedTest.groovy | 37 ------------------- 1 file changed, 37 deletions(-) delete mode 100644 dd-java-agent/instrumentation/jdbc/src/test/groovy/OracleInjectionForkedTest.groovy diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/OracleInjectionForkedTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/OracleInjectionForkedTest.groovy deleted file mode 100644 index 6d52ad1a3e0..00000000000 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/OracleInjectionForkedTest.groovy +++ /dev/null @@ -1,37 +0,0 @@ -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.api.config.TraceInstrumentationConfig -import test.TestConnection -import test.TestDatabaseMetaData -import test.TestStatement - -class OracleInjectionForkedTest extends AgentTestRunner { - - @Override - void configurePreAgent() { - super.configurePreAgent() - - injectSysConfig(TraceInstrumentationConfig.DB_DBM_PROPAGATION_MODE_MODE, "full") - injectSysConfig("service.name", "my_service_name") - } - - static query = "SELECT 1 FROM DUAL" - static serviceInjection = "ddps='my_service_name',dddbs='oracle',ddh='localhost',dddb='testdb'" - - def "Oracle no trace injection with full propagation mode"() { - setup: - def connection = new TestConnection(false) - def metadata = new TestDatabaseMetaData() - metadata.setURL("jdbc:oracle:thin:@localhost:1521:testdb") - connection.setMetaData(metadata) - - when: - def statement = connection.createStatement() as TestStatement - statement.executeQuery(query) - - then: - // Should only have service metadata, not traceparent, because Oracle uses V$SESSION.ACTION - assert statement.sql == "/*${serviceInjection}*/ ${query}" - // Verify that the SQL does NOT contain traceparent - assert !statement.sql.contains("traceparent") - } -}