Skip to content

Commit ad17c0e

Browse files
stillyastillya
committed
Add new transaction instrumeter
- fix review comments - add seperate transaction instrumenter
1 parent fb74aa2 commit ad17c0e

File tree

23 files changed

+396
-184
lines changed

23 files changed

+396
-184
lines changed

‎instrumentation-api-incubator/src/main/jflex/SqlSanitizer.jflex‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -347,12 +347,6 @@ WHITESPACE = [ \t\r\n]+
347347
}
348348
}
349349

350-
private class Commit extends Operation {
351-
}
352-
353-
private class Rollback extends Operation {
354-
}
355-
356350
private class Create extends DdlOperation {
357351
}
358352

@@ -491,20 +485,6 @@ WHITESPACE = [ \t\r\n]+
491485
appendCurrentFragment();
492486
if (isOverLimit()) return YYEOF;
493487
}
494-
"COMMIT" {
495-
if (!insideComment) {
496-
setOperation(new Commit());
497-
}
498-
appendCurrentFragment();
499-
if (isOverLimit()) return YYEOF;
500-
}
501-
"ROLLBACK" {
502-
if (!insideComment) {
503-
setOperation(new Rollback());
504-
}
505-
appendCurrentFragment();
506-
if (isOverLimit()) return YYEOF;
507-
}
508488

509489
{COMMA} {
510490
if (!insideComment && !extractionDone) {

‎instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlStatementSanitizerTest.java‎

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -71,17 +71,6 @@ void checkDdlOperationStatementsAreOk(
7171
assertThat(result.getMainIdentifier()).isEqualTo(expected.getMainIdentifier());
7272
}
7373

74-
@ParameterizedTest
75-
@ArgumentsSource(TransactionArgs.class)
76-
void checkTransactionOperationStatementsAreOk(
77-
String actual, Function<String, SqlStatementInfo> expectFunc) {
78-
SqlStatementInfo result = SqlStatementSanitizer.create(true).sanitize(actual);
79-
SqlStatementInfo expected = expectFunc.apply(actual);
80-
assertThat(result.getFullStatement()).isEqualTo(expected.getFullStatement());
81-
assertThat(result.getOperation()).isEqualTo(expected.getOperation());
82-
assertThat(result.getMainIdentifier()).isEqualTo(expected.getMainIdentifier());
83-
}
84-
8574
@Test
8675
void lotsOfTicksDontCauseStackOverflowOrLongRuntimes() {
8776
String s = "'";
@@ -438,21 +427,4 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
438427
"CREATE PROCEDURE p AS SELECT * FROM table GO", expect("CREATE PROCEDURE", null)));
439428
}
440429
}
441-
442-
static class TransactionArgs implements ArgumentsProvider {
443-
static Function<String, SqlStatementInfo> expect(String operation, String identifier) {
444-
return sql -> SqlStatementInfo.create(sql, operation, identifier);
445-
}
446-
447-
@Override
448-
public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
449-
return Stream.of(
450-
Arguments.of("COMMIT", expect("COMMIT", null)),
451-
Arguments.of("commit", expect("COMMIT", null)),
452-
Arguments.of("/*COMMIT*/", expect(null, null)),
453-
Arguments.of("ROLLBACK", expect("ROLLBACK", null)),
454-
Arguments.of("rollback", expect("ROLLBACK", null)),
455-
Arguments.of("/*ROLLBACK*/", expect(null, null)));
456-
}
457-
}
458430
}

‎instrumentation/jdbc/README.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Settings for the JDBC instrumentation
22

3-
| System property | Type | Default | Description |
4-
|---------------------------------------------------------|---------|---------|-------------------------------------------------------|
5-
| `otel.instrumentation.jdbc.statement-sanitizer.enabled` | Boolean | `true` | Enables the DB statement sanitization. |
6-
| `otel.instrumentation.jdbc.experimental.txn.enabled` | Boolean | `false` | Enables the experimental transaction instrumentation. |
3+
| System property | Type | Default | Description |
4+
|--------------------------------------------------------------|---------|---------|------------------------------------------------------------------------------------------|
5+
| `otel.instrumentation.jdbc.statement-sanitizer.enabled` | Boolean | `true` | Enables the DB statement sanitization. |
6+
| `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‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,6 @@ tasks {
9494

9595
tasks {
9696
withType<Test>().configureEach {
97-
jvmArgs("-Dotel.instrumentation.jdbc.experimental.txn.enabled=true")
97+
jvmArgs("-Dotel.instrumentation.jdbc.experimental.transaction.enabled=true")
9898
}
9999
}

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

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext;
99
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
1010
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
11-
import static io.opentelemetry.javaagent.instrumentation.jdbc.JdbcSingletons.statementInstrumenter;
11+
import static io.opentelemetry.javaagent.instrumentation.jdbc.JdbcSingletons.transactionInstrumenter;
1212
import static net.bytebuddy.matcher.ElementMatchers.isPublic;
1313
import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith;
1414
import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -19,10 +19,9 @@
1919

2020
import io.opentelemetry.context.Context;
2121
import io.opentelemetry.context.Scope;
22-
import io.opentelemetry.instrumentation.jdbc.internal.DbRequest;
2322
import io.opentelemetry.instrumentation.jdbc.internal.JdbcData;
2423
import io.opentelemetry.instrumentation.jdbc.internal.JdbcUtils;
25-
import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig;
24+
import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest;
2625
import io.opentelemetry.javaagent.bootstrap.jdbc.DbInfo;
2726
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
2827
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
@@ -35,10 +34,6 @@
3534

3635
public class ConnectionInstrumentation implements TypeInstrumentation {
3736

38-
private static final boolean TXN_ENABLED =
39-
AgentInstrumentationConfig.get()
40-
.getBoolean("otel.instrumentation.jdbc.experimental.txn.enabled", false);
41-
4237
@Override
4338
public ElementMatcher<ClassLoader> classLoaderOptimization() {
4439
return hasClassesNamed("java.sql.Connection");
@@ -57,11 +52,9 @@ public void transform(TypeTransformer transformer) {
5752
// Also include CallableStatement, which is a sub type of PreparedStatement
5853
.and(returns(implementsInterface(named("java.sql.PreparedStatement")))),
5954
ConnectionInstrumentation.class.getName() + "$PrepareAdvice");
60-
if (TXN_ENABLED) {
6155
transformer.applyAdviceToMethod(
6256
namedOneOf("commit", "rollback").and(takesNoArguments()).and(isPublic()),
6357
ConnectionInstrumentation.class.getName() + "$TxnAdvice");
64-
}
6558
}
6659

6760
@SuppressWarnings("unused")
@@ -92,28 +85,28 @@ public static void onEnter(
9285
if (dbInfo == null) {
9386
return;
9487
}
95-
DbRequest request = DbRequest.create(dbInfo, methodName.toUpperCase(Locale.ROOT));
88+
TransactionRequest request =
89+
TransactionRequest.create(dbInfo, methodName.toUpperCase(Locale.ROOT));
9690

97-
if (!statementInstrumenter().shouldStart(parentContext, request)) {
91+
if (!transactionInstrumenter().shouldStart(parentContext, request)) {
9892
return;
9993
}
10094

101-
request = DbRequest.create(request.getDbInfo(), methodName.toUpperCase(Locale.ROOT));
102-
context = statementInstrumenter().start(parentContext, request);
95+
context = transactionInstrumenter().start(parentContext, request);
10396
scope = context.makeCurrent();
10497
}
10598

10699
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
107100
public static void stopSpan(
108101
@Advice.Thrown Throwable throwable,
109-
@Advice.Local("otelRequest") DbRequest request,
102+
@Advice.Local("otelRequest") TransactionRequest request,
110103
@Advice.Local("otelContext") Context context,
111104
@Advice.Local("otelScope") Scope scope) {
112105
if (scope == null) {
113106
return;
114107
}
115108
scope.close();
116-
statementInstrumenter().end(context, request, null, throwable);
109+
transactionInstrumenter().end(context, request, null, throwable);
117110
}
118111
}
119112
}

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

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616
import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
1717
import io.opentelemetry.instrumentation.api.semconv.network.ServerAttributesExtractor;
1818
import io.opentelemetry.instrumentation.jdbc.internal.DbRequest;
19-
import io.opentelemetry.instrumentation.jdbc.internal.JdbcAttributesGetter;
20-
import io.opentelemetry.instrumentation.jdbc.internal.JdbcNetworkAttributesGetter;
19+
import io.opentelemetry.instrumentation.jdbc.internal.JdbcStatementAttributesGetter;
20+
import io.opentelemetry.instrumentation.jdbc.internal.JdbcTransactionAttributesGetter;
21+
import io.opentelemetry.instrumentation.jdbc.internal.StatementNetworkAttributesGetter;
22+
import io.opentelemetry.instrumentation.jdbc.internal.TransactionNetworkAttributesGetter;
23+
import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest;
2124
import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig;
2225
import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig;
2326
import io.opentelemetry.javaagent.bootstrap.jdbc.DbInfo;
@@ -27,32 +30,60 @@ public final class JdbcSingletons {
2730
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.jdbc";
2831

2932
private static final Instrumenter<DbRequest, Void> STATEMENT_INSTRUMENTER;
33+
private static final Instrumenter<TransactionRequest, Void> TRANSACTION_INSTRUMENTER;
3034
public static final Instrumenter<DataSource, DbInfo> DATASOURCE_INSTRUMENTER =
3135
createDataSourceInstrumenter(GlobalOpenTelemetry.get(), true);
3236

3337
static {
34-
JdbcAttributesGetter dbAttributesGetter = new JdbcAttributesGetter();
35-
JdbcNetworkAttributesGetter netAttributesGetter = new JdbcNetworkAttributesGetter();
38+
JdbcStatementAttributesGetter statementAttributesGetter = new JdbcStatementAttributesGetter();
39+
JdbcTransactionAttributesGetter transactionAttributesGetter =
40+
new JdbcTransactionAttributesGetter();
41+
StatementNetworkAttributesGetter statementNetAttributesGetter =
42+
new StatementNetworkAttributesGetter();
43+
TransactionNetworkAttributesGetter transactionNetAttributesGetter =
44+
new TransactionNetworkAttributesGetter();
3645

3746
STATEMENT_INSTRUMENTER =
3847
Instrumenter.<DbRequest, Void>builder(
3948
GlobalOpenTelemetry.get(),
4049
INSTRUMENTATION_NAME,
41-
DbClientSpanNameExtractor.create(dbAttributesGetter))
50+
DbClientSpanNameExtractor.create(statementAttributesGetter))
4251
.addAttributesExtractor(
43-
SqlClientAttributesExtractor.builder(dbAttributesGetter)
52+
SqlClientAttributesExtractor.builder(statementAttributesGetter)
4453
.setStatementSanitizationEnabled(
4554
AgentInstrumentationConfig.get()
4655
.getBoolean(
4756
"otel.instrumentation.jdbc.statement-sanitizer.enabled",
4857
AgentCommonConfig.get().isStatementSanitizationEnabled()))
4958
.build())
50-
.addAttributesExtractor(ServerAttributesExtractor.create(netAttributesGetter))
59+
.addAttributesExtractor(ServerAttributesExtractor.create(statementNetAttributesGetter))
5160
.addAttributesExtractor(
5261
PeerServiceAttributesExtractor.create(
53-
netAttributesGetter, AgentCommonConfig.get().getPeerServiceResolver()))
62+
statementNetAttributesGetter, AgentCommonConfig.get().getPeerServiceResolver()))
5463
.addOperationMetrics(DbClientMetrics.get())
5564
.buildInstrumenter(SpanKindExtractor.alwaysClient());
65+
66+
TRANSACTION_INSTRUMENTER =
67+
Instrumenter.<TransactionRequest, Void>builder(
68+
GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, TransactionRequest::spanName)
69+
.addAttributesExtractor(
70+
SqlClientAttributesExtractor.builder(transactionAttributesGetter).build())
71+
.addAttributesExtractor(
72+
ServerAttributesExtractor.create(transactionNetAttributesGetter))
73+
.addAttributesExtractor(
74+
PeerServiceAttributesExtractor.create(
75+
transactionNetAttributesGetter,
76+
AgentCommonConfig.get().getPeerServiceResolver()))
77+
.addOperationMetrics(DbClientMetrics.get())
78+
.setEnabled(
79+
AgentInstrumentationConfig.get()
80+
.getBoolean(
81+
"otel.instrumentation.jdbc.experimental.transaction.enabled", false))
82+
.buildInstrumenter(SpanKindExtractor.alwaysClient());
83+
}
84+
85+
public static Instrumenter<TransactionRequest, Void> transactionInstrumenter() {
86+
return TRANSACTION_INSTRUMENTER;
5687
}
5788

5889
public static Instrumenter<DbRequest, Void> statementInstrumenter() {

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

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION;
2020
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION_BATCH_SIZE;
2121
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION_NAME;
22-
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_QUERY_TEXT;
2322
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SQL_TABLE;
2423
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_STATEMENT;
2524
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SYSTEM;
@@ -1681,18 +1680,16 @@ DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url),
16811680
equalTo(maybeStable(DB_OPERATION), "INSERT"),
16821681
equalTo(maybeStable(DB_SQL_TABLE), tableName)),
16831682
span ->
1684-
span.hasName("COMMIT " + dbNameLower)
1683+
span.hasName("COMMIT")
16851684
.hasKind(SpanKind.CLIENT)
16861685
.hasParent(trace.getSpan(0))
16871686
.hasAttributesSatisfyingExactly(
16881687
equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)),
16891688
equalTo(maybeStable(DB_NAME), dbNameLower),
16901689
equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username),
1691-
equalTo(DB_STATEMENT, emitStableDatabaseSemconv() ? null : "COMMIT"),
1692-
equalTo(DB_QUERY_TEXT, emitStableDatabaseSemconv() ? "COMMIT" : null),
16931690
equalTo(
1694-
DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url),
1695-
equalTo(maybeStable(DB_OPERATION), "COMMIT"))));
1691+
DB_CONNECTION_STRING,
1692+
emitStableDatabaseSemconv() ? null : url))));
16961693
} finally {
16971694
connection.setAutoCommit(originalAutoCommit);
16981695
}
@@ -1745,20 +1742,16 @@ DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url),
17451742
equalTo(maybeStable(DB_OPERATION), "INSERT"),
17461743
equalTo(maybeStable(DB_SQL_TABLE), tableName)),
17471744
span ->
1748-
span.hasName("ROLLBACK " + dbNameLower)
1745+
span.hasName("ROLLBACK")
17491746
.hasKind(SpanKind.CLIENT)
17501747
.hasParent(trace.getSpan(0))
17511748
.hasAttributesSatisfyingExactly(
17521749
equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)),
17531750
equalTo(maybeStable(DB_NAME), dbNameLower),
17541751
equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username),
17551752
equalTo(
1756-
DB_STATEMENT, emitStableDatabaseSemconv() ? null : "ROLLBACK"),
1757-
equalTo(
1758-
DB_QUERY_TEXT, emitStableDatabaseSemconv() ? "ROLLBACK" : null),
1759-
equalTo(
1760-
DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url),
1761-
equalTo(maybeStable(DB_OPERATION), "ROLLBACK"))));
1753+
DB_CONNECTION_STRING,
1754+
emitStableDatabaseSemconv() ? null : url))));
17621755

17631756
Statement selectStatement = connection.createStatement();
17641757
cleanup.deferCleanup(selectStatement);

‎instrumentation/jdbc/library/build.gradle.kts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,6 @@ tasks {
6262

6363
tasks {
6464
withType<Test>().configureEach {
65-
jvmArgs("-Dotel.instrumentation.jdbc.experimental.txn.enabled=true")
65+
jvmArgs("-Dotel.instrumentation.jdbc.experimental.transaction.enabled=true")
6666
}
6767
}

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,13 @@
2424

2525
import io.opentelemetry.api.OpenTelemetry;
2626
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
27+
import io.opentelemetry.instrumentation.api.internal.ConfigPropertiesUtil;
2728
import io.opentelemetry.instrumentation.api.internal.EmbeddedInstrumentationProperties;
2829
import io.opentelemetry.instrumentation.jdbc.internal.DbRequest;
2930
import io.opentelemetry.instrumentation.jdbc.internal.JdbcConnectionUrlParser;
3031
import io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory;
3132
import io.opentelemetry.instrumentation.jdbc.internal.OpenTelemetryConnection;
33+
import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest;
3234
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
3335
import java.sql.Connection;
3436
import java.sql.Driver;
@@ -244,7 +246,13 @@ public Connection connect(String url, Properties info) throws SQLException {
244246

245247
Instrumenter<DbRequest, Void> statementInstrumenter =
246248
JdbcInstrumenterFactory.createStatementInstrumenter(openTelemetry);
247-
return OpenTelemetryConnection.create(connection, dbInfo, statementInstrumenter);
249+
Instrumenter<TransactionRequest, Void> transactionInstrumenter =
250+
JdbcInstrumenterFactory.createTransactionInstrumenter(
251+
openTelemetry,
252+
ConfigPropertiesUtil.getBoolean(
253+
"otel.instrumentation.jdbc.experimental.transaction.enabled", false));
254+
return OpenTelemetryConnection.create(
255+
connection, dbInfo, statementInstrumenter, transactionInstrumenter);
248256
}
249257

250258
@Override

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import io.opentelemetry.api.OpenTelemetry;
99
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
1010
import io.opentelemetry.instrumentation.jdbc.internal.DbRequest;
11+
import io.opentelemetry.instrumentation.jdbc.internal.TransactionRequest;
1112
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
1213
import javax.sql.DataSource;
1314

@@ -26,16 +27,22 @@ public static JdbcTelemetryBuilder builder(OpenTelemetry openTelemetry) {
2627

2728
private final Instrumenter<DataSource, DbInfo> dataSourceInstrumenter;
2829
private final Instrumenter<DbRequest, Void> statementInstrumenter;
30+
private final Instrumenter<TransactionRequest, Void> transactionInstrumenter;
2931

3032
JdbcTelemetry(
3133
Instrumenter<DataSource, DbInfo> dataSourceInstrumenter,
32-
Instrumenter<DbRequest, Void> statementInstrumenter) {
34+
Instrumenter<DbRequest, Void> statementInstrumenter,
35+
Instrumenter<TransactionRequest, Void> transactionInstrumenter) {
3336
this.dataSourceInstrumenter = dataSourceInstrumenter;
3437
this.statementInstrumenter = statementInstrumenter;
38+
this.transactionInstrumenter = transactionInstrumenter;
3539
}
3640

3741
public DataSource wrap(DataSource dataSource) {
3842
return new OpenTelemetryDataSource(
39-
dataSource, this.dataSourceInstrumenter, this.statementInstrumenter);
43+
dataSource,
44+
this.dataSourceInstrumenter,
45+
this.statementInstrumenter,
46+
this.transactionInstrumenter);
4047
}
4148
}

0 commit comments

Comments
 (0)