Skip to content

Commit d7fd7ac

Browse files
authored
fix generic url parsing in jdbc instrumentation (#9777)
* fix generic url parsing in jdbc instrumentation * add test * extract DBM specific test to a different class * fix existing test that was not asserting the correct thing * just realized there was already a test on this that I could reuse * Revert "extract DBM specific test to a different class" This reverts commit b776e20. * Revert "add test" This reverts commit f992419.
1 parent 35a6e6a commit d7fd7ac

File tree

5 files changed

+30
-25
lines changed

5 files changed

+30
-25
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,7 @@ public static DBInfo parse(String connectionUrl, final Properties props) {
870870
// Delegate to specific parser
871871
return typeParsers.get(baseType).doParse(jdbcUrl, parsedProps).build();
872872
}
873-
return GENERIC_URL_LIKE.doParse(connectionUrl, parsedProps).build();
873+
return GENERIC_URL_LIKE.doParse(jdbcUrl, parsedProps).build();
874874
} catch (final Exception e) {
875875
ExceptionLogger.LOGGER.debug("Error parsing URL", e);
876876
return parsedProps.build();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class DBMInjectionForkedTest extends InstrumentationSpecification {
2020
}
2121

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

2626
def "prepared stmt"() {

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1+
import static datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionUrlParser.extractDBInfo
2+
13
import datadog.trace.agent.test.InstrumentationSpecification
24
import datadog.trace.bootstrap.instrumentation.jdbc.DBInfo
35
import spock.lang.Shared
46

5-
import static datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionUrlParser.extractDBInfo
6-
77
class JDBCConnectionUrlParserTest extends InstrumentationSpecification {
88

99
@Shared
@@ -207,6 +207,8 @@ class JDBCConnectionUrlParserTest extends InstrumentationSpecification {
207207
// sybase
208208
"jdbc:sybase:Tds:dbhostname:2638?ServiceName=demo" | null | "sybase" | "tds" | null | "dbhostname" | 2638 | "demo" | null
209209
"jdbc:sybase:Tds:dbhostname:2638/dbname" | null | "sybase" | "tds" | null | "dbhostname" | 2638 | null | "dbname"
210+
// unknown DB type
211+
"jdbc:testdb://myhost:9999/testdatabase" | null | "testdb" | null | null | "myhost" | 9999 | null | "testdatabase"
210212
expected = new DBInfo.Builder().type(type).subtype(subtype).user(user).instance(instance).db(db).host(host).port(port).build()
211213
}
212214
}

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

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
2+
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
3+
import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE
4+
import static datadog.trace.api.config.TraceInstrumentationConfig.JDBC_POOL_WAITING_ENABLED
5+
16
import com.mchange.v2.c3p0.ComboPooledDataSource
27
import com.zaxxer.hikari.HikariConfig
38
import com.zaxxer.hikari.HikariDataSource
@@ -6,18 +11,6 @@ import datadog.trace.api.Config
611
import datadog.trace.api.DDSpanTypes
712
import datadog.trace.bootstrap.instrumentation.api.InstrumentationTags
813
import datadog.trace.bootstrap.instrumentation.api.Tags
9-
import org.apache.commons.dbcp2.BasicDataSource
10-
import org.apache.commons.pool2.PooledObject
11-
import org.apache.commons.pool2.PooledObjectFactory
12-
import org.apache.commons.pool2.impl.DefaultPooledObject
13-
import org.apache.commons.pool2.impl.GenericObjectPool
14-
import org.apache.derby.jdbc.EmbeddedDataSource
15-
import org.h2.jdbcx.JdbcDataSource
16-
import spock.lang.Shared
17-
import test.TestConnection
18-
import test.WrappedConnection
19-
20-
import javax.sql.DataSource
2114
import java.sql.CallableStatement
2215
import java.sql.Connection
2316
import java.sql.Driver
@@ -28,11 +21,17 @@ import java.sql.SQLTimeoutException
2821
import java.sql.SQLTransientConnectionException
2922
import java.sql.Statement
3023
import java.time.Duration
31-
32-
import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
33-
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
34-
import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST_SPLIT_BY_INSTANCE
35-
import static datadog.trace.api.config.TraceInstrumentationConfig.JDBC_POOL_WAITING_ENABLED
24+
import javax.sql.DataSource
25+
import org.apache.commons.dbcp2.BasicDataSource
26+
import org.apache.commons.pool2.PooledObject
27+
import org.apache.commons.pool2.PooledObjectFactory
28+
import org.apache.commons.pool2.impl.DefaultPooledObject
29+
import org.apache.commons.pool2.impl.GenericObjectPool
30+
import org.apache.derby.jdbc.EmbeddedDataSource
31+
import org.h2.jdbcx.JdbcDataSource
32+
import spock.lang.Shared
33+
import test.TestConnection
34+
import test.WrappedConnection
3635

3736
abstract class JDBCInstrumentationTest extends VersionedNamingTestBase {
3837

@@ -724,14 +723,15 @@ abstract class JDBCInstrumentationTest extends VersionedNamingTestBase {
724723
errored false
725724
measured true
726725
tags {
726+
"$Tags.PEER_HOSTNAME" "localhost"
727727
"$Tags.COMPONENT" "java-jdbc-statement"
728728
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
729729
"$Tags.DB_TYPE" database
730730
"$Tags.DB_OPERATION" CharSequence
731731
if (addDbmTag) {
732732
"$InstrumentationTags.DBM_TRACE_INJECTED" true
733733
}
734-
defaultTagsNoPeerService()
734+
defaultTags()
735735
}
736736
}
737737
}

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
2+
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
3+
14
import datadog.trace.agent.test.InstrumentationSpecification
25
import datadog.trace.api.DDSpanTypes
36
import datadog.trace.bootstrap.instrumentation.api.Tags
@@ -7,9 +10,6 @@ import test.TestPreparedStatement
710
import test.WrappedConnection
811
import test.WrappedPreparedStatement
912

10-
import static datadog.trace.agent.test.utils.TraceUtils.basicSpan
11-
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
12-
1313
/**
1414
* This tests all combinations of wrapped/unwrapped connections and prepared statements
1515
* H2 classes are called out because the don't implement the Wrapper interface. They are based an older spec leading to AbstractMethodError
@@ -255,6 +255,9 @@ class JDBCWrappedInterfacesTest extends InstrumentationSpecification {
255255
childOfPrevious()
256256
errored false
257257
tags {
258+
if (database == "testdb") {
259+
"$Tags.PEER_HOSTNAME" "localhost"
260+
}
258261
"$Tags.COMPONENT" "java-jdbc-prepared_statement"
259262
"$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT
260263
"$Tags.DB_TYPE" "${database}"

0 commit comments

Comments
 (0)