cache connection url parsing since it typically takes place within a lock

richardstartin · richardstartin · commit 3546ec27a8d9 · 2021-03-30T12:30:18.000+01:00
diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/jdbc/JDBCConnectionUrlParser.java
@@ -2,6 +2,10 @@
 
 import static datadog.trace.bootstrap.instrumentation.jdbc.DBInfo.DEFAULT;
 
+import datadog.trace.api.Function;
+import datadog.trace.api.Pair;
+import datadog.trace.api.cache.DDCache;
+import datadog.trace.api.cache.DDCaches;
 import datadog.trace.bootstrap.ExceptionLogger;
 import de.thetaphi.forbiddenapis.SuppressForbidden;
 import java.io.UnsupportedEncodingException;
@@ -692,6 +696,17 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) {
     }
   }
 
+  private static final DDCache<Pair<String, Properties>, DBInfo> CACHED_DB_INFO =
+      DDCaches.newFixedSizeCache(32);
+  private static final Function<Pair<String, Properties>, DBInfo> PARSE =
+      new Function<Pair<String, Properties>, DBInfo>() {
+
+        @Override
+        public DBInfo apply(Pair<String, Properties> input) {
+          return parse(input.getLeft(), input.getRight());
+        }
+      };
+
   private final String[] typeKeys;
 
   JDBCConnectionUrlParser(final String... typeKeys) {
@@ -700,6 +715,10 @@ DBInfo.Builder doParse(final String jdbcUrl, final DBInfo.Builder builder) {
 
   abstract DBInfo.Builder doParse(String jdbcUrl, final DBInfo.Builder builder);
 
+  public static DBInfo extractDBInfo(String connectionUrl, Properties props) {
+    return CACHED_DB_INFO.computeIfAbsent(Pair.of(connectionUrl, props), PARSE);
+  }
+
   public static DBInfo parse(String connectionUrl, final Properties props) {
     if (connectionUrl == null) {
       return DEFAULT;
diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DriverInstrumentation.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/DriverInstrumentation.java
@@ -70,7 +70,7 @@ public static void addDBInfo(
         // Exception was probably thrown.
         return;
       }
-      final DBInfo dbInfo = JDBCConnectionUrlParser.parse(url, props);
+      final DBInfo dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, props);
       InstrumentationContext.get(Connection.class, DBInfo.class).put(connection, dbInfo);
     }
   }
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
@@ -89,10 +89,10 @@ public AgentSpan onConnection(
           final String url = metaData.getURL();
           if (url != null) {
             try {
-              dbInfo = JDBCConnectionUrlParser.parse(url, connection.getClientInfo());
+              dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, connection.getClientInfo());
             } catch (final Throwable ex) {
               // getClientInfo is likely not allowed.
-              dbInfo = JDBCConnectionUrlParser.parse(url, null);
+              dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, null);
             }
           } else {
             dbInfo = DBInfo.DEFAULT;
diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCConnectionUrlParserTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/JDBCConnectionUrlParserTest.groovy
@@ -2,7 +2,7 @@ import datadog.trace.agent.test.AgentTestRunner
 import datadog.trace.bootstrap.instrumentation.jdbc.DBInfo
 import spock.lang.Shared
 
-import static datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionUrlParser.parse
+import static datadog.trace.bootstrap.instrumentation.jdbc.JDBCConnectionUrlParser.extractDBInfo
 
 class JDBCConnectionUrlParserTest extends AgentTestRunner {
 
@@ -24,7 +24,7 @@ class JDBCConnectionUrlParserTest extends AgentTestRunner {
 
   def "invalid url returns default"() {
     expect:
-    parse(url, null) == DBInfo.DEFAULT
+    extractDBInfo(url, null) == DBInfo.DEFAULT
 
     where:
     url            | _
@@ -37,7 +37,7 @@ class JDBCConnectionUrlParserTest extends AgentTestRunner {
 
   def "verify #type:#subtype parsing of #url"() {
     setup:
-    def info = parse(url, props)
+    def info = extractDBInfo(url, props)
 
     expect:
     info.url == expected.url

Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ public static void addDBInfo(`
`70`	`70`	`// Exception was probably thrown.`
`71`	`71`	`return;`
`72`	`72`	`}`
`73`		`- final DBInfo dbInfo = JDBCConnectionUrlParser.parse(url, props);`
	`73`	`+ final DBInfo dbInfo = JDBCConnectionUrlParser.extractDBInfo(url, props);`
`74`	`74`	`InstrumentationContext.get(Connection.class, DBInfo.class).put(connection, dbInfo);`
`75`	`75`	`}`
`76`	`76`	`}`