Fix duplicate trace injection for SQL Server and Oracle

Author: lu-zhengda

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);

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")
+  }
+}

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" | ""

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")
   }
 }