Added instrumentation for transaction commit/rollback in jdbc

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.transaction.enabled` | Boolean | `false` | Enables experimental instrumentation to create spans for COMMIT and ROLLBACK operations. |

instrumentation/jdbc/javaagent/build.gradle.kts

@@ -91,3 +91,9 @@ tasks {
     dependsOn(testSlickStableSemconv)
   }
 }
+
+tasks {
+  withType<Test>().configureEach {
+    jvmArgs("-Dotel.instrumentation.jdbc.experimental.transaction.enabled=true")
+  }
+}

instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java

@@ -5,17 +5,27 @@
 
 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.transactionInstrumenter;
+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.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;
@@ -40,6 +50,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() + "$TransactionAdvice");
   }
 
   @SuppressWarnings("unused")
@@ -51,4 +64,39 @@ public static void addDbInfo(
       JdbcData.preparedStatement.set(statement, sql);
     }
   }
+
+  @SuppressWarnings("unused")
+  public static class TransactionAdvice {
+
+    @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();
+      DbRequest request =
+          DbRequest.createTransaction(connection, methodName.toUpperCase(Locale.ROOT));
+
+      if (request == null || !transactionInstrumenter().shouldStart(parentContext, request)) {
+        return;
+      }
+
+      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("otelContext") Context context,
+        @Advice.Local("otelScope") Scope scope) {
+      if (scope == null) {
+        return;
+      }
+      scope.close();
+      transactionInstrumenter().end(context, request, null, throwable);
+    }
+  }
 }

instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java

@@ -8,51 +8,50 @@
 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.JdbcAttributesGetter;
+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<DbRequest, Void> STATEMENT_INSTRUMENTER;
+  private static final Instrumenter<DbRequest, Void> TRANSACTION_INSTRUMENTER;
   public static final Instrumenter<DataSource, DbInfo> DATASOURCE_INSTRUMENTER =
       createDataSourceInstrumenter(GlobalOpenTelemetry.get(), true);
 
   static {
-    JdbcAttributesGetter dbAttributesGetter = new JdbcAttributesGetter();
     JdbcNetworkAttributesGetter netAttributesGetter = new JdbcNetworkAttributesGetter();
+    AttributesExtractor<DbRequest, Void> peerServiceExtractor =
+        PeerServiceAttributesExtractor.create(
+            netAttributesGetter, AgentCommonConfig.get().getPeerServiceResolver());
 
     STATEMENT_INSTRUMENTER =
-        Instrumenter.<DbRequest, Void>builder(
-                GlobalOpenTelemetry.get(),
-                INSTRUMENTATION_NAME,
-                DbClientSpanNameExtractor.create(dbAttributesGetter))
-            .addAttributesExtractor(
-                SqlClientAttributesExtractor.builder(dbAttributesGetter)
-                    .setStatementSanitizationEnabled(
-                        AgentInstrumentationConfig.get()
-                            .getBoolean(
-                                "otel.instrumentation.jdbc.statement-sanitizer.enabled",
-                                AgentCommonConfig.get().isStatementSanitizationEnabled()))
-                    .build())
-            .addAttributesExtractor(ServerAttributesExtractor.create(netAttributesGetter))
-            .addAttributesExtractor(
-                PeerServiceAttributesExtractor.create(
-                    netAttributesGetter, 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 =
+        JdbcInstrumenterFactory.createTransactionInstrumenter(
+            GlobalOpenTelemetry.get(),
+            Collections.singletonList(peerServiceExtractor),
+            AgentInstrumentationConfig.get()
+                .getBoolean("otel.instrumentation.jdbc.experimental.transaction.enabled", false));
+  }
+
+  public static Instrumenter<DbRequest, Void> transactionInstrumenter() {
+    return TRANSACTION_INSTRUMENTER;
   }
 
   public static Instrumenter<DbRequest, Void> statementInstrumenter() {

instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/JdbcInstrumentationTest.java

@@ -1632,4 +1632,128 @@ 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);
+
+    connection.setAutoCommit(false);
+
+    testing.waitForTraces(1);
+    testing.clearData();
+
+    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(maybeStable(DB_OPERATION), "COMMIT"),
+                            equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username),
+                            equalTo(
+                                DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url))));
+  }
+
+  @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);
+
+    connection.setAutoCommit(false);
+
+    testing.waitForTraces(1);
+    testing.clearData();
+
+    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(maybeStable(DB_OPERATION), "ROLLBACK"),
+                            equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username),
+                            equalTo(
+                                DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url))));
+  }
+
+  static Stream<Arguments> 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:"));
+  }
 }

instrumentation/jdbc/library/build.gradle.kts

@@ -59,3 +59,9 @@ tasks {
     dependsOn(testStableSemconv)
   }
 }
+
+tasks {
+  withType<Test>().configureEach {
+    jvmArgs("-Dotel.instrumentation.jdbc.experimental.transaction.enabled=true")
+  }
+}

instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java

@@ -244,7 +244,10 @@ public Connection connect(String url, Properties info) throws SQLException {
 
     Instrumenter<DbRequest, Void> statementInstrumenter =
         JdbcInstrumenterFactory.createStatementInstrumenter(openTelemetry);
-    return OpenTelemetryConnection.create(connection, dbInfo, statementInstrumenter);
+    Instrumenter<DbRequest, Void> transactionInstrumenter =
+        JdbcInstrumenterFactory.createTransactionInstrumenter(openTelemetry);
+    return OpenTelemetryConnection.create(
+        connection, dbInfo, statementInstrumenter, transactionInstrumenter);
   }
 
   @Override

instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetry.java

@@ -26,16 +26,22 @@ public static JdbcTelemetryBuilder builder(OpenTelemetry openTelemetry) {
 
   private final Instrumenter<DataSource, DbInfo> dataSourceInstrumenter;
   private final Instrumenter<DbRequest, Void> statementInstrumenter;
+  private final Instrumenter<DbRequest, Void> transactionInstrumenter;
 
   JdbcTelemetry(
       Instrumenter<DataSource, DbInfo> dataSourceInstrumenter,
-      Instrumenter<DbRequest, Void> statementInstrumenter) {
+      Instrumenter<DbRequest, Void> statementInstrumenter,
+      Instrumenter<DbRequest, Void> 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);
   }
 }

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. Disabled 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));
   }
 }