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

CHANGELOG.asciidoc

@@ -34,6 +34,7 @@ endif::[]
 * When the `MANIFEST.MF` of the main jar contains the `Implementation-Version` attribute, it is used as the default service version (except for application servers) - {pull}1922[#1922]
 * Added support for overwritting the service version per classloader - {pull}1726[#1726]
 * Support for the Java LDAP client - {pull}2355[#2355]
+* Add the instance name to `context.destination.service.resource` to JDBC spans, if use_jdbc_service_resource_auto_inference is set to true - {pull}1928[#1928]
 
 [float]
 ===== Bug fixes

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

@@ -0,0 +1,39 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package co.elastic.apm.agent.jdbc;
+
+import org.stagemonitor.configuration.ConfigurationOption;
+import org.stagemonitor.configuration.ConfigurationOptionProvider;
+
+public class JDBCConfiguration extends ConfigurationOptionProvider {
+
+    private static final String JDBC_CATEGORY = "JDBC";
+
+    private final ConfigurationOption<Boolean> use_instance_for_db_resource = ConfigurationOption.booleanOption()
+        .key("use_instance_for_db_resource")
+        .configurationCategory(JDBC_CATEGORY)
+        .description("If set to `true`, the agent adds the db instance name to `destination.service.resource` of a JDBC span.")
+        .dynamic(false)
+        .tags("added[1.26.0]", "internal")
+        .buildWithDefault(false);
+
+    public boolean getUseInstanceForDbResource() {
+        return use_instance_for_db_resource.get();
+    }
+}

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

@@ -20,9 +20,11 @@
 
 import co.elastic.apm.agent.db.signature.Scanner;
 import co.elastic.apm.agent.db.signature.SignatureParser;
+import co.elastic.apm.agent.impl.GlobalTracer;
 import co.elastic.apm.agent.impl.context.Destination;
 import co.elastic.apm.agent.impl.transaction.AbstractSpan;
 import co.elastic.apm.agent.impl.transaction.Span;
+import co.elastic.apm.agent.jdbc.JDBCConfiguration;
 import co.elastic.apm.agent.jdbc.JdbcFilter;
 import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap;
 import co.elastic.apm.agent.sdk.logging.Logger;
@@ -44,6 +46,10 @@ public class JdbcHelper {
     public static final String DB_SPAN_TYPE = "db";
     public static final String DB_SPAN_ACTION = "query";
 
+    private static final boolean useInstanceForDbResource = GlobalTracer.getTracerImpl()
+        .getConfig(JDBCConfiguration.class)
+        .getUseInstanceForDbResource();
+
     private static final JdbcHelper INSTANCE = new JdbcHelper();
 
     public static JdbcHelper get() {
@@ -87,7 +93,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,8 +127,14 @@ public Span createJdbcSpan(@Nullable String sql, Object statement, @Nullable Abs
                 .withPort(connectionMetaData.getPort());
             destination.getService()
                 .withName(vendor)
-                .withResource(vendor)
                 .withType(DB_SPAN_TYPE);
+
+            destination.getService().withResource(vendor);
+            if (useInstanceForDbResource && connectionMetaData.getInstance() != null) {
+                destination.getService().getResource()
+                    .append('/')
+                    .append(connectionMetaData.getInstance());
+            }
         }
         span.withSubtype(vendor).withAction(DB_SPAN_ACTION);
 

apm-agent-plugins/apm-jdbc-plugin/src/main/resources/META-INF/services/org.stagemonitor.configuration.ConfigurationOptionProvider

@@ -0,0 +1 @@
+co.elastic.apm.agent.jdbc.JDBCConfiguration

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);
@@ -419,6 +423,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);
@@ -444,7 +449,11 @@ private Span assertSpanRecorded(String rawSql, boolean preparedStatement, long e
         }
 
         Destination.Service service = destination.getService();
-        assertThat(service.getResource().toString()).isEqualTo(expectedDbVendor);
+        if (tracer.getConfig(JDBCConfiguration.class).getUseInstanceForDbResource() && 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")
@@ -460,6 +469,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");
@@ -480,7 +490,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", "mssql"},
-            {"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", "mssql", "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");
     }
 
 }

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

@@ -0,0 +1,37 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package co.elastic.apm.agent.jdbc;
+
+import org.junit.BeforeClass;
+
+import java.sql.DriverManager;
+
+import static org.mockito.Mockito.when;
+
+public class JdbcServiceResourceAutoInferenceTest extends AbstractJdbcInstrumentationTest {
+
+    @BeforeClass
+    public static void setUseJDBCServiceResourceAutoInference() {
+        when(config.getConfig(JDBCConfiguration.class).getUseInstanceForDbResource()).thenReturn(true);
+    }
+
+    public JdbcServiceResourceAutoInferenceTest() throws Exception {
+        super(DriverManager.getConnection("jdbc:h2:mem:test"), "h2", "TEST");
+    }
+}