From e595df989499aeb54d42e6476005acaccc0f15e7 Mon Sep 17 00:00:00 2001 From: Allen Zhou Date: Thu, 12 Mar 2026 15:16:49 -0400 Subject: [PATCH 1/3] 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 --- ...BMCompatibleConnectionInstrumentation.java | 2 +- .../instrumentation/jdbc/JDBCDecorator.java | 17 +++-- .../jdbc/StatementInstrumentation.java | 9 ++- .../groovy/OracleInjectionForkedTest.groovy | 69 +++++++++++++++++++ .../RemoteJDBCInstrumentationTest.groovy | 36 ++++++++++ 5 files changed, 126 insertions(+), 7 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/DBMCompatibleConnectionInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DBMCompatibleConnectionInstrumentation.java index 486a92dcbf7..c1944141df8 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,7 @@ 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.getDbInstance(dbInfo), 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..ac915229531 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,20 @@ 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; + if (dbInfo.getInstance() != null) { + return dbInfo.getInstance(); + } + return dbService(dbInfo.getType(), null); + } + + 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..61c2f8f2203 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,17 @@ public static AgentScope onEnter( appendComment = true; } + String dbService = DECORATE.getDbService(dbInfo); + if (dbService != null) { + dbService = traceConfig(span).getServiceMapping().getOrDefault(dbService, dbService); + } sql = SQLCommenter.inject( sql, - span.getServiceName(), + dbService, dbInfo.getType(), dbInfo.getHost(), - dbInfo.getDb(), + DECORATE.getDbInstance(dbInfo), 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..917da18ca66 --- /dev/null +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/OracleInjectionForkedTest.groovy @@ -0,0 +1,69 @@ +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" + // Oracle URL with SID "BENEDB" + static oracleUrl = "jdbc:oracle:thin:@localhost:1521:BENEDB" + // Expected: dddbs should be the SID, not the generic "oracle" type. + // Note: the URL parser lowercases the full URL before extraction, so the SID is lowercase. + static serviceInjection = "ddps='my_service_name',dddbs='benedb',ddh='localhost',dddb='benedb'" + + TestConnection createOracleConnection() { + def connection = new TestConnection(false) + def metadata = new TestDatabaseMetaData() + metadata.setURL(oracleUrl) + connection.setMetaData(metadata) + return connection + } +} + +class OracleInjectionForkedTest extends OracleInjectionTestBase { + + def "Oracle prepared statement injects instance name in dddbs and dddb"() { + setup: + def connection = createOracleConnection() + + when: + def statement = connection.prepareStatement(query) as TestPreparedStatement + statement.execute() + + then: + // dddbs must be the Oracle SID "BENEDB", not the generic type "oracle" + // dddb must also be present with the SID value + assert statement.sql == "/*${serviceInjection}*/ ${query}" + } + + def "Oracle single statement injects instance name in dddbs and dddb"() { + setup: + def connection = createOracleConnection() + + when: + def statement = connection.createStatement() as TestStatement + statement.executeQuery(query) + + then: + // Oracle uses v$session.action for trace context, so no traceparent in comment + // dddbs must be the Oracle SID "BENEDB", not the generic type "oracle" + // dddb must also be present with the SID value + assert statement.sql == "/*${serviceInjection}*/ ${query}" + } +} 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..ec8f328d392 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 { From 48487e281e787737bbe9bc7e343ea5df01dc4fbd Mon Sep 17 00:00:00 2001 From: Allen Zhou Date: Thu, 12 Mar 2026 15:32:21 -0400 Subject: [PATCH 2/3] linter --- .../jdbc/DBMCompatibleConnectionInstrumentation.java | 8 +++++++- .../src/test/groovy/RemoteJDBCInstrumentationTest.groovy | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) 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 c1944141df8..cdca91a712c 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(), DECORATE.getDbInstance(dbInfo), null, append); + sql, + dbService, + dbInfo.getType(), + dbInfo.getHost(), + DECORATE.getDbInstance(dbInfo), + null, + append); return inputSql; } 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 ec8f328d392..a8b7978094a 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/RemoteJDBCInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/RemoteJDBCInstrumentationTest.groovy @@ -1169,7 +1169,7 @@ class RemoteDBMTraceInjectedForkedTest extends RemoteJDBCInstrumentationTest { "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 From 5f04c0f9bfcfdd57650c14921f92bb322d9c1086 Mon Sep 17 00:00:00 2001 From: Allen Zhou Date: Fri, 13 Mar 2026 11:31:54 -0400 Subject: [PATCH 3/3] scope changes to oracle only and add service-name URL format test --- ...BMCompatibleConnectionInstrumentation.java | 2 +- .../instrumentation/jdbc/JDBCDecorator.java | 11 ++++-- .../jdbc/StatementInstrumentation.java | 18 +++++++-- .../groovy/OracleInjectionForkedTest.groovy | 38 +++++++++++-------- 4 files changed, 46 insertions(+), 23 deletions(-) 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 cdca91a712c..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 @@ -136,7 +136,7 @@ public static String onEnter( dbService, dbInfo.getType(), dbInfo.getHost(), - DECORATE.getDbInstance(dbInfo), + 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 ac915229531..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 @@ -208,10 +208,15 @@ public String getDbService(final DBInfo dbInfo) { if (null == dbInfo) { return null; } - if (dbInfo.getInstance() != null) { - return dbInfo.getInstance(); + // 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(), null); + return dbService(dbInfo.getType(), dbInstance(dbInfo)); } public String getDbInstance(final DBInfo dbInfo) { 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 61c2f8f2203..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 @@ -146,9 +146,19 @@ public static AgentScope onEnter( appendComment = true; } - String dbService = DECORATE.getDbService(dbInfo); - if (dbService != null) { - dbService = traceConfig(span).getServiceMapping().getOrDefault(dbService, dbService); + 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( @@ -156,7 +166,7 @@ public static AgentScope onEnter( dbService, dbInfo.getType(), dbInfo.getHost(), - DECORATE.getDbInstance(dbInfo), + 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 index 917da18ca66..e13fbcdc734 100644 --- a/dd-java-agent/instrumentation/jdbc/src/test/groovy/OracleInjectionForkedTest.groovy +++ b/dd-java-agent/instrumentation/jdbc/src/test/groovy/OracleInjectionForkedTest.groovy @@ -21,16 +21,18 @@ abstract class OracleInjectionTestBase extends InstrumentationSpecification { } static query = "SELECT 1" - // Oracle URL with SID "BENEDB" - static oracleUrl = "jdbc:oracle:thin:@localhost:1521:BENEDB" - // Expected: dddbs should be the SID, not the generic "oracle" type. - // Note: the URL parser lowercases the full URL before extraction, so the SID is lowercase. - static serviceInjection = "ddps='my_service_name',dddbs='benedb',ddh='localhost',dddb='benedb'" - TestConnection createOracleConnection() { + // 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(oracleUrl) + metadata.setURL(url) connection.setMetaData(metadata) return connection } @@ -40,21 +42,24 @@ class OracleInjectionForkedTest extends OracleInjectionTestBase { def "Oracle prepared statement injects instance name in dddbs and dddb"() { setup: - def connection = createOracleConnection() + def connection = createOracleConnection(url) when: def statement = connection.prepareStatement(query) as TestPreparedStatement statement.execute() then: - // dddbs must be the Oracle SID "BENEDB", not the generic type "oracle" - // dddb must also be present with the SID value - assert statement.sql == "/*${serviceInjection}*/ ${query}" + 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() + def connection = createOracleConnection(url) when: def statement = connection.createStatement() as TestStatement @@ -62,8 +67,11 @@ class OracleInjectionForkedTest extends OracleInjectionTestBase { then: // Oracle uses v$session.action for trace context, so no traceparent in comment - // dddbs must be the Oracle SID "BENEDB", not the generic type "oracle" - // dddb must also be present with the SID value - assert statement.sql == "/*${serviceInjection}*/ ${query}" + statement.sql == "/*${expected}*/ ${query}" + + where: + url | expected + sidUrl | sidInjection + serviceNameUrl | serviceNameInjection } }