Skip to content

Commit 7093d6d

Browse files
lauritjaydelucaotelbot[bot]
authored
Add sqlcommenter support for jdbc and r2dbc (#13714)
Co-authored-by: Jay DeLuca <[email protected]> Co-authored-by: otelbot <[email protected]>
1 parent 54b01fd commit 7093d6d

File tree

34 files changed

+1274
-170
lines changed

34 files changed

+1274
-170
lines changed

‎instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/config/internal/CommonConfig.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ public final class CommonConfig {
3434
private final Set<String> knownHttpRequestMethods;
3535
private final EnduserConfig enduserConfig;
3636
private final boolean statementSanitizationEnabled;
37+
private final boolean sqlCommenterEnabled;
3738
private final boolean emitExperimentalHttpClientTelemetry;
3839
private final boolean emitExperimentalHttpServerTelemetry;
3940
private final boolean redactQueryParameters;
@@ -87,6 +88,9 @@ public CommonConfig(InstrumentationConfig config) {
8788
new ArrayList<>(HttpConstants.KNOWN_METHODS)));
8889
statementSanitizationEnabled =
8990
config.getBoolean("otel.instrumentation.common.db-statement-sanitizer.enabled", true);
91+
sqlCommenterEnabled =
92+
config.getBoolean(
93+
"otel.instrumentation.common.experimental.db-sqlcommenter.enabled", false);
9094
emitExperimentalHttpClientTelemetry =
9195
config.getBoolean("otel.instrumentation.http.client.emit-experimental-telemetry", false);
9296
redactQueryParameters =
@@ -138,6 +142,10 @@ public boolean isStatementSanitizationEnabled() {
138142
return statementSanitizationEnabled;
139143
}
140144

145+
public boolean isSqlCommenterEnabled() {
146+
return sqlCommenterEnabled;
147+
}
148+
141149
public boolean shouldEmitExperimentalHttpClientTelemetry() {
142150
return emitExperimentalHttpClientTelemetry;
143151
}

‎instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/SqlCommenterUtil.java‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.instrumentation.api.incubator.semconv.db.internal;
7+
8+
import io.opentelemetry.api.trace.Span;
9+
import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator;
10+
import io.opentelemetry.context.Context;
11+
import java.io.UnsupportedEncodingException;
12+
import java.net.URLEncoder;
13+
import javax.annotation.Nullable;
14+
15+
/**
16+
* This class is internal and experimental. Its APIs are unstable and can change at any time. Its
17+
* APIs (or a version of them) may be promoted to the public stable API in the future, but no
18+
* guarantees are made.
19+
*/
20+
public final class SqlCommenterUtil {
21+
22+
/**
23+
* Append comment containing tracing information at the end of the query. See <a
24+
* href="https://google.github.io/sqlcommenter/spec/">sqlcommenter</a> for the description of the
25+
* algorithm.
26+
*/
27+
public static String processQuery(String query) {
28+
if (!Span.current().getSpanContext().isValid()) {
29+
return query;
30+
}
31+
// skip queries that contain comments
32+
if (containsSqlComment(query)) {
33+
return query;
34+
}
35+
36+
class State {
37+
@Nullable String traceparent;
38+
@Nullable String tracestate;
39+
}
40+
41+
State state = new State();
42+
43+
W3CTraceContextPropagator.getInstance()
44+
.inject(
45+
Context.current(),
46+
state,
47+
(carrier, key, value) -> {
48+
if (carrier == null) {
49+
return;
50+
}
51+
if ("traceparent".equals(key)) {
52+
carrier.traceparent = value;
53+
} else if ("tracestate".equals(key)) {
54+
carrier.tracestate = value;
55+
}
56+
});
57+
try {
58+
// we know that the traceparent doesn't contain anything that needs to be encoded
59+
query += " /*traceparent='" + state.traceparent + "'";
60+
if (state.tracestate != null) {
61+
query += ", tracestate=" + serialize(state.tracestate);
62+
}
63+
query += "*/";
64+
} catch (UnsupportedEncodingException exception) {
65+
// this exception should never happen as UTF-8 encoding is always available
66+
}
67+
return query;
68+
}
69+
70+
private static boolean containsSqlComment(String query) {
71+
return query.contains("--") || query.contains("/*");
72+
}
73+
74+
private static String serialize(String value) throws UnsupportedEncodingException {
75+
// specification requires percent encoding, here we use the java build in url encoder that
76+
// encodes space as '+' instead of '%20' as required
77+
String result = URLEncoder.encode(value, "UTF-8").replace("+", "%20");
78+
// specification requires escaping ' with a backslash, we skip this because URLEncoder already
79+
// encodes the '
80+
return "'" + result + "'";
81+
}
82+
83+
private SqlCommenterUtil() {}
84+
}

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.instrumentation.api.incubator.semconv.db.internal;
7+
8+
import static org.assertj.core.api.Assertions.assertThat;
9+
10+
import io.opentelemetry.api.trace.Span;
11+
import io.opentelemetry.api.trace.SpanContext;
12+
import io.opentelemetry.api.trace.TraceFlags;
13+
import io.opentelemetry.api.trace.TraceState;
14+
import io.opentelemetry.context.Context;
15+
import io.opentelemetry.context.Scope;
16+
import org.junit.jupiter.params.ParameterizedTest;
17+
import org.junit.jupiter.params.provider.ValueSource;
18+
19+
class SqlCommenterUtilTest {
20+
21+
@ParameterizedTest
22+
@ValueSource(strings = {"SELECT /**/ 1", "SELECT 1 --", "SELECT '/*'"})
23+
void skipQueriesWithComments(String query) {
24+
Context parent =
25+
Context.root()
26+
.with(
27+
Span.wrap(
28+
SpanContext.create(
29+
"ff01020304050600ff0a0b0c0d0e0f00",
30+
"090a0b0c0d0e0f00",
31+
TraceFlags.getSampled(),
32+
TraceState.getDefault())));
33+
34+
try (Scope ignore = parent.makeCurrent()) {
35+
assertThat(SqlCommenterUtil.processQuery(query)).isEqualTo(query);
36+
}
37+
}
38+
39+
@ParameterizedTest
40+
@ValueSource(booleans = {true, false})
41+
void sqlCommenter(boolean hasTraceState) {
42+
TraceState state =
43+
hasTraceState ? TraceState.builder().put("test", "test'").build() : TraceState.getDefault();
44+
Context parent =
45+
Context.root()
46+
.with(
47+
Span.wrap(
48+
SpanContext.create(
49+
"ff01020304050600ff0a0b0c0d0e0f00",
50+
"090a0b0c0d0e0f00",
51+
TraceFlags.getSampled(),
52+
state)));
53+
54+
try (Scope ignore = parent.makeCurrent()) {
55+
assertThat(SqlCommenterUtil.processQuery("SELECT 1"))
56+
.isEqualTo(
57+
hasTraceState
58+
? "SELECT 1 /*traceparent='00-ff01020304050600ff0a0b0c0d0e0f00-090a0b0c0d0e0f00-01', tracestate='test%3Dtest%27'*/"
59+
: "SELECT 1 /*traceparent='00-ff01020304050600ff0a0b0c0d0e0f00-090a0b0c0d0e0f00-01'*/");
60+
}
61+
}
62+
}

‎instrumentation/jdbc/README.md‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
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.capture-query-parameters` | Boolean | `false` | Enable the capture of query parameters as span attributes. Enabling this option disables the statement sanitization. <p>WARNING: captured query parameters may contain sensitive information such as passwords, personally identifiable information or protected health info. |
7-
| `otel.instrumentation.jdbc.experimental.transaction.enabled` | Boolean | `false` | Enables experimental instrumentation to create spans for COMMIT and ROLLBACK operations. |
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.capture-query-parameters` | Boolean | `false` | Enable the capture of query parameters as span attributes. Enabling this option disables the statement sanitization. <p>WARNING: captured query parameters may contain sensitive information such as passwords, personally identifiable information or protected health info. |
7+
| `otel.instrumentation.jdbc.experimental.transaction.enabled` | Boolean | `false` | Enables experimental instrumentation to create spans for COMMIT and ROLLBACK operations. |
8+
| `otel.instrumentation.jdbc.experimental.sqlcommenter.enabled` | Boolean | `false` | Enables augmenting queries with a comment containing the tracing information. See [sqlcommenter](https://google.github.io/sqlcommenter/) for more info. WARNING: augmenting queries with tracing context will make query texts unique, which may have adverse impact on database performance. Consult with database experts before enabling. |

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,21 @@ tasks {
6666
include("**/SlickTest.*")
6767
}
6868

69+
val testSqlCommenter by registering(Test::class) {
70+
filter {
71+
includeTestsMatching("SqlCommenterTest")
72+
}
73+
include("**/SqlCommenterTest.*")
74+
jvmArgs("-Dotel.instrumentation.jdbc.experimental.sqlcommenter.enabled=true")
75+
}
76+
6977
val testStableSemconv by registering(Test::class) {
7078
testClassesDirs = sourceSets.test.get().output.classesDirs
7179
classpath = sourceSets.test.get().runtimeClasspath
7280

7381
filter {
7482
excludeTestsMatching("SlickTest")
83+
excludeTestsMatching("SqlCommenterTest")
7584
excludeTestsMatching("PreparedStatementParametersTest")
7685
}
7786
jvmArgs("-Dotel.instrumentation.jdbc-datasource.enabled=true")
@@ -102,13 +111,18 @@ tasks {
102111
test {
103112
filter {
104113
excludeTestsMatching("SlickTest")
114+
excludeTestsMatching("SqlCommenterTest")
105115
excludeTestsMatching("PreparedStatementParametersTest")
106116
}
107117
jvmArgs("-Dotel.instrumentation.jdbc-datasource.enabled=true")
108118
}
109119

110120
check {
111-
dependsOn(testSlick, testStableSemconv, testSlickStableSemconv, testCaptureParameters)
121+
dependsOn(testSlick)
122+
dependsOn(testSqlCommenter)
123+
dependsOn(testStableSemconv)
124+
dependsOn(testSlickStableSemconv)
125+
dependsOn(testCaptureParameters)
112126
}
113127
}
114128

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

Lines changed: 68 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,21 @@
1818
import static net.bytebuddy.matcher.ElementMatchers.takesNoArguments;
1919

2020
import io.opentelemetry.context.Context;
21+
import io.opentelemetry.context.ContextKey;
22+
import io.opentelemetry.context.ImplicitContextKeyed;
2123
import io.opentelemetry.context.Scope;
2224
import io.opentelemetry.instrumentation.jdbc.internal.DbRequest;
2325
import io.opentelemetry.instrumentation.jdbc.internal.JdbcData;
26+
import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge;
2427
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
2528
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
2629
import java.sql.Connection;
2730
import java.sql.PreparedStatement;
2831
import java.util.Locale;
2932
import javax.annotation.Nullable;
3033
import net.bytebuddy.asm.Advice;
34+
import net.bytebuddy.asm.Advice.AssignReturned;
35+
import net.bytebuddy.asm.Advice.AssignReturned.ToArguments.ToArgument;
3136
import net.bytebuddy.description.type.TypeDescription;
3237
import net.bytebuddy.matcher.ElementMatcher;
3338

@@ -48,7 +53,7 @@ public void transform(TypeTransformer transformer) {
4853
transformer.applyAdviceToMethod(
4954
nameStartsWith("prepare")
5055
.and(takesArgument(0, String.class))
51-
// Also include CallableStatement, which is a sub type of PreparedStatement
56+
// Also include CallableStatement, which is a subtype of PreparedStatement
5257
.and(returns(implementsInterface(named("java.sql.PreparedStatement")))),
5358
ConnectionInstrumentation.class.getName() + "$PrepareAdvice");
5459
transformer.applyAdviceToMethod(
@@ -59,14 +64,72 @@ public void transform(TypeTransformer transformer) {
5964
@SuppressWarnings("unused")
6065
public static class PrepareAdvice {
6166

62-
@Advice.OnMethodExit(suppress = Throwable.class)
67+
public static final class PrepareContext implements ImplicitContextKeyed {
68+
69+
private static final ContextKey<PrepareContext> KEY =
70+
ContextKey.named("jdbc-prepare-context");
71+
72+
private final String originalSql;
73+
74+
private PrepareContext(String originalSql) {
75+
this.originalSql = originalSql;
76+
}
77+
78+
public String get() {
79+
return originalSql;
80+
}
81+
82+
@Nullable
83+
public static PrepareContext get(Context context) {
84+
return context.get(KEY);
85+
}
86+
87+
public static Context init(Context context, String originalSql) {
88+
if (context.get(KEY) != null) {
89+
return context;
90+
}
91+
return context.with(new PrepareContext(originalSql));
92+
}
93+
94+
@Override
95+
public Context storeInContext(Context context) {
96+
return context.with(KEY, this);
97+
}
98+
}
99+
100+
@AssignReturned.ToArguments(@ToArgument(value = 0, index = 0))
101+
@Advice.OnMethodEnter(suppress = Throwable.class)
102+
public static Object[] processSql(@Advice.Argument(0) String sql) {
103+
Context context = Java8BytecodeBridge.currentContext();
104+
if (PrepareContext.get(context) == null) {
105+
// process sql only in the outermost prepare call and save the original sql in context
106+
String processSql = JdbcSingletons.processSql(sql);
107+
Scope scope = PrepareContext.init(context, sql).makeCurrent();
108+
return new Object[] {processSql, scope};
109+
} else {
110+
return new Object[] {sql, null};
111+
}
112+
}
113+
114+
@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
63115
public static void addDbInfo(
64-
@Advice.Argument(0) String sql, @Advice.Return PreparedStatement statement) {
65-
if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) {
116+
@Advice.Return PreparedStatement statement,
117+
@Advice.Enter Object[] enterResult,
118+
@Advice.Thrown Throwable error) {
119+
Context context = Java8BytecodeBridge.currentContext();
120+
PrepareContext prepareContext = PrepareContext.get(context);
121+
Scope scope = (Scope) enterResult[1];
122+
if (scope != null) {
123+
scope.close();
124+
}
125+
if (error != null
126+
|| prepareContext == null
127+
|| JdbcSingletons.isWrapper(statement, PreparedStatement.class)) {
66128
return;
67129
}
68130

69-
JdbcData.preparedStatement.set(statement, sql);
131+
String originalSql = prepareContext.get();
132+
JdbcData.preparedStatement.set(statement, originalSql);
70133
}
71134
}
72135

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import static io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory.createDataSourceInstrumenter;
99

1010
import io.opentelemetry.api.GlobalOpenTelemetry;
11+
import io.opentelemetry.instrumentation.api.incubator.semconv.db.internal.SqlCommenterUtil;
1112
import io.opentelemetry.instrumentation.api.incubator.semconv.net.PeerServiceAttributesExtractor;
1213
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
1314
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
@@ -28,6 +29,11 @@ public final class JdbcSingletons {
2829
private static final Instrumenter<DbRequest, Void> TRANSACTION_INSTRUMENTER;
2930
public static final Instrumenter<DataSource, DbInfo> DATASOURCE_INSTRUMENTER =
3031
createDataSourceInstrumenter(GlobalOpenTelemetry.get(), true);
32+
private static final boolean SQLCOMMENTER_ENABLED =
33+
AgentInstrumentationConfig.get()
34+
.getBoolean(
35+
"otel.instrumentation.jdbc.experimental.sqlcommenter.enabled",
36+
AgentCommonConfig.get().isSqlCommenterEnabled());
3137
public static final boolean CAPTURE_QUERY_PARAMETERS;
3238

3339
static {
@@ -97,5 +103,9 @@ private static <T extends Wrapper> boolean isWrapperInternal(T object, Class<T>
97103
return false;
98104
}
99105

106+
public static String processSql(String sql) {
107+
return SQLCOMMENTER_ENABLED ? SqlCommenterUtil.processQuery(sql) : sql;
108+
}
109+
100110
private JdbcSingletons() {}
101111
}

0 commit comments

Comments
 (0)