From be242641dfd01849f473dc79e1e22b12f59caf3d Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 26 Nov 2024 14:29:26 +0200 Subject: [PATCH 1/4] Instrument jdbc batch queries --- .../semconv/db/DbClientSpanNameExtractor.java | 37 ++- .../api/incubator/semconv/db/MultiQuery.java | 75 ++++++ .../db/SqlClientAttributesExtractor.java | 87 +++++-- .../semconv/db/SqlClientAttributesGetter.java | 5 + .../MultiQuerySqlClientAttributesGetter.java | 27 ++ .../db/DbClientSpanNameExtractorTest.java | 53 ++++ .../db/SqlClientAttributesExtractorTest.java | 235 ++++++++++++++--- .../PreparedStatementInstrumentation.java | 19 +- .../jdbc/StatementInstrumentation.java | 99 ++++++++ .../jdbc/test/JdbcInstrumentationTest.java | 237 +++++++++++++++++- .../jdbc/internal/DbRequest.java | 34 ++- .../jdbc/internal/JdbcAttributesGetter.java | 17 +- .../jdbc/internal/JdbcData.java | 77 ++++++ .../OpenTelemetryPreparedStatement.java | 1 + .../jdbc/internal/OpenTelemetryStatement.java | 35 +-- .../jdbc/datasource/JdbcTelemetryTest.java | 45 ++++ 16 files changed, 998 insertions(+), 85 deletions(-) create mode 100644 instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/MultiQuery.java create mode 100644 instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/MultiQuerySqlClientAttributesGetter.java diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientSpanNameExtractor.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientSpanNameExtractor.java index 58eb91b08b09..b14d4a08908e 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientSpanNameExtractor.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientSpanNameExtractor.java @@ -5,7 +5,9 @@ package io.opentelemetry.instrumentation.api.incubator.semconv.db; +import io.opentelemetry.instrumentation.api.incubator.semconv.db.internal.MultiQuerySqlClientAttributesGetter; import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor; +import io.opentelemetry.instrumentation.api.internal.SemconvStability; public abstract class DbClientSpanNameExtractor implements SpanNameExtractor { @@ -93,10 +95,39 @@ private SqlClientSpanNameExtractor(SqlClientAttributesGetter getter) { @Override public String extract(REQUEST request) { + boolean isMultiQuery = false; + MultiQuerySqlClientAttributesGetter multiGetter = null; + if (getter instanceof MultiQuerySqlClientAttributesGetter) { + multiGetter = (MultiQuerySqlClientAttributesGetter) getter; + isMultiQuery = multiGetter.getRawQueryTexts(request).size() > 1; + } + + Long batchSize = getter.getBatchSize(request); + boolean isBatch = batchSize != null && batchSize > 1; + String namespace = getter.getDbNamespace(request); - SqlStatementInfo sanitizedStatement = sanitizer.sanitize(getter.getRawQueryText(request)); - return computeSpanName( - namespace, sanitizedStatement.getOperation(), sanitizedStatement.getMainIdentifier()); + if (!isBatch || (!SemconvStability.emitStableDatabaseSemconv() && !isMultiQuery)) { + SqlStatementInfo sanitizedStatement = sanitizer.sanitize(getter.getRawQueryText(request)); + return computeSpanName( + namespace, sanitizedStatement.getOperation(), sanitizedStatement.getMainIdentifier()); + } else if (SemconvStability.emitStableDatabaseSemconv()) { + if (!isMultiQuery) { // batch query with single unique query + SqlStatementInfo sanitizedStatement = sanitizer.sanitize(getter.getRawQueryText(request)); + return computeSpanName( + namespace, + "BATCH " + sanitizedStatement.getOperation(), + sanitizedStatement.getMainIdentifier()); + } else { // batch query with multiple unique queries + MultiQuery multiQuery = MultiQuery.analyze(multiGetter.getRawQueryTexts(request), false); + + return computeSpanName( + namespace, + multiQuery.getOperation() != null ? "BATCH " + multiQuery.getOperation() : "BATCH", + multiQuery.getMainIdentifier()); + } + } else { + return computeSpanName(namespace, null, null); + } } } } diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/MultiQuery.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/MultiQuery.java new file mode 100644 index 000000000000..1dc939ebcb0d --- /dev/null +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/MultiQuery.java @@ -0,0 +1,75 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.incubator.semconv.db; + +import java.util.Collection; +import java.util.LinkedHashSet; +import java.util.Set; + +class MultiQuery { + private static final SqlStatementSanitizer sanitizer = SqlStatementSanitizer.create(true); + + private final String mainIdentifier; + private final String operation; + private final Set statements; + + private MultiQuery(String mainIdentifier, String operation, Set statements) { + this.mainIdentifier = mainIdentifier; + this.operation = operation; + this.statements = statements; + } + + static MultiQuery analyze( + Collection rawQueryTexts, boolean statementSanitizationEnabled) { + UniqueValue uniqueMainIdentifier = new UniqueValue(); + UniqueValue uniqueOperation = new UniqueValue(); + Set uniqueStatements = new LinkedHashSet<>(); + for (String rawQueryText : rawQueryTexts) { + SqlStatementInfo sanitizedStatement = sanitizer.sanitize(rawQueryText); + String mainIdentifier = sanitizedStatement.getMainIdentifier(); + uniqueMainIdentifier.set(mainIdentifier); + String operation = sanitizedStatement.getOperation(); + uniqueOperation.set(operation); + uniqueStatements.add( + statementSanitizationEnabled ? sanitizedStatement.getFullStatement() : rawQueryText); + } + + return new MultiQuery( + uniqueMainIdentifier.getValue(), uniqueOperation.getValue(), uniqueStatements); + } + + public String getMainIdentifier() { + return mainIdentifier; + } + + public String getOperation() { + return operation; + } + + public Set getStatements() { + return statements; + } + + private static class UniqueValue { + private String value; + private boolean valid = true; + + void set(String value) { + if (!valid) { + return; + } + if (this.value == null) { + this.value = value; + } else if (!this.value.equals(value)) { + valid = false; + } + } + + String getValue() { + return valid ? value : null; + } + } +} diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractor.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractor.java index fc5f29efe81b..8d4eae487889 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractor.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractor.java @@ -10,8 +10,10 @@ import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.incubator.semconv.db.internal.MultiQuerySqlClientAttributesGetter; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import io.opentelemetry.instrumentation.api.internal.SemconvStability; +import java.util.Collection; /** * Extractor of private static final AttributeKey DB_QUERY_TEXT = AttributeKey.stringKey("db.query.text"); private static final AttributeKey DB_COLLECTION_NAME = AttributeKey.stringKey("db.collection.name"); + private static final AttributeKey DB_OPERATION_BATCH_SIZE = + AttributeKey.longKey("db.operation.batch.size"); /** Creates the SQL client attributes extractor with default configuration. */ public static AttributesExtractor create( @@ -52,7 +56,7 @@ public static SqlClientAttributesExtractorBuilder oldSemconvTableAttribute; @@ -71,30 +75,73 @@ public static SqlClientAttributesExtractorBuilder multiGetter = null; + if (getter instanceof MultiQuerySqlClientAttributesGetter) { + multiGetter = (MultiQuerySqlClientAttributesGetter) getter; + isMultiQuery = multiGetter.getRawQueryTexts(request).size() > 1; } - if (!SQL_CALL.equals(operation)) { + + Long batchSize = getter.getBatchSize(request); + boolean isBatch = batchSize != null && batchSize > 1; + + if (!isMultiQuery) { + String rawQueryText = getter.getRawQueryText(request); + SqlStatementInfo sanitizedStatement = sanitizer.sanitize(rawQueryText); + String operation = sanitizedStatement.getOperation(); if (SemconvStability.emitStableDatabaseSemconv()) { - internalSet(attributes, DB_COLLECTION_NAME, sanitizedStatement.getMainIdentifier()); + internalSet( + attributes, + DB_QUERY_TEXT, + statementSanitizationEnabled ? sanitizedStatement.getFullStatement() : rawQueryText); + if (operation != null) { + internalSet(attributes, DB_OPERATION_NAME, (isBatch ? "BATCH " : "") + operation); + } } if (SemconvStability.emitOldDatabaseSemconv()) { - internalSet(attributes, oldSemconvTableAttribute, sanitizedStatement.getMainIdentifier()); + internalSet( + attributes, + DB_STATEMENT, + statementSanitizationEnabled ? sanitizedStatement.getFullStatement() : rawQueryText); + internalSet(attributes, DB_OPERATION, operation); + } + if (!SQL_CALL.equals(operation)) { + if (SemconvStability.emitStableDatabaseSemconv()) { + internalSet(attributes, DB_COLLECTION_NAME, sanitizedStatement.getMainIdentifier()); + } + if (SemconvStability.emitOldDatabaseSemconv()) { + internalSet(attributes, oldSemconvTableAttribute, sanitizedStatement.getMainIdentifier()); + } + } + if (SemconvStability.emitStableDatabaseSemconv() && isBatch) { + internalSet(attributes, DB_OPERATION_BATCH_SIZE, batchSize); + } + } else if (SemconvStability.emitStableDatabaseSemconv()) { + MultiQuery multiQuery = + MultiQuery.analyze(multiGetter.getRawQueryTexts(request), statementSanitizationEnabled); + + internalSet(attributes, DB_QUERY_TEXT, join(";", multiQuery.getStatements())); + String operation = + multiQuery.getOperation() != null ? "BATCH " + multiQuery.getOperation() : "BATCH"; + internalSet(attributes, DB_OPERATION_NAME, operation); + + if (multiQuery.getMainIdentifier() != null + && (multiQuery.getOperation() == null || !SQL_CALL.equals(multiQuery.getOperation()))) { + internalSet(attributes, DB_COLLECTION_NAME, multiQuery.getMainIdentifier()); + } + internalSet(attributes, DB_OPERATION_BATCH_SIZE, batchSize); + } + } + + // String.join is not available on android + private static String join(String delimiter, Collection collection) { + StringBuilder builder = new StringBuilder(); + for (String string : collection) { + if (builder.length() != 0) { + builder.append(delimiter); } + builder.append(string); } + return builder.toString(); } } diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesGetter.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesGetter.java index d0817d63e460..fc33375b3745 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesGetter.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesGetter.java @@ -38,4 +38,9 @@ default String getRawStatement(REQUEST request) { default String getRawQueryText(REQUEST request) { return getRawStatement(request); } + + // TODO: make this required to implement + default Long getBatchSize(REQUEST request) { + return null; + } } diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/MultiQuerySqlClientAttributesGetter.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/MultiQuerySqlClientAttributesGetter.java new file mode 100644 index 000000000000..64e00b39e4fd --- /dev/null +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/MultiQuerySqlClientAttributesGetter.java @@ -0,0 +1,27 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.api.incubator.semconv.db.internal; + +import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlClientAttributesExtractor; +import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlClientAttributesGetter; +import java.util.Collection; + +/** + * An extended version of {@link SqlClientAttributesGetter} for getting SQL database client + * attributes for operations that run multiple distinct queries. + * + *

This class is internal and is hence not for public use. Its APIs are unstable and can change + * at any time. + */ +public interface MultiQuerySqlClientAttributesGetter + extends SqlClientAttributesGetter { + + /** + * Get the raw SQL statements. The value returned by this method is later sanitized by the {@link + * SqlClientAttributesExtractor} before being set as span attribute. + */ + Collection getRawQueryTexts(REQUEST request); +} diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientSpanNameExtractorTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientSpanNameExtractorTest.java index 1e05cef5a37e..0f369bceb0e6 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientSpanNameExtractorTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientSpanNameExtractorTest.java @@ -8,7 +8,11 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.when; +import io.opentelemetry.instrumentation.api.incubator.semconv.db.internal.MultiQuerySqlClientAttributesGetter; import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor; +import io.opentelemetry.instrumentation.api.internal.SemconvStability; +import java.util.Arrays; +import java.util.Collections; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -18,6 +22,7 @@ class DbClientSpanNameExtractorTest { @Mock DbClientAttributesGetter dbAttributesGetter; @Mock SqlClientAttributesGetter sqlAttributesGetter; + @Mock MultiQuerySqlClientAttributesGetter multiQuerySqlClientAttributesGetter; @Test void shouldExtractFullSpanName() { @@ -132,5 +137,53 @@ void shouldFallBackToDefaultSpanName() { assertEquals("DB Query", spanName); } + @Test + void shouldExtractFullSpanNameForBatch() { + // given + DbRequest dbRequest = new DbRequest(); + + when(multiQuerySqlClientAttributesGetter.getRawQueryTexts(dbRequest)) + .thenReturn(Arrays.asList("INSERT INTO table VALUES(1)", "INSERT INTO table VALUES(2)")); + when(multiQuerySqlClientAttributesGetter.getDbNamespace(dbRequest)).thenReturn("database"); + when(multiQuerySqlClientAttributesGetter.getBatchSize(dbRequest)).thenReturn(2L); + + SpanNameExtractor underTest = + DbClientSpanNameExtractor.create(multiQuerySqlClientAttributesGetter); + + // when + String spanName = underTest.extract(dbRequest); + + // then + assertEquals( + SemconvStability.emitStableDatabaseSemconv() ? "BATCH INSERT database.table" : "database", + spanName); + } + + @Test + void shouldExtractFullSpanNameForSingleQueryBatch() { + // given + DbRequest dbRequest = new DbRequest(); + + when(multiQuerySqlClientAttributesGetter.getRawQueryTexts(dbRequest)) + .thenReturn(Collections.singletonList("INSERT INTO table VALUES(?)")); + if (SemconvStability.emitStableDatabaseSemconv()) { + when(multiQuerySqlClientAttributesGetter.getRawQueryText(dbRequest)) + .thenReturn("INSERT INTO table VALUES(?)"); + } + when(multiQuerySqlClientAttributesGetter.getDbNamespace(dbRequest)).thenReturn("database"); + when(multiQuerySqlClientAttributesGetter.getBatchSize(dbRequest)).thenReturn(2L); + + SpanNameExtractor underTest = + DbClientSpanNameExtractor.create(multiQuerySqlClientAttributesGetter); + + // when + String spanName = underTest.extract(dbRequest); + + // then + assertEquals( + SemconvStability.emitStableDatabaseSemconv() ? "BATCH INSERT database.table" : "database", + spanName); + } + static class DbRequest {} } diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractorTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractorTest.java index f0b31197a3e2..c72bf1ff5e93 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractorTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractorTest.java @@ -12,9 +12,12 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.incubator.semconv.db.internal.MultiQuerySqlClientAttributesGetter; import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import io.opentelemetry.instrumentation.api.internal.SemconvStability; import io.opentelemetry.semconv.incubating.DbIncubatingAttributes; +import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -23,34 +26,56 @@ @SuppressWarnings("deprecation") // using deprecated semconv class SqlClientAttributesExtractorTest { - static final class TestAttributesGetter - implements SqlClientAttributesGetter> { + static class TestAttributesGetter implements SqlClientAttributesGetter> { @Override - public String getRawQueryText(Map map) { - return map.get("db.statement"); + public String getRawQueryText(Map map) { + return read(map, "db.statement"); } @Override - public String getDbSystem(Map map) { - return map.get("db.system"); + public String getDbSystem(Map map) { + return read(map, "db.system"); } @Deprecated @Override - public String getUser(Map map) { - return map.get("db.user"); + public String getUser(Map map) { + return read(map, "db.user"); } @Override - public String getDbNamespace(Map map) { - return map.get("db.name"); + public String getDbNamespace(Map map) { + return read(map, "db.name"); } @Deprecated @Override - public String getConnectionString(Map map) { - return map.get("db.connection_string"); + public String getConnectionString(Map map) { + return read(map, "db.connection_string"); + } + + @Override + public Long getBatchSize(Map map) { + return read(map, "db.operation.batch.size", Long.class); + } + + protected String read(Map map, String key) { + return read(map, key, String.class); + } + + protected T read(Map map, String key, Class clazz) { + return clazz.cast(map.get(key)); + } + } + + static class TestMultiAttributesGetter extends TestAttributesGetter + implements MultiQuerySqlClientAttributesGetter> { + + @SuppressWarnings("unchecked") + @Override + public Collection getRawQueryTexts(Map map) { + return (Collection) map.get("db.statements"); } } @@ -58,7 +83,7 @@ public String getConnectionString(Map map) { @Test void shouldExtractAllAttributes() { // given - Map request = new HashMap<>(); + Map request = new HashMap<>(); request.put("db.system", "myDb"); request.put("db.user", "username"); request.put("db.name", "potatoes"); @@ -67,7 +92,7 @@ void shouldExtractAllAttributes() { Context context = Context.root(); - AttributesExtractor, Void> underTest = + AttributesExtractor, Void> underTest = SqlClientAttributesExtractor.create(new TestAttributesGetter()); // when @@ -88,10 +113,10 @@ void shouldExtractAllAttributes() { entry(DbIncubatingAttributes.DB_STATEMENT, "SELECT * FROM potato WHERE id=?"), entry(DbIncubatingAttributes.DB_OPERATION, "SELECT"), entry(DbIncubatingAttributes.DB_SQL_TABLE, "potato"), - entry(stringKey("db.namespace"), "potatoes"), - entry(stringKey("db.query.text"), "SELECT * FROM potato WHERE id=?"), - entry(stringKey("db.operation.name"), "SELECT"), - entry(stringKey("db.collection.name"), "potato")); + entry(DbIncubatingAttributes.DB_NAMESPACE, "potatoes"), + entry(DbIncubatingAttributes.DB_QUERY_TEXT, "SELECT * FROM potato WHERE id=?"), + entry(DbIncubatingAttributes.DB_OPERATION_NAME, "SELECT"), + entry(DbIncubatingAttributes.DB_COLLECTION_NAME, "potato")); } else if (SemconvStability.emitOldDatabaseSemconv()) { assertThat(startAttributes.build()) .containsOnly( @@ -106,10 +131,10 @@ void shouldExtractAllAttributes() { assertThat(startAttributes.build()) .containsOnly( entry(DbIncubatingAttributes.DB_SYSTEM, "myDb"), - entry(stringKey("db.namespace"), "potatoes"), - entry(stringKey("db.query.text"), "SELECT * FROM potato WHERE id=?"), - entry(stringKey("db.operation.name"), "SELECT"), - entry(stringKey("db.collection.name"), "potato")); + entry(DbIncubatingAttributes.DB_NAMESPACE, "potatoes"), + entry(DbIncubatingAttributes.DB_QUERY_TEXT, "SELECT * FROM potato WHERE id=?"), + entry(DbIncubatingAttributes.DB_OPERATION_NAME, "SELECT"), + entry(DbIncubatingAttributes.DB_COLLECTION_NAME, "potato")); } assertThat(endAttributes.build().isEmpty()).isTrue(); @@ -118,12 +143,12 @@ void shouldExtractAllAttributes() { @Test void shouldNotExtractTableIfAttributeIsNotSet() { // given - Map request = new HashMap<>(); + Map request = new HashMap<>(); request.put("db.statement", "SELECT *"); Context context = Context.root(); - AttributesExtractor, Void> underTest = + AttributesExtractor, Void> underTest = SqlClientAttributesExtractor.create(new TestAttributesGetter()); // when @@ -155,13 +180,13 @@ void shouldNotExtractTableIfAttributeIsNotSet() { @SuppressWarnings("deprecation") // to support old database semantic conventions void shouldExtractTableToSpecifiedKey() { // given - Map request = new HashMap<>(); + Map request = new HashMap<>(); request.put("db.statement", "SELECT * FROM table"); Context context = Context.root(); - AttributesExtractor, Void> underTest = - SqlClientAttributesExtractor., Void>builder(new TestAttributesGetter()) + AttributesExtractor, Void> underTest = + SqlClientAttributesExtractor., Void>builder(new TestAttributesGetter()) .setTableAttribute(DbIncubatingAttributes.DB_CASSANDRA_TABLE) .build(); @@ -197,7 +222,7 @@ void shouldExtractTableToSpecifiedKey() { @Test void shouldExtractNoAttributesIfNoneAreAvailable() { // when - AttributesExtractor, Void> underTest = + AttributesExtractor, Void> underTest = SqlClientAttributesExtractor.create(new TestAttributesGetter()); // when @@ -207,4 +232,158 @@ void shouldExtractNoAttributesIfNoneAreAvailable() { // then assertThat(attributes.build().isEmpty()).isTrue(); } + + @Test + void shouldExtractSingleQueryBatchAttributes() { + // given + Map request = new HashMap<>(); + request.put("db.name", "potatoes"); + request.put("db.statement", "INSERT INTO potato VALUES(?)"); + request.put("db.statements", Collections.emptyList()); + request.put("db.operation.batch.size", 2L); + + Context context = Context.root(); + + AttributesExtractor, Void> underTest = + SqlClientAttributesExtractor.create(new TestMultiAttributesGetter()); + + // when + AttributesBuilder startAttributes = Attributes.builder(); + underTest.onStart(startAttributes, context, request); + + AttributesBuilder endAttributes = Attributes.builder(); + underTest.onEnd(endAttributes, context, request, null, null); + + // then + if (SemconvStability.emitStableDatabaseSemconv() && SemconvStability.emitOldDatabaseSemconv()) { + assertThat(startAttributes.build()) + .containsOnly( + entry(DbIncubatingAttributes.DB_NAME, "potatoes"), + entry(DbIncubatingAttributes.DB_STATEMENT, "INSERT INTO potato VALUES(?)"), + entry(DbIncubatingAttributes.DB_OPERATION, "INSERT"), + entry(DbIncubatingAttributes.DB_SQL_TABLE, "potato"), + entry(DbIncubatingAttributes.DB_NAMESPACE, "potatoes"), + entry(DbIncubatingAttributes.DB_QUERY_TEXT, "INSERT INTO potato VALUES(?)"), + entry(DbIncubatingAttributes.DB_OPERATION_NAME, "BATCH INSERT"), + entry(DbIncubatingAttributes.DB_COLLECTION_NAME, "potato"), + entry(DbIncubatingAttributes.DB_OPERATION_BATCH_SIZE, 2L)); + } else if (SemconvStability.emitOldDatabaseSemconv()) { + assertThat(startAttributes.build()) + .containsOnly( + entry(DbIncubatingAttributes.DB_NAME, "potatoes"), + entry(DbIncubatingAttributes.DB_STATEMENT, "INSERT INTO potato VALUES(?)"), + entry(DbIncubatingAttributes.DB_OPERATION, "INSERT"), + entry(DbIncubatingAttributes.DB_SQL_TABLE, "potato")); + } else if (SemconvStability.emitStableDatabaseSemconv()) { + assertThat(startAttributes.build()) + .containsOnly( + entry(DbIncubatingAttributes.DB_NAMESPACE, "potatoes"), + entry(DbIncubatingAttributes.DB_QUERY_TEXT, "INSERT INTO potato VALUES(?)"), + entry(DbIncubatingAttributes.DB_OPERATION_NAME, "BATCH INSERT"), + entry(DbIncubatingAttributes.DB_COLLECTION_NAME, "potato"), + entry(DbIncubatingAttributes.DB_OPERATION_BATCH_SIZE, 2L)); + } + + assertThat(endAttributes.build().isEmpty()).isTrue(); + } + + @Test + void shouldExtractMultiQueryBatchAttributes() { + // given + Map request = new HashMap<>(); + request.put("db.name", "potatoes"); + request.put( + "db.statements", + Arrays.asList("INSERT INTO potato VALUES(1)", "INSERT INTO potato VALUES(2)")); + request.put("db.operation.batch.size", 2L); + + Context context = Context.root(); + + AttributesExtractor, Void> underTest = + SqlClientAttributesExtractor.create(new TestMultiAttributesGetter()); + + // when + AttributesBuilder startAttributes = Attributes.builder(); + underTest.onStart(startAttributes, context, request); + + AttributesBuilder endAttributes = Attributes.builder(); + underTest.onEnd(endAttributes, context, request, null, null); + + // then + if (SemconvStability.emitStableDatabaseSemconv() && SemconvStability.emitOldDatabaseSemconv()) { + assertThat(startAttributes.build()) + .containsOnly( + entry(DbIncubatingAttributes.DB_NAME, "potatoes"), + entry(DbIncubatingAttributes.DB_NAMESPACE, "potatoes"), + entry(DbIncubatingAttributes.DB_QUERY_TEXT, "INSERT INTO potato VALUES(?)"), + entry(DbIncubatingAttributes.DB_OPERATION_NAME, "BATCH INSERT"), + entry(DbIncubatingAttributes.DB_COLLECTION_NAME, "potato"), + entry(DbIncubatingAttributes.DB_OPERATION_BATCH_SIZE, 2L)); + } else if (SemconvStability.emitOldDatabaseSemconv()) { + assertThat(startAttributes.build()) + .containsOnly(entry(DbIncubatingAttributes.DB_NAME, "potatoes")); + } else if (SemconvStability.emitStableDatabaseSemconv()) { + assertThat(startAttributes.build()) + .containsOnly( + entry(DbIncubatingAttributes.DB_NAMESPACE, "potatoes"), + entry(DbIncubatingAttributes.DB_QUERY_TEXT, "INSERT INTO potato VALUES(?)"), + entry(DbIncubatingAttributes.DB_OPERATION_NAME, "BATCH INSERT"), + entry(DbIncubatingAttributes.DB_COLLECTION_NAME, "potato"), + entry(DbIncubatingAttributes.DB_OPERATION_BATCH_SIZE, 2L)); + } + + assertThat(endAttributes.build().isEmpty()).isTrue(); + } + + @Test + void shouldIgnoreBatchSizeOne() { + // given + Map request = new HashMap<>(); + request.put("db.name", "potatoes"); + request.put("db.statement", "INSERT INTO potato VALUES(?)"); + request.put("db.statements", Collections.emptyList()); + request.put("db.operation.batch.size", 1L); + + Context context = Context.root(); + + AttributesExtractor, Void> underTest = + SqlClientAttributesExtractor.create(new TestMultiAttributesGetter()); + + // when + AttributesBuilder startAttributes = Attributes.builder(); + underTest.onStart(startAttributes, context, request); + + AttributesBuilder endAttributes = Attributes.builder(); + underTest.onEnd(endAttributes, context, request, null, null); + + // then + if (SemconvStability.emitStableDatabaseSemconv() && SemconvStability.emitOldDatabaseSemconv()) { + assertThat(startAttributes.build()) + .containsOnly( + entry(DbIncubatingAttributes.DB_NAME, "potatoes"), + entry(DbIncubatingAttributes.DB_STATEMENT, "INSERT INTO potato VALUES(?)"), + entry(DbIncubatingAttributes.DB_OPERATION, "INSERT"), + entry(DbIncubatingAttributes.DB_SQL_TABLE, "potato"), + entry(DbIncubatingAttributes.DB_NAMESPACE, "potatoes"), + entry(DbIncubatingAttributes.DB_QUERY_TEXT, "INSERT INTO potato VALUES(?)"), + entry(DbIncubatingAttributes.DB_OPERATION_NAME, "INSERT"), + entry(DbIncubatingAttributes.DB_COLLECTION_NAME, "potato")); + } else if (SemconvStability.emitOldDatabaseSemconv()) { + assertThat(startAttributes.build()) + .containsOnly( + entry(DbIncubatingAttributes.DB_NAME, "potatoes"), + entry(DbIncubatingAttributes.DB_STATEMENT, "INSERT INTO potato VALUES(?)"), + entry(DbIncubatingAttributes.DB_OPERATION, "INSERT"), + entry(DbIncubatingAttributes.DB_SQL_TABLE, "potato")); + } else if (SemconvStability.emitStableDatabaseSemconv()) { + assertThat(startAttributes.build()) + .containsOnly( + entry(DbIncubatingAttributes.DB_NAMESPACE, "potatoes"), + entry(DbIncubatingAttributes.DB_QUERY_TEXT, "INSERT INTO potato VALUES(?)"), + entry(DbIncubatingAttributes.DB_OPERATION_NAME, "INSERT"), + entry(DbIncubatingAttributes.DB_COLLECTION_NAME, "potato")); + } + + assertThat(endAttributes.build().isEmpty()).isTrue(); + } } diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java index 6b6034e067b6..57174d423355 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java @@ -12,7 +12,9 @@ import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; +import static net.bytebuddy.matcher.ElementMatchers.takesNoArguments; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; @@ -42,8 +44,14 @@ public ElementMatcher typeMatcher() { @Override public void transform(TypeTransformer transformer) { transformer.applyAdviceToMethod( - nameStartsWith("execute").and(takesArguments(0)).and(isPublic()), + nameStartsWith("execute") + .and(not(named("executeBatch"))) + .and(takesArguments(0)) + .and(isPublic()), PreparedStatementInstrumentation.class.getName() + "$PreparedStatementAdvice"); + transformer.applyAdviceToMethod( + named("addBatch").and(takesNoArguments()).and(isPublic()), + PreparedStatementInstrumentation.class.getName() + "$AddBatchAdvice"); } @SuppressWarnings("unused") @@ -102,4 +110,13 @@ public static void stopSpan( } } } + + @SuppressWarnings("unused") + public static class AddBatchAdvice { + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void addBatch(@Advice.This PreparedStatement statement) { + JdbcData.addPreparedStatementBatch(statement); + } + } } diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java index 4eeb0ef82bf9..92ca126861fb 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java @@ -13,13 +13,16 @@ import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesNoArguments; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.jdbc.internal.DbRequest; +import io.opentelemetry.instrumentation.jdbc.internal.JdbcData; import io.opentelemetry.javaagent.bootstrap.CallDepth; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.sql.PreparedStatement; import java.sql.Statement; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -42,6 +45,15 @@ public void transform(TypeTransformer transformer) { transformer.applyAdviceToMethod( nameStartsWith("execute").and(takesArgument(0, String.class)).and(isPublic()), StatementInstrumentation.class.getName() + "$StatementAdvice"); + transformer.applyAdviceToMethod( + named("addBatch").and(takesArgument(0, String.class)).and(isPublic()), + StatementInstrumentation.class.getName() + "$AddBatchAdvice"); + transformer.applyAdviceToMethod( + named("clearBatch").and(isPublic()), + StatementInstrumentation.class.getName() + "$ClearBatchAdvice"); + transformer.applyAdviceToMethod( + named("executeBatch").and(takesNoArguments()).and(isPublic()), + StatementInstrumentation.class.getName() + "$ExecuteBatchAdvice"); } @SuppressWarnings("unused") @@ -95,4 +107,91 @@ public static void stopSpan( } } } + + @SuppressWarnings("unused") + public static class AddBatchAdvice { + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void addBatch(@Advice.This Statement statement, @Advice.Argument(0) String sql) { + if (statement instanceof PreparedStatement) { + return; + } + JdbcData.addStatementBatch(statement, sql); + } + } + + @SuppressWarnings("unused") + public static class ClearBatchAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void clearBatch(@Advice.This Statement statement) { + JdbcData.clearBatch(statement); + } + } + + @SuppressWarnings("unused") + public static class ExecuteBatchAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.This Statement statement, + @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelRequest") DbRequest request, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + // Connection#getMetaData() may execute a Statement or PreparedStatement to retrieve DB info + // this happens before the DB CLIENT span is started (and put in the current context), so this + // instrumentation runs again and the shouldStartSpan() check always returns true - and so on + // until we get a StackOverflowError + // using CallDepth prevents this, because this check happens before Connection#getMetadata() + // is called - the first recursive Statement call is just skipped and we do not create a span + // for it + callDepth = CallDepth.forClass(Statement.class); + if (callDepth.getAndIncrement() > 0) { + return; + } + + Context parentContext = currentContext(); + if (statement instanceof PreparedStatement) { + Long batchSize = JdbcData.getPreparedStatementBatchSize((PreparedStatement) statement); + String sql = JdbcData.preparedStatement.get((PreparedStatement) statement); + if (sql == null) { + return; + } + request = DbRequest.create(statement, sql, batchSize); + } else { + JdbcData.StatementBatchInfo batchInfo = JdbcData.getStatementBatchInfo(statement); + if (batchInfo == null) { + request = DbRequest.create(statement, null); + } else { + request = + DbRequest.create(statement, batchInfo.getStatements(), batchInfo.getBatchSize()); + } + } + + if (request == null || !statementInstrumenter().shouldStart(parentContext, request)) { + return; + } + + context = statementInstrumenter().start(parentContext, request); + scope = context.makeCurrent(); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopSpan( + @Advice.Thrown Throwable throwable, + @Advice.Local("otelCallDepth") CallDepth callDepth, + @Advice.Local("otelRequest") DbRequest request, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + if (callDepth.decrementAndGet() > 0) { + return; + } + + if (scope != null) { + scope.close(); + statementInstrumenter().end(context, request, null, throwable); + } + } + } } diff --git a/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/JdbcInstrumentationTest.java b/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/JdbcInstrumentationTest.java index 649b5c480e45..e698e7b51c86 100644 --- a/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/JdbcInstrumentationTest.java +++ b/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/JdbcInstrumentationTest.java @@ -11,6 +11,7 @@ import static io.opentelemetry.semconv.ServerAttributes.SERVER_ADDRESS; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_NAME; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION_BATCH_SIZE; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SQL_TABLE; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_STATEMENT; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SYSTEM; @@ -490,7 +491,7 @@ void testPreparedStatementExecute( span.hasName(spanName) .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) - .hasAttributesSatisfying( + .hasAttributesSatisfyingExactly( equalTo(DB_SYSTEM, system), equalTo(maybeStable(DB_NAME), dbNameLower), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), @@ -527,7 +528,7 @@ void testPreparedStatementQuery( span.hasName(spanName) .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) - .hasAttributesSatisfying( + .hasAttributesSatisfyingExactly( equalTo(DB_SYSTEM, system), equalTo(maybeStable(DB_NAME), dbNameLower), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), @@ -564,7 +565,7 @@ void testPreparedCall( span.hasName(spanName) .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) - .hasAttributesSatisfying( + .hasAttributesSatisfyingExactly( equalTo(DB_SYSTEM, system), equalTo(maybeStable(DB_NAME), dbNameLower), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), @@ -700,7 +701,7 @@ void testStatementUpdate( span.hasName(spanName) .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) - .hasAttributesSatisfying( + .hasAttributesSatisfyingExactly( equalTo(DB_SYSTEM, system), equalTo(maybeStable(DB_NAME), dbNameLower), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), @@ -802,7 +803,7 @@ void testPreparedStatementUpdate( span.hasName(spanName) .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) - .hasAttributesSatisfying( + .hasAttributesSatisfyingExactly( equalTo(DB_SYSTEM, system), equalTo(maybeStable(DB_NAME), dbNameLower), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), @@ -909,7 +910,7 @@ void testConnectionConstructorThrowing( span.hasName(spanName) .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) - .hasAttributesSatisfying( + .hasAttributesSatisfyingExactly( equalTo(DB_SYSTEM, system), equalTo(maybeStable(DB_NAME), dbNameLower), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), @@ -975,7 +976,7 @@ void testGetConnection( .hasName(datasource.getClass().getSimpleName() + ".getConnection") .hasKind(SpanKind.INTERNAL) .hasParent(trace.getSpan(0)) - .hasAttributesSatisfying( + .hasAttributesSatisfyingExactly( equalTo( CodeIncubatingAttributes.CODE_NAMESPACE, datasource.getClass().getName()), @@ -992,7 +993,7 @@ void testGetConnection( span.hasName(datasource.getClass().getSimpleName() + ".getConnection") .hasKind(SpanKind.INTERNAL) .hasParent(trace.getSpan(1)) - .hasAttributesSatisfying( + .hasAttributesSatisfyingExactly( equalTo( CodeIncubatingAttributes.CODE_NAMESPACE, datasource.getClass().getName()), @@ -1034,7 +1035,7 @@ void testGetClientInfoException(String query) throws SQLException { span.hasName("DB Query") .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) - .hasAttributesSatisfying( + .hasAttributesSatisfyingExactly( equalTo(DB_SYSTEM, "other_sql"), equalTo(maybeStable(DB_STATEMENT), "testing ?"), equalTo( @@ -1117,7 +1118,7 @@ void testProduceProperSpanName( span.hasName(spanName) .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) - .hasAttributesSatisfying( + .hasAttributesSatisfyingExactly( equalTo(DB_SYSTEM, "other_sql"), equalTo(maybeStable(DB_NAME), databaseName), equalTo( @@ -1167,7 +1168,7 @@ void testConnectionCached(String connectionPoolName) throws SQLException { span -> span.hasName("SELECT INFORMATION_SCHEMA.SYSTEM_USERS") .hasKind(SpanKind.CLIENT) - .hasAttributesSatisfying( + .hasAttributesSatisfyingExactly( equalTo(DB_SYSTEM, "hsqldb"), equalTo(maybeStable(DB_NAME), dbNameLower), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : "SA"), @@ -1241,7 +1242,7 @@ void testHandleRecursiveStatements( span.hasName("SELECT table") .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) - .hasAttributesSatisfying( + .hasAttributesSatisfyingExactly( equalTo(DB_SYSTEM, "other_sql"), equalTo( DB_CONNECTION_STRING, @@ -1302,4 +1303,216 @@ void testProxyPreparedStatement() throws SQLException { .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)))); } + + static Stream batchStream() throws SQLException { + return Stream.of( + Arguments.of("h2", new org.h2.Driver().connect(jdbcUrls.get("h2"), null), null, "h2:mem:"), + Arguments.of( + "derby", + new EmbeddedDriver().connect(jdbcUrls.get("derby"), null), + "APP", + "derby:memory:"), + Arguments.of( + "hsqldb", new JDBCDriver().connect(jdbcUrls.get("hsqldb"), null), "SA", "hsqldb:mem:")); + } + + @ParameterizedTest + @MethodSource("batchStream") + void testBatch(String system, Connection connection, String username, String url) + throws SQLException { + String tableName = "simple_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(); + 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", () -> 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 INSERT jdbcunittest." + tableName + : "jdbcunittest") + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(0)) + .hasAttributesSatisfyingExactly( + equalTo(DB_SYSTEM, system), + equalTo(maybeStable(DB_NAME), dbNameLower), + equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), + equalTo(DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), + equalTo( + maybeStable(DB_STATEMENT), + emitStableDatabaseSemconv() + ? "INSERT INTO " + tableName + " VALUES(?)" + : null), + equalTo( + maybeStable(DB_OPERATION), + emitStableDatabaseSemconv() ? "BATCH INSERT" : null), + equalTo( + maybeStable(DB_SQL_TABLE), + emitStableDatabaseSemconv() ? tableName : null), + equalTo( + DB_OPERATION_BATCH_SIZE, + emitStableDatabaseSemconv() ? 2L : null)))); + } + + @ParameterizedTest + @MethodSource("batchStream") + void testMultiBatch(String system, Connection connection, String username, String url) + throws SQLException { + 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(createTable1); + + 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 INSERT jdbcunittest" + : "jdbcunittest") + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(0)) + .hasAttributesSatisfyingExactly( + equalTo(DB_SYSTEM, system), + equalTo(maybeStable(DB_NAME), dbNameLower), + 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), + equalTo( + maybeStable(DB_OPERATION), + emitStableDatabaseSemconv() ? "BATCH INSERT" : null), + equalTo( + DB_OPERATION_BATCH_SIZE, + emitStableDatabaseSemconv() ? 2L : null)))); + } + + @ParameterizedTest + @MethodSource("batchStream") + void testSingleItemBatch(String system, Connection connection, String username, String url) + throws SQLException { + 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(); + cleanup.deferCleanup(statement); + statement.addBatch("INSERT INTO " + tableName + " VALUES(1)"); + testing.runWithSpan( + "parent", () -> assertThat(statement.executeBatch()).isEqualTo(new int[] {1})); + + testing.waitAndAssertTraces( + trace -> + trace.hasSpansSatisfyingExactly( + span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(), + span -> + span.hasName("INSERT jdbcunittest." + tableName) + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(0)) + .hasAttributesSatisfyingExactly( + equalTo(DB_SYSTEM, system), + equalTo(maybeStable(DB_NAME), dbNameLower), + equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), + equalTo(DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), + equalTo( + maybeStable(DB_STATEMENT), + "INSERT INTO " + tableName + " VALUES(?)"), + equalTo(maybeStable(DB_OPERATION), "INSERT"), + equalTo(maybeStable(DB_SQL_TABLE), tableName)))); + } + + @ParameterizedTest + @MethodSource("batchStream") + void testPreparedBatch(String system, Connection connection, String username, String url) + throws SQLException { + String tableName = "prepared_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(); + + PreparedStatement statement = + connection.prepareStatement("INSERT INTO " + tableName + " VALUES(?)"); + cleanup.deferCleanup(statement); + statement.setInt(1, 1); + statement.addBatch(); + statement.clearBatch(); + statement.setInt(1, 1); + statement.addBatch(); + statement.setInt(1, 2); + statement.addBatch(); + 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 INSERT jdbcunittest." + tableName + : "INSERT jdbcunittest." + tableName) + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(0)) + .hasAttributesSatisfyingExactly( + equalTo(DB_SYSTEM, system), + equalTo(maybeStable(DB_NAME), dbNameLower), + equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), + equalTo(DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), + equalTo( + maybeStable(DB_STATEMENT), + "INSERT INTO " + tableName + " VALUES(?)"), + equalTo( + maybeStable(DB_OPERATION), + emitStableDatabaseSemconv() ? "BATCH INSERT" : "INSERT"), + equalTo(maybeStable(DB_SQL_TABLE), tableName), + equalTo( + DB_OPERATION_BATCH_SIZE, + emitStableDatabaseSemconv() ? 2L : null)))); + } } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/DbRequest.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/DbRequest.java index f1360d9b1cac..a5e8e0559178 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/DbRequest.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/DbRequest.java @@ -13,6 +13,8 @@ import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.Statement; +import java.util.Collection; +import java.util.Collections; import javax.annotation.Nullable; /** @@ -29,19 +31,45 @@ public static DbRequest create(PreparedStatement statement) { @Nullable public static DbRequest create(Statement statement, String dbStatementString) { + return create(statement, dbStatementString, null); + } + + @Nullable + public static DbRequest create(Statement statement, String dbStatementString, Long batchSize) { + Connection connection = connectionFromStatement(statement); + if (connection == null) { + return null; + } + + return create(extractDbInfo(connection), dbStatementString, batchSize); + } + + public static DbRequest create( + Statement statement, Collection queryTexts, Long batchSize) { Connection connection = connectionFromStatement(statement); if (connection == null) { return null; } - return create(extractDbInfo(connection), dbStatementString); + return create(extractDbInfo(connection), queryTexts, batchSize); } public static DbRequest create(DbInfo dbInfo, String queryText) { - return new AutoValue_DbRequest(dbInfo, queryText); + return create(dbInfo, queryText, null); + } + + public static DbRequest create(DbInfo dbInfo, String queryText, Long batchSize) { + return create(dbInfo, Collections.singletonList(queryText), batchSize); + } + + public static DbRequest create(DbInfo dbInfo, Collection queryTexts, Long batchSize) { + return new AutoValue_DbRequest(dbInfo, queryTexts, batchSize); } public abstract DbInfo getDbInfo(); - public abstract String getQueryText(); + public abstract Collection getQueryTexts(); + + @Nullable + public abstract Long getBatchSize(); } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcAttributesGetter.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcAttributesGetter.java index b5623f4d99ce..5fc05275a0c6 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcAttributesGetter.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcAttributesGetter.java @@ -5,15 +5,16 @@ package io.opentelemetry.instrumentation.jdbc.internal; -import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlClientAttributesGetter; +import io.opentelemetry.instrumentation.api.incubator.semconv.db.internal.MultiQuerySqlClientAttributesGetter; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; +import java.util.Collection; import javax.annotation.Nullable; /** * This class is internal and is hence not for public use. Its APIs are unstable and can change at * any time. */ -public final class JdbcAttributesGetter implements SqlClientAttributesGetter { +public final class JdbcAttributesGetter implements MultiQuerySqlClientAttributesGetter { @Nullable @Override @@ -45,6 +46,16 @@ public String getConnectionString(DbRequest request) { @Nullable @Override public String getRawQueryText(DbRequest request) { - return request.getQueryText(); + return request.getQueryTexts().iterator().next(); + } + + @Override + public Collection getRawQueryTexts(DbRequest request) { + return request.getQueryTexts(); + } + + @Override + public Long getBatchSize(DbRequest request) { + return request.getBatchSize(); } } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcData.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcData.java index 84433208ceef..04d458554235 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcData.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcData.java @@ -10,6 +10,9 @@ import java.lang.ref.WeakReference; import java.sql.Connection; import java.sql.PreparedStatement; +import java.sql.Statement; +import java.util.Collection; +import java.util.LinkedHashSet; import java.util.Map; import java.util.WeakHashMap; @@ -26,6 +29,11 @@ public final class JdbcData { VirtualField.find(Connection.class, DbInfo.class); public static final VirtualField preparedStatement = VirtualField.find(PreparedStatement.class, String.class); + private static final VirtualField statementBatch = + VirtualField.find(Statement.class, StatementBatchInfo.class); + private static final VirtualField + preparedStatementBatch = + VirtualField.find(PreparedStatement.class, PreparedStatementBatchInfo.class); private JdbcData() {} @@ -50,4 +58,73 @@ public static DbInfo intern(DbInfo dbInfo) { return dbInfo; } } + + public static void addStatementBatch(Statement statement, String sql) { + StatementBatchInfo batchInfo = statementBatch.get(statement); + if (batchInfo == null) { + batchInfo = new StatementBatchInfo(); + statementBatch.set(statement, batchInfo); + } + batchInfo.add(sql); + } + + public static void addPreparedStatementBatch(PreparedStatement statement) { + PreparedStatementBatchInfo batchInfo = preparedStatementBatch.get(statement); + if (batchInfo == null) { + batchInfo = new PreparedStatementBatchInfo(); + preparedStatementBatch.set(statement, batchInfo); + } + batchInfo.add(); + } + + public static void clearBatch(Statement statement) { + if (statement instanceof PreparedStatement) { + preparedStatementBatch.set((PreparedStatement) statement, null); + } else { + statementBatch.set(statement, null); + } + } + + public static StatementBatchInfo getStatementBatchInfo(Statement statement) { + return statementBatch.get(statement); + } + + public static Long getPreparedStatementBatchSize(PreparedStatement statement) { + PreparedStatementBatchInfo batchInfo = preparedStatementBatch.get(statement); + return batchInfo != null ? batchInfo.getBatchSize() : null; + } + + /** + * This class is internal and is hence not for public use. Its APIs are unstable and can change at + * any time. + */ + public static final class StatementBatchInfo { + private final Collection statements = new LinkedHashSet<>(); + private long batchSize; + + void add(String sql) { + statements.add(sql); + batchSize++; + } + + public Collection getStatements() { + return statements; + } + + public long getBatchSize() { + return batchSize; + } + } + + private static final class PreparedStatementBatchInfo { + private long batchSize; + + void add() { + batchSize++; + } + + long getBatchSize() { + return batchSize; + } + } } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java index 691e61304a1d..bb5f8ec837e5 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java @@ -234,6 +234,7 @@ public void setObject(int parameterIndex, Object x, int targetSqlType, int scale @Override public void addBatch() throws SQLException { delegate.addBatch(); + batchSize++; } @SuppressWarnings("UngroupedOverloads") diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java index d2d5a8b6ca94..156d47b02d5d 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java @@ -25,11 +25,13 @@ import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; import java.sql.Connection; +import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.SQLWarning; import java.sql.Statement; -import java.util.ArrayList; +import java.util.Collection; +import java.util.LinkedHashSet; /** * This class is internal and is hence not for public use. Its APIs are unstable and can change at @@ -43,7 +45,8 @@ public class OpenTelemetryStatement implements Statement { protected final String query; protected final Instrumenter instrumenter; - private final ArrayList batchCommands = new ArrayList<>(); + private final Collection batchCommands = new LinkedHashSet<>(); + protected long batchSize; OpenTelemetryStatement( S delegate, @@ -113,7 +116,7 @@ public boolean execute(String sql, String[] columnNames) throws SQLException { @Override public int[] executeBatch() throws SQLException { - return wrapCall(buildSqlForBatch(), delegate::executeBatch); + return wrapBatchCall(delegate::executeBatch); } @Override @@ -231,12 +234,14 @@ public int getResultSetType() throws SQLException { public void addBatch(String sql) throws SQLException { delegate.addBatch(sql); batchCommands.add(sql); + batchSize++; } @Override public void clearBatch() throws SQLException { delegate.clearBatch(); batchCommands.clear(); + batchSize = 0; } @Override @@ -291,8 +296,13 @@ public boolean isWrapperFor(Class iface) throws SQLException { protected T wrapCall(String sql, ThrowingSupplier callable) throws E { - Context parentContext = Context.current(); DbRequest request = DbRequest.create(dbInfo, sql); + return wrapCall(request, callable); + } + + private T wrapCall(DbRequest request, ThrowingSupplier callable) + throws E { + Context parentContext = Context.current(); if (!this.instrumenter.shouldStart(parentContext, request)) { return callable.call(); @@ -310,16 +320,11 @@ protected T wrapCall(String sql, ThrowingSupplier return result; } - private String buildSqlForBatch() { - StringBuilder sqlBuilder = new StringBuilder(); - if (query != null) { - sqlBuilder.append(query); - } - - for (String batchCommand : batchCommands) { - sqlBuilder.append(batchCommand); - } - - return sqlBuilder.toString(); + protected T wrapBatchCall(ThrowingSupplier callable) throws E { + DbRequest request = + this instanceof PreparedStatement + ? DbRequest.create(dbInfo, query, batchSize) + : DbRequest.create(dbInfo, batchCommands, batchSize); + return wrapCall(request, callable); } } diff --git a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryTest.java b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryTest.java index 1f3db9d51bfa..748ecdfe4a23 100644 --- a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryTest.java +++ b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryTest.java @@ -7,9 +7,14 @@ import static io.opentelemetry.instrumentation.testing.junit.db.SemconvStabilityUtil.maybeStable; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_NAME; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION_BATCH_SIZE; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SQL_TABLE; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_STATEMENT; import static org.assertj.core.api.Assertions.assertThat; +import io.opentelemetry.instrumentation.api.internal.SemconvStability; import io.opentelemetry.instrumentation.jdbc.internal.OpenTelemetryConnection; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension; @@ -134,4 +139,44 @@ void statementReturnsWrappedConnection() throws SQLException { CallableStatement callableStatement = connection.prepareCall("SELECT 1"); assertThat(callableStatement.getConnection()).isInstanceOf(OpenTelemetryConnection.class); } + + @Test + void batchStatement() throws SQLException { + JdbcTelemetry telemetry = JdbcTelemetry.builder(testing.getOpenTelemetry()).build(); + DataSource dataSource = telemetry.wrap(new TestDataSource()); + + testing.runWithSpan( + "parent", + () -> { + Statement statement = dataSource.getConnection().createStatement(); + statement.addBatch("INSERT INTO invalid VALUES(1)"); + statement.clearBatch(); + statement.addBatch("INSERT INTO test VALUES(1)"); + statement.addBatch("INSERT INTO test VALUES(2)"); + statement.executeBatch(); + }); + + testing.waitAndAssertTraces( + trace -> + trace.hasSpansSatisfyingExactly( + span -> span.hasName("parent"), + span -> span.hasName("TestDataSource.getConnection"), + span -> + span.hasName( + SemconvStability.emitStableDatabaseSemconv() + ? "BATCH INSERT dbname.test" + : "dbname") + .hasAttributesSatisfying( + equalTo(maybeStable(DB_NAME), "dbname"), + equalTo( + maybeStable(DB_OPERATION), + SemconvStability.emitStableDatabaseSemconv() + ? "BATCH INSERT" + : "INSERT"), + equalTo(maybeStable(DB_SQL_TABLE), "test"), + equalTo(maybeStable(DB_STATEMENT), "INSERT INTO test VALUES(?)"), + equalTo( + DB_OPERATION_BATCH_SIZE, + SemconvStability.emitStableDatabaseSemconv() ? 2L : null)))); + } } From f0306cec07e2d70a3e7d0f6b729dfac3914e17d0 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 26 Nov 2024 15:15:32 +0200 Subject: [PATCH 2/4] fix test --- .../jdbc/datasource/JdbcTelemetryTest.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryTest.java b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryTest.java index 748ecdfe4a23..e64e39a81690 100644 --- a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryTest.java +++ b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryTest.java @@ -172,9 +172,15 @@ void batchStatement() throws SQLException { maybeStable(DB_OPERATION), SemconvStability.emitStableDatabaseSemconv() ? "BATCH INSERT" - : "INSERT"), - equalTo(maybeStable(DB_SQL_TABLE), "test"), - equalTo(maybeStable(DB_STATEMENT), "INSERT INTO test VALUES(?)"), + : null), + equalTo( + maybeStable(DB_SQL_TABLE), + SemconvStability.emitStableDatabaseSemconv() ? "test" : null), + equalTo( + maybeStable(DB_STATEMENT), + SemconvStability.emitStableDatabaseSemconv() + ? "INSERT INTO test VALUES(?)" + : null), equalTo( DB_OPERATION_BATCH_SIZE, SemconvStability.emitStableDatabaseSemconv() ? 2L : null)))); From 0f5dbbc95241538873516c08c5d82c7d132caf8b Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Thu, 5 Dec 2024 10:01:40 +0200 Subject: [PATCH 3/4] address review comment --- .../jdbc/internal/OpenTelemetryPreparedStatement.java | 10 ++++++++++ .../jdbc/internal/OpenTelemetryStatement.java | 10 +++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java index bb5f8ec837e5..ac7f024511e1 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java @@ -364,4 +364,14 @@ public void setSQLXML(int parameterIndex, SQLXML xmlObject) throws SQLException public void clearParameters() throws SQLException { delegate.clearParameters(); } + + @Override + public int[] executeBatch() throws SQLException { + return wrapBatchCall(delegate::executeBatch); + } + + private T wrapBatchCall(ThrowingSupplier callable) throws E { + DbRequest request = DbRequest.create(dbInfo, query, batchSize); + return wrapCall(request, callable); + } } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java index 156d47b02d5d..3b4ab2b28502 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java @@ -25,7 +25,6 @@ import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; import java.sql.Connection; -import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.SQLWarning; @@ -300,7 +299,7 @@ protected T wrapCall(String sql, ThrowingSupplier return wrapCall(request, callable); } - private T wrapCall(DbRequest request, ThrowingSupplier callable) + protected T wrapCall(DbRequest request, ThrowingSupplier callable) throws E { Context parentContext = Context.current(); @@ -320,11 +319,8 @@ private T wrapCall(DbRequest request, ThrowingSupplier< return result; } - protected T wrapBatchCall(ThrowingSupplier callable) throws E { - DbRequest request = - this instanceof PreparedStatement - ? DbRequest.create(dbInfo, query, batchSize) - : DbRequest.create(dbInfo, batchCommands, batchSize); + private T wrapBatchCall(ThrowingSupplier callable) throws E { + DbRequest request = DbRequest.create(dbInfo, batchCommands, batchSize); return wrapCall(request, callable); } } From 6ff9ee6f0d7ac7c57dd713eeb5e6d5779f70fa09 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Mon, 9 Dec 2024 15:50:07 +0200 Subject: [PATCH 4/4] use list --- .../instrumentation/jdbc/internal/JdbcData.java | 9 ++++----- .../jdbc/internal/OpenTelemetryStatement.java | 6 +++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcData.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcData.java index 04d458554235..08393a02df72 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcData.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcData.java @@ -11,8 +11,9 @@ import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.Statement; +import java.util.ArrayList; import java.util.Collection; -import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; import java.util.WeakHashMap; @@ -99,12 +100,10 @@ public static Long getPreparedStatementBatchSize(PreparedStatement statement) { * any time. */ public static final class StatementBatchInfo { - private final Collection statements = new LinkedHashSet<>(); - private long batchSize; + private final List statements = new ArrayList<>(); void add(String sql) { statements.add(sql); - batchSize++; } public Collection getStatements() { @@ -112,7 +111,7 @@ public Collection getStatements() { } public long getBatchSize() { - return batchSize; + return statements.size(); } } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java index 3b4ab2b28502..3cb62cebd6e2 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java @@ -29,8 +29,8 @@ import java.sql.SQLException; import java.sql.SQLWarning; import java.sql.Statement; -import java.util.Collection; -import java.util.LinkedHashSet; +import java.util.ArrayList; +import java.util.List; /** * This class is internal and is hence not for public use. Its APIs are unstable and can change at @@ -44,7 +44,7 @@ public class OpenTelemetryStatement implements Statement { protected final String query; protected final Instrumenter instrumenter; - private final Collection batchCommands = new LinkedHashSet<>(); + private final List batchCommands = new ArrayList<>(); protected long batchSize; OpenTelemetryStatement(