Skip to content

Commit ba2c056

Browse files
authored
Add a feature flag to disable extra calls to the DB in JDBC instrumentation (#9774)
1 parent 6b0cd83 commit ba2c056

File tree

8 files changed

+492
-23
lines changed

8 files changed

+492
-23
lines changed

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public void methodAdvice(MethodTransformer transformer) {
6464
}
6565

6666
public static class DriverAdvice {
67+
6768
@Advice.OnMethodExit(suppress = Throwable.class)
6869
public static void addDBInfo(
6970
@Advice.Argument(0) final String url,
@@ -75,8 +76,10 @@ public static void addDBInfo(
7576
}
7677
String connectionUrl = url;
7778
Properties connectionProps = props;
78-
if (null == props
79-
|| !Boolean.parseBoolean(props.getProperty("oracle.jdbc.useShardingDriverConnection"))) {
79+
if (JDBCDecorator.FETCH_DB_METADATA_ON_CONNECT
80+
&& (null == props
81+
|| !Boolean.parseBoolean(
82+
props.getProperty("oracle.jdbc.useShardingDriverConnection")))) {
8083
try {
8184
DatabaseMetaData metaData = connection.getMetaData();
8285
connectionUrl = metaData.getURL();

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.sql.SQLException;
2929
import java.sql.Statement;
3030
import java.util.HashSet;
31+
import java.util.Properties;
3132
import java.util.Set;
3233
import org.slf4j.Logger;
3334
import org.slf4j.LoggerFactory;
@@ -61,6 +62,10 @@ public class JDBCDecorator extends DatabaseClientDecorator<DBInfo> {
6162
Config.get().isDbmTracePreparedStatements();
6263
public static final boolean DBM_ALWAYS_APPEND_SQL_COMMENT =
6364
Config.get().isDbmAlwaysAppendSqlComment();
65+
public static final boolean FETCH_DB_METADATA_ON_CONNECT =
66+
Config.get().isDbMetadataFetchingOnConnectEnabled();
67+
private static final boolean FETCH_DB_METADATA_ON_QUERY =
68+
Config.get().isDbMetadataFetchingOnQueryEnabled();
6469

6570
private volatile boolean warnedAboutDBMPropagationMode = false; // to log a warning only once
6671

@@ -183,7 +188,7 @@ public static DBInfo parseDBInfo(
183188
} catch (Throwable ignore) {
184189
}
185190
if (dbInfo == null) {
186-
// couldn't find DBInfo anywhere, so fall back to default
191+
// couldn't find DBInfo from a previous call anywhere, so we try to fetch it from the DB
187192
dbInfo = parseDBInfoFromConnection(connection);
188193
}
189194
// store the DBInfo on the outermost connection instance to avoid future searches
@@ -202,7 +207,7 @@ public String getDbService(final DBInfo dbInfo) {
202207
}
203208

204209
public static DBInfo parseDBInfoFromConnection(final Connection connection) {
205-
if (connection == null) {
210+
if (connection == null || !FETCH_DB_METADATA_ON_QUERY) {
206211
// we can log here, but it risks to be too verbose
207212
return DBInfo.DEFAULT;
208213
}
@@ -211,16 +216,19 @@ public static DBInfo parseDBInfoFromConnection(final Connection connection) {
211216
final DatabaseMetaData metaData = connection.getMetaData();
212217
final String url;
213218
if (metaData != null && (url = metaData.getURL()) != null) {
219+
Properties clientInfo = null;
214220
try {
215-
dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, connection.getClientInfo());
221+
clientInfo = connection.getClientInfo();
216222
} catch (final Throwable ex) {
217-
// getClientInfo is likely not allowed.
218-
dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, null);
223+
// getClientInfo is likely not allowed, we can still extract info from the url alone
224+
log.debug("Could not get client info from DB", ex);
219225
}
226+
dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, clientInfo);
220227
} else {
221228
dbInfo = DBInfo.DEFAULT;
222229
}
223230
} catch (final SQLException se) {
231+
log.debug("Could not get metadata from DB", se);
224232
dbInfo = DBInfo.DEFAULT;
225233
}
226234
return dbInfo;
Lines changed: 271 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
2+
import static datadog.trace.api.config.TraceInstrumentationConfig.DB_METADATA_FETCHING_ON_CONNECT
3+
4+
import datadog.trace.agent.test.InstrumentationSpecification
5+
import datadog.trace.api.DDSpanTypes
6+
import datadog.trace.bootstrap.instrumentation.api.Tags
7+
import java.sql.Connection
8+
import java.sql.DatabaseMetaData
9+
import java.sql.SQLException
10+
import java.sql.Statement
11+
import test.TestConnection
12+
import test.TestDatabaseMetaData
13+
import test.TestDriver
14+
15+
abstract class DriverInstrumentationMetadataFetchingTestBase extends InstrumentationSpecification {
16+
17+
def "test dbInfo extraction from metadata or URL with username"() {
18+
setup:
19+
def originalUrl = "jdbc:postgresql://original-host:1234/originaldb"
20+
def metadataUrl = "jdbc:postgresql://metadata-host:5678/metadatadb"
21+
22+
def metadata = new TestDatabaseMetaData() {
23+
@Override
24+
String getURL() throws SQLException {
25+
return metadataUrl
26+
}
27+
28+
@Override
29+
String getUserName() throws SQLException {
30+
return "metadata-user"
31+
}
32+
}
33+
34+
def testConnection = new TestConnection(false)
35+
testConnection.setMetaData(metadata)
36+
37+
def driver = new TestDriver() {
38+
@Override
39+
Connection connect(String url, Properties info) {
40+
return testConnection
41+
}
42+
}
43+
44+
def props = new Properties()
45+
props.put("user", "original-user")
46+
47+
// Expected values depend on whether metadata fetching is enabled
48+
def expectedDbInstance = shouldFetchMetadataOnConnect() ? "metadatadb" : "originaldb"
49+
def expectedDbUser = shouldFetchMetadataOnConnect() ? "metadata-user" : "original-user"
50+
51+
when:
52+
def connection = driver.connect(originalUrl, props)
53+
Statement statement = connection.createStatement()
54+
runUnderTrace("parent") {
55+
statement.execute("SELECT 1")
56+
}
57+
58+
then:
59+
assertTraces(1) {
60+
trace(2) {
61+
span(0) {
62+
operationName "parent"
63+
}
64+
span(1) {
65+
operationName "postgresql.query"
66+
spanType DDSpanTypes.SQL
67+
childOf span(0)
68+
tags(false) {
69+
"$Tags.DB_INSTANCE" expectedDbInstance
70+
"$Tags.DB_USER" expectedDbUser
71+
}
72+
}
73+
}
74+
}
75+
76+
cleanup:
77+
statement?.close()
78+
connection?.close()
79+
}
80+
81+
def "test driver connect with null metadata URL"() {
82+
setup:
83+
def originalUrl = "jdbc:postgresql://original-host:5432/originaldb"
84+
85+
def metadata = new TestDatabaseMetaData() {
86+
@Override
87+
String getURL() throws SQLException {
88+
return null
89+
}
90+
}
91+
92+
def testConnection = new TestConnection(false)
93+
testConnection.setMetaData(metadata)
94+
95+
def driver = new TestDriver() {
96+
@Override
97+
Connection connect(String url, Properties info) {
98+
return testConnection
99+
}
100+
}
101+
102+
def props = new Properties()
103+
104+
when:
105+
def connection = driver.connect(originalUrl, props)
106+
Statement statement = connection.createStatement()
107+
runUnderTrace("parent") {
108+
statement.execute("SELECT 1")
109+
}
110+
111+
then:
112+
assertTraces(1) {
113+
trace(2) {
114+
span(0) {
115+
operationName "parent"
116+
}
117+
span(1) {
118+
operationName "postgresql.query"
119+
spanType DDSpanTypes.SQL
120+
childOf span(0)
121+
tags(false) {
122+
// Should fallback to original URL regardless of flag when metadata URL is null
123+
"$Tags.DB_INSTANCE" "originaldb"
124+
}
125+
}
126+
}
127+
}
128+
129+
cleanup:
130+
statement?.close()
131+
connection?.close()
132+
}
133+
134+
def "test driver connect with metadata exception"() {
135+
setup:
136+
def originalUrl = "jdbc:postgresql://original-host:5432/originaldb"
137+
138+
def testConnection = new TestConnection(false) {
139+
@Override
140+
DatabaseMetaData getMetaData() throws SQLException {
141+
throw new SQLException("Test exception")
142+
}
143+
}
144+
145+
def driver = new TestDriver() {
146+
@Override
147+
Connection connect(String url, Properties info) {
148+
return testConnection
149+
}
150+
}
151+
152+
def props = new Properties()
153+
154+
when:
155+
def connection = driver.connect(originalUrl, props)
156+
Statement statement = connection.createStatement()
157+
runUnderTrace("parent") {
158+
statement.execute("SELECT 1")
159+
}
160+
161+
then:
162+
assertTraces(1) {
163+
trace(2) {
164+
span(0) {
165+
operationName "parent"
166+
}
167+
span(1) {
168+
operationName "postgresql.query"
169+
spanType DDSpanTypes.SQL
170+
childOf span(0)
171+
tags(false) {
172+
// Should fallback to original URL when exception occurs
173+
"$Tags.DB_INSTANCE" "originaldb"
174+
}
175+
}
176+
}
177+
}
178+
179+
cleanup:
180+
statement?.close()
181+
connection?.close()
182+
}
183+
184+
def "test driver connect with Oracle sharding driver"() {
185+
setup:
186+
def originalUrl = "jdbc:oracle:thin:@original-host:1521:orcl"
187+
def metadataUrl = "jdbc:oracle:thin:@metadata-host:1521:orcl"
188+
189+
def metadata = new TestDatabaseMetaData()
190+
metadata.setURL(metadataUrl)
191+
192+
def testConnection = new TestConnection(false)
193+
testConnection.setMetaData(metadata)
194+
195+
def driver = new TestDriver() {
196+
@Override
197+
Connection connect(String url, Properties info) {
198+
return testConnection
199+
}
200+
}
201+
202+
def props = new Properties()
203+
props.setProperty("oracle.jdbc.useShardingDriverConnection", "true")
204+
205+
when:
206+
def connection = driver.connect(originalUrl, props)
207+
Statement statement = connection.createStatement()
208+
runUnderTrace("parent") {
209+
statement.execute("SELECT 1")
210+
}
211+
212+
then:
213+
assertTraces(1) {
214+
trace(2) {
215+
span(0) {
216+
operationName "parent"
217+
}
218+
span(1) {
219+
operationName "oracle.query"
220+
spanType DDSpanTypes.SQL
221+
childOf span(0)
222+
tags(false) {
223+
// Should use original URL for Oracle sharding driver regardless of flag
224+
"$Tags.DB_INSTANCE" "orcl"
225+
}
226+
}
227+
}
228+
}
229+
230+
cleanup:
231+
statement?.close()
232+
connection?.close()
233+
}
234+
235+
abstract boolean shouldFetchMetadataOnConnect()
236+
}
237+
238+
/**
239+
* Test with metadata fetching enabled on connect (default behavior)
240+
*/
241+
class DriverInstrumentationWithMetadataOnConnectForkedTest extends DriverInstrumentationMetadataFetchingTestBase {
242+
243+
@Override
244+
void configurePreAgent() {
245+
super.configurePreAgent()
246+
injectSysConfig(DB_METADATA_FETCHING_ON_CONNECT, "true")
247+
}
248+
249+
@Override
250+
boolean shouldFetchMetadataOnConnect() {
251+
return true
252+
}
253+
}
254+
255+
/**
256+
* Test with metadata fetching disabled on connect
257+
*/
258+
class DriverInstrumentationWithoutMetadataOnConnectForkedTest extends DriverInstrumentationMetadataFetchingTestBase {
259+
260+
@Override
261+
void configurePreAgent() {
262+
super.configurePreAgent()
263+
injectSysConfig(DB_METADATA_FETCHING_ON_CONNECT, "false")
264+
}
265+
266+
@Override
267+
boolean shouldFetchMetadataOnConnect() {
268+
return false
269+
}
270+
}
271+

0 commit comments

Comments
 (0)