diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DBMCompatibleConnectionInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DBMCompatibleConnectionInstrumentation.java index 486a92dcbf7..5e22e098e22 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DBMCompatibleConnectionInstrumentation.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DBMCompatibleConnectionInstrumentation.java @@ -132,7 +132,13 @@ public static String onEnter( DECORATE.DBM_ALWAYS_APPEND_SQL_COMMENT || "sqlserver".equals(dbInfo.getType()); sql = SQLCommenter.inject( - sql, dbService, dbInfo.getType(), dbInfo.getHost(), dbInfo.getDb(), null, append); + sql, + dbService, + dbInfo.getType(), + dbInfo.getHost(), + DECORATE.isOracle(dbInfo) ? DECORATE.getDbInstance(dbInfo) : dbInfo.getDb(), + null, + append); return inputSql; } diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java index 094199d28fd..24e0f07b58d 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/JDBCDecorator.java @@ -205,11 +205,25 @@ 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 String getDbInstance(final DBInfo dbInfo) { + if (null == dbInfo) { + return null; + } + return dbInstance(dbInfo); } public static DBInfo parseDBInfoFromConnection(final Connection connection) { 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 06d353202ba..c251bb2d2a9 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 @@ -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,13 +146,27 @@ public static AgentScope onEnter( appendComment = true; } + final String dbService; + final String dbName; + if (isOracle) { + String oracleService = DECORATE.getDbService(dbInfo); + if (oracleService != null) { + oracleService = + traceConfig(span).getServiceMapping().getOrDefault(oracleService, oracleService); + } + dbService = oracleService; + dbName = DECORATE.getDbInstance(dbInfo); + } else { + dbService = span.getServiceName(); + dbName = dbInfo.getDb(); + } sql = SQLCommenter.inject( sql, - span.getServiceName(), + dbService, dbInfo.getType(), dbInfo.getHost(), - dbInfo.getDb(), + dbName, injectTraceInComment ? traceParent : null, appendComment); } 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..e13fbcdc734 --- /dev/null +++ b/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 + } +} diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/RemoteJDBCInstrumentationTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/RemoteJDBCInstrumentationTest.groovy index 681674cd922..a8b7978094a 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/RemoteJDBCInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/RemoteJDBCInstrumentationTest.groovy @@ -1146,6 +1146,42 @@ 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 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 {