diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcAdviceScope.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcAdviceScope.java index 1685d3b5a81a..fa6982e7927e 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcAdviceScope.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcAdviceScope.java @@ -7,6 +7,7 @@ import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; import static io.opentelemetry.javaagent.instrumentation.jdbc.JdbcSingletons.statementInstrumenter; +import static java.util.Collections.emptyList; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; @@ -83,11 +84,11 @@ private static DbRequest createBatchRequest(Statement statement) { } Long batchSize = JdbcData.getPreparedStatementBatchSize((PreparedStatement) statement); Map parameters = JdbcData.getParameters((PreparedStatement) statement); - return DbRequest.create(statement, sql, batchSize, parameters, true); + return DbRequest.create(statement, sql, batchSize != null ? batchSize : 0L, parameters, true); } else { JdbcData.StatementBatchInfo batchInfo = JdbcData.getStatementBatchInfo(statement); if (batchInfo == null) { - return DbRequest.create(statement, null); + return DbRequest.create(statement, emptyList(), 0L, false); } else { return DbRequest.create( statement, batchInfo.getQueryTexts(), batchInfo.getBatchSize(), false); diff --git a/instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java b/instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java index 5d5f71a5d426..f3aea8c079d7 100644 --- a/instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java +++ b/instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/testing/AbstractJdbcInstrumentationTest.java @@ -1879,82 +1879,76 @@ void testProxyPreparedStatement() throws SQLException { .hasParent(trace.getSpan(0)))); } - static Stream batchStream() throws SQLException { + static Stream batchCasesStream() { return Stream.of( - Arguments.of("h2", new org.h2.Driver().connect(JDBC_URLS.get("h2"), null), null, "h2:mem:"), - Arguments.of( - "derby", - new EmbeddedDriver().connect(JDBC_URLS.get("derby"), null), - "APP", - "derby:memory:"), - Arguments.of( - "hsqldb", new JDBCDriver().connect(JDBC_URLS.get("hsqldb"), null), "SA", "hsqldb:mem:"), - Arguments.of( - "sqlite", - new JDBC().connect(JDBC_URLS.get("sqlite"), new Properties()), - null, - "sqlite:memory:")); + Arguments.argumentSet( + "empty", + BatchScenario.builder() + .spanName(DATABASE_NAME_LOWER) + .oldSpanName(DATABASE_NAME_LOWER) + .build()), + Arguments.argumentSet( + "single", + BatchScenario.builder() + .addQuery("INSERT INTO batch_test (id, num) VALUES (1, 1)") + .spanName("INSERT batch_test") + .oldSpanName("INSERT " + DATABASE_NAME_LOWER + ".batch_test") + .queryText("INSERT INTO batch_test (id, num) VALUES (?, ?)") + .oldStatement("INSERT INTO batch_test (id, num) VALUES (?, ?)") + .summary("INSERT batch_test") + .oldOperation("INSERT") + .oldTable("batch_test") + .build()), + Arguments.argumentSet( + "twoSameOperation", + BatchScenario.builder() + .addQuery("INSERT INTO batch_test (id, num) VALUES (1, 1)") + .addQuery("INSERT INTO batch_test (id, num) VALUES (2, 2)") + .spanName("BATCH INSERT batch_test") + .oldSpanName(DATABASE_NAME_LOWER) + .queryText("INSERT INTO batch_test (id, num) VALUES (?, ?)") + .summary("BATCH INSERT batch_test") + .batchSize(2) + .build()), + Arguments.argumentSet( + "twoDifferentOperations", + BatchScenario.builder() + .addQuery("INSERT INTO batch_test (id, num) VALUES (1, 1)") + .addQuery("UPDATE batch_test SET num = 5 WHERE id = 1") + .spanName("BATCH") + .oldSpanName(DATABASE_NAME_LOWER) + .queryText( + "INSERT INTO batch_test (id, num) VALUES (?, ?); UPDATE batch_test SET num = ? WHERE id = ?") + .summary("BATCH") + .batchSize(2) + .build())); } @ParameterizedTest - @MethodSource("batchStream") - void testBatch(String system, Connection connection, String username, String url) - throws SQLException { - testBatchImpl( - system, - wrap(connection), - username, - url, - "simple_batch_test", - statement -> assertThat(statement.executeBatch()).isEqualTo(new int[] {1, 1})); - } - - @ParameterizedTest - @MethodSource("batchStream") - void testLargeBatch(String system, Connection connection, String username, String url) - throws SQLException { - - Statement createTable = wrap(connection).createStatement(); - cleanup.deferCleanup(createTable); - createTable.execute( - "CREATE TABLE simple_batch_test_large (id INTEGER not NULL, PRIMARY KEY ( id ))"); - Statement statement = wrap(connection).createStatement(); - cleanup.deferCleanup(statement); - statement.addBatch("INSERT INTO simple_batch_test_large VALUES(1)"); - statement.addBatch("INSERT INTO simple_batch_test_large VALUES(2)"); - - if (testLatestDeps() || "sqlite".equals(system)) { - assertThat(statement.executeLargeBatch()).isEqualTo(new long[] {1, 1}); - } else { - // Older drivers don't support JDBC 4.2, expect UnsupportedOperationException - // This is the correct behavior - instrumentation should not change driver behavior - assertThatThrownBy(statement::executeLargeBatch) - .isInstanceOf(UnsupportedOperationException.class); - } - } + @MethodSource("batchCasesStream") + void testStatementBatch(BatchScenario scenario) throws SQLException { + Connection connection = wrap(new org.h2.Driver().connect(JDBC_URLS.get("h2"), null)); + cleanup.deferCleanup(connection); - private void testBatchImpl( - String system, - Connection connection, - String username, - String url, - String tableName, - ThrowingConsumer action) - throws SQLException { + // recreate a fresh batch_test table for each scenario so that batch row ids can be reused + // without worrying about collisions from previous scenarios + Statement dropTable = connection.createStatement(); + dropTable.execute("DROP TABLE IF EXISTS batch_test"); + cleanup.deferCleanup(dropTable); Statement createTable = connection.createStatement(); - createTable.execute("CREATE TABLE " + tableName + " (id INTEGER not NULL, PRIMARY KEY ( id ))"); + createTable.execute( + "CREATE TABLE batch_test (id INTEGER not NULL, num INTEGER, PRIMARY KEY ( id ))"); cleanup.deferCleanup(createTable); - - testing().waitForTraces(1); + testing().waitForTraces(2); testing().clearData(); Statement statement = connection.createStatement(); cleanup.deferCleanup(statement); - statement.addBatch("INSERT INTO non_existent_table VALUES(1)"); - statement.clearBatch(); - statement.addBatch("INSERT INTO " + tableName + " VALUES(1)"); - statement.addBatch("INSERT INTO " + tableName + " VALUES(2)"); - testing().runWithSpan("parent", () -> action.accept(statement)); + for (String sql : scenario.queries) { + statement.addBatch(sql); + } + + testing().runWithSpan("parent", statement::executeBatch); testing() .waitAndAssertTraces( @@ -1964,137 +1958,74 @@ private void testBatchImpl( span -> span.hasName( emitStableDatabaseSemconv() - ? "BATCH INSERT " + tableName - : "jdbcunittest") + ? scenario.spanName + : scenario.oldSpanName) .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) .hasAttributesSatisfyingExactly( - equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)), + equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName("h2")), equalTo(maybeStable(DB_NAME), DATABASE_NAME_LOWER), - equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), equalTo( - DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), + DB_CONNECTION_STRING, + emitStableDatabaseSemconv() ? null : "h2:mem:"), equalTo( maybeStable(DB_STATEMENT), emitStableDatabaseSemconv() - ? "INSERT INTO " + tableName + " VALUES(?)" - : null), + ? scenario.queryText + : scenario.oldStatement), equalTo( DB_QUERY_SUMMARY, - emitStableDatabaseSemconv() - ? "BATCH INSERT " + tableName - : null), + emitStableDatabaseSemconv() ? scenario.summary : null), equalTo( - DB_OPERATION_BATCH_SIZE, - emitStableDatabaseSemconv() ? 2L : null)))); - } - - @ParameterizedTest - @MethodSource("batchStream") - void testMultiBatch(String system, Connection conn, String username, String url) - throws SQLException { - Connection connection = wrap(conn); - String tableName1 = "multi_batch_test_1"; - String tableName2 = "multi_batch_test_2"; - Statement createTable1 = connection.createStatement(); - createTable1.execute( - "CREATE TABLE " + tableName1 + " (id INTEGER not NULL, PRIMARY KEY ( id ))"); - cleanup.deferCleanup(createTable1); - Statement createTable2 = connection.createStatement(); - createTable2.execute( - "CREATE TABLE " + tableName2 + " (id INTEGER not NULL, PRIMARY KEY ( id ))"); - cleanup.deferCleanup(createTable2); - - testing().waitForTraces(2); - testing().clearData(); - - Statement statement = connection.createStatement(); - cleanup.deferCleanup(statement); - statement.addBatch("INSERT INTO " + tableName1 + " VALUES(1)"); - statement.addBatch("INSERT INTO " + tableName2 + " VALUES(2)"); - testing() - .runWithSpan( - "parent", () -> assertThat(statement.executeBatch()).isEqualTo(new int[] {1, 1})); - - testing() - .waitAndAssertTraces( - trace -> - trace.hasSpansSatisfyingExactly( - span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(), - span -> - span.hasName(emitStableDatabaseSemconv() ? "BATCH" : "jdbcunittest") - .hasKind(SpanKind.CLIENT) - .hasParent(trace.getSpan(0)) - .hasAttributesSatisfyingExactly( - equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)), - equalTo(maybeStable(DB_NAME), DATABASE_NAME_LOWER), - equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), - equalTo( - DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), - equalTo( - maybeStable(DB_STATEMENT), - emitStableDatabaseSemconv() - ? "INSERT INTO " - + tableName1 - + " VALUES(?); INSERT INTO multi_batch_test_2 VALUES(?)" - : null), + maybeStable(DB_OPERATION), + emitStableDatabaseSemconv() ? null : scenario.oldOperation), equalTo( - DB_QUERY_SUMMARY, emitStableDatabaseSemconv() ? "BATCH" : null), - equalTo(maybeStable(DB_OPERATION), null), + maybeStable(DB_SQL_TABLE), + emitStableDatabaseSemconv() ? null : scenario.oldTable), equalTo( DB_OPERATION_BATCH_SIZE, - emitStableDatabaseSemconv() ? 2L : null)))); + emitStableDatabaseSemconv() ? scenario.batchSize : null)))); + } + + static Stream batchStream() throws SQLException { + return Stream.of( + Arguments.of("h2", new org.h2.Driver().connect(JDBC_URLS.get("h2"), null), null, "h2:mem:"), + Arguments.of( + "derby", + new EmbeddedDriver().connect(JDBC_URLS.get("derby"), null), + "APP", + "derby:memory:"), + Arguments.of( + "hsqldb", new JDBCDriver().connect(JDBC_URLS.get("hsqldb"), null), "SA", "hsqldb:mem:"), + Arguments.of( + "sqlite", + new JDBC().connect(JDBC_URLS.get("sqlite"), new Properties()), + null, + "sqlite:memory:")); } @ParameterizedTest @MethodSource("batchStream") - void testSingleItemBatch(String system, Connection conn, String username, String url) + void testLargeBatch(String system, Connection connection, String username, String url) throws SQLException { - Connection connection = wrap(conn); - String tableName = "single_item_batch_test"; - Statement createTable = connection.createStatement(); - createTable.execute("CREATE TABLE " + tableName + " (id INTEGER not NULL, PRIMARY KEY ( id ))"); - cleanup.deferCleanup(createTable); - testing().waitForTraces(1); - testing().clearData(); - - Statement statement = connection.createStatement(); + Statement createTable = wrap(connection).createStatement(); + cleanup.deferCleanup(createTable); + createTable.execute( + "CREATE TABLE simple_batch_test_large (id INTEGER not NULL, PRIMARY KEY ( id ))"); + Statement statement = wrap(connection).createStatement(); cleanup.deferCleanup(statement); - statement.addBatch("INSERT INTO " + tableName + " VALUES(1)"); - testing() - .runWithSpan("parent", () -> assertThat(statement.executeBatch()).isEqualTo(new int[] {1})); + statement.addBatch("INSERT INTO simple_batch_test_large VALUES(1)"); + statement.addBatch("INSERT INTO simple_batch_test_large VALUES(2)"); - testing() - .waitAndAssertTraces( - trace -> - trace.hasSpansSatisfyingExactly( - span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(), - span -> - span.hasName( - emitStableDatabaseSemconv() - ? "INSERT " + tableName - : "INSERT jdbcunittest." + tableName) - .hasKind(SpanKind.CLIENT) - .hasParent(trace.getSpan(0)) - .hasAttributesSatisfyingExactly( - equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)), - equalTo(maybeStable(DB_NAME), DATABASE_NAME_LOWER), - equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), - equalTo( - DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), - equalTo( - maybeStable(DB_STATEMENT), - "INSERT INTO " + tableName + " VALUES(?)"), - equalTo( - DB_QUERY_SUMMARY, - emitStableDatabaseSemconv() ? "INSERT " + tableName : null), - equalTo( - maybeStable(DB_OPERATION), - emitStableDatabaseSemconv() ? null : "INSERT"), - equalTo( - maybeStable(DB_SQL_TABLE), - emitStableDatabaseSemconv() ? null : tableName)))); + if (testLatestDeps() || "sqlite".equals(system)) { + statement.executeLargeBatch(); + } else { + // Older drivers don't support JDBC 4.2, expect UnsupportedOperationException + // This is the correct behavior - instrumentation should not change driver behavior + assertThatThrownBy(statement::executeLargeBatch) + .isInstanceOf(UnsupportedOperationException.class); + } } @ParameterizedTest @@ -2120,9 +2051,7 @@ void testPreparedBatch(String system, Connection conn, String username, String u statement.addBatch(); statement.setInt(1, 2); statement.addBatch(); - testing() - .runWithSpan( - "parent", () -> assertThat(statement.executeBatch()).isEqualTo(new int[] {1, 1})); + testing().runWithSpan("parent", statement::executeBatch); testing() .waitAndAssertTraces( @@ -2446,4 +2375,93 @@ void testStatementWrapper() throws SQLException { .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(1)))); } + + private static final class BatchScenario { + final List queries; + final String spanName; + final String oldSpanName; + final String queryText; + final String oldStatement; + final String summary; + final String oldOperation; + final String oldTable; + final Long batchSize; + + BatchScenario(Builder builder) { + this.queries = builder.queries; + this.spanName = builder.spanName; + this.oldSpanName = builder.oldSpanName; + this.queryText = builder.queryText; + this.oldStatement = builder.oldStatement; + this.summary = builder.summary; + this.oldOperation = builder.oldOperation; + this.oldTable = builder.oldTable; + this.batchSize = builder.batchSize; + } + + static Builder builder() { + return new Builder(); + } + + static final class Builder { + private final List queries = new ArrayList<>(); + private String spanName; + private String oldSpanName; + private String queryText; + private String oldStatement; + private String summary; + private String oldOperation; + private String oldTable; + private Long batchSize; + + Builder addQuery(String query) { + queries.add(query); + return this; + } + + Builder spanName(String spanName) { + this.spanName = spanName; + return this; + } + + Builder oldSpanName(String oldSpanName) { + this.oldSpanName = oldSpanName; + return this; + } + + Builder queryText(String queryText) { + this.queryText = queryText; + return this; + } + + Builder oldStatement(String oldStatement) { + this.oldStatement = oldStatement; + return this; + } + + Builder summary(String summary) { + this.summary = summary; + return this; + } + + Builder oldOperation(String oldOperation) { + this.oldOperation = oldOperation; + return this; + } + + Builder oldTable(String oldTable) { + this.oldTable = oldTable; + return this; + } + + Builder batchSize(long batchSize) { + this.batchSize = batchSize; + return this; + } + + BatchScenario build() { + return new BatchScenario(this); + } + } + } } diff --git a/instrumentation/r2dbc-1.0/testing/src/main/java/io/opentelemetry/instrumentation/r2dbc/v1_0/AbstractR2dbcStatementTest.java b/instrumentation/r2dbc-1.0/testing/src/main/java/io/opentelemetry/instrumentation/r2dbc/v1_0/AbstractR2dbcStatementTest.java index 0732c04799f5..53a95d869368 100644 --- a/instrumentation/r2dbc-1.0/testing/src/main/java/io/opentelemetry/instrumentation/r2dbc/v1_0/AbstractR2dbcStatementTest.java +++ b/instrumentation/r2dbc-1.0/testing/src/main/java/io/opentelemetry/instrumentation/r2dbc/v1_0/AbstractR2dbcStatementTest.java @@ -14,8 +14,8 @@ import static io.opentelemetry.semconv.DbAttributes.DB_OPERATION_BATCH_SIZE; import static io.opentelemetry.semconv.DbAttributes.DB_OPERATION_NAME; import static io.opentelemetry.semconv.DbAttributes.DB_QUERY_SUMMARY; -import static io.opentelemetry.semconv.DbAttributes.DB_QUERY_TEXT; import static io.opentelemetry.semconv.DbAttributes.DB_SYSTEM_NAME; +import static io.opentelemetry.semconv.ErrorAttributes.ERROR_TYPE; import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS; import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_CONNECTION_STRING; @@ -32,19 +32,24 @@ import static io.r2dbc.spi.ConnectionFactoryOptions.PASSWORD; import static io.r2dbc.spi.ConnectionFactoryOptions.PORT; import static io.r2dbc.spi.ConnectionFactoryOptions.USER; -import static org.junit.jupiter.api.Assumptions.assumeTrue; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.catchThrowable; import static org.junit.jupiter.api.Named.named; import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.opentelemetry.api.trace.SpanKind; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import io.r2dbc.spi.Batch; import io.r2dbc.spi.ConnectionFactories; import io.r2dbc.spi.ConnectionFactory; import io.r2dbc.spi.ConnectionFactoryOptions; import java.time.Duration; +import java.util.ArrayList; import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; import java.util.stream.Stream; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Test; @@ -298,10 +303,10 @@ void testMetrics() { SERVER_PORT); } - @Test - void testBatchQueries() { - assumeTrue(emitStableDatabaseSemconv()); - + @SuppressWarnings("deprecation") // using deprecated semconv + @ParameterizedTest + @MethodSource("batchScenarios") + void batchQueries(BatchScenario scenario) { DbSystemProps props = systems.get(MARIADB.system); startContainer(props); ConnectionFactory connectionFactory = @@ -316,44 +321,178 @@ void testBatchQueries() { .option(CONNECT_TIMEOUT, Duration.ofSeconds(30)) .build()); - getTesting() - .runWithSpan( - "parent", - () -> { - Mono.from(connectionFactory.create()) - .flatMapMany( - connection -> - Flux.from( - connection - .createBatch() - .add("SELECT 1") - .add("SELECT 2") - .execute()) - .flatMap(result -> result.map((row, metadata) -> "")) - .concatWith(Mono.from(connection.close()).cast(String.class))) - .blockLast(Duration.ofMinutes(1)); - }); + // recreate a fresh batch_test table for each scenario so that batch row ids can be reused + // without worrying about collisions from previous scenarios; the table also lets the collection + // name be captured (in db.query.summary and, under old semconv, db.sql.table) + recreateBatchTestTable(connectionFactory); + getTesting().waitForTraces(2); + getTesting().clearData(); + + Throwable thrown = + catchThrowable( + () -> + getTesting() + .runWithSpan( + "parent", + () -> { + Mono.from(connectionFactory.create()) + .flatMapMany( + connection -> { + Batch batch = connection.createBatch(); + for (String query : scenario.queries) { + batch.add(query); + } + return Flux.from(batch.execute()) + .flatMap(result -> result.map((row, metadata) -> "")) + .concatWith( + Mono.from(connection.close()).cast(String.class)); + }) + .blockLast(Duration.ofMinutes(1)); + })); + + String connectionString = MARIADB.system + "://localhost:" + port; + + if (scenario.queries.isEmpty()) { + // an empty batch fails to execute but still produces a client span + assertThat(thrown).isInstanceOf(NoSuchElementException.class); + getTesting() + .waitAndAssertTraces( + trace -> + trace.hasSpansSatisfyingExactly( + span -> span.hasName("parent").hasKind(SpanKind.INTERNAL), + span -> + span.hasName(DB) + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(0)) + .hasAttributesSatisfyingExactly( + equalTo( + DB_CONNECTION_STRING, + emitStableDatabaseSemconv() ? null : connectionString), + equalTo(maybeStable(DB_SYSTEM), MARIADB.system), + equalTo(maybeStable(DB_NAME), DB), + equalTo(DB_USER, emitStableDatabaseSemconv() ? null : USER_DB), + equalTo( + maybeStable(DB_STATEMENT), + emitStableDatabaseSemconv() ? null : ""), + equalTo(DB_OPERATION_BATCH_SIZE, null), + equalTo(maybeStablePeerService(), "test-peer-service"), + equalTo(SERVER_ADDRESS, container.getHost()), + equalTo(SERVER_PORT, port), + equalTo( + ERROR_TYPE, + emitStableDatabaseSemconv() + ? "java.util.NoSuchElementException" + : null)))); + return; + } + assertThat(thrown).isNull(); getTesting() .waitAndAssertTraces( trace -> trace.hasSpansSatisfyingExactly( span -> span.hasName("parent").hasKind(SpanKind.INTERNAL), span -> - span.hasName("BATCH SELECT") + span.hasName( + emitStableDatabaseSemconv() + ? scenario.spanName + : scenario.oldSpanName) .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) .hasAttributesSatisfyingExactly( - equalTo(DB_SYSTEM_NAME, MARIADB.system), - equalTo(DB_NAMESPACE, DB), - equalTo(DB_QUERY_TEXT, "SELECT ?"), - equalTo(DB_QUERY_SUMMARY, "BATCH SELECT"), - equalTo(DB_OPERATION_BATCH_SIZE, 2), + equalTo( + DB_CONNECTION_STRING, + emitStableDatabaseSemconv() ? null : connectionString), + equalTo(maybeStable(DB_SYSTEM), MARIADB.system), + equalTo(maybeStable(DB_NAME), DB), + equalTo(DB_USER, emitStableDatabaseSemconv() ? null : USER_DB), + equalTo( + maybeStable(DB_STATEMENT), + emitStableDatabaseSemconv() + ? scenario.queryText + : scenario.oldStatement), + equalTo( + DB_QUERY_SUMMARY, + emitStableDatabaseSemconv() ? scenario.summary : null), + equalTo( + maybeStable(DB_OPERATION), + emitStableDatabaseSemconv() ? null : scenario.oldOperation), + equalTo( + maybeStable(DB_SQL_TABLE), + emitStableDatabaseSemconv() ? null : scenario.oldCollection), + equalTo( + DB_OPERATION_BATCH_SIZE, + emitStableDatabaseSemconv() ? scenario.batchSize : null), equalTo(maybeStablePeerService(), "test-peer-service"), equalTo(SERVER_ADDRESS, container.getHost()), equalTo(SERVER_PORT, port)))); } + private static Stream batchScenarios() { + return Stream.of( + Arguments.argumentSet("empty", BatchScenario.builder().build()), + Arguments.argumentSet( + "single", + BatchScenario.builder() + .addQuery("INSERT INTO batch_test (id, num) VALUES (1, 1)") + .spanName("INSERT batch_test") + .oldSpanName("INSERT " + DB + ".batch_test") + .summary("INSERT batch_test") + .queryText("INSERT INTO batch_test (id, num) VALUES (?, ?)") + .oldStatement("INSERT INTO batch_test (id, num) VALUES (?, ?)") + .oldOperation("INSERT") + .oldCollection("batch_test") + .build()), + Arguments.argumentSet( + "twoSameOperation", + BatchScenario.builder() + .addQuery("INSERT INTO batch_test (id, num) VALUES (1, 1)") + .addQuery("INSERT INTO batch_test (id, num) VALUES (2, 2)") + .spanName("BATCH INSERT batch_test") + .oldSpanName("INSERT " + DB + ".batch_test") + .summary("BATCH INSERT batch_test") + .queryText("INSERT INTO batch_test (id, num) VALUES (?, ?)") + .oldStatement( + "INSERT INTO batch_test (id, num) VALUES (?, ?); INSERT INTO batch_test (id, num) VALUES (?, ?)") + .oldOperation("INSERT") + .oldCollection("batch_test") + .batchSize(2) + .build()), + Arguments.argumentSet( + "twoDifferentOperations", + BatchScenario.builder() + .addQuery("INSERT INTO batch_test (id, num) VALUES (1, 1)") + .addQuery("UPDATE batch_test SET num = 5 WHERE id = 1") + .spanName("BATCH") + .oldSpanName("INSERT " + DB + ".batch_test") + .summary("BATCH") + .queryText( + "INSERT INTO batch_test (id, num) VALUES (?, ?); UPDATE batch_test SET num = ? WHERE id = ?") + .oldStatement( + "INSERT INTO batch_test (id, num) VALUES (?, ?); UPDATE batch_test SET num = ? WHERE id = ?") + .oldOperation("INSERT") + .oldCollection("batch_test") + .batchSize(2) + .build())); + } + + private void recreateBatchTestTable(ConnectionFactory connectionFactory) { + Mono.from(connectionFactory.create()) + .flatMapMany( + connection -> + Mono.from(connection.createStatement("DROP TABLE IF EXISTS batch_test").execute()) + .flatMapMany(result -> result.map((row, metadata) -> "")) + .concatWith( + Mono.from( + connection + .createStatement( + "CREATE TABLE batch_test (id INTEGER PRIMARY KEY, num INTEGER)") + .execute()) + .flatMapMany(result -> result.map((row, metadata) -> ""))) + .concatWith(Mono.from(connection.close()).cast(String.class))) + .blockLast(Duration.ofMinutes(1)); + } + private static class Parameter { private final String system; @@ -407,4 +546,93 @@ private DbSystemProps envVariables(String... keyValues) { return this; } } + + private static final class BatchScenario { + final List queries; + final String spanName; + final String oldSpanName; + final String summary; + final String queryText; + final String oldStatement; + final String oldOperation; + final String oldCollection; + final Long batchSize; + + BatchScenario(Builder builder) { + this.queries = builder.queries; + this.spanName = builder.spanName; + this.oldSpanName = builder.oldSpanName; + this.summary = builder.summary; + this.queryText = builder.queryText; + this.oldStatement = builder.oldStatement; + this.oldOperation = builder.oldOperation; + this.oldCollection = builder.oldCollection; + this.batchSize = builder.batchSize; + } + + static Builder builder() { + return new Builder(); + } + + static final class Builder { + private final List queries = new ArrayList<>(); + private String spanName; + private String oldSpanName; + private String summary; + private String queryText; + private String oldStatement; + private String oldOperation; + private String oldCollection; + private Long batchSize; + + Builder addQuery(String query) { + queries.add(query); + return this; + } + + Builder spanName(String spanName) { + this.spanName = spanName; + return this; + } + + Builder oldSpanName(String oldSpanName) { + this.oldSpanName = oldSpanName; + return this; + } + + Builder summary(String summary) { + this.summary = summary; + return this; + } + + Builder queryText(String queryText) { + this.queryText = queryText; + return this; + } + + Builder oldStatement(String oldStatement) { + this.oldStatement = oldStatement; + return this; + } + + Builder oldOperation(String oldOperation) { + this.oldOperation = oldOperation; + return this; + } + + Builder oldCollection(String oldCollection) { + this.oldCollection = oldCollection; + return this; + } + + Builder batchSize(long batchSize) { + this.batchSize = batchSize; + return this; + } + + BatchScenario build() { + return new BatchScenario(this); + } + } + } } diff --git a/instrumentation/vertx/vertx-sql-client/vertx-sql-client-4.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/vertx/sqlclient/v4_0/VertxSqlClientTest.java b/instrumentation/vertx/vertx-sql-client/vertx-sql-client-4.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/vertx/sqlclient/v4_0/VertxSqlClientTest.java index b80f6d7468f6..8499d3c4aa0e 100644 --- a/instrumentation/vertx/vertx-sql-client/vertx-sql-client-4.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/vertx/sqlclient/v4_0/VertxSqlClientTest.java +++ b/instrumentation/vertx/vertx-sql-client/vertx-sql-client-4.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/vertx/sqlclient/v4_0/VertxSqlClientTest.java @@ -29,6 +29,8 @@ import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_USER; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DbSystemNameIncubatingValues.POSTGRESQL; import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; @@ -50,12 +52,17 @@ import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.function.Consumer; +import java.util.stream.Stream; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testcontainers.containers.GenericContainer; @@ -280,17 +287,27 @@ private static void assertPreparedSelect() { equalTo(SERVER_PORT, port)))); } - @Test - void testBatch() throws Exception { - testing - .runWithSpan( - "parent", - () -> - pool.preparedQuery("insert into test values ($1, $2) returning *") - .executeBatch(asList(Tuple.of(3, "Three"), Tuple.of(4, "Four")))) - .toCompletionStage() - .toCompletableFuture() - .get(30, SECONDS); + @ParameterizedTest + @MethodSource("batchScenarios") + void testBatch(BatchScenario scenario) throws Exception { + // recreate a fresh batch_test table for each scenario so that batch row ids can be reused + // without worrying about collisions from previous scenarios + recreateBatchTestTable(); + testing.waitForTraces(2); + testing.clearData(); + + // an empty batch is rejected before sending, so its execution fails; non-empty batches succeed + try { + testing + .runWithSpan( + "parent", + () -> pool.preparedQuery(scenario.preparedQuery).executeBatch(scenario.tuples)) + .toCompletionStage() + .toCompletableFuture() + .get(30, SECONDS); + } catch (ExecutionException ignored) { + // an empty batch fails to execute; the failure is recorded on the client span + } testing.waitAndAssertTraces( trace -> @@ -299,8 +316,8 @@ void testBatch() throws Exception { span -> span.hasName( emitStableDatabaseSemconv() - ? "BATCH insert test" - : "INSERT tempdb.test") + ? scenario.stableSpanName + : "INSERT tempdb.batch_test") .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) .hasAttributesSatisfyingExactly( @@ -309,25 +326,66 @@ void testBatch() throws Exception { emitStableDatabaseSemconv() ? POSTGRESQL : null), equalTo(maybeStable(DB_NAME), DB), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : USER_DB), - equalTo( - maybeStable(DB_STATEMENT), - "insert into test values ($1, $2) returning *"), + equalTo(maybeStable(DB_STATEMENT), scenario.preparedQuery), equalTo( DB_QUERY_SUMMARY, - emitStableDatabaseSemconv() ? "BATCH insert test" : null), + emitStableDatabaseSemconv() ? scenario.querySummary : null), equalTo( - DB_OPERATION_BATCH_SIZE, emitStableDatabaseSemconv() ? 2L : null), + DB_OPERATION_BATCH_SIZE, + emitStableDatabaseSemconv() ? scenario.batchSize : null), equalTo( maybeStable(DB_OPERATION), emitStableDatabaseSemconv() ? null : "INSERT"), equalTo( maybeStable(DB_SQL_TABLE), - emitStableDatabaseSemconv() ? null : "test"), + emitStableDatabaseSemconv() ? null : "batch_test"), + equalTo( + ERROR_TYPE, + emitStableDatabaseSemconv() ? scenario.errorType : null), equalTo(maybeStablePeerService(), "test-peer-service"), equalTo(SERVER_ADDRESS, host), equalTo(SERVER_PORT, port)))); } + private static void recreateBatchTestTable() throws Exception { + pool.query("drop table if exists batch_test") + .execute() + .compose(r -> pool.query("create table batch_test(id int primary key, num int)").execute()) + .toCompletionStage() + .toCompletableFuture() + .get(30, SECONDS); + } + + private static Stream batchScenarios() { + return Stream.of( + Arguments.argumentSet( + "empty", + BatchScenario.builder() + .preparedQuery("insert into batch_test values ($1, $2) returning *") + .tuples(emptyList()) + .stableSpanName("insert batch_test") + .querySummary("insert batch_test") + .errorType("io.vertx.core.impl.NoStackTraceThrowable") + .build()), + Arguments.argumentSet( + "single", + BatchScenario.builder() + .preparedQuery("insert into batch_test values ($1, $2) returning *") + .tuples(singletonList(Tuple.of(1, 1))) + .stableSpanName("insert batch_test") + .querySummary("insert batch_test") + .build()), + Arguments.argumentSet( + "twoSameOperation", + BatchScenario.builder() + .preparedQuery("insert into batch_test values ($1, $2) returning *") + .tuples(asList(Tuple.of(1, 1), Tuple.of(2, 2))) + .stableSpanName("BATCH insert batch_test") + .querySummary("BATCH insert batch_test") + .batchSize(2) + .build())); + } + @Test void testWithTransaction() throws Exception { testing @@ -508,4 +566,69 @@ void testConcurrency() throws Exception { .hasParent(trace.getSpan(0)))); testing.waitAndAssertTraces(assertions); } + + private static final class BatchScenario { + final String preparedQuery; + final List tuples; + final String stableSpanName; + final String querySummary; + final Long batchSize; + final String errorType; + + BatchScenario(Builder builder) { + this.preparedQuery = builder.preparedQuery; + this.tuples = builder.tuples; + this.stableSpanName = builder.stableSpanName; + this.querySummary = builder.querySummary; + this.batchSize = builder.batchSize; + this.errorType = builder.errorType; + } + + static Builder builder() { + return new Builder(); + } + + static final class Builder { + private String preparedQuery; + private List tuples; + private String stableSpanName; + private String querySummary; + private Long batchSize; + private String errorType; + + Builder preparedQuery(String preparedQuery) { + this.preparedQuery = preparedQuery; + return this; + } + + Builder tuples(List tuples) { + this.tuples = tuples; + return this; + } + + Builder stableSpanName(String stableSpanName) { + this.stableSpanName = stableSpanName; + return this; + } + + Builder querySummary(String querySummary) { + this.querySummary = querySummary; + return this; + } + + Builder batchSize(long batchSize) { + this.batchSize = batchSize; + return this; + } + + Builder errorType(String errorType) { + this.errorType = errorType; + return this; + } + + BatchScenario build() { + return new BatchScenario(this); + } + } + } } diff --git a/instrumentation/vertx/vertx-sql-client/vertx-sql-client-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/vertx/sqlclient/v5_0/VertxSqlClientTest.java b/instrumentation/vertx/vertx-sql-client/vertx-sql-client-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/vertx/sqlclient/v5_0/VertxSqlClientTest.java index d9b4bd36a82a..a06686379599 100644 --- a/instrumentation/vertx/vertx-sql-client/vertx-sql-client-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/vertx/sqlclient/v5_0/VertxSqlClientTest.java +++ b/instrumentation/vertx/vertx-sql-client/vertx-sql-client-5.0/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/vertx/sqlclient/v5_0/VertxSqlClientTest.java @@ -29,6 +29,8 @@ import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_USER; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DbSystemNameIncubatingValues.POSTGRESQL; import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; @@ -50,12 +52,17 @@ import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.function.Consumer; +import java.util.stream.Stream; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testcontainers.containers.GenericContainer; @@ -281,17 +288,27 @@ private static void assertPreparedSelect() { equalTo(SERVER_PORT, port)))); } - @Test - void testBatch() throws Exception { - testing - .runWithSpan( - "parent", - () -> - pool.preparedQuery("insert into test values ($1, $2) returning *") - .executeBatch(asList(Tuple.of(3, "Three"), Tuple.of(4, "Four")))) - .toCompletionStage() - .toCompletableFuture() - .get(30, SECONDS); + @ParameterizedTest + @MethodSource("batchScenarios") + void testBatch(BatchScenario scenario) throws Exception { + // recreate a fresh batch_test table for each scenario so that batch row ids can be reused + // without worrying about collisions from previous scenarios + recreateBatchTestTable(); + testing.waitForTraces(2); + testing.clearData(); + + // an empty batch is rejected before sending, so its execution fails; non-empty batches succeed + try { + testing + .runWithSpan( + "parent", + () -> pool.preparedQuery(scenario.preparedQuery).executeBatch(scenario.tuples)) + .toCompletionStage() + .toCompletableFuture() + .get(30, SECONDS); + } catch (ExecutionException ignored) { + // an empty batch fails to execute; the failure is recorded on the client span + } testing.waitAndAssertTraces( trace -> @@ -300,8 +317,8 @@ void testBatch() throws Exception { span -> span.hasName( emitStableDatabaseSemconv() - ? "BATCH insert test" - : "INSERT tempdb.test") + ? scenario.stableSpanName + : "INSERT tempdb.batch_test") .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) .hasAttributesSatisfyingExactly( @@ -310,25 +327,66 @@ void testBatch() throws Exception { emitStableDatabaseSemconv() ? POSTGRESQL : null), equalTo(maybeStable(DB_NAME), DB), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : USER_DB), - equalTo( - maybeStable(DB_STATEMENT), - "insert into test values ($1, $2) returning *"), + equalTo(maybeStable(DB_STATEMENT), scenario.preparedQuery), equalTo( DB_QUERY_SUMMARY, - emitStableDatabaseSemconv() ? "BATCH insert test" : null), + emitStableDatabaseSemconv() ? scenario.querySummary : null), equalTo( - DB_OPERATION_BATCH_SIZE, emitStableDatabaseSemconv() ? 2L : null), + DB_OPERATION_BATCH_SIZE, + emitStableDatabaseSemconv() ? scenario.batchSize : null), equalTo( maybeStable(DB_OPERATION), emitStableDatabaseSemconv() ? null : "INSERT"), equalTo( maybeStable(DB_SQL_TABLE), - emitStableDatabaseSemconv() ? null : "test"), + emitStableDatabaseSemconv() ? null : "batch_test"), + equalTo( + ERROR_TYPE, + emitStableDatabaseSemconv() ? scenario.errorType : null), equalTo(maybeStablePeerService(), "test-peer-service"), equalTo(SERVER_ADDRESS, host), equalTo(SERVER_PORT, port)))); } + private static void recreateBatchTestTable() throws Exception { + pool.query("drop table if exists batch_test") + .execute() + .compose(r -> pool.query("create table batch_test(id int primary key, num int)").execute()) + .toCompletionStage() + .toCompletableFuture() + .get(30, SECONDS); + } + + private static Stream batchScenarios() { + return Stream.of( + Arguments.argumentSet( + "empty", + BatchScenario.builder() + .preparedQuery("insert into batch_test values ($1, $2) returning *") + .tuples(emptyList()) + .stableSpanName("insert batch_test") + .querySummary("insert batch_test") + .errorType("io.vertx.core.VertxException") + .build()), + Arguments.argumentSet( + "single", + BatchScenario.builder() + .preparedQuery("insert into batch_test values ($1, $2) returning *") + .tuples(singletonList(Tuple.of(1, 1))) + .stableSpanName("insert batch_test") + .querySummary("insert batch_test") + .build()), + Arguments.argumentSet( + "twoSameOperation", + BatchScenario.builder() + .preparedQuery("insert into batch_test values ($1, $2) returning *") + .tuples(asList(Tuple.of(1, 1), Tuple.of(2, 2))) + .stableSpanName("BATCH insert batch_test") + .querySummary("BATCH insert batch_test") + .batchSize(2) + .build())); + } + @Test void testWithTransaction() throws Exception { testing @@ -510,4 +568,69 @@ void testConcurrency() throws Exception { .hasParent(trace.getSpan(0)))); testing.waitAndAssertTraces(assertions); } + + private static final class BatchScenario { + final String preparedQuery; + final List tuples; + final String stableSpanName; + final String querySummary; + final Long batchSize; + final String errorType; + + BatchScenario(Builder builder) { + this.preparedQuery = builder.preparedQuery; + this.tuples = builder.tuples; + this.stableSpanName = builder.stableSpanName; + this.querySummary = builder.querySummary; + this.batchSize = builder.batchSize; + this.errorType = builder.errorType; + } + + static Builder builder() { + return new Builder(); + } + + static final class Builder { + private String preparedQuery; + private List tuples; + private String stableSpanName; + private String querySummary; + private Long batchSize; + private String errorType; + + Builder preparedQuery(String preparedQuery) { + this.preparedQuery = preparedQuery; + return this; + } + + Builder tuples(List tuples) { + this.tuples = tuples; + return this; + } + + Builder stableSpanName(String stableSpanName) { + this.stableSpanName = stableSpanName; + return this; + } + + Builder querySummary(String querySummary) { + this.querySummary = querySummary; + return this; + } + + Builder batchSize(long batchSize) { + this.batchSize = batchSize; + return this; + } + + Builder errorType(String errorType) { + this.errorType = errorType; + return this; + } + + BatchScenario build() { + return new BatchScenario(this); + } + } + } }