Fix Oracle DBM trace correlation (#10829)

Fix Oracle DBM trace correlation by injecting the database instance name into dddbs and dddb SQL comment tags instead of the generic type string and null

linter

scope changes to oracle only and add service-name URL format test

test fix

simplify implementation, getDb returns instance if db not found

Co-authored-by: allen.zhou <allen.zhou@datadoghq.com>

Author: azhou-datadog

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

@@ -193,7 +193,7 @@ public String getInstance() {
   }
 
   public String getDb() {
-    return db;
+    return db != null ? db : instance;
   }
 
   public String getHost() {

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

@@ -205,11 +205,18 @@ public static DBInfo parseDBInfo(
   }
 
   public String getDbService(final DBInfo dbInfo) {
-    String dbService = null;
-    if (null != dbInfo) {
-      dbService = dbService(dbInfo.getType(), dbInstance(dbInfo));
+    if (null == dbInfo) {
+      return null;
     }
-    return dbService;
+    // For Oracle, the URL parser sets instance (SID/service name) but never db.
+    // Without this, dddbs defaults to the generic type string "oracle" which breaks
+    // DBM trace correlation. Other databases (e.g. SQL Server) rely on the type-based
+    // service name for DBM correlation and must not be changed here.
+    if ("oracle".equals(dbInfo.getType()) && dbInfo.getInstance() != null) {
+      String service = dbClientService(dbInfo.getInstance());
+      return service != null ? service : dbInfo.getInstance();
+    }
+    return dbService(dbInfo.getType(), dbInstance(dbInfo));
   }
 
   public static DBInfo parseDBInfoFromConnection(final Connection connection) {

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

@@ -5,6 +5,7 @@
 import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.traceConfig;
 import static datadog.trace.bootstrap.instrumentation.api.InstrumentationTags.DBM_TRACE_INJECTED;
 import static datadog.trace.instrumentation.jdbc.JDBCDecorator.DATABASE_QUERY;
 import static datadog.trace.instrumentation.jdbc.JDBCDecorator.DECORATE;
@@ -145,10 +146,21 @@ public static AgentScope onEnter(
             appendComment = true;
           }
 
+          final String dbService;
+          if (isOracle) {
+            String oracleService = DECORATE.getDbService(dbInfo);
+            if (oracleService != null) {
+              oracleService =
+                  traceConfig(span).getServiceMapping().getOrDefault(oracleService, oracleService);
+            }
+            dbService = oracleService;
+          } else {
+            dbService = span.getServiceName();
+          }
           sql =
               SQLCommenter.inject(
                   sql,
-                  span.getServiceName(),
+                  dbService,
                   dbInfo.getType(),
                   dbInfo.getHost(),
                   dbInfo.getDb(),

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

@@ -0,0 +1,77 @@
+import datadog.trace.agent.test.InstrumentationSpecification
+import datadog.trace.api.config.TraceInstrumentationConfig
+import test.TestConnection
+import test.TestDatabaseMetaData
+import test.TestPreparedStatement
+import test.TestStatement
+
+/**
+ * Tests that Oracle DBM SQL comment injection produces the correct dddbs and dddb tags.
+ *
+ * Bug 1: dddbs was populated with the generic type string "oracle" instead of the SID/service name.
+ * Bug 2: dddb was never injected because the Oracle URL parser sets instance, not db.
+ */
+abstract class OracleInjectionTestBase extends InstrumentationSpecification {
+  @Override
+  void configurePreAgent() {
+    super.configurePreAgent()
+
+    injectSysConfig(TraceInstrumentationConfig.DB_DBM_PROPAGATION_MODE_MODE, "full")
+    injectSysConfig("service.name", "my_service_name")
+  }
+
+  static query = "SELECT 1"
+
+  // Note: the URL parser lowercases the full URL before extraction, so identifiers are lowercase.
+  static sidUrl = "jdbc:oracle:thin:@localhost:1521:BENEDB"
+  static serviceNameUrl = "jdbc:oracle:thin:@//localhost:1521/MYSERVICE"
+
+  static sidInjection = "ddps='my_service_name',dddbs='benedb',ddh='localhost',dddb='benedb'"
+  static serviceNameInjection = "ddps='my_service_name',dddbs='myservice',ddh='localhost',dddb='myservice'"
+
+  TestConnection createOracleConnection(String url) {
+    def connection = new TestConnection(false)
+    def metadata = new TestDatabaseMetaData()
+    metadata.setURL(url)
+    connection.setMetaData(metadata)
+    return connection
+  }
+}
+
+class OracleInjectionForkedTest extends OracleInjectionTestBase {
+
+  def "Oracle prepared statement injects instance name in dddbs and dddb"() {
+    setup:
+    def connection = createOracleConnection(url)
+
+    when:
+    def statement = connection.prepareStatement(query) as TestPreparedStatement
+    statement.execute()
+
+    then:
+    statement.sql == "/*${expected}*/ ${query}"
+
+    where:
+    url            | expected
+    sidUrl         | sidInjection
+    serviceNameUrl | serviceNameInjection
+  }
+
+  def "Oracle single statement injects instance name in dddbs and dddb"() {
+    setup:
+    def connection = createOracleConnection(url)
+
+    when:
+    def statement = connection.createStatement() as TestStatement
+    statement.executeQuery(query)
+
+    then:
+    // Oracle uses v$session.action for trace context, so no traceparent in comment
+    statement.sql == "/*${expected}*/ ${query}"
+
+    where:
+    url            | expected
+    sidUrl         | sidInjection
+    serviceNameUrl | serviceNameInjection
+  }
+}

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

@@ -1146,6 +1146,43 @@ class RemoteDBMTraceInjectedForkedTest extends RemoteJDBCInstrumentationTest {
     final databaseNaming = new DatabaseNamingV1()
     return databaseNaming.normalizedName(dbType)
   }
+
+  def "Oracle DBM comment contains instance name in dddbs and dddb, not generic type string"() {
+    setup:
+    // Use a query text unlikely to already be in v$sql cursor cache
+    def markerQuery = "SELECT 1729 /* oracle-dbm-fix-verify */ FROM dual"
+    def conn = connectTo(ORACLE, peerConnectionProps(ORACLE))
+
+    when:
+    def stmt = conn.createStatement()
+    runUnderTrace("parent") {
+      stmt.execute(markerQuery)
+    }
+    TEST_WRITER.waitForTraces(1)
+
+    then:
+    // Connect as system to read v$sql — system shares the same password as the test user
+    // in the gvenzl/oracle-free image (both are set via ORACLE_PASSWORD).
+    def adminUrl = "jdbc:oracle:thin:@//${oracle.getHost()}:${oracle.getMappedPort(1521)}/freepdb1"
+    def adminConn = java.sql.DriverManager.getConnection(adminUrl, "system", oracle.getPassword())
+    def rs = adminConn.createStatement().executeQuery(
+      "SELECT sql_fulltext FROM v\$sql " +
+      "WHERE sql_fulltext LIKE '%1729%' AND sql_fulltext LIKE '%dddbs%' " +
+      "AND sql_fulltext LIKE '%oracle-dbm-fix-verify%' " +
+      "AND ROWNUM = 1"
+      )
+    assert rs.next() : "Instrumented Oracle query not found in v\$sql — DBM comment may be missing"
+    def sqlText = rs.getString(1)
+    // dddbs and dddb should both carry the PDB/service name, not the generic "oracle" type string
+    assert sqlText.contains("dddbs='freepdb1'") : "Expected dddbs='freepdb1' in SQL comment, got: ${sqlText}"
+    assert sqlText.contains("dddb='freepdb1'")  : "Expected dddb='freepdb1' in SQL comment, got: ${sqlText}"
+    assert !sqlText.contains("dddbs='oracle'")  : "dddbs must not be the generic type string 'oracle': ${sqlText}"
+
+    cleanup:
+    adminConn?.close()
+    stmt?.close()
+    conn?.close()
+  }
 }
 
 class RemoteDBMTraceInjectedForkedTestTracePreparedStatements extends RemoteJDBCInstrumentationTest {