Use the resource auto-inference to set context.destination.service.resource for JDBC spans (#1913)

CHANGELOG.asciidoc

@@ -68,6 +68,7 @@ service map and downstream service in the dependencies table - {pull}1898[#1898]
 * Fix `ClassCastException` with `ConnnectionMetaData` and multiple classloaders - {pull}1864[#1864]
 * Fix NPE in `co.elastic.apm.agent.servlet.helper.ServletTransactionCreationHelper.getClassloader` - {pull}1861[#1861]
 * Fix for Jboss JMX unexpected notifications - {pull}1895[#1895]
+* Use the resource auto-inference to set `context.destination.service.resource` for JDBC spans  - {pull}1914[#1914]
 
 [[release-notes-1.x]]
 === Java Agent version 1.x

apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java

@@ -87,7 +87,7 @@ public Span createJdbcSpan(@Nullable String sql, Object statement, @Nullable Abs
             return null;
         }
 
-        Span span = parent.createSpan().activate();
+        Span span = parent.createExitSpan().activate();
         if (sql.isEmpty()) {
             span.withName("empty query");
         } else if (span.isSampled()) {
@@ -121,7 +121,6 @@ public Span createJdbcSpan(@Nullable String sql, Object statement, @Nullable Abs
                 .withPort(connectionMetaData.getPort());
             destination.getService()
                 .withName(vendor)
-                .withResource(vendor)
                 .withType(DB_SPAN_TYPE);
         }
         span.withSubtype(vendor).withAction(DB_SPAN_ACTION);

apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/AbstractJdbcInstrumentationTest.java

@@ -59,6 +59,7 @@ public abstract class AbstractJdbcInstrumentationTest extends AbstractInstrument
     private static final long PREPARED_STMT_TIMEOUT = 10000;
 
     private final String expectedDbVendor;
+    private final String expectedDbInstance;
     private Connection connection;
     @Nullable
     private PreparedStatement preparedStatement;
@@ -67,9 +68,10 @@ public abstract class AbstractJdbcInstrumentationTest extends AbstractInstrument
     private final Transaction transaction;
     private final SignatureParser signatureParser;
 
-    AbstractJdbcInstrumentationTest(Connection connection, String expectedDbVendor) throws Exception {
+    AbstractJdbcInstrumentationTest(Connection connection, String expectedDbVendor, String expectedDbInstance) throws Exception {
         this.connection = connection;
         this.expectedDbVendor = expectedDbVendor;
+        this.expectedDbInstance= expectedDbInstance;
         connection.createStatement().execute("CREATE TABLE ELASTIC_APM (FOO INT NOT NULL, BAR VARCHAR(255))");
         connection.createStatement().execute("ALTER TABLE ELASTIC_APM ADD PRIMARY KEY (FOO)");
         transaction = startTestRootTransaction("jdbc-test");
@@ -78,6 +80,8 @@ public abstract class AbstractJdbcInstrumentationTest extends AbstractInstrument
 
     @Before
     public void setUp() throws Exception {
+        reporter.disableCheckDestinationAddress();
+
         preparedStatement = EXECUTOR_SERVICE.submit(new Callable<PreparedStatement>() {
             public PreparedStatement call() throws Exception {
                 return connection.prepareStatement(PREPARED_STATEMENT_SQL);
@@ -394,6 +398,7 @@ private Span assertSpanRecorded(String rawSql, boolean preparedStatement, long e
         Span span = reporter.getFirstSpan();
         StringBuilder processedSql = new StringBuilder();
         signatureParser.querySignature(rawSql, processedSql, preparedStatement);
+        assertThat(span.isExit()).isTrue();
         assertThat(span.getNameAsString()).isEqualTo(processedSql.toString());
         assertThat(span.getType()).isEqualTo(DB_SPAN_TYPE);
         assertThat(span.getSubtype()).isEqualTo(expectedDbVendor);
@@ -419,7 +424,11 @@ private Span assertSpanRecorded(String rawSql, boolean preparedStatement, long e
         }
 
         Destination.Service service = destination.getService();
-        assertThat(service.getResource().toString()).isEqualTo(expectedDbVendor);
+        if (expectedDbInstance != null) {
+            assertThat(service.getResource().toString()).isEqualTo(expectedDbVendor + "/" + expectedDbInstance);
+        } else {
+            assertThat(service.getResource().toString()).isEqualTo(expectedDbVendor);
+        }
 
         assertThat(span.getOutcome())
             .describedAs("span outcome should be explicitly set to either failure or success")
@@ -435,6 +444,7 @@ private void assertSpanRecordedWithoutConnection(String rawSql, boolean prepared
         Span jdbcSpan = reporter.getFirstSpan();
         StringBuilder processedSql = new StringBuilder();
         signatureParser.querySignature(rawSql, processedSql, preparedStatement);
+        assertThat(jdbcSpan.isExit()).isTrue();
         assertThat(jdbcSpan.getNameAsString()).isEqualTo(processedSql.toString());
         assertThat(jdbcSpan.getType()).isEqualTo(DB_SPAN_TYPE);
         assertThat(jdbcSpan.getSubtype()).isEqualTo("unknown");
@@ -455,7 +465,7 @@ private void assertSpanRecordedWithoutConnection(String rawSql, boolean prepared
         assertThat(destination.getPort()).isLessThanOrEqualTo(0);
 
         Destination.Service service = destination.getService();
-        assertThat(service.getResource()).isNullOrEmpty();
+        assertThat(service.getResource().toString()).isEqualTo("unknown");
     }
 
     private static long[] toLongArray(int[] a) {

apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/DataSourceIT.java

@@ -39,7 +39,7 @@ public class DataSourceIT extends AbstractJdbcInstrumentationTest {
     private static final String URL = "jdbc:h2:mem:test";
 
     public DataSourceIT(Supplier<DataSource> dataSourceSupplier) throws Exception {
-        super(dataSourceSupplier.get().getConnection(), "h2");
+        super(dataSourceSupplier.get().getConnection(), "h2", "TEST");
     }
 
     @Parameterized.Parameters(name = "{0}")

apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/JdbcDbIT.java

@@ -31,20 +31,20 @@ public class JdbcDbIT extends AbstractJdbcInstrumentationTest {
         System.setProperty("oracle.jdbc.timezoneAsRegion", "false");
     }
 
-    public JdbcDbIT(String url, String expectedDbVendor) throws Exception {
-        super(DriverManager.getConnection(url), expectedDbVendor);
+    public JdbcDbIT(String url, String expectedDbVendor, String expectedDbInstance) throws Exception {
+        super(DriverManager.getConnection(url), expectedDbVendor, expectedDbInstance);
     }
 
     @Parameterized.Parameters(name = "{1} {0}")
     public static Iterable<Object[]> data() {
         return Arrays.asList(new Object[][]{
-            {"jdbc:tc:mysql:5://hostname/databasename", "mysql"},
-            {"jdbc:tc:postgresql:9://hostname/databasename", "postgresql"},
-            {"jdbc:tc:postgresql:10://hostname/databasename", "postgresql"},
-            {"jdbc:tc:mariadb:10://hostname/databasename", "mariadb"},
-            {"jdbc:tc:sqlserver:2017-CU12://hostname/databasename", "sqlserver"},
-            {"jdbc:tc:db2:11.5.0.0a://hostname/databasename", "db2"},
-            {"jdbc:tc:oracle://hostname/databasename", "oracle"},
+            {"jdbc:tc:mysql:5://hostname/databasename", "mysql", "databasename"},
+            {"jdbc:tc:postgresql:9://hostname/databasename", "postgresql", "databasename"},
+            {"jdbc:tc:postgresql:10://hostname/databasename", "postgresql", "databasename"},
+            {"jdbc:tc:mariadb:10://hostname/databasename", "mariadb", "databasename"},
+            {"jdbc:tc:sqlserver:2017-CU12://hostname/databasename", "sqlserver", "master"},
+            {"jdbc:tc:db2:11.5.0.0a://hostname/databasename", "db2", null},
+            {"jdbc:tc:oracle://hostname/databasename", "oracle", null},
         });
     }
 

apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/JdbcInstrumentationTest.java

@@ -23,7 +23,7 @@
 public class JdbcInstrumentationTest extends AbstractJdbcInstrumentationTest {
 
     public JdbcInstrumentationTest() throws Exception {
-        super(DriverManager.getConnection("jdbc:h2:mem:test"), "h2");
+        super(DriverManager.getConnection("jdbc:h2:mem:test"), "h2", "TEST");
     }
 
 }