From a4aa3c681786e37e5bfd97fcfd548ac955b97b5d Mon Sep 17 00:00:00 2001 From: Tobias Stadler Date: Mon, 12 Jul 2021 16:48:21 +0200 Subject: [PATCH 1/2] Use the resource auto-inference to set context.destination.service.resource for JDBC spans (#1913) --- .../apm/agent/jdbc/helper/JdbcHelper.java | 3 +-- .../jdbc/AbstractJdbcInstrumentationTest.java | 16 +++++++++++++--- .../elastic/apm/agent/jdbc/DataSourceIT.java | 2 +- .../co/elastic/apm/agent/jdbc/JdbcDbIT.java | 18 +++++++++--------- .../agent/jdbc/JdbcInstrumentationTest.java | 2 +- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java index 7158b367c5..2e23180680 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/helper/JdbcHelper.java +++ b/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); diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/AbstractJdbcInstrumentationTest.java b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/AbstractJdbcInstrumentationTest.java index 207456e0b3..6688625716 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/AbstractJdbcInstrumentationTest.java +++ b/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() { 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) { diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/DataSourceIT.java b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/DataSourceIT.java index b561ed3d45..5d03c9d935 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/DataSourceIT.java +++ b/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 dataSourceSupplier) throws Exception { - super(dataSourceSupplier.get().getConnection(), "h2"); + super(dataSourceSupplier.get().getConnection(), "h2", "TEST"); } @Parameterized.Parameters(name = "{0}") diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/JdbcDbIT.java b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/JdbcDbIT.java index 5cdae8a581..6718e80345 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/JdbcDbIT.java +++ b/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 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}, }); } diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/JdbcInstrumentationTest.java b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/JdbcInstrumentationTest.java index 929ec523d1..0103308d51 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/JdbcInstrumentationTest.java +++ b/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"); } } From 186e5f635a70091f686582bd62abc28f7685763b Mon Sep 17 00:00:00 2001 From: Tobias Stadler Date: Mon, 12 Jul 2021 17:09:15 +0200 Subject: [PATCH 2/2] Added #1914 to the changelog --- CHANGELOG.asciidoc | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 29c978b05b..42acc29bbe 100644 --- a/CHANGELOG.asciidoc +++ b/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