-
Notifications
You must be signed in to change notification settings - Fork 1k
Add sqlcommenter support for jdbc and r2dbc #13714
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.
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
+1,274
−170
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
152657e
Add sqlcommenter support for jdbc and r2dbc
laurit b901ff0
add doc
laurit 7109f37
merge
laurit 8ca85ed
add warning and spring configuration
laurit ee15119
Update instrumentation/jdbc/library/src/test/java/io/opentelemetry/in…
laurit 112a937
Merge branch 'main' into sqlcommenter
laurit 370a516
use system property to configure OpenTelemetryDriver
laurit c6b15e7
merge
laurit 7e223e3
merge
laurit a10672d
Merge branch 'main' into sqlcommenter
laurit 51cc03a
merge
laurit ff5aa12
fix merge
laurit 11dde45
address review comments
laurit fd8d93f
fix
laurit bb065bc
./gradlew spotlessApply
otelbot[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
84 changes: 84 additions & 0 deletions
84
.../io/opentelemetry/instrumentation/api/incubator/semconv/db/internal/SqlCommenterUtil.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.instrumentation.api.incubator.semconv.db.internal; | ||
|
||
import io.opentelemetry.api.trace.Span; | ||
import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; | ||
import io.opentelemetry.context.Context; | ||
import java.io.UnsupportedEncodingException; | ||
import java.net.URLEncoder; | ||
import javax.annotation.Nullable; | ||
|
||
/** | ||
* This class is internal and experimental. Its APIs are unstable and can change at any time. Its | ||
* APIs (or a version of them) may be promoted to the public stable API in the future, but no | ||
* guarantees are made. | ||
*/ | ||
public final class SqlCommenterUtil { | ||
|
||
/** | ||
* Append comment containing tracing information at the end of the query. See <a | ||
* href="https://google.github.io/sqlcommenter/spec/">sqlcommenter</a> for the description of the | ||
* algorithm. | ||
*/ | ||
public static String processQuery(String query) { | ||
if (!Span.current().getSpanContext().isValid()) { | ||
return query; | ||
} | ||
// skip queries that contain comments | ||
if (containsSqlComment(query)) { | ||
return query; | ||
} | ||
|
||
class State { | ||
@Nullable String traceparent; | ||
@Nullable String tracestate; | ||
} | ||
|
||
State state = new State(); | ||
|
||
W3CTraceContextPropagator.getInstance() | ||
.inject( | ||
Context.current(), | ||
state, | ||
(carrier, key, value) -> { | ||
if (carrier == null) { | ||
return; | ||
} | ||
if ("traceparent".equals(key)) { | ||
carrier.traceparent = value; | ||
} else if ("tracestate".equals(key)) { | ||
carrier.tracestate = value; | ||
} | ||
}); | ||
try { | ||
// we know that the traceparent doesn't contain anything that needs to be encoded | ||
query += " /*traceparent='" + state.traceparent + "'"; | ||
if (state.tracestate != null) { | ||
query += ", tracestate=" + serialize(state.tracestate); | ||
} | ||
query += "*/"; | ||
} catch (UnsupportedEncodingException exception) { | ||
// this exception should never happen as UTF-8 encoding is always available | ||
} | ||
return query; | ||
} | ||
|
||
private static boolean containsSqlComment(String query) { | ||
return query.contains("--") || query.contains("/*"); | ||
} | ||
|
||
private static String serialize(String value) throws UnsupportedEncodingException { | ||
// specification requires percent encoding, here we use the java build in url encoder that | ||
// encodes space as '+' instead of '%20' as required | ||
String result = URLEncoder.encode(value, "UTF-8").replace("+", "%20"); | ||
// specification requires escaping ' with a backslash, we skip this because URLEncoder already | ||
// encodes the ' | ||
return "'" + result + "'"; | ||
} | ||
|
||
private SqlCommenterUtil() {} | ||
} |
62 changes: 62 additions & 0 deletions
62
...opentelemetry/instrumentation/api/incubator/semconv/db/internal/SqlCommenterUtilTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.instrumentation.api.incubator.semconv.db.internal; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
import io.opentelemetry.api.trace.Span; | ||
import io.opentelemetry.api.trace.SpanContext; | ||
import io.opentelemetry.api.trace.TraceFlags; | ||
import io.opentelemetry.api.trace.TraceState; | ||
import io.opentelemetry.context.Context; | ||
import io.opentelemetry.context.Scope; | ||
import org.junit.jupiter.params.ParameterizedTest; | ||
import org.junit.jupiter.params.provider.ValueSource; | ||
|
||
class SqlCommenterUtilTest { | ||
|
||
@ParameterizedTest | ||
@ValueSource(strings = {"SELECT /**/ 1", "SELECT 1 --", "SELECT '/*'"}) | ||
void skipQueriesWithComments(String query) { | ||
Context parent = | ||
Context.root() | ||
.with( | ||
Span.wrap( | ||
SpanContext.create( | ||
"ff01020304050600ff0a0b0c0d0e0f00", | ||
"090a0b0c0d0e0f00", | ||
TraceFlags.getSampled(), | ||
TraceState.getDefault()))); | ||
|
||
try (Scope ignore = parent.makeCurrent()) { | ||
assertThat(SqlCommenterUtil.processQuery(query)).isEqualTo(query); | ||
} | ||
} | ||
|
||
@ParameterizedTest | ||
@ValueSource(booleans = {true, false}) | ||
void sqlCommenter(boolean hasTraceState) { | ||
TraceState state = | ||
hasTraceState ? TraceState.builder().put("test", "test'").build() : TraceState.getDefault(); | ||
Context parent = | ||
Context.root() | ||
.with( | ||
Span.wrap( | ||
SpanContext.create( | ||
"ff01020304050600ff0a0b0c0d0e0f00", | ||
"090a0b0c0d0e0f00", | ||
TraceFlags.getSampled(), | ||
state))); | ||
|
||
try (Scope ignore = parent.makeCurrent()) { | ||
assertThat(SqlCommenterUtil.processQuery("SELECT 1")) | ||
.isEqualTo( | ||
hasTraceState | ||
? "SELECT 1 /*traceparent='00-ff01020304050600ff0a0b0c0d0e0f00-090a0b0c0d0e0f00-01', tracestate='test%3Dtest%27'*/" | ||
: "SELECT 1 /*traceparent='00-ff01020304050600ff0a0b0c0d0e0f00-090a0b0c0d0e0f00-01'*/"); | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
# 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.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. | | ||
| `otel.instrumentation.jdbc.experimental.transaction.enabled` | Boolean | `false` | Enables experimental instrumentation to create spans for COMMIT and ROLLBACK operations. | | ||
| System property | Type | Default | Description | | ||
|-------------------------------------------------------------------|---------|---------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | ||
| `otel.instrumentation.jdbc.statement-sanitizer.enabled` | Boolean | `true` | Enables the DB statement sanitization. | | ||
| `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. | | ||
| `otel.instrumentation.jdbc.experimental.transaction.enabled` | Boolean | `false` | Enables experimental instrumentation to create spans for COMMIT and ROLLBACK operations. | | ||
| `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. | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the
tracestate
, which is the only value passed to this method, can't contain a spacereplace("+", "%20")
isn't really needed. I chose to add it just in case we decide to add something else here later.