From 83795a3a21ba32e48f469f25f4edd005829eaba7 Mon Sep 17 00:00:00 2001 From: stillya Date: Mon, 14 Apr 2025 16:38:19 +0500 Subject: [PATCH 1/9] feat: contribute for #10381 To enable trace when transaction commit or rollback in jdbc under experimental flag. --- .../src/main/jflex/SqlSanitizer.jflex | 20 +++ .../semconv/db/SqlStatementSanitizerTest.java | 28 ++++ instrumentation/jdbc/README.md | 7 +- .../jdbc/javaagent/build.gradle.kts | 6 + .../jdbc/ConnectionInstrumentation.java | 65 ++++++++ .../jdbc/test/JdbcInstrumentationTest.java | 149 ++++++++++++++++++ instrumentation/jdbc/library/build.gradle.kts | 6 + .../internal/OpenTelemetryConnection.java | 48 +++++- .../internal/OpenTelemetryConnectionTest.java | 67 ++++++++ 9 files changed, 390 insertions(+), 6 deletions(-) diff --git a/instrumentation-api-incubator/src/main/jflex/SqlSanitizer.jflex b/instrumentation-api-incubator/src/main/jflex/SqlSanitizer.jflex index 83a59fc0abc9..1044375ae37b 100644 --- a/instrumentation-api-incubator/src/main/jflex/SqlSanitizer.jflex +++ b/instrumentation-api-incubator/src/main/jflex/SqlSanitizer.jflex @@ -347,6 +347,12 @@ WHITESPACE = [ \t\r\n]+ } } + private class Commit extends Operation { + } + + private class Rollback extends Operation { + } + private class Create extends DdlOperation { } @@ -485,6 +491,20 @@ WHITESPACE = [ \t\r\n]+ appendCurrentFragment(); if (isOverLimit()) return YYEOF; } + "COMMIT" { + if (!insideComment) { + setOperation(new Commit()); + } + appendCurrentFragment(); + if (isOverLimit()) return YYEOF; + } + "ROLLBACK" { + if (!insideComment) { + setOperation(new Rollback()); + } + appendCurrentFragment(); + if (isOverLimit()) return YYEOF; + } {COMMA} { if (!insideComment && !extractionDone) { diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlStatementSanitizerTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlStatementSanitizerTest.java index 8771ac1ed7c4..454875d23ca5 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlStatementSanitizerTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlStatementSanitizerTest.java @@ -71,6 +71,17 @@ void checkDdlOperationStatementsAreOk( assertThat(result.getMainIdentifier()).isEqualTo(expected.getMainIdentifier()); } + @ParameterizedTest + @ArgumentsSource(TransactionArgs.class) + void checkTransactionOperationStatementsAreOk( + String actual, Function expectFunc) { + SqlStatementInfo result = SqlStatementSanitizer.create(true).sanitize(actual); + SqlStatementInfo expected = expectFunc.apply(actual); + assertThat(result.getFullStatement()).isEqualTo(expected.getFullStatement()); + assertThat(result.getOperation()).isEqualTo(expected.getOperation()); + assertThat(result.getMainIdentifier()).isEqualTo(expected.getMainIdentifier()); + } + @Test void lotsOfTicksDontCauseStackOverflowOrLongRuntimes() { String s = "'"; @@ -427,4 +438,21 @@ public Stream provideArguments(ExtensionContext context) { "CREATE PROCEDURE p AS SELECT * FROM table GO", expect("CREATE PROCEDURE", null))); } } + + static class TransactionArgs implements ArgumentsProvider { + static Function expect(String operation, String identifier) { + return sql -> SqlStatementInfo.create(sql, operation, identifier); + } + + @Override + public Stream provideArguments(ExtensionContext context) { + return Stream.of( + Arguments.of("COMMIT", expect("COMMIT", null)), + Arguments.of("commit", expect("COMMIT", null)), + Arguments.of("/*COMMIT*/", expect(null, null)), + Arguments.of("ROLLBACK", expect("ROLLBACK", null)), + Arguments.of("rollback", expect("ROLLBACK", null)), + Arguments.of("/*ROLLBACK*/", expect(null, null))); + } + } } diff --git a/instrumentation/jdbc/README.md b/instrumentation/jdbc/README.md index 7e3440fd8c01..1841f4657ea5 100644 --- a/instrumentation/jdbc/README.md +++ b/instrumentation/jdbc/README.md @@ -1,5 +1,6 @@ # Settings for the JDBC instrumentation -| System property | Type | Default | Description | -|---------------------------------------------------------|---------|---------|----------------------------------------| -| `otel.instrumentation.jdbc.statement-sanitizer.enabled` | Boolean | `true` | Enables the DB statement sanitization. | +| System property | Type | Default | Description | +|---------------------------------------------------------|---------|---------|-------------------------------------------------------| +| `otel.instrumentation.jdbc.statement-sanitizer.enabled` | Boolean | `true` | Enables the DB statement sanitization. | +| `otel.instrumentation.jdbc.experimental.txn.enabled` | Boolean | `false` | Enables the experimental transaction instrumentation. | diff --git a/instrumentation/jdbc/javaagent/build.gradle.kts b/instrumentation/jdbc/javaagent/build.gradle.kts index ac4617836eaf..43ebc079eddd 100644 --- a/instrumentation/jdbc/javaagent/build.gradle.kts +++ b/instrumentation/jdbc/javaagent/build.gradle.kts @@ -91,3 +91,9 @@ tasks { dependsOn(testSlickStableSemconv) } } + +tasks { + withType().configureEach { + jvmArgs("-Dotel.instrumentation.jdbc.experimental.txn.enabled=true") + } +} diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java index f739db400778..27cb634db86e 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java @@ -5,23 +5,40 @@ package io.opentelemetry.javaagent.instrumentation.jdbc; +import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.instrumentation.jdbc.JdbcSingletons.statementInstrumenter; +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.namedOneOf; import static net.bytebuddy.matcher.ElementMatchers.returns; 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.instrumentation.jdbc.internal.JdbcUtils; +import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig; +import io.opentelemetry.javaagent.bootstrap.jdbc.DbInfo; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.sql.Connection; import java.sql.PreparedStatement; +import java.util.Locale; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; public class ConnectionInstrumentation implements TypeInstrumentation { + private static final boolean TXN_ENABLED = + AgentInstrumentationConfig.get() + .getBoolean("otel.instrumentation.jdbc.experimental.txn.enabled", false); + @Override public ElementMatcher classLoaderOptimization() { return hasClassesNamed("java.sql.Connection"); @@ -40,6 +57,11 @@ public void transform(TypeTransformer transformer) { // Also include CallableStatement, which is a sub type of PreparedStatement .and(returns(implementsInterface(named("java.sql.PreparedStatement")))), ConnectionInstrumentation.class.getName() + "$PrepareAdvice"); + if (TXN_ENABLED) { + transformer.applyAdviceToMethod( + namedOneOf("commit", "rollback").and(takesNoArguments()).and(isPublic()), + ConnectionInstrumentation.class.getName() + "$TxnAdvice"); + } } @SuppressWarnings("unused") @@ -51,4 +73,47 @@ public static void addDbInfo( JdbcData.preparedStatement.set(statement, sql); } } + + @SuppressWarnings("unused") + public static class TxnAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter( + @Advice.This Connection connection, + @Advice.Origin("#m") String methodName, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + Context parentContext = currentContext(); + DbInfo dbInfo = null; + Connection realConnection = JdbcUtils.unwrapConnection(connection); + if (realConnection != null) { + dbInfo = JdbcUtils.extractDbInfo(realConnection); + } + if (dbInfo == null) { + return; + } + DbRequest request = DbRequest.create(dbInfo, methodName.toUpperCase(Locale.ROOT)); + + if (!statementInstrumenter().shouldStart(parentContext, request)) { + return; + } + + request = DbRequest.create(request.getDbInfo(), methodName.toUpperCase(Locale.ROOT)); + 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("otelRequest") DbRequest request, + @Advice.Local("otelContext") Context context, + @Advice.Local("otelScope") Scope scope) { + if (scope == null) { + return; + } + 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 369a09f31fcf..efa3096f2830 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 @@ -19,6 +19,7 @@ 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_OPERATION_NAME; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_QUERY_TEXT; 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; @@ -1632,4 +1633,152 @@ void testPreparedBatch(String system, Connection connection, String username, St DB_OPERATION_BATCH_SIZE, emitStableDatabaseSemconv() ? 2L : null)))); } + + @ParameterizedTest + @MethodSource("transactionOperationsStream") + void testCommitTransaction(String system, Connection connection, String username, String url) + throws SQLException { + + String tableName = "TXN_COMMIT_TEST_" + system.toUpperCase(Locale.ROOT); + Statement createTable = connection.createStatement(); + createTable.execute("CREATE TABLE " + tableName + " (id INTEGER not NULL, PRIMARY KEY ( id ))"); + cleanup.deferCleanup(createTable); + + boolean originalAutoCommit = connection.getAutoCommit(); + connection.setAutoCommit(false); + + testing.waitForTraces(1); + testing.clearData(); + + try { + Statement insertStatement = connection.createStatement(); + cleanup.deferCleanup(insertStatement); + + testing.runWithSpan( + "parent", + () -> { + insertStatement.executeUpdate("INSERT INTO " + tableName + " VALUES(1)"); + connection.commit(); + }); + + 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(maybeStable(DB_SYSTEM), maybeStableDbSystemName(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)), + span -> + span.hasName("COMMIT " + dbNameLower) + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(0)) + .hasAttributesSatisfyingExactly( + equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)), + equalTo(maybeStable(DB_NAME), dbNameLower), + equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), + equalTo(DB_STATEMENT, emitStableDatabaseSemconv() ? null : "COMMIT"), + equalTo(DB_QUERY_TEXT, emitStableDatabaseSemconv() ? "COMMIT" : null), + equalTo( + DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), + equalTo(maybeStable(DB_OPERATION), "COMMIT")))); + } finally { + connection.setAutoCommit(originalAutoCommit); + } + } + + @ParameterizedTest + @MethodSource("transactionOperationsStream") + void testRollbackTransaction(String system, Connection connection, String username, String url) + throws SQLException { + + String tableName = "TXN_ROLLBACK_TEST_" + system.toUpperCase(Locale.ROOT); + Statement createTable = connection.createStatement(); + createTable.execute("CREATE TABLE " + tableName + " (id INTEGER not NULL, PRIMARY KEY ( id ))"); + cleanup.deferCleanup(createTable); + + boolean originalAutoCommit = connection.getAutoCommit(); + connection.setAutoCommit(false); + + testing.waitForTraces(1); + testing.clearData(); + + try { + Statement insertStatement = connection.createStatement(); + cleanup.deferCleanup(insertStatement); + + testing.runWithSpan( + "parent", + () -> { + insertStatement.executeUpdate("INSERT INTO " + tableName + " VALUES(1)"); + connection.rollback(); + }); + + 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(maybeStable(DB_SYSTEM), maybeStableDbSystemName(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)), + span -> + span.hasName("ROLLBACK " + dbNameLower) + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(0)) + .hasAttributesSatisfyingExactly( + equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)), + equalTo(maybeStable(DB_NAME), dbNameLower), + equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), + equalTo( + DB_STATEMENT, emitStableDatabaseSemconv() ? null : "ROLLBACK"), + equalTo( + DB_QUERY_TEXT, emitStableDatabaseSemconv() ? "ROLLBACK" : null), + equalTo( + DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), + equalTo(maybeStable(DB_OPERATION), "ROLLBACK")))); + + Statement selectStatement = connection.createStatement(); + cleanup.deferCleanup(selectStatement); + ResultSet resultSet = selectStatement.executeQuery("SELECT COUNT(*) FROM " + tableName); + resultSet.next(); + assertThat(resultSet.getInt(1)).isEqualTo(0); + } finally { + connection.setAutoCommit(originalAutoCommit); + } + } + + static Stream transactionOperationsStream() 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:")); + } } diff --git a/instrumentation/jdbc/library/build.gradle.kts b/instrumentation/jdbc/library/build.gradle.kts index e61a0cbacbef..5b3004e0fa88 100644 --- a/instrumentation/jdbc/library/build.gradle.kts +++ b/instrumentation/jdbc/library/build.gradle.kts @@ -59,3 +59,9 @@ tasks { dependsOn(testStableSemconv) } } + +tasks { + withType().configureEach { + jvmArgs("-Dotel.instrumentation.jdbc.experimental.txn.enabled=true") + } +} diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java index 8fe5ca26ab1d..e3f8ce448316 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java @@ -20,7 +20,10 @@ package io.opentelemetry.instrumentation.jdbc.internal; +import io.opentelemetry.context.Context; +import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; import java.sql.Array; import java.sql.Blob; @@ -48,6 +51,8 @@ */ public class OpenTelemetryConnection implements Connection { + private static final boolean TXN_ENABLED = + ConfigPropertiesUtil.getBoolean("otel.instrumentation.jdbc.experimental.txn.enabled", false); private static final boolean hasJdbc43 = hasJdbc43(); protected final Connection delegate; private final DbInfo dbInfo; @@ -173,7 +178,11 @@ public CallableStatement prepareCall( @Override public void commit() throws SQLException { - delegate.commit(); + if (TXN_ENABLED) { + wrapCall(delegate::commit, "COMMIT"); + } else { + delegate.commit(); + } } @Override @@ -279,13 +288,21 @@ public Savepoint setSavepoint(String name) throws SQLException { @SuppressWarnings("UngroupedOverloads") @Override public void rollback() throws SQLException { - delegate.rollback(); + if (TXN_ENABLED) { + wrapCall(delegate::rollback, "ROLLBACK"); + } else { + delegate.rollback(); + } } @SuppressWarnings("UngroupedOverloads") @Override public void rollback(Savepoint savepoint) throws SQLException { - delegate.rollback(savepoint); + if (TXN_ENABLED) { + wrapCall(() -> delegate.rollback(savepoint), "ROLLBACK"); + } else { + delegate.rollback(savepoint); + } } @Override @@ -435,4 +452,29 @@ public void setShardingKey(ShardingKey shardingKey) throws SQLException { delegate.setShardingKey(shardingKey); } } + + protected void wrapCall(ThrowingSupplier callable, String statement) + throws E { + Context parentContext = Context.current(); + DbRequest request = DbRequest.create(dbInfo, statement); + if (!this.statementInstrumenter.shouldStart(parentContext, request)) { + callable.call(); + return; + } + + Context context = this.statementInstrumenter.start(parentContext, request); + try (Scope ignored = context.makeCurrent()) { + callable.call(); + } catch (Throwable t) { + this.statementInstrumenter.end(context, request, null, t); + throw t; + } + this.statementInstrumenter.end(context, request, null, null); + } + + protected interface ThrowingSupplier { + String statement = null; + + void call() throws E; + } } diff --git a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java index a5d5eeea5ff6..fd79c789b84a 100644 --- a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java +++ b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java @@ -201,6 +201,48 @@ void testVerifyPrepareCallReturnsOtelWrapper() throws Exception { connection.close(); } + @Test + void testVerifyCommit() throws Exception { + Instrumenter instrumenter = + createStatementInstrumenter(testing.getOpenTelemetry()); + DbInfo dbInfo = getDbInfo(); + OpenTelemetryConnection connection = + new OpenTelemetryConnection(new TestConnection(), dbInfo, instrumenter); + String query = "COMMIT"; + Statement statement = connection.createStatement(); + + testing.runWithSpan( + "parent", + () -> { + assertThat(statement.execute(query)).isTrue(); + }); + transactionTraceAssertion(dbInfo, query); + + statement.close(); + connection.close(); + } + + @Test + void testVerifyRollback() throws Exception { + Instrumenter instrumenter = + createStatementInstrumenter(testing.getOpenTelemetry()); + DbInfo dbInfo = getDbInfo(); + OpenTelemetryConnection connection = + new OpenTelemetryConnection(new TestConnection(), dbInfo, instrumenter); + String query = "ROLLBACK"; + Statement statement = connection.createStatement(); + + testing.runWithSpan( + "parent", + () -> { + assertThat(statement.execute(query)).isTrue(); + }); + transactionTraceAssertion(dbInfo, query); + + statement.close(); + connection.close(); + } + private static DbInfo getDbInfo() { return DbInfo.builder() .system("my_system") @@ -239,4 +281,29 @@ private static void jdbcTraceAssertion(DbInfo dbInfo, String query) { equalTo(SERVER_ADDRESS, dbInfo.getHost()), equalTo(SERVER_PORT, dbInfo.getPort())))); } + + @SuppressWarnings("deprecation") // old semconv + private static void transactionTraceAssertion(DbInfo dbInfo, String query) { + testing.waitAndAssertTraces( + trace -> + trace.hasSpansSatisfyingExactly( + span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(), + span -> + span.hasName(query + " " + dbInfo.getName()) + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(0)) + .hasAttributesSatisfyingExactly( + equalTo( + maybeStable(DB_SYSTEM), + maybeStableDbSystemName(dbInfo.getSystem())), + equalTo(maybeStable(DB_NAME), dbInfo.getName()), + equalTo(DB_USER, emitStableDatabaseSemconv() ? null : dbInfo.getUser()), + equalTo( + DB_CONNECTION_STRING, + emitStableDatabaseSemconv() ? null : dbInfo.getShortUrl()), + equalTo(maybeStable(DB_STATEMENT), query), + equalTo(maybeStable(DB_OPERATION), query), + equalTo(SERVER_ADDRESS, dbInfo.getHost()), + equalTo(SERVER_PORT, dbInfo.getPort())))); + } } From fb2276447db21bf9e4baa368dcb6c292cfef333d Mon Sep 17 00:00:00 2001 From: stillya Date: Sun, 27 Apr 2025 23:45:25 +0500 Subject: [PATCH 2/9] Add new transaction instrumeter - fix review comments - add seperate transaction instrumenter --- .../src/main/jflex/SqlSanitizer.jflex | 20 ---- .../semconv/db/SqlStatementSanitizerTest.java | 28 ------ instrumentation/jdbc/README.md | 8 +- .../jdbc/javaagent/build.gradle.kts | 2 +- .../jdbc/ConnectionInstrumentation.java | 23 ++--- .../instrumentation/jdbc/JdbcSingletons.java | 47 +++++++-- .../jdbc/test/JdbcInstrumentationTest.java | 19 ++-- instrumentation/jdbc/library/build.gradle.kts | 2 +- .../jdbc/OpenTelemetryDriver.java | 10 +- .../jdbc/datasource/JdbcTelemetry.java | 11 ++- .../jdbc/datasource/JdbcTelemetryBuilder.java | 12 ++- .../datasource/OpenTelemetryDataSource.java | 12 ++- .../internal/JdbcInstrumenterFactory.java | 29 ++++-- .../internal/JdbcNetworkAttributesGetter.java | 11 ++- ...ava => JdbcStatementAttributesGetter.java} | 2 +- .../JdbcTransactionAttributesGetter.java | 69 +++++++++++++ .../internal/OpenTelemetryConnection.java | 60 ++++++------ .../StatementNetworkAttributesGetter.java | 30 ++++++ .../TransactionNetworkAttributesGetter.java | 31 ++++++ .../jdbc/internal/TransactionRequest.java | 29 ++++++ .../jdbc/datasource/JdbcTelemetryTest.java | 25 +++++ .../internal/OpenTelemetryConnectionTest.java | 98 +++++++++++-------- 22 files changed, 395 insertions(+), 183 deletions(-) rename instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/{JdbcAttributesGetter.java => JdbcStatementAttributesGetter.java} (93%) create mode 100644 instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcTransactionAttributesGetter.java create mode 100644 instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/StatementNetworkAttributesGetter.java create mode 100644 instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionNetworkAttributesGetter.java create mode 100644 instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionRequest.java diff --git a/instrumentation-api-incubator/src/main/jflex/SqlSanitizer.jflex b/instrumentation-api-incubator/src/main/jflex/SqlSanitizer.jflex index 1044375ae37b..83a59fc0abc9 100644 --- a/instrumentation-api-incubator/src/main/jflex/SqlSanitizer.jflex +++ b/instrumentation-api-incubator/src/main/jflex/SqlSanitizer.jflex @@ -347,12 +347,6 @@ WHITESPACE = [ \t\r\n]+ } } - private class Commit extends Operation { - } - - private class Rollback extends Operation { - } - private class Create extends DdlOperation { } @@ -491,20 +485,6 @@ WHITESPACE = [ \t\r\n]+ appendCurrentFragment(); if (isOverLimit()) return YYEOF; } - "COMMIT" { - if (!insideComment) { - setOperation(new Commit()); - } - appendCurrentFragment(); - if (isOverLimit()) return YYEOF; - } - "ROLLBACK" { - if (!insideComment) { - setOperation(new Rollback()); - } - appendCurrentFragment(); - if (isOverLimit()) return YYEOF; - } {COMMA} { if (!insideComment && !extractionDone) { diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlStatementSanitizerTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlStatementSanitizerTest.java index 454875d23ca5..8771ac1ed7c4 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlStatementSanitizerTest.java +++ b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlStatementSanitizerTest.java @@ -71,17 +71,6 @@ void checkDdlOperationStatementsAreOk( assertThat(result.getMainIdentifier()).isEqualTo(expected.getMainIdentifier()); } - @ParameterizedTest - @ArgumentsSource(TransactionArgs.class) - void checkTransactionOperationStatementsAreOk( - String actual, Function expectFunc) { - SqlStatementInfo result = SqlStatementSanitizer.create(true).sanitize(actual); - SqlStatementInfo expected = expectFunc.apply(actual); - assertThat(result.getFullStatement()).isEqualTo(expected.getFullStatement()); - assertThat(result.getOperation()).isEqualTo(expected.getOperation()); - assertThat(result.getMainIdentifier()).isEqualTo(expected.getMainIdentifier()); - } - @Test void lotsOfTicksDontCauseStackOverflowOrLongRuntimes() { String s = "'"; @@ -438,21 +427,4 @@ public Stream provideArguments(ExtensionContext context) { "CREATE PROCEDURE p AS SELECT * FROM table GO", expect("CREATE PROCEDURE", null))); } } - - static class TransactionArgs implements ArgumentsProvider { - static Function expect(String operation, String identifier) { - return sql -> SqlStatementInfo.create(sql, operation, identifier); - } - - @Override - public Stream provideArguments(ExtensionContext context) { - return Stream.of( - Arguments.of("COMMIT", expect("COMMIT", null)), - Arguments.of("commit", expect("COMMIT", null)), - Arguments.of("/*COMMIT*/", expect(null, null)), - Arguments.of("ROLLBACK", expect("ROLLBACK", null)), - Arguments.of("rollback", expect("ROLLBACK", null)), - Arguments.of("/*ROLLBACK*/", expect(null, null))); - } - } } diff --git a/instrumentation/jdbc/README.md b/instrumentation/jdbc/README.md index 1841f4657ea5..7cffb26d4e92 100644 --- a/instrumentation/jdbc/README.md +++ b/instrumentation/jdbc/README.md @@ -1,6 +1,6 @@ # Settings for the JDBC instrumentation -| System property | Type | Default | Description | -|---------------------------------------------------------|---------|---------|-------------------------------------------------------| -| `otel.instrumentation.jdbc.statement-sanitizer.enabled` | Boolean | `true` | Enables the DB statement sanitization. | -| `otel.instrumentation.jdbc.experimental.txn.enabled` | Boolean | `false` | Enables the experimental transaction instrumentation. | +| System property | Type | Default | Description | +|--------------------------------------------------------------|---------|---------|------------------------------------------------------------------------------------------| +| `otel.instrumentation.jdbc.statement-sanitizer.enabled` | Boolean | `true` | Enables the DB statement sanitization. | +| `otel.instrumentation.jdbc.experimental.transaction.enabled` | Boolean | `false` | Enables experimental instrumentation to create spans for COMMIT and ROLLBACK operations. | \ No newline at end of file diff --git a/instrumentation/jdbc/javaagent/build.gradle.kts b/instrumentation/jdbc/javaagent/build.gradle.kts index 43ebc079eddd..10720fb42a6c 100644 --- a/instrumentation/jdbc/javaagent/build.gradle.kts +++ b/instrumentation/jdbc/javaagent/build.gradle.kts @@ -94,6 +94,6 @@ tasks { tasks { withType().configureEach { - jvmArgs("-Dotel.instrumentation.jdbc.experimental.txn.enabled=true") + jvmArgs("-Dotel.instrumentation.jdbc.experimental.transaction.enabled=true") } } diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java index 27cb634db86e..c49c16be2058 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java @@ -8,7 +8,7 @@ import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; -import static io.opentelemetry.javaagent.instrumentation.jdbc.JdbcSingletons.statementInstrumenter; +import static io.opentelemetry.javaagent.instrumentation.jdbc.JdbcSingletons.transactionInstrumenter; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -19,10 +19,9 @@ 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.instrumentation.jdbc.internal.JdbcUtils; -import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig; +import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest; import io.opentelemetry.javaagent.bootstrap.jdbc.DbInfo; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; @@ -35,10 +34,6 @@ public class ConnectionInstrumentation implements TypeInstrumentation { - private static final boolean TXN_ENABLED = - AgentInstrumentationConfig.get() - .getBoolean("otel.instrumentation.jdbc.experimental.txn.enabled", false); - @Override public ElementMatcher classLoaderOptimization() { return hasClassesNamed("java.sql.Connection"); @@ -57,11 +52,9 @@ public void transform(TypeTransformer transformer) { // Also include CallableStatement, which is a sub type of PreparedStatement .and(returns(implementsInterface(named("java.sql.PreparedStatement")))), ConnectionInstrumentation.class.getName() + "$PrepareAdvice"); - if (TXN_ENABLED) { transformer.applyAdviceToMethod( namedOneOf("commit", "rollback").and(takesNoArguments()).and(isPublic()), ConnectionInstrumentation.class.getName() + "$TxnAdvice"); - } } @SuppressWarnings("unused") @@ -92,28 +85,28 @@ public static void onEnter( if (dbInfo == null) { return; } - DbRequest request = DbRequest.create(dbInfo, methodName.toUpperCase(Locale.ROOT)); + TransactionRequest request = + TransactionRequest.create(dbInfo, methodName.toUpperCase(Locale.ROOT)); - if (!statementInstrumenter().shouldStart(parentContext, request)) { + if (!transactionInstrumenter().shouldStart(parentContext, request)) { return; } - request = DbRequest.create(request.getDbInfo(), methodName.toUpperCase(Locale.ROOT)); - context = statementInstrumenter().start(parentContext, request); + context = transactionInstrumenter().start(parentContext, request); scope = context.makeCurrent(); } @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( @Advice.Thrown Throwable throwable, - @Advice.Local("otelRequest") DbRequest request, + @Advice.Local("otelRequest") TransactionRequest request, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { if (scope == null) { return; } scope.close(); - statementInstrumenter().end(context, request, null, throwable); + transactionInstrumenter().end(context, request, null, throwable); } } } diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java index e9c5290aa217..3a131146b816 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java @@ -16,8 +16,11 @@ import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor; import io.opentelemetry.instrumentation.api.semconv.network.ServerAttributesExtractor; import io.opentelemetry.instrumentation.jdbc.internal.DbRequest; -import io.opentelemetry.instrumentation.jdbc.internal.JdbcAttributesGetter; -import io.opentelemetry.instrumentation.jdbc.internal.JdbcNetworkAttributesGetter; +import io.opentelemetry.instrumentation.jdbc.internal.JdbcStatementAttributesGetter; +import io.opentelemetry.instrumentation.jdbc.internal.JdbcTransactionAttributesGetter; +import io.opentelemetry.instrumentation.jdbc.internal.StatementNetworkAttributesGetter; +import io.opentelemetry.instrumentation.jdbc.internal.TransactionNetworkAttributesGetter; +import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest; import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig; import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig; import io.opentelemetry.javaagent.bootstrap.jdbc.DbInfo; @@ -27,32 +30,60 @@ public final class JdbcSingletons { private static final String INSTRUMENTATION_NAME = "io.opentelemetry.jdbc"; private static final Instrumenter STATEMENT_INSTRUMENTER; + private static final Instrumenter TRANSACTION_INSTRUMENTER; public static final Instrumenter DATASOURCE_INSTRUMENTER = createDataSourceInstrumenter(GlobalOpenTelemetry.get(), true); static { - JdbcAttributesGetter dbAttributesGetter = new JdbcAttributesGetter(); - JdbcNetworkAttributesGetter netAttributesGetter = new JdbcNetworkAttributesGetter(); + JdbcStatementAttributesGetter statementAttributesGetter = new JdbcStatementAttributesGetter(); + JdbcTransactionAttributesGetter transactionAttributesGetter = + new JdbcTransactionAttributesGetter(); + StatementNetworkAttributesGetter statementNetAttributesGetter = + new StatementNetworkAttributesGetter(); + TransactionNetworkAttributesGetter transactionNetAttributesGetter = + new TransactionNetworkAttributesGetter(); STATEMENT_INSTRUMENTER = Instrumenter.builder( GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, - DbClientSpanNameExtractor.create(dbAttributesGetter)) + DbClientSpanNameExtractor.create(statementAttributesGetter)) .addAttributesExtractor( - SqlClientAttributesExtractor.builder(dbAttributesGetter) + SqlClientAttributesExtractor.builder(statementAttributesGetter) .setStatementSanitizationEnabled( AgentInstrumentationConfig.get() .getBoolean( "otel.instrumentation.jdbc.statement-sanitizer.enabled", AgentCommonConfig.get().isStatementSanitizationEnabled())) .build()) - .addAttributesExtractor(ServerAttributesExtractor.create(netAttributesGetter)) + .addAttributesExtractor(ServerAttributesExtractor.create(statementNetAttributesGetter)) .addAttributesExtractor( PeerServiceAttributesExtractor.create( - netAttributesGetter, AgentCommonConfig.get().getPeerServiceResolver())) + statementNetAttributesGetter, AgentCommonConfig.get().getPeerServiceResolver())) .addOperationMetrics(DbClientMetrics.get()) .buildInstrumenter(SpanKindExtractor.alwaysClient()); + + TRANSACTION_INSTRUMENTER = + Instrumenter.builder( + GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, TransactionRequest::spanName) + .addAttributesExtractor( + SqlClientAttributesExtractor.builder(transactionAttributesGetter).build()) + .addAttributesExtractor( + ServerAttributesExtractor.create(transactionNetAttributesGetter)) + .addAttributesExtractor( + PeerServiceAttributesExtractor.create( + transactionNetAttributesGetter, + AgentCommonConfig.get().getPeerServiceResolver())) + .addOperationMetrics(DbClientMetrics.get()) + .setEnabled( + AgentInstrumentationConfig.get() + .getBoolean( + "otel.instrumentation.jdbc.experimental.transaction.enabled", false)) + .buildInstrumenter(SpanKindExtractor.alwaysClient()); + } + + public static Instrumenter transactionInstrumenter() { + return TRANSACTION_INSTRUMENTER; } public static Instrumenter statementInstrumenter() { 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 efa3096f2830..02755d9f7cd5 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 @@ -19,7 +19,6 @@ 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_OPERATION_NAME; -import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_QUERY_TEXT; 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; @@ -1681,18 +1680,16 @@ DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), equalTo(maybeStable(DB_OPERATION), "INSERT"), equalTo(maybeStable(DB_SQL_TABLE), tableName)), span -> - span.hasName("COMMIT " + dbNameLower) + span.hasName("COMMIT") .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) .hasAttributesSatisfyingExactly( equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)), equalTo(maybeStable(DB_NAME), dbNameLower), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), - equalTo(DB_STATEMENT, emitStableDatabaseSemconv() ? null : "COMMIT"), - equalTo(DB_QUERY_TEXT, emitStableDatabaseSemconv() ? "COMMIT" : null), equalTo( - DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), - equalTo(maybeStable(DB_OPERATION), "COMMIT")))); + DB_CONNECTION_STRING, + emitStableDatabaseSemconv() ? null : url)))); } finally { connection.setAutoCommit(originalAutoCommit); } @@ -1745,7 +1742,7 @@ DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), equalTo(maybeStable(DB_OPERATION), "INSERT"), equalTo(maybeStable(DB_SQL_TABLE), tableName)), span -> - span.hasName("ROLLBACK " + dbNameLower) + span.hasName("ROLLBACK") .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) .hasAttributesSatisfyingExactly( @@ -1753,12 +1750,8 @@ DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), equalTo(maybeStable(DB_NAME), dbNameLower), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), equalTo( - DB_STATEMENT, emitStableDatabaseSemconv() ? null : "ROLLBACK"), - equalTo( - DB_QUERY_TEXT, emitStableDatabaseSemconv() ? "ROLLBACK" : null), - equalTo( - DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), - equalTo(maybeStable(DB_OPERATION), "ROLLBACK")))); + DB_CONNECTION_STRING, + emitStableDatabaseSemconv() ? null : url)))); Statement selectStatement = connection.createStatement(); cleanup.deferCleanup(selectStatement); diff --git a/instrumentation/jdbc/library/build.gradle.kts b/instrumentation/jdbc/library/build.gradle.kts index 5b3004e0fa88..03e0f372c10d 100644 --- a/instrumentation/jdbc/library/build.gradle.kts +++ b/instrumentation/jdbc/library/build.gradle.kts @@ -62,6 +62,6 @@ tasks { tasks { withType().configureEach { - jvmArgs("-Dotel.instrumentation.jdbc.experimental.txn.enabled=true") + jvmArgs("-Dotel.instrumentation.jdbc.experimental.transaction.enabled=true") } } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java index d50039ca9bdd..b2e1ca6a27fb 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java @@ -24,11 +24,13 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil; import io.opentelemetry.instrumentation.api.internal.EmbeddedInstrumentationProperties; import io.opentelemetry.instrumentation.jdbc.internal.DbRequest; import io.opentelemetry.instrumentation.jdbc.internal.JdbcConnectionUrlParser; import io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory; import io.opentelemetry.instrumentation.jdbc.internal.OpenTelemetryConnection; +import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; import java.sql.Connection; import java.sql.Driver; @@ -244,7 +246,13 @@ public Connection connect(String url, Properties info) throws SQLException { Instrumenter statementInstrumenter = JdbcInstrumenterFactory.createStatementInstrumenter(openTelemetry); - return OpenTelemetryConnection.create(connection, dbInfo, statementInstrumenter); + Instrumenter transactionInstrumenter = + JdbcInstrumenterFactory.createTransactionInstrumenter( + openTelemetry, + ConfigPropertiesUtil.getBoolean( + "otel.instrumentation.jdbc.experimental.transaction.enabled", false)); + return OpenTelemetryConnection.create( + connection, dbInfo, statementInstrumenter, transactionInstrumenter); } @Override diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetry.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetry.java index cc857b823563..c0e85580792f 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetry.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetry.java @@ -8,6 +8,7 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.jdbc.internal.DbRequest; +import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; import javax.sql.DataSource; @@ -26,16 +27,22 @@ public static JdbcTelemetryBuilder builder(OpenTelemetry openTelemetry) { private final Instrumenter dataSourceInstrumenter; private final Instrumenter statementInstrumenter; + private final Instrumenter transactionInstrumenter; JdbcTelemetry( Instrumenter dataSourceInstrumenter, - Instrumenter statementInstrumenter) { + Instrumenter statementInstrumenter, + Instrumenter transactionInstrumenter) { this.dataSourceInstrumenter = dataSourceInstrumenter; this.statementInstrumenter = statementInstrumenter; + this.transactionInstrumenter = transactionInstrumenter; } public DataSource wrap(DataSource dataSource) { return new OpenTelemetryDataSource( - dataSource, this.dataSourceInstrumenter, this.statementInstrumenter); + dataSource, + this.dataSourceInstrumenter, + this.statementInstrumenter, + this.transactionInstrumenter); } } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryBuilder.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryBuilder.java index 825b29547334..347f763e0dc4 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryBuilder.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryBuilder.java @@ -16,6 +16,7 @@ public final class JdbcTelemetryBuilder { private boolean dataSourceInstrumenterEnabled = true; private boolean statementInstrumenterEnabled = true; private boolean statementSanitizationEnabled = true; + private boolean transactionInstrumenterEnabled = false; JdbcTelemetryBuilder(OpenTelemetry openTelemetry) { this.openTelemetry = openTelemetry; @@ -42,12 +43,21 @@ public JdbcTelemetryBuilder setStatementSanitizationEnabled(boolean enabled) { return this; } + /** Configures whether spans are created for JDBC Transactions. Enabled by default. */ + @CanIgnoreReturnValue + public JdbcTelemetryBuilder setTransactionInstrumenterEnabled(boolean enabled) { + this.transactionInstrumenterEnabled = enabled; + return this; + } + /** Returns a new {@link JdbcTelemetry} with the settings of this {@link JdbcTelemetryBuilder}. */ public JdbcTelemetry build() { return new JdbcTelemetry( JdbcInstrumenterFactory.createDataSourceInstrumenter( openTelemetry, dataSourceInstrumenterEnabled), JdbcInstrumenterFactory.createStatementInstrumenter( - openTelemetry, statementInstrumenterEnabled, statementSanitizationEnabled)); + openTelemetry, statementInstrumenterEnabled, statementSanitizationEnabled), + JdbcInstrumenterFactory.createTransactionInstrumenter( + openTelemetry, transactionInstrumenterEnabled)); } } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java index deb0425ff223..a2507a520eaf 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java @@ -22,6 +22,7 @@ import static io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory.createDataSourceInstrumenter; import static io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory.createStatementInstrumenter; +import static io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory.createTransactionInstrumenter; import static io.opentelemetry.instrumentation.jdbc.internal.JdbcUtils.computeDbInfo; import io.opentelemetry.api.GlobalOpenTelemetry; @@ -33,6 +34,7 @@ import io.opentelemetry.instrumentation.jdbc.internal.DbRequest; import io.opentelemetry.instrumentation.jdbc.internal.OpenTelemetryConnection; import io.opentelemetry.instrumentation.jdbc.internal.ThrowingSupplier; +import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; import java.io.PrintWriter; import java.sql.Connection; @@ -47,6 +49,7 @@ public class OpenTelemetryDataSource implements DataSource, AutoCloseable { private final DataSource delegate; private final Instrumenter dataSourceInstrumenter; private final Instrumenter statementInstrumenter; + private final Instrumenter transactionInstrumenter; private volatile DbInfo cachedDbInfo; /** @@ -71,6 +74,7 @@ public OpenTelemetryDataSource(DataSource delegate, OpenTelemetry openTelemetry) this.delegate = delegate; this.dataSourceInstrumenter = createDataSourceInstrumenter(openTelemetry, true); this.statementInstrumenter = createStatementInstrumenter(openTelemetry); + this.transactionInstrumenter = createTransactionInstrumenter(openTelemetry, false); } /** @@ -83,24 +87,26 @@ public OpenTelemetryDataSource(DataSource delegate, OpenTelemetry openTelemetry) OpenTelemetryDataSource( DataSource delegate, Instrumenter dataSourceInstrumenter, - Instrumenter statementInstrumenter) { + Instrumenter statementInstrumenter, + Instrumenter transactionInstrumenter) { this.delegate = delegate; this.dataSourceInstrumenter = dataSourceInstrumenter; this.statementInstrumenter = statementInstrumenter; + this.transactionInstrumenter = transactionInstrumenter; } @Override public Connection getConnection() throws SQLException { Connection connection = wrapCall(delegate::getConnection); DbInfo dbInfo = getDbInfo(connection); - return OpenTelemetryConnection.create(connection, dbInfo, statementInstrumenter); + return OpenTelemetryConnection.create(connection, dbInfo, statementInstrumenter, transactionInstrumenter); } @Override public Connection getConnection(String username, String password) throws SQLException { Connection connection = wrapCall(() -> delegate.getConnection(username, password)); DbInfo dbInfo = getDbInfo(connection); - return OpenTelemetryConnection.create(connection, dbInfo, statementInstrumenter); + return OpenTelemetryConnection.create(connection, dbInfo, statementInstrumenter, transactionInstrumenter); } @Override diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java index 861e1dd11759..94a7ac9cb887 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java @@ -25,9 +25,14 @@ */ public final class JdbcInstrumenterFactory { public static final String INSTRUMENTATION_NAME = "io.opentelemetry.jdbc"; - private static final JdbcAttributesGetter dbAttributesGetter = new JdbcAttributesGetter(); - private static final JdbcNetworkAttributesGetter netAttributesGetter = - new JdbcNetworkAttributesGetter(); + private static final JdbcStatementAttributesGetter statementAttributesGetter = + new JdbcStatementAttributesGetter(); + private static final JdbcTransactionAttributesGetter transactionAttributesGetter = + new JdbcTransactionAttributesGetter(); + private static final StatementNetworkAttributesGetter statementNetAttributesGetter = + new StatementNetworkAttributesGetter(); + private static final TransactionNetworkAttributesGetter transactionNetAttributesGetter = + new TransactionNetworkAttributesGetter(); public static Instrumenter createStatementInstrumenter() { return createStatementInstrumenter(GlobalOpenTelemetry.get()); @@ -47,12 +52,12 @@ public static Instrumenter createStatementInstrumenter( return Instrumenter.builder( openTelemetry, INSTRUMENTATION_NAME, - DbClientSpanNameExtractor.create(dbAttributesGetter)) + DbClientSpanNameExtractor.create(statementAttributesGetter)) .addAttributesExtractor( - SqlClientAttributesExtractor.builder(dbAttributesGetter) + SqlClientAttributesExtractor.builder(statementAttributesGetter) .setStatementSanitizationEnabled(statementSanitizationEnabled) .build()) - .addAttributesExtractor(ServerAttributesExtractor.create(netAttributesGetter)) + .addAttributesExtractor(ServerAttributesExtractor.create(statementNetAttributesGetter)) .addOperationMetrics(DbClientMetrics.get()) .setEnabled(enabled) .buildInstrumenter(SpanKindExtractor.alwaysClient()); @@ -69,5 +74,17 @@ public static Instrumenter createDataSourceInstrumenter( .buildInstrumenter(); } + public static Instrumenter createTransactionInstrumenter( + OpenTelemetry openTelemetry, boolean enabled) { + return Instrumenter.builder( + openTelemetry, INSTRUMENTATION_NAME, TransactionRequest::spanName) + .addAttributesExtractor( + SqlClientAttributesExtractor.builder(transactionAttributesGetter).build()) + .addAttributesExtractor(ServerAttributesExtractor.create(transactionNetAttributesGetter)) + .addOperationMetrics(DbClientMetrics.get()) + .setEnabled(enabled) + .buildInstrumenter(SpanKindExtractor.alwaysClient()); + } + private JdbcInstrumenterFactory() {} } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcNetworkAttributesGetter.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcNetworkAttributesGetter.java index a09ca312672b..23ba03b82f68 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcNetworkAttributesGetter.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcNetworkAttributesGetter.java @@ -6,23 +6,24 @@ package io.opentelemetry.instrumentation.jdbc.internal; import io.opentelemetry.instrumentation.api.semconv.network.ServerAttributesGetter; +import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; 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 JdbcNetworkAttributesGetter implements ServerAttributesGetter { +public final class JdbcNetworkAttributesGetter implements ServerAttributesGetter { @Nullable @Override - public String getServerAddress(DbRequest request) { - return request.getDbInfo().getHost(); + public String getServerAddress(DbInfo request) { + return request.getHost(); } @Nullable @Override - public Integer getServerPort(DbRequest request) { - return request.getDbInfo().getPort(); + public Integer getServerPort(DbInfo request) { + return request.getPort(); } } 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/JdbcStatementAttributesGetter.java similarity index 93% rename from instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcAttributesGetter.java rename to instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcStatementAttributesGetter.java index 74cd1c52d51a..0e0293985650 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/JdbcStatementAttributesGetter.java @@ -15,7 +15,7 @@ * 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 JdbcStatementAttributesGetter implements SqlClientAttributesGetter { @Nullable @Override diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcTransactionAttributesGetter.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcTransactionAttributesGetter.java new file mode 100644 index 000000000000..14d6d73ec15c --- /dev/null +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcTransactionAttributesGetter.java @@ -0,0 +1,69 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jdbc.internal; + +import static java.util.Collections.emptySet; + +import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlClientAttributesGetter; +import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; +import java.sql.SQLException; +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 class JdbcTransactionAttributesGetter + implements SqlClientAttributesGetter { + + @Nullable + @Override + public String getDbSystem(TransactionRequest request) { + return request.getDbInfo().getSystem(); + } + + @Deprecated + @Nullable + @Override + public String getUser(TransactionRequest request) { + return request.getDbInfo().getUser(); + } + + @Nullable + @Override + public String getDbNamespace(TransactionRequest request) { + DbInfo dbInfo = request.getDbInfo(); + return dbInfo.getName() == null ? dbInfo.getDb() : dbInfo.getName(); + } + + @Deprecated + @Nullable + @Override + public String getConnectionString(TransactionRequest request) { + return request.getDbInfo().getShortUrl(); + } + + @Override + public Collection getRawQueryTexts(TransactionRequest request) { + return emptySet(); + } + + @Nullable + @Override + public Long getBatchSize(TransactionRequest request) { + return null; + } + + @Nullable + @Override + public String getResponseStatus(@Nullable Void response, @Nullable Throwable error) { + if (error instanceof SQLException) { + return Integer.toString(((SQLException) error).getErrorCode()); + } + return null; + } +} diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java index e3f8ce448316..0e3f8603ec95 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java @@ -23,7 +23,6 @@ import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; import java.sql.Array; import java.sql.Blob; @@ -50,19 +49,22 @@ * any time. */ public class OpenTelemetryConnection implements Connection { - - private static final boolean TXN_ENABLED = - ConfigPropertiesUtil.getBoolean("otel.instrumentation.jdbc.experimental.txn.enabled", false); + private static final boolean hasJdbc43 = hasJdbc43(); protected final Connection delegate; private final DbInfo dbInfo; protected final Instrumenter statementInstrumenter; + protected final Instrumenter transactionInstrumenter; protected OpenTelemetryConnection( - Connection delegate, DbInfo dbInfo, Instrumenter statementInstrumenter) { + Connection delegate, + DbInfo dbInfo, + Instrumenter statementInstrumenter, + Instrumenter transactionInstrumenter) { this.delegate = delegate; this.dbInfo = dbInfo; this.statementInstrumenter = statementInstrumenter; + this.transactionInstrumenter = transactionInstrumenter; } // visible for testing @@ -76,11 +78,16 @@ static boolean hasJdbc43() { } public static Connection create( - Connection delegate, DbInfo dbInfo, Instrumenter statementInstrumenter) { + Connection delegate, + DbInfo dbInfo, + Instrumenter statementInstrumenter, + Instrumenter transactionInstrumenter) { if (hasJdbc43) { - return new OpenTelemetryConnectionJdbc43(delegate, dbInfo, statementInstrumenter); + return new OpenTelemetryConnectionJdbc43( + delegate, dbInfo, statementInstrumenter, transactionInstrumenter); } - return new OpenTelemetryConnection(delegate, dbInfo, statementInstrumenter); + return new OpenTelemetryConnection( + delegate, dbInfo, statementInstrumenter, transactionInstrumenter); } @Override @@ -178,11 +185,7 @@ public CallableStatement prepareCall( @Override public void commit() throws SQLException { - if (TXN_ENABLED) { - wrapCall(delegate::commit, "COMMIT"); - } else { - delegate.commit(); - } + wrapCall(delegate::commit, "COMMIT"); } @Override @@ -288,21 +291,13 @@ public Savepoint setSavepoint(String name) throws SQLException { @SuppressWarnings("UngroupedOverloads") @Override public void rollback() throws SQLException { - if (TXN_ENABLED) { - wrapCall(delegate::rollback, "ROLLBACK"); - } else { - delegate.rollback(); - } + wrapCall(delegate::rollback, "ROLLBACK"); } @SuppressWarnings("UngroupedOverloads") @Override public void rollback(Savepoint savepoint) throws SQLException { - if (TXN_ENABLED) { - wrapCall(() -> delegate.rollback(savepoint), "ROLLBACK"); - } else { - delegate.rollback(savepoint); - } + wrapCall(() -> delegate.rollback(savepoint), "ROLLBACK"); } @Override @@ -410,8 +405,11 @@ public DbInfo getDbInfo() { // JDBC 4.3 static class OpenTelemetryConnectionJdbc43 extends OpenTelemetryConnection { OpenTelemetryConnectionJdbc43( - Connection delegate, DbInfo dbInfo, Instrumenter statementInstrumenter) { - super(delegate, dbInfo, statementInstrumenter); + Connection delegate, + DbInfo dbInfo, + Instrumenter statementInstrumenter, + Instrumenter transactionInstrumenter) { + super(delegate, dbInfo, statementInstrumenter, transactionInstrumenter); } @SuppressWarnings("Since15") @@ -453,23 +451,23 @@ public void setShardingKey(ShardingKey shardingKey) throws SQLException { } } - protected void wrapCall(ThrowingSupplier callable, String statement) + protected void wrapCall(ThrowingSupplier callable, String operation) throws E { Context parentContext = Context.current(); - DbRequest request = DbRequest.create(dbInfo, statement); - if (!this.statementInstrumenter.shouldStart(parentContext, request)) { + TransactionRequest request = TransactionRequest.create(dbInfo, operation); + if (!this.transactionInstrumenter.shouldStart(parentContext, request)) { callable.call(); return; } - Context context = this.statementInstrumenter.start(parentContext, request); + Context context = this.transactionInstrumenter.start(parentContext, request); try (Scope ignored = context.makeCurrent()) { callable.call(); } catch (Throwable t) { - this.statementInstrumenter.end(context, request, null, t); + this.transactionInstrumenter.end(context, request, null, t); throw t; } - this.statementInstrumenter.end(context, request, null, null); + this.transactionInstrumenter.end(context, request, null, null); } protected interface ThrowingSupplier { diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/StatementNetworkAttributesGetter.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/StatementNetworkAttributesGetter.java new file mode 100644 index 000000000000..0b7011cfb3bd --- /dev/null +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/StatementNetworkAttributesGetter.java @@ -0,0 +1,30 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jdbc.internal; + +import io.opentelemetry.instrumentation.api.semconv.network.ServerAttributesGetter; +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 StatementNetworkAttributesGetter implements ServerAttributesGetter { + private final JdbcNetworkAttributesGetter jdbcNetworkAttributesGetter = + new JdbcNetworkAttributesGetter(); + + @Nullable + @Override + public String getServerAddress(DbRequest request) { + return jdbcNetworkAttributesGetter.getServerAddress(request.getDbInfo()); + } + + @Nullable + @Override + public Integer getServerPort(DbRequest request) { + return jdbcNetworkAttributesGetter.getServerPort(request.getDbInfo()); + } +} diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionNetworkAttributesGetter.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionNetworkAttributesGetter.java new file mode 100644 index 000000000000..0c6da65177f7 --- /dev/null +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionNetworkAttributesGetter.java @@ -0,0 +1,31 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jdbc.internal; + +import io.opentelemetry.instrumentation.api.semconv.network.ServerAttributesGetter; +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 TransactionNetworkAttributesGetter + implements ServerAttributesGetter { + private final JdbcNetworkAttributesGetter jdbcNetworkAttributesGetter = + new JdbcNetworkAttributesGetter(); + + @Nullable + @Override + public String getServerAddress(TransactionRequest request) { + return jdbcNetworkAttributesGetter.getServerAddress(request.getDbInfo()); + } + + @Nullable + @Override + public Integer getServerPort(TransactionRequest request) { + return jdbcNetworkAttributesGetter.getServerPort(request.getDbInfo()); + } +} diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionRequest.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionRequest.java new file mode 100644 index 000000000000..9fc1c98786b1 --- /dev/null +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionRequest.java @@ -0,0 +1,29 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jdbc.internal; + +import com.google.auto.value.AutoValue; +import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; + +/** + * This class is internal and is hence not for public use. Its APIs are unstable and can change at + * any time. + */ +@AutoValue +public abstract class TransactionRequest { + + public static TransactionRequest create(DbInfo dbInfo, String operation) { + return new AutoValue_TransactionRequest(dbInfo, operation); + } + + public abstract DbInfo getDbInfo(); + + public abstract String operation(); + + public static String spanName(TransactionRequest request) { + return request.operation(); + } +} 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 f3ec381ade4a..145f8e07df7d 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 @@ -130,6 +130,7 @@ void buildWithAllInstrumentersDisabled() throws SQLException { JdbcTelemetry.builder(testing.getOpenTelemetry()) .setDataSourceInstrumenterEnabled(false) .setStatementInstrumenterEnabled(false) + .setTransactionInstrumenterEnabled(false) .build(); DataSource dataSource = telemetry.wrap(new TestDataSource()); @@ -178,6 +179,30 @@ void buildWithStatementInstrumenterDisabled() throws SQLException { span -> span.hasName("TestDataSource.getConnection"))); } + @Test + void buildWithTransactionInstrumenterDisabled() throws SQLException { + JdbcTelemetry telemetry = + JdbcTelemetry.builder(testing.getOpenTelemetry()) + .setTransactionInstrumenterEnabled(false) + .build(); + + DataSource dataSource = telemetry.wrap(new TestDataSource()); + + testing.runWithSpan( + "parent", + () -> { + Connection connection = dataSource.getConnection(); + connection.commit(); + connection.rollback(); + }); + + testing.waitAndAssertTraces( + trace -> + trace.hasSpansSatisfyingExactly( + span -> span.hasName("parent"), + span -> span.hasName("TestDataSource.getConnection"))); + } + @Test void buildWithSanitizationDisabled() throws SQLException { JdbcTelemetry telemetry = diff --git a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java index fd79c789b84a..d0d7be64dfbc 100644 --- a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java +++ b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java @@ -7,6 +7,7 @@ import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableDatabaseSemconv; import static io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory.createStatementInstrumenter; +import static io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory.createTransactionInstrumenter; import static io.opentelemetry.instrumentation.testing.junit.db.SemconvStabilityUtil.maybeStable; import static io.opentelemetry.instrumentation.testing.junit.db.SemconvStabilityUtil.maybeStableDbSystemName; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; @@ -43,11 +44,14 @@ class OpenTelemetryConnectionTest { @Test void testVerifyCreateStatement() throws SQLException { - Instrumenter instrumenter = + Instrumenter statementInstrumenter = createStatementInstrumenter(testing.getOpenTelemetry()); + Instrumenter transactionInstrumenter = + createTransactionInstrumenter(testing.getOpenTelemetry(), true); DbInfo dbInfo = getDbInfo(); OpenTelemetryConnection connection = - new OpenTelemetryConnection(new TestConnection(), dbInfo, instrumenter); + new OpenTelemetryConnection( + new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); String query = "SELECT * FROM users"; Statement statement = connection.createStatement(); @@ -67,26 +71,32 @@ void testVerifyCreateStatement() throws SQLException { @Test void testVerifyCreateStatementReturnsOtelWrapper() throws Exception { OpenTelemetry ot = OpenTelemetry.propagating(ContextPropagators.noop()); - Instrumenter instrumenter = createStatementInstrumenter(ot); + Instrumenter statementInstrumenter = createStatementInstrumenter(ot); + Instrumenter transactionInstrumenter = + createTransactionInstrumenter(ot, true); OpenTelemetryConnection connection = - new OpenTelemetryConnection(new TestConnection(), DbInfo.DEFAULT, instrumenter); + new OpenTelemetryConnection( + new TestConnection(), DbInfo.DEFAULT, statementInstrumenter, transactionInstrumenter); assertThat(connection.createStatement()).isInstanceOf(OpenTelemetryStatement.class); assertThat(connection.createStatement(0, 0)).isInstanceOf(OpenTelemetryStatement.class); assertThat(connection.createStatement(0, 0, 0)).isInstanceOf(OpenTelemetryStatement.class); assertThat(((OpenTelemetryStatement) connection.createStatement()).instrumenter) - .isEqualTo(instrumenter); + .isEqualTo(statementInstrumenter); connection.close(); } @Test void testVerifyPrepareStatement() throws SQLException { - Instrumenter instrumenter = + Instrumenter statementInstrumenter = createStatementInstrumenter(testing.getOpenTelemetry()); + Instrumenter transactionInstrumenter = + createTransactionInstrumenter(testing.getOpenTelemetry(), true); DbInfo dbInfo = getDbInfo(); OpenTelemetryConnection connection = - new OpenTelemetryConnection(new TestConnection(), dbInfo, instrumenter); + new OpenTelemetryConnection( + new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); String query = "SELECT * FROM users"; PreparedStatement statement = connection.prepareStatement(query); @@ -107,11 +117,14 @@ void testVerifyPrepareStatement() throws SQLException { @Test void testVerifyPrepareStatementQuery() throws SQLException { - Instrumenter instrumenter = + Instrumenter statementInstrumenter = createStatementInstrumenter(testing.getOpenTelemetry()); + Instrumenter transactionInstrumenter = + createTransactionInstrumenter(testing.getOpenTelemetry(), true); DbInfo dbInfo = getDbInfo(); OpenTelemetryConnection connection = - new OpenTelemetryConnection(new TestConnection(), dbInfo, instrumenter); + new OpenTelemetryConnection( + new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); String query = "SELECT * FROM users"; PreparedStatement statement = connection.prepareStatement(query); @@ -133,9 +146,12 @@ void testVerifyPrepareStatementQuery() throws SQLException { @Test void testVerifyPrepareStatementReturnsOtelWrapper() throws Exception { OpenTelemetry ot = OpenTelemetry.propagating(ContextPropagators.noop()); - Instrumenter instrumenter = createStatementInstrumenter(ot); + Instrumenter statementInstrumenter = createStatementInstrumenter(ot); + Instrumenter transactionInstrumenter = + createTransactionInstrumenter(ot, true); OpenTelemetryConnection connection = - new OpenTelemetryConnection(new TestConnection(), DbInfo.DEFAULT, instrumenter); + new OpenTelemetryConnection( + new TestConnection(), DbInfo.DEFAULT, statementInstrumenter, transactionInstrumenter); String query = "SELECT * FROM users"; assertThat(connection.prepareStatement(query)) @@ -152,18 +168,21 @@ void testVerifyPrepareStatementReturnsOtelWrapper() throws Exception { .isInstanceOf(OpenTelemetryPreparedStatement.class); assertThat( ((OpenTelemetryStatement) connection.prepareStatement(query)).instrumenter) - .isEqualTo(instrumenter); + .isEqualTo(statementInstrumenter); connection.close(); } @Test void testVerifyPrepareCall() throws SQLException { - Instrumenter instrumenter = + Instrumenter statementInstrumenter = createStatementInstrumenter(testing.getOpenTelemetry()); + Instrumenter transactionInstrumenter = + createTransactionInstrumenter(testing.getOpenTelemetry(), true); DbInfo dbInfo = getDbInfo(); OpenTelemetryConnection connection = - new OpenTelemetryConnection(new TestConnection(), dbInfo, instrumenter); + new OpenTelemetryConnection( + new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); String query = "SELECT * FROM users"; PreparedStatement statement = connection.prepareCall(query); @@ -183,9 +202,12 @@ void testVerifyPrepareCall() throws SQLException { @Test void testVerifyPrepareCallReturnsOtelWrapper() throws Exception { OpenTelemetry ot = OpenTelemetry.propagating(ContextPropagators.noop()); - Instrumenter instrumenter = createStatementInstrumenter(ot); + Instrumenter statementInstrumenter = createStatementInstrumenter(ot); + Instrumenter transactionInstrumenter = + createTransactionInstrumenter(ot, true); OpenTelemetryConnection connection = - new OpenTelemetryConnection(new TestConnection(), DbInfo.DEFAULT, instrumenter); + new OpenTelemetryConnection( + new TestConnection(), DbInfo.DEFAULT, statementInstrumenter, transactionInstrumenter); String query = "SELECT * FROM users"; assertThat(connection.prepareCall(query)).isInstanceOf(OpenTelemetryCallableStatement.class); @@ -196,50 +218,42 @@ void testVerifyPrepareCallReturnsOtelWrapper() throws Exception { assertThat(connection.prepareCall(query, 0, 0, 0)) .isInstanceOf(OpenTelemetryCallableStatement.class); assertThat(((OpenTelemetryStatement) connection.prepareCall(query)).instrumenter) - .isEqualTo(instrumenter); + .isEqualTo(statementInstrumenter); connection.close(); } @Test void testVerifyCommit() throws Exception { - Instrumenter instrumenter = + Instrumenter statementInstrumenter = createStatementInstrumenter(testing.getOpenTelemetry()); + Instrumenter transactionInstrumenter = + createTransactionInstrumenter(testing.getOpenTelemetry(), true); DbInfo dbInfo = getDbInfo(); OpenTelemetryConnection connection = - new OpenTelemetryConnection(new TestConnection(), dbInfo, instrumenter); - String query = "COMMIT"; - Statement statement = connection.createStatement(); + new OpenTelemetryConnection( + new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); - testing.runWithSpan( - "parent", - () -> { - assertThat(statement.execute(query)).isTrue(); - }); - transactionTraceAssertion(dbInfo, query); + testing.runWithSpan("parent", connection::commit); + transactionTraceAssertion(dbInfo, "COMMIT"); - statement.close(); connection.close(); } @Test void testVerifyRollback() throws Exception { - Instrumenter instrumenter = + Instrumenter statementInstrumenter = createStatementInstrumenter(testing.getOpenTelemetry()); + Instrumenter transactionInstrumenter = + createTransactionInstrumenter(testing.getOpenTelemetry(), true); DbInfo dbInfo = getDbInfo(); OpenTelemetryConnection connection = - new OpenTelemetryConnection(new TestConnection(), dbInfo, instrumenter); - String query = "ROLLBACK"; - Statement statement = connection.createStatement(); + new OpenTelemetryConnection( + new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); - testing.runWithSpan( - "parent", - () -> { - assertThat(statement.execute(query)).isTrue(); - }); - transactionTraceAssertion(dbInfo, query); + testing.runWithSpan("parent", () -> connection.rollback()); + transactionTraceAssertion(dbInfo, "ROLLBACK"); - statement.close(); connection.close(); } @@ -283,13 +297,13 @@ private static void jdbcTraceAssertion(DbInfo dbInfo, String query) { } @SuppressWarnings("deprecation") // old semconv - private static void transactionTraceAssertion(DbInfo dbInfo, String query) { + private static void transactionTraceAssertion(DbInfo dbInfo, String operation) { testing.waitAndAssertTraces( trace -> trace.hasSpansSatisfyingExactly( span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(), span -> - span.hasName(query + " " + dbInfo.getName()) + span.hasName(operation) .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) .hasAttributesSatisfyingExactly( @@ -301,8 +315,6 @@ private static void transactionTraceAssertion(DbInfo dbInfo, String query) { equalTo( DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : dbInfo.getShortUrl()), - equalTo(maybeStable(DB_STATEMENT), query), - equalTo(maybeStable(DB_OPERATION), query), equalTo(SERVER_ADDRESS, dbInfo.getHost()), equalTo(SERVER_PORT, dbInfo.getPort())))); } From 2de16b2c75ddffdd47fc713b0e231a884aa70f56 Mon Sep 17 00:00:00 2001 From: stillya Date: Mon, 28 Apr 2025 00:04:31 +0500 Subject: [PATCH 3/9] polish commit --- .../instrumentation/jdbc/ConnectionInstrumentation.java | 6 +++--- .../jdbc/datasource/OpenTelemetryDataSource.java | 6 ++++-- .../jdbc/internal/JdbcStatementAttributesGetter.java | 3 ++- .../jdbc/internal/OpenTelemetryConnection.java | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java index c49c16be2058..c713a3cced63 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java @@ -52,9 +52,9 @@ public void transform(TypeTransformer transformer) { // Also include CallableStatement, which is a sub type of PreparedStatement .and(returns(implementsInterface(named("java.sql.PreparedStatement")))), ConnectionInstrumentation.class.getName() + "$PrepareAdvice"); - transformer.applyAdviceToMethod( - namedOneOf("commit", "rollback").and(takesNoArguments()).and(isPublic()), - ConnectionInstrumentation.class.getName() + "$TxnAdvice"); + transformer.applyAdviceToMethod( + namedOneOf("commit", "rollback").and(takesNoArguments()).and(isPublic()), + ConnectionInstrumentation.class.getName() + "$TxnAdvice"); } @SuppressWarnings("unused") diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java index a2507a520eaf..7d534e8881c0 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java @@ -99,14 +99,16 @@ public OpenTelemetryDataSource(DataSource delegate, OpenTelemetry openTelemetry) public Connection getConnection() throws SQLException { Connection connection = wrapCall(delegate::getConnection); DbInfo dbInfo = getDbInfo(connection); - return OpenTelemetryConnection.create(connection, dbInfo, statementInstrumenter, transactionInstrumenter); + return OpenTelemetryConnection.create( + connection, dbInfo, statementInstrumenter, transactionInstrumenter); } @Override public Connection getConnection(String username, String password) throws SQLException { Connection connection = wrapCall(() -> delegate.getConnection(username, password)); DbInfo dbInfo = getDbInfo(connection); - return OpenTelemetryConnection.create(connection, dbInfo, statementInstrumenter, transactionInstrumenter); + return OpenTelemetryConnection.create( + connection, dbInfo, statementInstrumenter, transactionInstrumenter); } @Override diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcStatementAttributesGetter.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcStatementAttributesGetter.java index 0e0293985650..ec5798b933d3 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcStatementAttributesGetter.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcStatementAttributesGetter.java @@ -15,7 +15,8 @@ * This class is internal and is hence not for public use. Its APIs are unstable and can change at * any time. */ -public final class JdbcStatementAttributesGetter implements SqlClientAttributesGetter { +public final class JdbcStatementAttributesGetter + implements SqlClientAttributesGetter { @Nullable @Override diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java index 0e3f8603ec95..028ed594ac16 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java @@ -49,7 +49,7 @@ * any time. */ public class OpenTelemetryConnection implements Connection { - + private static final boolean hasJdbc43 = hasJdbc43(); protected final Connection delegate; private final DbInfo dbInfo; From 510c1b0ca66db85ddc44684283c90fe42596499b Mon Sep 17 00:00:00 2001 From: stillya Date: Mon, 28 Apr 2025 00:22:12 +0500 Subject: [PATCH 4/9] polish commit --- instrumentation/jdbc/README.md | 2 +- .../jdbc/internal/OpenTelemetryConnectionTest.java | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/instrumentation/jdbc/README.md b/instrumentation/jdbc/README.md index 7cffb26d4e92..720aaefbd5e3 100644 --- a/instrumentation/jdbc/README.md +++ b/instrumentation/jdbc/README.md @@ -3,4 +3,4 @@ | System property | Type | Default | Description | |--------------------------------------------------------------|---------|---------|------------------------------------------------------------------------------------------| | `otel.instrumentation.jdbc.statement-sanitizer.enabled` | Boolean | `true` | Enables the DB statement sanitization. | -| `otel.instrumentation.jdbc.experimental.transaction.enabled` | Boolean | `false` | Enables experimental instrumentation to create spans for COMMIT and ROLLBACK operations. | \ No newline at end of file +| `otel.instrumentation.jdbc.experimental.transaction.enabled` | Boolean | `false` | Enables experimental instrumentation to create spans for COMMIT and ROLLBACK operations. | diff --git a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java index 10bc27fd6082..5a2d7e4b72e8 100644 --- a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java +++ b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java @@ -117,11 +117,14 @@ void testVerifyPrepareStatement() throws SQLException { @Test void testVerifyPrepareStatementUpdate() throws SQLException { - Instrumenter instrumenter = + Instrumenter statementInstrumenter = createStatementInstrumenter(testing.getOpenTelemetry()); + Instrumenter transactionInstrumenter = + createTransactionInstrumenter(testing.getOpenTelemetry(), true); DbInfo dbInfo = getDbInfo(); OpenTelemetryConnection connection = - new OpenTelemetryConnection(new TestConnection(), dbInfo, instrumenter); + new OpenTelemetryConnection( + new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); String query = "UPDATE users SET name = name"; PreparedStatement statement = connection.prepareStatement(query); From ac5ea540ffaef78476308c0c47593ceb368dce0c Mon Sep 17 00:00:00 2001 From: stillya Date: Mon, 28 Apr 2025 19:35:58 +0500 Subject: [PATCH 5/9] review fixes --- .../jdbc/ConnectionInstrumentation.java | 4 +- .../jdbc/test/JdbcInstrumentationTest.java | 172 ++++++++---------- .../jdbc/datasource/JdbcTelemetryBuilder.java | 2 +- .../internal/JdbcInstrumenterFactory.java | 1 - 4 files changed, 79 insertions(+), 100 deletions(-) diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java index c713a3cced63..ae9dfd1e0f2d 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java @@ -54,7 +54,7 @@ public void transform(TypeTransformer transformer) { ConnectionInstrumentation.class.getName() + "$PrepareAdvice"); transformer.applyAdviceToMethod( namedOneOf("commit", "rollback").and(takesNoArguments()).and(isPublic()), - ConnectionInstrumentation.class.getName() + "$TxnAdvice"); + ConnectionInstrumentation.class.getName() + "$TransactionAdvice"); } @SuppressWarnings("unused") @@ -68,7 +68,7 @@ public static void addDbInfo( } @SuppressWarnings("unused") - public static class TxnAdvice { + public static class TransactionAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter( 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 02755d9f7cd5..aa441cc77c9b 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 @@ -1643,56 +1643,49 @@ void testCommitTransaction(String system, Connection connection, String username createTable.execute("CREATE TABLE " + tableName + " (id INTEGER not NULL, PRIMARY KEY ( id ))"); cleanup.deferCleanup(createTable); - boolean originalAutoCommit = connection.getAutoCommit(); connection.setAutoCommit(false); testing.waitForTraces(1); testing.clearData(); - try { - Statement insertStatement = connection.createStatement(); - cleanup.deferCleanup(insertStatement); - - testing.runWithSpan( - "parent", - () -> { - insertStatement.executeUpdate("INSERT INTO " + tableName + " VALUES(1)"); - connection.commit(); - }); - - 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(maybeStable(DB_SYSTEM), maybeStableDbSystemName(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)), - span -> - span.hasName("COMMIT") - .hasKind(SpanKind.CLIENT) - .hasParent(trace.getSpan(0)) - .hasAttributesSatisfyingExactly( - equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)), - equalTo(maybeStable(DB_NAME), dbNameLower), - equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), - equalTo( - DB_CONNECTION_STRING, - emitStableDatabaseSemconv() ? null : url)))); - } finally { - connection.setAutoCommit(originalAutoCommit); - } + Statement insertStatement = connection.createStatement(); + cleanup.deferCleanup(insertStatement); + + testing.runWithSpan( + "parent", + () -> { + insertStatement.executeUpdate("INSERT INTO " + tableName + " VALUES(1)"); + connection.commit(); + }); + + 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(maybeStable(DB_SYSTEM), maybeStableDbSystemName(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)), + span -> + span.hasName("COMMIT") + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(0)) + .hasAttributesSatisfyingExactly( + equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)), + equalTo(maybeStable(DB_NAME), dbNameLower), + equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), + equalTo( + DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url)))); } @ParameterizedTest @@ -1705,62 +1698,49 @@ void testRollbackTransaction(String system, Connection connection, String userna createTable.execute("CREATE TABLE " + tableName + " (id INTEGER not NULL, PRIMARY KEY ( id ))"); cleanup.deferCleanup(createTable); - boolean originalAutoCommit = connection.getAutoCommit(); connection.setAutoCommit(false); testing.waitForTraces(1); testing.clearData(); - try { - Statement insertStatement = connection.createStatement(); - cleanup.deferCleanup(insertStatement); - - testing.runWithSpan( - "parent", - () -> { - insertStatement.executeUpdate("INSERT INTO " + tableName + " VALUES(1)"); - connection.rollback(); - }); - - 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(maybeStable(DB_SYSTEM), maybeStableDbSystemName(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)), - span -> - span.hasName("ROLLBACK") - .hasKind(SpanKind.CLIENT) - .hasParent(trace.getSpan(0)) - .hasAttributesSatisfyingExactly( - equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)), - equalTo(maybeStable(DB_NAME), dbNameLower), - equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), - equalTo( - DB_CONNECTION_STRING, - emitStableDatabaseSemconv() ? null : url)))); - - Statement selectStatement = connection.createStatement(); - cleanup.deferCleanup(selectStatement); - ResultSet resultSet = selectStatement.executeQuery("SELECT COUNT(*) FROM " + tableName); - resultSet.next(); - assertThat(resultSet.getInt(1)).isEqualTo(0); - } finally { - connection.setAutoCommit(originalAutoCommit); - } + Statement insertStatement = connection.createStatement(); + cleanup.deferCleanup(insertStatement); + + testing.runWithSpan( + "parent", + () -> { + insertStatement.executeUpdate("INSERT INTO " + tableName + " VALUES(1)"); + connection.rollback(); + }); + + 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(maybeStable(DB_SYSTEM), maybeStableDbSystemName(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)), + span -> + span.hasName("ROLLBACK") + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(0)) + .hasAttributesSatisfyingExactly( + equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)), + equalTo(maybeStable(DB_NAME), dbNameLower), + equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), + equalTo( + DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url)))); } static Stream transactionOperationsStream() throws SQLException { diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryBuilder.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryBuilder.java index 347f763e0dc4..bdb173928f32 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryBuilder.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryBuilder.java @@ -43,7 +43,7 @@ public JdbcTelemetryBuilder setStatementSanitizationEnabled(boolean enabled) { return this; } - /** Configures whether spans are created for JDBC Transactions. Enabled by default. */ + /** Configures whether spans are created for JDBC Transactions. Disabled by default. */ @CanIgnoreReturnValue public JdbcTelemetryBuilder setTransactionInstrumenterEnabled(boolean enabled) { this.transactionInstrumenterEnabled = enabled; diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java index 94a7ac9cb887..d32178ad5da8 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java @@ -81,7 +81,6 @@ public static Instrumenter createTransactionInstrument .addAttributesExtractor( SqlClientAttributesExtractor.builder(transactionAttributesGetter).build()) .addAttributesExtractor(ServerAttributesExtractor.create(transactionNetAttributesGetter)) - .addOperationMetrics(DbClientMetrics.get()) .setEnabled(enabled) .buildInstrumenter(SpanKindExtractor.alwaysClient()); } From 114ccac324a61634759c07c77f0348be64010c6c Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 6 May 2025 14:32:18 +0300 Subject: [PATCH 6/9] reduce the amount of code --- .../jdbc/ConnectionInstrumentation.java | 20 +-- .../instrumentation/jdbc/JdbcSingletons.java | 80 ++++------- .../jdbc/OpenTelemetryDriver.java | 9 +- .../jdbc/datasource/JdbcTelemetry.java | 5 +- .../datasource/OpenTelemetryDataSource.java | 5 +- .../jdbc/internal/DbRequest.java | 25 +++- ...sGetter.java => JdbcAttributesGetter.java} | 7 +- .../internal/JdbcInstrumenterFactory.java | 63 ++++++--- .../internal/JdbcNetworkAttributesGetter.java | 11 +- .../JdbcTransactionAttributesGetter.java | 69 ---------- .../internal/OpenTelemetryConnection.java | 12 +- .../StatementNetworkAttributesGetter.java | 30 ----- .../TransactionNetworkAttributesGetter.java | 31 ----- .../jdbc/internal/TransactionRequest.java | 29 ---- .../internal/OpenTelemetryConnectionTest.java | 126 ++++++------------ 15 files changed, 151 insertions(+), 371 deletions(-) rename instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/{JdbcStatementAttributesGetter.java => JdbcAttributesGetter.java} (86%) delete mode 100644 instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcTransactionAttributesGetter.java delete mode 100644 instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/StatementNetworkAttributesGetter.java delete mode 100644 instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionNetworkAttributesGetter.java delete mode 100644 instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionRequest.java diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java index ae9dfd1e0f2d..3f356fb6db32 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java @@ -19,10 +19,8 @@ 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.instrumentation.jdbc.internal.JdbcUtils; -import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest; -import io.opentelemetry.javaagent.bootstrap.jdbc.DbInfo; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import java.sql.Connection; @@ -77,18 +75,10 @@ public static void onEnter( @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { Context parentContext = currentContext(); - DbInfo dbInfo = null; - Connection realConnection = JdbcUtils.unwrapConnection(connection); - if (realConnection != null) { - dbInfo = JdbcUtils.extractDbInfo(realConnection); - } - if (dbInfo == null) { - return; - } - TransactionRequest request = - TransactionRequest.create(dbInfo, methodName.toUpperCase(Locale.ROOT)); + DbRequest request = + DbRequest.createTransaction(connection, methodName.toUpperCase(Locale.ROOT)); - if (!transactionInstrumenter().shouldStart(parentContext, request)) { + if (request == null || !transactionInstrumenter().shouldStart(parentContext, request)) { return; } @@ -99,7 +89,7 @@ public static void onEnter( @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void stopSpan( @Advice.Thrown Throwable throwable, - @Advice.Local("otelRequest") TransactionRequest request, + @Advice.Local("otelRequest") DbRequest request, @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { if (scope == null) { diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java index 3a131146b816..78716a6a430d 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java @@ -8,81 +8,51 @@ import static io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory.createDataSourceInstrumenter; import io.opentelemetry.api.GlobalOpenTelemetry; -import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientMetrics; -import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientSpanNameExtractor; -import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlClientAttributesExtractor; import io.opentelemetry.instrumentation.api.incubator.semconv.net.PeerServiceAttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor; -import io.opentelemetry.instrumentation.api.semconv.network.ServerAttributesExtractor; import io.opentelemetry.instrumentation.jdbc.internal.DbRequest; -import io.opentelemetry.instrumentation.jdbc.internal.JdbcStatementAttributesGetter; -import io.opentelemetry.instrumentation.jdbc.internal.JdbcTransactionAttributesGetter; -import io.opentelemetry.instrumentation.jdbc.internal.StatementNetworkAttributesGetter; -import io.opentelemetry.instrumentation.jdbc.internal.TransactionNetworkAttributesGetter; -import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest; +import io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory; +import io.opentelemetry.instrumentation.jdbc.internal.JdbcNetworkAttributesGetter; import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig; import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig; import io.opentelemetry.javaagent.bootstrap.jdbc.DbInfo; +import java.util.Collections; import javax.sql.DataSource; public final class JdbcSingletons { - private static final String INSTRUMENTATION_NAME = "io.opentelemetry.jdbc"; - private static final Instrumenter STATEMENT_INSTRUMENTER; - private static final Instrumenter TRANSACTION_INSTRUMENTER; + private static final Instrumenter TRANSACTION_INSTRUMENTER; public static final Instrumenter DATASOURCE_INSTRUMENTER = createDataSourceInstrumenter(GlobalOpenTelemetry.get(), true); static { - JdbcStatementAttributesGetter statementAttributesGetter = new JdbcStatementAttributesGetter(); - JdbcTransactionAttributesGetter transactionAttributesGetter = - new JdbcTransactionAttributesGetter(); - StatementNetworkAttributesGetter statementNetAttributesGetter = - new StatementNetworkAttributesGetter(); - TransactionNetworkAttributesGetter transactionNetAttributesGetter = - new TransactionNetworkAttributesGetter(); + JdbcNetworkAttributesGetter netAttributesGetter = new JdbcNetworkAttributesGetter(); + AttributesExtractor peerServiceExtractor = + PeerServiceAttributesExtractor.create( + netAttributesGetter, AgentCommonConfig.get().getPeerServiceResolver()); STATEMENT_INSTRUMENTER = - Instrumenter.builder( - GlobalOpenTelemetry.get(), - INSTRUMENTATION_NAME, - DbClientSpanNameExtractor.create(statementAttributesGetter)) - .addAttributesExtractor( - SqlClientAttributesExtractor.builder(statementAttributesGetter) - .setStatementSanitizationEnabled( - AgentInstrumentationConfig.get() - .getBoolean( - "otel.instrumentation.jdbc.statement-sanitizer.enabled", - AgentCommonConfig.get().isStatementSanitizationEnabled())) - .build()) - .addAttributesExtractor(ServerAttributesExtractor.create(statementNetAttributesGetter)) - .addAttributesExtractor( - PeerServiceAttributesExtractor.create( - statementNetAttributesGetter, AgentCommonConfig.get().getPeerServiceResolver())) - .addOperationMetrics(DbClientMetrics.get()) - .buildInstrumenter(SpanKindExtractor.alwaysClient()); + JdbcInstrumenterFactory.createStatementInstrumenter( + GlobalOpenTelemetry.get(), + Collections.singletonList(peerServiceExtractor), + true, + AgentInstrumentationConfig.get() + .getBoolean( + "otel.instrumentation.jdbc.statement-sanitizer.enabled", + AgentCommonConfig.get().isStatementSanitizationEnabled())); TRANSACTION_INSTRUMENTER = - Instrumenter.builder( - GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, TransactionRequest::spanName) - .addAttributesExtractor( - SqlClientAttributesExtractor.builder(transactionAttributesGetter).build()) - .addAttributesExtractor( - ServerAttributesExtractor.create(transactionNetAttributesGetter)) - .addAttributesExtractor( - PeerServiceAttributesExtractor.create( - transactionNetAttributesGetter, - AgentCommonConfig.get().getPeerServiceResolver())) - .addOperationMetrics(DbClientMetrics.get()) - .setEnabled( - AgentInstrumentationConfig.get() - .getBoolean( - "otel.instrumentation.jdbc.experimental.transaction.enabled", false)) - .buildInstrumenter(SpanKindExtractor.alwaysClient()); + JdbcInstrumenterFactory.createTransactionInstrumenter( + GlobalOpenTelemetry.get(), + Collections.singletonList(peerServiceExtractor), + AgentInstrumentationConfig.get() + .getBoolean( + "otel.instrumentation.jdbc.experimental.transaction.enabled", + AgentCommonConfig.get().isStatementSanitizationEnabled())); } - public static Instrumenter transactionInstrumenter() { + public static Instrumenter transactionInstrumenter() { return TRANSACTION_INSTRUMENTER; } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java index b2e1ca6a27fb..73f0e2f71434 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java @@ -24,13 +24,11 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil; import io.opentelemetry.instrumentation.api.internal.EmbeddedInstrumentationProperties; import io.opentelemetry.instrumentation.jdbc.internal.DbRequest; import io.opentelemetry.instrumentation.jdbc.internal.JdbcConnectionUrlParser; import io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory; import io.opentelemetry.instrumentation.jdbc.internal.OpenTelemetryConnection; -import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; import java.sql.Connection; import java.sql.Driver; @@ -246,11 +244,8 @@ public Connection connect(String url, Properties info) throws SQLException { Instrumenter statementInstrumenter = JdbcInstrumenterFactory.createStatementInstrumenter(openTelemetry); - Instrumenter transactionInstrumenter = - JdbcInstrumenterFactory.createTransactionInstrumenter( - openTelemetry, - ConfigPropertiesUtil.getBoolean( - "otel.instrumentation.jdbc.experimental.transaction.enabled", false)); + Instrumenter transactionInstrumenter = + JdbcInstrumenterFactory.createTransactionInstrumenter(openTelemetry); return OpenTelemetryConnection.create( connection, dbInfo, statementInstrumenter, transactionInstrumenter); } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetry.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetry.java index c0e85580792f..dd86484ca689 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetry.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetry.java @@ -8,7 +8,6 @@ import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.jdbc.internal.DbRequest; -import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; import javax.sql.DataSource; @@ -27,12 +26,12 @@ public static JdbcTelemetryBuilder builder(OpenTelemetry openTelemetry) { private final Instrumenter dataSourceInstrumenter; private final Instrumenter statementInstrumenter; - private final Instrumenter transactionInstrumenter; + private final Instrumenter transactionInstrumenter; JdbcTelemetry( Instrumenter dataSourceInstrumenter, Instrumenter statementInstrumenter, - Instrumenter transactionInstrumenter) { + Instrumenter transactionInstrumenter) { this.dataSourceInstrumenter = dataSourceInstrumenter; this.statementInstrumenter = statementInstrumenter; this.transactionInstrumenter = transactionInstrumenter; diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java index 7d534e8881c0..021897e02ae7 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java @@ -34,7 +34,6 @@ import io.opentelemetry.instrumentation.jdbc.internal.DbRequest; import io.opentelemetry.instrumentation.jdbc.internal.OpenTelemetryConnection; import io.opentelemetry.instrumentation.jdbc.internal.ThrowingSupplier; -import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; import java.io.PrintWriter; import java.sql.Connection; @@ -49,7 +48,7 @@ public class OpenTelemetryDataSource implements DataSource, AutoCloseable { private final DataSource delegate; private final Instrumenter dataSourceInstrumenter; private final Instrumenter statementInstrumenter; - private final Instrumenter transactionInstrumenter; + private final Instrumenter transactionInstrumenter; private volatile DbInfo cachedDbInfo; /** @@ -88,7 +87,7 @@ public OpenTelemetryDataSource(DataSource delegate, OpenTelemetry openTelemetry) DataSource delegate, Instrumenter dataSourceInstrumenter, Instrumenter statementInstrumenter, - Instrumenter transactionInstrumenter) { + Instrumenter transactionInstrumenter) { this.delegate = delegate; this.dataSourceInstrumenter = dataSourceInstrumenter; this.statementInstrumenter = statementInstrumenter; 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 a5e8e0559178..cb16d91542e8 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 @@ -63,7 +63,26 @@ public static DbRequest create(DbInfo dbInfo, String queryText, Long batchSize) } public static DbRequest create(DbInfo dbInfo, Collection queryTexts, Long batchSize) { - return new AutoValue_DbRequest(dbInfo, queryTexts, batchSize); + return new AutoValue_DbRequest(dbInfo, queryTexts, batchSize, null); + } + + public static DbRequest create( + DbInfo dbInfo, Collection queryTexts, Long batchSize, String operation) { + return new AutoValue_DbRequest(dbInfo, queryTexts, batchSize, operation); + } + + @Nullable + public static DbRequest createTransaction(Connection connection, String operation) { + Connection realConnection = JdbcUtils.unwrapConnection(connection); + if (realConnection == null) { + return null; + } + + return createTransaction(JdbcUtils.extractDbInfo(realConnection), operation); + } + + public static DbRequest createTransaction(DbInfo dbInfo, String operation) { + return create(dbInfo, Collections.emptyList(), null, operation); } public abstract DbInfo getDbInfo(); @@ -72,4 +91,8 @@ public static DbRequest create(DbInfo dbInfo, Collection queryTexts, Lon @Nullable public abstract Long getBatchSize(); + + // used for transaction instrumentation + @Nullable + public abstract String getOperation(); } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcStatementAttributesGetter.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcAttributesGetter.java similarity index 86% rename from instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcStatementAttributesGetter.java rename to instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcAttributesGetter.java index ec5798b933d3..f01d836ab72d 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcStatementAttributesGetter.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcAttributesGetter.java @@ -11,12 +11,7 @@ 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 JdbcStatementAttributesGetter - implements SqlClientAttributesGetter { +final class JdbcAttributesGetter implements SqlClientAttributesGetter { @Nullable @Override diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java index d32178ad5da8..ec7ff4f760d3 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java @@ -5,18 +5,21 @@ package io.opentelemetry.instrumentation.jdbc.internal; -import io.opentelemetry.api.GlobalOpenTelemetry; +import static java.util.Collections.emptyList; + import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor; import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeSpanNameExtractor; import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientMetrics; import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientSpanNameExtractor; import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlClientAttributesExtractor; +import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor; import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil; import io.opentelemetry.instrumentation.api.semconv.network.ServerAttributesExtractor; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; +import java.util.List; import javax.sql.DataSource; /** @@ -25,18 +28,9 @@ */ public final class JdbcInstrumenterFactory { public static final String INSTRUMENTATION_NAME = "io.opentelemetry.jdbc"; - private static final JdbcStatementAttributesGetter statementAttributesGetter = - new JdbcStatementAttributesGetter(); - private static final JdbcTransactionAttributesGetter transactionAttributesGetter = - new JdbcTransactionAttributesGetter(); - private static final StatementNetworkAttributesGetter statementNetAttributesGetter = - new StatementNetworkAttributesGetter(); - private static final TransactionNetworkAttributesGetter transactionNetAttributesGetter = - new TransactionNetworkAttributesGetter(); - - public static Instrumenter createStatementInstrumenter() { - return createStatementInstrumenter(GlobalOpenTelemetry.get()); - } + private static final JdbcAttributesGetter dbAttributesGetter = new JdbcAttributesGetter(); + private static final JdbcNetworkAttributesGetter netAttributesGetter = + new JdbcNetworkAttributesGetter(); public static Instrumenter createStatementInstrumenter( OpenTelemetry openTelemetry) { @@ -49,15 +43,25 @@ public static Instrumenter createStatementInstrumenter( public static Instrumenter createStatementInstrumenter( OpenTelemetry openTelemetry, boolean enabled, boolean statementSanitizationEnabled) { + return createStatementInstrumenter( + openTelemetry, emptyList(), enabled, statementSanitizationEnabled); + } + + public static Instrumenter createStatementInstrumenter( + OpenTelemetry openTelemetry, + List> extractors, + boolean enabled, + boolean statementSanitizationEnabled) { return Instrumenter.builder( openTelemetry, INSTRUMENTATION_NAME, - DbClientSpanNameExtractor.create(statementAttributesGetter)) + DbClientSpanNameExtractor.create(dbAttributesGetter)) .addAttributesExtractor( - SqlClientAttributesExtractor.builder(statementAttributesGetter) + SqlClientAttributesExtractor.builder(dbAttributesGetter) .setStatementSanitizationEnabled(statementSanitizationEnabled) .build()) - .addAttributesExtractor(ServerAttributesExtractor.create(statementNetAttributesGetter)) + .addAttributesExtractor(ServerAttributesExtractor.create(netAttributesGetter)) + .addAttributesExtractors(extractors) .addOperationMetrics(DbClientMetrics.get()) .setEnabled(enabled) .buildInstrumenter(SpanKindExtractor.alwaysClient()); @@ -74,13 +78,28 @@ public static Instrumenter createDataSourceInstrumenter( .buildInstrumenter(); } - public static Instrumenter createTransactionInstrumenter( + public static Instrumenter createTransactionInstrumenter( + OpenTelemetry openTelemetry) { + return createTransactionInstrumenter( + openTelemetry, + ConfigPropertiesUtil.getBoolean( + "otel.instrumentation.jdbc.experimental.transaction.enabled", false)); + } + + public static Instrumenter createTransactionInstrumenter( OpenTelemetry openTelemetry, boolean enabled) { - return Instrumenter.builder( - openTelemetry, INSTRUMENTATION_NAME, TransactionRequest::spanName) - .addAttributesExtractor( - SqlClientAttributesExtractor.builder(transactionAttributesGetter).build()) - .addAttributesExtractor(ServerAttributesExtractor.create(transactionNetAttributesGetter)) + return createTransactionInstrumenter(openTelemetry, emptyList(), enabled); + } + + public static Instrumenter createTransactionInstrumenter( + OpenTelemetry openTelemetry, + List> extractors, + boolean enabled) { + return Instrumenter.builder( + openTelemetry, INSTRUMENTATION_NAME, DbRequest::getOperation) + .addAttributesExtractor(SqlClientAttributesExtractor.builder(dbAttributesGetter).build()) + .addAttributesExtractor(ServerAttributesExtractor.create(netAttributesGetter)) + .addAttributesExtractors(extractors) .setEnabled(enabled) .buildInstrumenter(SpanKindExtractor.alwaysClient()); } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcNetworkAttributesGetter.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcNetworkAttributesGetter.java index 23ba03b82f68..a09ca312672b 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcNetworkAttributesGetter.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcNetworkAttributesGetter.java @@ -6,24 +6,23 @@ package io.opentelemetry.instrumentation.jdbc.internal; import io.opentelemetry.instrumentation.api.semconv.network.ServerAttributesGetter; -import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; 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 JdbcNetworkAttributesGetter implements ServerAttributesGetter { +public final class JdbcNetworkAttributesGetter implements ServerAttributesGetter { @Nullable @Override - public String getServerAddress(DbInfo request) { - return request.getHost(); + public String getServerAddress(DbRequest request) { + return request.getDbInfo().getHost(); } @Nullable @Override - public Integer getServerPort(DbInfo request) { - return request.getPort(); + public Integer getServerPort(DbRequest request) { + return request.getDbInfo().getPort(); } } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcTransactionAttributesGetter.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcTransactionAttributesGetter.java deleted file mode 100644 index 14d6d73ec15c..000000000000 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcTransactionAttributesGetter.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.jdbc.internal; - -import static java.util.Collections.emptySet; - -import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlClientAttributesGetter; -import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; -import java.sql.SQLException; -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 class JdbcTransactionAttributesGetter - implements SqlClientAttributesGetter { - - @Nullable - @Override - public String getDbSystem(TransactionRequest request) { - return request.getDbInfo().getSystem(); - } - - @Deprecated - @Nullable - @Override - public String getUser(TransactionRequest request) { - return request.getDbInfo().getUser(); - } - - @Nullable - @Override - public String getDbNamespace(TransactionRequest request) { - DbInfo dbInfo = request.getDbInfo(); - return dbInfo.getName() == null ? dbInfo.getDb() : dbInfo.getName(); - } - - @Deprecated - @Nullable - @Override - public String getConnectionString(TransactionRequest request) { - return request.getDbInfo().getShortUrl(); - } - - @Override - public Collection getRawQueryTexts(TransactionRequest request) { - return emptySet(); - } - - @Nullable - @Override - public Long getBatchSize(TransactionRequest request) { - return null; - } - - @Nullable - @Override - public String getResponseStatus(@Nullable Void response, @Nullable Throwable error) { - if (error instanceof SQLException) { - return Integer.toString(((SQLException) error).getErrorCode()); - } - return null; - } -} diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java index 028ed594ac16..4874ed4a6f5e 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java @@ -54,13 +54,13 @@ public class OpenTelemetryConnection implements Connection { protected final Connection delegate; private final DbInfo dbInfo; protected final Instrumenter statementInstrumenter; - protected final Instrumenter transactionInstrumenter; + protected final Instrumenter transactionInstrumenter; protected OpenTelemetryConnection( Connection delegate, DbInfo dbInfo, Instrumenter statementInstrumenter, - Instrumenter transactionInstrumenter) { + Instrumenter transactionInstrumenter) { this.delegate = delegate; this.dbInfo = dbInfo; this.statementInstrumenter = statementInstrumenter; @@ -81,7 +81,7 @@ public static Connection create( Connection delegate, DbInfo dbInfo, Instrumenter statementInstrumenter, - Instrumenter transactionInstrumenter) { + Instrumenter transactionInstrumenter) { if (hasJdbc43) { return new OpenTelemetryConnectionJdbc43( delegate, dbInfo, statementInstrumenter, transactionInstrumenter); @@ -408,7 +408,7 @@ static class OpenTelemetryConnectionJdbc43 extends OpenTelemetryConnection { Connection delegate, DbInfo dbInfo, Instrumenter statementInstrumenter, - Instrumenter transactionInstrumenter) { + Instrumenter transactionInstrumenter) { super(delegate, dbInfo, statementInstrumenter, transactionInstrumenter); } @@ -454,7 +454,7 @@ public void setShardingKey(ShardingKey shardingKey) throws SQLException { protected void wrapCall(ThrowingSupplier callable, String operation) throws E { Context parentContext = Context.current(); - TransactionRequest request = TransactionRequest.create(dbInfo, operation); + DbRequest request = DbRequest.createTransaction(dbInfo, operation); if (!this.transactionInstrumenter.shouldStart(parentContext, request)) { callable.call(); return; @@ -471,8 +471,6 @@ protected void wrapCall(ThrowingSupplier callable, Stri } protected interface ThrowingSupplier { - String statement = null; - void call() throws E; } } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/StatementNetworkAttributesGetter.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/StatementNetworkAttributesGetter.java deleted file mode 100644 index 0b7011cfb3bd..000000000000 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/StatementNetworkAttributesGetter.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.jdbc.internal; - -import io.opentelemetry.instrumentation.api.semconv.network.ServerAttributesGetter; -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 StatementNetworkAttributesGetter implements ServerAttributesGetter { - private final JdbcNetworkAttributesGetter jdbcNetworkAttributesGetter = - new JdbcNetworkAttributesGetter(); - - @Nullable - @Override - public String getServerAddress(DbRequest request) { - return jdbcNetworkAttributesGetter.getServerAddress(request.getDbInfo()); - } - - @Nullable - @Override - public Integer getServerPort(DbRequest request) { - return jdbcNetworkAttributesGetter.getServerPort(request.getDbInfo()); - } -} diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionNetworkAttributesGetter.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionNetworkAttributesGetter.java deleted file mode 100644 index 0c6da65177f7..000000000000 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionNetworkAttributesGetter.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.jdbc.internal; - -import io.opentelemetry.instrumentation.api.semconv.network.ServerAttributesGetter; -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 TransactionNetworkAttributesGetter - implements ServerAttributesGetter { - private final JdbcNetworkAttributesGetter jdbcNetworkAttributesGetter = - new JdbcNetworkAttributesGetter(); - - @Nullable - @Override - public String getServerAddress(TransactionRequest request) { - return jdbcNetworkAttributesGetter.getServerAddress(request.getDbInfo()); - } - - @Nullable - @Override - public Integer getServerPort(TransactionRequest request) { - return jdbcNetworkAttributesGetter.getServerPort(request.getDbInfo()); - } -} diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionRequest.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionRequest.java deleted file mode 100644 index 9fc1c98786b1..000000000000 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionRequest.java +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.instrumentation.jdbc.internal; - -import com.google.auto.value.AutoValue; -import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; - -/** - * This class is internal and is hence not for public use. Its APIs are unstable and can change at - * any time. - */ -@AutoValue -public abstract class TransactionRequest { - - public static TransactionRequest create(DbInfo dbInfo, String operation) { - return new AutoValue_TransactionRequest(dbInfo, operation); - } - - public abstract DbInfo getDbInfo(); - - public abstract String operation(); - - public static String spanName(TransactionRequest request) { - return request.operation(); - } -} diff --git a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java index 5a2d7e4b72e8..328557924836 100644 --- a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java +++ b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java @@ -44,14 +44,7 @@ class OpenTelemetryConnectionTest { @Test void testVerifyCreateStatement() throws SQLException { - Instrumenter statementInstrumenter = - createStatementInstrumenter(testing.getOpenTelemetry()); - Instrumenter transactionInstrumenter = - createTransactionInstrumenter(testing.getOpenTelemetry(), true); - DbInfo dbInfo = getDbInfo(); - OpenTelemetryConnection connection = - new OpenTelemetryConnection( - new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); + OpenTelemetryConnection connection = getConnection(); String query = "SELECT * FROM users"; Statement statement = connection.createStatement(); @@ -61,7 +54,7 @@ void testVerifyCreateStatement() throws SQLException { assertThat(statement.execute(query)).isTrue(); }); - jdbcTraceAssertion(dbInfo, query); + jdbcTraceAssertion(connection.getDbInfo(), query); statement.close(); connection.close(); @@ -70,33 +63,21 @@ void testVerifyCreateStatement() throws SQLException { @SuppressWarnings("unchecked") @Test void testVerifyCreateStatementReturnsOtelWrapper() throws Exception { - OpenTelemetry ot = OpenTelemetry.propagating(ContextPropagators.noop()); - Instrumenter statementInstrumenter = createStatementInstrumenter(ot); - Instrumenter transactionInstrumenter = - createTransactionInstrumenter(ot, true); - OpenTelemetryConnection connection = - new OpenTelemetryConnection( - new TestConnection(), DbInfo.DEFAULT, statementInstrumenter, transactionInstrumenter); + OpenTelemetry openTelemetry = OpenTelemetry.propagating(ContextPropagators.noop()); + OpenTelemetryConnection connection = getConnection(openTelemetry); assertThat(connection.createStatement()).isInstanceOf(OpenTelemetryStatement.class); assertThat(connection.createStatement(0, 0)).isInstanceOf(OpenTelemetryStatement.class); assertThat(connection.createStatement(0, 0, 0)).isInstanceOf(OpenTelemetryStatement.class); assertThat(((OpenTelemetryStatement) connection.createStatement()).instrumenter) - .isEqualTo(statementInstrumenter); + .isEqualTo(connection.statementInstrumenter); connection.close(); } @Test void testVerifyPrepareStatement() throws SQLException { - Instrumenter statementInstrumenter = - createStatementInstrumenter(testing.getOpenTelemetry()); - Instrumenter transactionInstrumenter = - createTransactionInstrumenter(testing.getOpenTelemetry(), true); - DbInfo dbInfo = getDbInfo(); - OpenTelemetryConnection connection = - new OpenTelemetryConnection( - new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); + OpenTelemetryConnection connection = getConnection(); String query = "SELECT * FROM users"; PreparedStatement statement = connection.prepareStatement(query); @@ -109,7 +90,7 @@ void testVerifyPrepareStatement() throws SQLException { assertThat(resultSet.getStatement()).isEqualTo(statement); }); - jdbcTraceAssertion(dbInfo, query); + jdbcTraceAssertion(connection.getDbInfo(), query); statement.close(); connection.close(); @@ -117,14 +98,7 @@ void testVerifyPrepareStatement() throws SQLException { @Test void testVerifyPrepareStatementUpdate() throws SQLException { - Instrumenter statementInstrumenter = - createStatementInstrumenter(testing.getOpenTelemetry()); - Instrumenter transactionInstrumenter = - createTransactionInstrumenter(testing.getOpenTelemetry(), true); - DbInfo dbInfo = getDbInfo(); - OpenTelemetryConnection connection = - new OpenTelemetryConnection( - new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); + OpenTelemetryConnection connection = getConnection(); String query = "UPDATE users SET name = name"; PreparedStatement statement = connection.prepareStatement(query); @@ -135,7 +109,7 @@ void testVerifyPrepareStatementUpdate() throws SQLException { assertThat(statement.getResultSet()).isNull(); }); - jdbcTraceAssertion(dbInfo, query, "UPDATE"); + jdbcTraceAssertion(connection.getDbInfo(), query, "UPDATE"); statement.close(); connection.close(); @@ -143,14 +117,7 @@ void testVerifyPrepareStatementUpdate() throws SQLException { @Test void testVerifyPrepareStatementQuery() throws SQLException { - Instrumenter statementInstrumenter = - createStatementInstrumenter(testing.getOpenTelemetry()); - Instrumenter transactionInstrumenter = - createTransactionInstrumenter(testing.getOpenTelemetry(), true); - DbInfo dbInfo = getDbInfo(); - OpenTelemetryConnection connection = - new OpenTelemetryConnection( - new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); + OpenTelemetryConnection connection = getConnection(); String query = "SELECT * FROM users"; PreparedStatement statement = connection.prepareStatement(query); @@ -162,7 +129,7 @@ void testVerifyPrepareStatementQuery() throws SQLException { assertThat(resultSet.getStatement()).isEqualTo(statement); }); - jdbcTraceAssertion(dbInfo, query); + jdbcTraceAssertion(connection.getDbInfo(), query); statement.close(); connection.close(); @@ -171,13 +138,9 @@ void testVerifyPrepareStatementQuery() throws SQLException { @SuppressWarnings("unchecked") @Test void testVerifyPrepareStatementReturnsOtelWrapper() throws Exception { - OpenTelemetry ot = OpenTelemetry.propagating(ContextPropagators.noop()); - Instrumenter statementInstrumenter = createStatementInstrumenter(ot); - Instrumenter transactionInstrumenter = - createTransactionInstrumenter(ot, true); - OpenTelemetryConnection connection = - new OpenTelemetryConnection( - new TestConnection(), DbInfo.DEFAULT, statementInstrumenter, transactionInstrumenter); + OpenTelemetry openTelemetry = OpenTelemetry.propagating(ContextPropagators.noop()); + OpenTelemetryConnection connection = getConnection(openTelemetry); + String query = "SELECT * FROM users"; assertThat(connection.prepareStatement(query)) @@ -194,21 +157,14 @@ void testVerifyPrepareStatementReturnsOtelWrapper() throws Exception { .isInstanceOf(OpenTelemetryPreparedStatement.class); assertThat( ((OpenTelemetryStatement) connection.prepareStatement(query)).instrumenter) - .isEqualTo(statementInstrumenter); + .isEqualTo(connection.statementInstrumenter); connection.close(); } @Test void testVerifyPrepareCall() throws SQLException { - Instrumenter statementInstrumenter = - createStatementInstrumenter(testing.getOpenTelemetry()); - Instrumenter transactionInstrumenter = - createTransactionInstrumenter(testing.getOpenTelemetry(), true); - DbInfo dbInfo = getDbInfo(); - OpenTelemetryConnection connection = - new OpenTelemetryConnection( - new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); + OpenTelemetryConnection connection = getConnection(); String query = "SELECT * FROM users"; PreparedStatement statement = connection.prepareCall(query); @@ -218,7 +174,7 @@ void testVerifyPrepareCall() throws SQLException { assertThat(statement.execute()).isTrue(); }); - jdbcTraceAssertion(dbInfo, query); + jdbcTraceAssertion(connection.getDbInfo(), query); statement.close(); connection.close(); @@ -227,13 +183,9 @@ void testVerifyPrepareCall() throws SQLException { @SuppressWarnings("unchecked") @Test void testVerifyPrepareCallReturnsOtelWrapper() throws Exception { - OpenTelemetry ot = OpenTelemetry.propagating(ContextPropagators.noop()); - Instrumenter statementInstrumenter = createStatementInstrumenter(ot); - Instrumenter transactionInstrumenter = - createTransactionInstrumenter(ot, true); - OpenTelemetryConnection connection = - new OpenTelemetryConnection( - new TestConnection(), DbInfo.DEFAULT, statementInstrumenter, transactionInstrumenter); + OpenTelemetry openTelemetry = OpenTelemetry.propagating(ContextPropagators.noop()); + OpenTelemetryConnection connection = getConnection(openTelemetry); + String query = "SELECT * FROM users"; assertThat(connection.prepareCall(query)).isInstanceOf(OpenTelemetryCallableStatement.class); @@ -244,41 +196,27 @@ void testVerifyPrepareCallReturnsOtelWrapper() throws Exception { assertThat(connection.prepareCall(query, 0, 0, 0)) .isInstanceOf(OpenTelemetryCallableStatement.class); assertThat(((OpenTelemetryStatement) connection.prepareCall(query)).instrumenter) - .isEqualTo(statementInstrumenter); + .isEqualTo(connection.statementInstrumenter); connection.close(); } @Test void testVerifyCommit() throws Exception { - Instrumenter statementInstrumenter = - createStatementInstrumenter(testing.getOpenTelemetry()); - Instrumenter transactionInstrumenter = - createTransactionInstrumenter(testing.getOpenTelemetry(), true); - DbInfo dbInfo = getDbInfo(); - OpenTelemetryConnection connection = - new OpenTelemetryConnection( - new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); + OpenTelemetryConnection connection = getConnection(); testing.runWithSpan("parent", connection::commit); - transactionTraceAssertion(dbInfo, "COMMIT"); + transactionTraceAssertion(connection.getDbInfo(), "COMMIT"); connection.close(); } @Test void testVerifyRollback() throws Exception { - Instrumenter statementInstrumenter = - createStatementInstrumenter(testing.getOpenTelemetry()); - Instrumenter transactionInstrumenter = - createTransactionInstrumenter(testing.getOpenTelemetry(), true); - DbInfo dbInfo = getDbInfo(); - OpenTelemetryConnection connection = - new OpenTelemetryConnection( - new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); + OpenTelemetryConnection connection = getConnection(); testing.runWithSpan("parent", () -> connection.rollback()); - transactionTraceAssertion(dbInfo, "ROLLBACK"); + transactionTraceAssertion(connection.getDbInfo(), "ROLLBACK"); connection.close(); } @@ -348,4 +286,18 @@ private static void transactionTraceAssertion(DbInfo dbInfo, String operation) { equalTo(SERVER_ADDRESS, dbInfo.getHost()), equalTo(SERVER_PORT, dbInfo.getPort())))); } + + private static OpenTelemetryConnection getConnection() { + return getConnection(testing.getOpenTelemetry()); + } + + private static OpenTelemetryConnection getConnection(OpenTelemetry openTelemetry) { + Instrumenter statementInstrumenter = + createStatementInstrumenter(openTelemetry); + Instrumenter transactionInstrumenter = + createTransactionInstrumenter(openTelemetry, true); + DbInfo dbInfo = getDbInfo(); + return new OpenTelemetryConnection( + new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); + } } From 5a4a0d23f2971d112b8344a9a958131aa0701955 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 6 May 2025 14:43:45 +0300 Subject: [PATCH 7/9] add db.operation attribute to transaction spans --- .../jdbc/test/JdbcInstrumentationTest.java | 2 + .../internal/JdbcInstrumenterFactory.java | 1 + .../TransactionAttributeExtractor.java | 42 +++++++++++++++++++ .../internal/OpenTelemetryConnectionTest.java | 1 + 4 files changed, 46 insertions(+) create mode 100644 instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionAttributeExtractor.java 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 aa441cc77c9b..2050cef44978 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 @@ -1683,6 +1683,7 @@ void testCommitTransaction(String system, Connection connection, String username .hasAttributesSatisfyingExactly( equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)), equalTo(maybeStable(DB_NAME), dbNameLower), + equalTo(maybeStable(DB_OPERATION), "COMMIT"), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), equalTo( DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url)))); @@ -1738,6 +1739,7 @@ void testRollbackTransaction(String system, Connection connection, String userna .hasAttributesSatisfyingExactly( equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)), equalTo(maybeStable(DB_NAME), dbNameLower), + equalTo(maybeStable(DB_OPERATION), "ROLLBACK"), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), equalTo( DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url)))); diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java index ec7ff4f760d3..626e62452fc8 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java @@ -98,6 +98,7 @@ public static Instrumenter createTransactionInstrumenter( return Instrumenter.builder( openTelemetry, INSTRUMENTATION_NAME, DbRequest::getOperation) .addAttributesExtractor(SqlClientAttributesExtractor.builder(dbAttributesGetter).build()) + .addAttributesExtractor(TransactionAttributeExtractor.INSTANCE) .addAttributesExtractor(ServerAttributesExtractor.create(netAttributesGetter)) .addAttributesExtractors(extractors) .setEnabled(enabled) diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionAttributeExtractor.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionAttributeExtractor.java new file mode 100644 index 000000000000..cb3553b1e7cc --- /dev/null +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/TransactionAttributeExtractor.java @@ -0,0 +1,42 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.jdbc.internal; + +import static io.opentelemetry.instrumentation.api.internal.AttributesExtractorUtil.internalSet; + +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.context.Context; +import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import io.opentelemetry.instrumentation.api.internal.SemconvStability; +import javax.annotation.Nullable; + +enum TransactionAttributeExtractor implements AttributesExtractor { + INSTANCE; + + // copied from DbIncubatingAttributes + private static final AttributeKey DB_OPERATION = AttributeKey.stringKey("db.operation"); + private static final AttributeKey DB_OPERATION_NAME = + AttributeKey.stringKey("db.operation.name"); + + @Override + public void onStart(AttributesBuilder attributes, Context parentContext, DbRequest request) { + if (SemconvStability.emitOldDatabaseSemconv()) { + internalSet(attributes, DB_OPERATION, request.getOperation()); + } + if (SemconvStability.emitStableDatabaseSemconv()) { + internalSet(attributes, DB_OPERATION_NAME, request.getOperation()); + } + } + + @Override + public void onEnd( + AttributesBuilder attributes, + Context context, + DbRequest request, + @Nullable Void unused, + @Nullable Throwable error) {} +} diff --git a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java index 328557924836..836525ff2ec3 100644 --- a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java +++ b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java @@ -279,6 +279,7 @@ private static void transactionTraceAssertion(DbInfo dbInfo, String operation) { maybeStable(DB_SYSTEM), maybeStableDbSystemName(dbInfo.getSystem())), equalTo(maybeStable(DB_NAME), dbInfo.getName()), + equalTo(maybeStable(DB_OPERATION), operation), equalTo(DB_USER, emitStableDatabaseSemconv() ? null : dbInfo.getUser()), equalTo( DB_CONNECTION_STRING, From 7bccecd81b966482de74ef4803dbad496a64badb Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 6 May 2025 15:46:25 +0300 Subject: [PATCH 8/9] disable by default --- .../javaagent/instrumentation/jdbc/JdbcSingletons.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java index 78716a6a430d..1b474d833d7a 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java @@ -47,9 +47,7 @@ public final class JdbcSingletons { GlobalOpenTelemetry.get(), Collections.singletonList(peerServiceExtractor), AgentInstrumentationConfig.get() - .getBoolean( - "otel.instrumentation.jdbc.experimental.transaction.enabled", - AgentCommonConfig.get().isStatementSanitizationEnabled())); + .getBoolean("otel.instrumentation.jdbc.experimental.transaction.enabled", false)); } public static Instrumenter transactionInstrumenter() { From 542f4b5777372d84248fc975a721757aeb64ad83 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Thu, 8 May 2025 15:18:22 +0300 Subject: [PATCH 9/9] spring configuration, metadata --- instrumentation/jdbc/metadata.yaml | 3 +++ .../instrumentation/jdbc/DataSourcePostProcessor.java | 4 ++++ .../META-INF/additional-spring-configuration-metadata.json | 6 ++++++ 3 files changed, 13 insertions(+) diff --git a/instrumentation/jdbc/metadata.yaml b/instrumentation/jdbc/metadata.yaml index e52ce68e1aef..23f5d943a8d2 100644 --- a/instrumentation/jdbc/metadata.yaml +++ b/instrumentation/jdbc/metadata.yaml @@ -11,6 +11,9 @@ configurations: - name: otel.instrumentation.common.db-statement-sanitizer.enabled description: Enables statement sanitization for database queries. default: true + - name: otel.instrumentation.jdbc.experimental.transaction.enabled + description: Enables experimental instrumentation to create spans for COMMIT and ROLLBACK operations. + default: false - name: otel.instrumentation.common.peer-service-mapping description: Used to specify a mapping from host names or IP addresses to peer services. default: "" diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/DataSourcePostProcessor.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/DataSourcePostProcessor.java index f29513d01bc5..22c437605de6 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/DataSourcePostProcessor.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/DataSourcePostProcessor.java @@ -55,6 +55,10 @@ public Object postProcessAfterInitialization(Object bean, String beanName) { InstrumentationConfigUtil.isStatementSanitizationEnabled( configPropertiesProvider.getObject(), "otel.instrumentation.jdbc.statement-sanitizer.enabled")) + .setTransactionInstrumenterEnabled( + configPropertiesProvider + .getObject() + .getBoolean("otel.instrumentation.jdbc.experimental.transaction.enabled", false)) .build() .wrap(dataSource); } diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/instrumentation/spring/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json index db72deca72bd..b4f7b9d91875 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -351,6 +351,12 @@ "description": "Enables the DB statement sanitization.", "defaultValue": true }, + { + "name": "otel.instrumentation.jdbc.experimental.transaction.enabled", + "type": "java.lang.Boolean", + "description": "Enables experimental instrumentation to create spans for COMMIT and ROLLBACK operations.", + "defaultValue": false + }, { "name": "otel.instrumentation.kafka.enabled", "type": "java.lang.Boolean",