Skip to content

Added instrumentation for transaction commit/rollback in jdbc #13709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
May 14, 2025
Merged

Added instrumentation for transaction commit/rollback in jdbc #13709

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
83795a3
feat: contribute for #10381
stillya Apr 14, 2025
140583e
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
stillya Apr 14, 2025
54fee2e
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
stillya Apr 15, 2025
fb74aa2
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
stillya Apr 22, 2025
fb22764
Add new transaction instrumeter
stillya Apr 27, 2025
2de16b2
polish commit
stillya Apr 27, 2025
12b544b
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
stillya Apr 27, 2025
510c1b0
polish commit
stillya Apr 27, 2025
ac5ea54
review fixes
stillya Apr 28, 2025
114ccac
reduce the amount of code
laurit May 6, 2025
1336ab2
Merge branch 'main' into feat/jdbc-txn-instrumentation
laurit May 6, 2025
5a4a0d2
add db.operation attribute to transaction spans
laurit May 6, 2025
7bccecd
disable by default
laurit May 6, 2025
a928021
Merge branch 'main' into feat/jdbc-txn-instrumentation
laurit May 8, 2025
542f4b5
spring configuration, metadata
laurit May 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions instrumentation/jdbc/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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. |
6 changes: 6 additions & 0 deletions instrumentation/jdbc/javaagent/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,9 @@ tasks {
dependsOn(testSlickStableSemconv)
}
}

tasks {
withType<Test>().configureEach {
jvmArgs("-Dotel.instrumentation.jdbc.experimental.transaction.enabled=true")
}
}
58 changes: 58 additions & 0 deletions .../main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,29 @@

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.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;
import java.sql.PreparedStatement;
import java.util.Locale;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
Expand All @@ -40,6 +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");
}

@SuppressWarnings("unused")
Expand All @@ -51,4 +66,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;
}
TransactionRequest request =
TransactionRequest.create(dbInfo, methodName.toUpperCase(Locale.ROOT));

if (!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") TransactionRequest request,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
if (scope == null) {
return;
}
scope.close();
transactionInstrumenter().end(context, request, null, throwable);
}
}
}
47 changes: 39 additions & 8 deletions ...vaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,32 +30,60 @@ 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<TransactionRequest, Void> TRANSACTION_INSTRUMENTER;
public static final Instrumenter<DataSource, DbInfo> 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.<DbRequest, Void>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.<TransactionRequest, Void>builder(
Copy link
Contributor

@laurit laurit Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could try to reuse the code in JdbcInstrumenterFactory

Copy link
Contributor Author

@stillya stillya Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, because we use PeerServiceAttributesExtractor, which is agent-specific.

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())
Copy link
Contributor

@laurit laurit Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addOperationMetrics(DbClientMetrics.get()) is responsible for creating metrics, you probably should not add it to the transaction isntrumenter

Copy link
Contributor Author

@stillya stillya Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

.setEnabled(
AgentInstrumentationConfig.get()
.getBoolean(
"otel.instrumentation.jdbc.experimental.transaction.enabled", false))
.buildInstrumenter(SpanKindExtractor.alwaysClient());
}

public static Instrumenter<TransactionRequest, Void> transactionInstrumenter() {
return TRANSACTION_INSTRUMENTER;
}

public static Instrumenter<DbRequest, Void> statementInstrumenter() {
Expand Down
142 changes: 142 additions & 0 deletions ...st/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/JdbcInstrumentationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1632,4 +1632,146 @@ 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")
.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);
}
}

@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")
.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);
Copy link
Contributor

@laurit laurit Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that this could cause test flakiness. executeQuery creates a span but since it is not asserted there is a possibility that the span arrives after the test has completed and exported data is reset before the next test. If that happens the next test will fail because there is an unexpected span. You could work around this by calling testing.clearData(); before this code (to clear the existing trace) and testing.waitForTraces(1); after this code to ensure that trace data gets cleared before the next test.

Copy link
Contributor Author

@stillya stillya Apr 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dropped these asserts since they're irrelevant to the test (we don't check rollback functionality).

} finally {
connection.setAutoCommit(originalAutoCommit);
}
}

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:"));
}
}
6 changes: 6 additions & 0 deletions instrumentation/jdbc/library/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,9 @@ tasks {
dependsOn(testStableSemconv)
}
}

tasks {
withType<Test>().configureEach {
jvmArgs("-Dotel.instrumentation.jdbc.experimental.transaction.enabled=true")
}
}
10 changes: 9 additions & 1 deletion ...jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -244,7 +246,13 @@ public Connection connect(String url, Properties info) throws SQLException {

Instrumenter<DbRequest, Void> statementInstrumenter =
JdbcInstrumenterFactory.createStatementInstrumenter(openTelemetry);
return OpenTelemetryConnection.create(connection, dbInfo, statementInstrumenter);
Instrumenter<TransactionRequest, Void> transactionInstrumenter =
JdbcInstrumenterFactory.createTransactionInstrumenter(
openTelemetry,
ConfigPropertiesUtil.getBoolean(
"otel.instrumentation.jdbc.experimental.transaction.enabled", false));
return OpenTelemetryConnection.create(
connection, dbInfo, statementInstrumenter, transactionInstrumenter);
}

@Override
Expand Down
Loading
Loading