Skip to content

fix generic url parsing in jdbc instrumentation #9777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Oct 21, 2025
Merged

fix generic url parsing in jdbc instrumentation #9777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
beb5d2b
fix generic url parsing in jdbc instrumentation
vandonr Oct 16, 2025
f992419
add test
vandonr Oct 16, 2025
b776e20
extract DBM specific test to a different class
vandonr Oct 16, 2025
de2a6c3
fix existing test that was not asserting the correct thing
vandonr Oct 16, 2025
0d5b2e4
just realized there was already a test on this that I could reuse
vandonr Oct 16, 2025
dbc4226
Revert "extract DBM specific test to a different class"
vandonr Oct 16, 2025
807915d
Revert "add test"
vandonr Oct 16, 2025
8a4ac88
fix other test
vandonr Oct 16, 2025
45b635c
fix lasts tests
vandonr Oct 20, 2025
2465781
Merge branch 'master' into vandonr/fix
vandonr Oct 20, 2025
fc22174
Merge branch 'master' into vandonr/fix
vandonr Oct 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...p/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ public static DBInfo parse(String connectionUrl, final Properties props) {
// Delegate to specific parser
return typeParsers.get(baseType).doParse(jdbcUrl, parsedProps).build();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we are using the correct variable

}
return GENERIC_URL_LIKE.doParse(connectionUrl, parsedProps).build();
return GENERIC_URL_LIKE.doParse(jdbcUrl, parsedProps).build();
} catch (final Exception e) {
ExceptionLogger.LOGGER.debug("Error parsing URL", e);
return parsedProps.build();
Expand Down
2 changes: 1 addition & 1 deletion dd-java-agent/instrumentation/jdbc/src/test/groovy/DBMInjectionForkedTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class DBMInjectionForkedTest extends InstrumentationSpecification {
}

static query = "SELECT 1"
static serviceInjection = "ddps='my_service_name',dddbs='remapped_testdb'"
static serviceInjection = "ddps='my_service_name',dddbs='remapped_testdb',ddh='localhost'"
static fullInjection = serviceInjection + ",traceparent='00-00000000000000000000000000000004-0000000000000003-01'"

def "prepared stmt"() {
Expand Down
6 changes: 4 additions & 2 deletions dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCConnectionUrlParserTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import static datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionUrlParser.extractDBInfo

import datadog.trace.agent.test.InstrumentationSpecification
import datadog.trace.bootstrap.instrumentation.jdbc.DBInfo
import spock.lang.Shared

import static datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionUrlParser.extractDBInfo

class JDBCConnectionUrlParserTest extends InstrumentationSpecification {

@Shared
Expand Down Expand Up @@ -207,6 +207,8 @@ class JDBCConnectionUrlParserTest extends InstrumentationSpecification {
// sybase
"jdbc:sybase:Tds:dbhostname:2638?ServiceName=demo" | null | "sybase" | "tds" | null | "dbhostname" | 2638 | "demo" | null
"jdbc:sybase:Tds:dbhostname:2638/dbname" | null | "sybase" | "tds" | null | "dbhostname" | 2638 | null | "dbname"
// unknown DB type
"jdbc:testdb://myhost:9999/testdatabase" | null | "testdb" | null | null | "myhost" | 9999 | null | "testdatabase"
expected = new DBInfo.Builder().type(type).subtype(subtype).user(user).instance(instance).db(db).host(host).port(port).build()
}
}
36 changes: 18 additions & 18 deletions dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCInstrumentationTestBase.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE
import static datadog.trace.api.config.TraceInstrumentationConfig.JDBC_POOL_WAITING_ENABLED

import com.mchange.v2.c3p0.ComboPooledDataSource
import com.zaxxer.hikari.HikariConfig
import com.zaxxer.hikari.HikariDataSource
Expand All @@ -6,18 +11,6 @@ import datadog.trace.api.Config
import datadog.trace.api.DDSpanTypes
import datadog.trace.bootstrap.instrumentation.api.InstrumentationTags
import datadog.trace.bootstrap.instrumentation.api.Tags
import org.apache.commons.dbcp2.BasicDataSource
import org.apache.commons.pool2.PooledObject
import org.apache.commons.pool2.PooledObjectFactory
import org.apache.commons.pool2.impl.DefaultPooledObject
import org.apache.commons.pool2.impl.GenericObjectPool
import org.apache.derby.jdbc.EmbeddedDataSource
import org.h2.jdbcx.JdbcDataSource
import spock.lang.Shared
import test.TestConnection
import test.WrappedConnection

import javax.sql.DataSource
import java.sql.CallableStatement
import java.sql.Connection
import java.sql.Driver
Expand All @@ -28,11 +21,17 @@ import java.sql.SQLTimeoutException
import java.sql.SQLTransientConnectionException
import java.sql.Statement
import java.time.Duration

import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE
import static datadog.trace.api.config.TraceInstrumentationConfig.JDBC_POOL_WAITING_ENABLED
import javax.sql.DataSource
import org.apache.commons.dbcp2.BasicDataSource
import org.apache.commons.pool2.PooledObject
import org.apache.commons.pool2.PooledObjectFactory
import org.apache.commons.pool2.impl.DefaultPooledObject
import org.apache.commons.pool2.impl.GenericObjectPool
import org.apache.derby.jdbc.EmbeddedDataSource
import org.h2.jdbcx.JdbcDataSource
import spock.lang.Shared
import test.TestConnection
import test.WrappedConnection

abstract class JDBCInstrumentationTest extends VersionedNamingTestBase {

Expand Down Expand Up @@ -724,14 +723,15 @@ abstract class JDBCInstrumentationTest extends VersionedNamingTestBase {
errored false
measured true
tags {
"$Tags.PEER_HOSTNAME" "localhost"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because of the fix, the url that is "testdb://localhost" now gets correctly parsed, which is used to fill the peer.hostname

"$Tags.COMPONENT" "java-jdbc-statement"
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
"$Tags.DB_TYPE" database
"$Tags.DB_OPERATION" CharSequence
if (addDbmTag) {
"$InstrumentationTags.DBM_TRACE_INJECTED" true
}
defaultTagsNoPeerService()
defaultTags()
}
}
}
Expand Down
9 changes: 6 additions & 3 deletions dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCWrappedInterfacesTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace

import datadog.trace.agent.test.InstrumentationSpecification
import datadog.trace.api.DDSpanTypes
import datadog.trace.bootstrap.instrumentation.api.Tags
Expand All @@ -7,9 +10,6 @@ import test.TestPreparedStatement
import test.WrappedConnection
import test.WrappedPreparedStatement

import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace

/**
* This tests all combinations of wrapped/unwrapped connections and prepared statements
* H2 classes are called out because the don't implement the Wrapper interface. They are based an older spec leading to AbstractMethodError
Expand Down Expand Up @@ -255,6 +255,9 @@ class JDBCWrappedInterfacesTest extends InstrumentationSpecification {
childOfPrevious()
errored false
tags {
if (database == "testdb") {
"$Tags.PEER_HOSTNAME" "localhost"
}
"$Tags.COMPONENT" "java-jdbc-prepared_statement"
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
"$Tags.DB_TYPE" "${database}"
Expand Down