Skip to content

Commit 8cde80a

Browse files
AlixBalaurit
AlixBa
and
laurit
authored
Added JDBC db.query.parameter span attributes (open-telemetry#13719)
Co-authored-by: Lauri Tulmin <[email protected]>
1 parent 97e5260 commit 8cde80a

File tree

27 files changed

+1161
-78
lines changed

27 files changed

+1161
-78
lines changed

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

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
import io.opentelemetry.context.Context;
1313
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
1414
import io.opentelemetry.instrumentation.api.internal.SemconvStability;
15+
import io.opentelemetry.semconv.AttributeKeyTemplate;
1516
import java.util.Collection;
17+
import java.util.Map;
1618

1719
/**
1820
* Extractor of <a
@@ -38,6 +40,8 @@ public final class SqlClientAttributesExtractor<REQUEST, RESPONSE>
3840
AttributeKey.stringKey("db.collection.name");
3941
private static final AttributeKey<Long> DB_OPERATION_BATCH_SIZE =
4042
AttributeKey.longKey("db.operation.batch.size");
43+
private static final AttributeKeyTemplate<String> DB_QUERY_PARAMETER =
44+
AttributeKeyTemplate.stringKeyTemplate("db.query.parameter");
4145

4246
/** Creates the SQL client attributes extractor with default configuration. */
4347
public static <REQUEST, RESPONSE> AttributesExtractor<REQUEST, RESPONSE> create(
@@ -58,14 +62,18 @@ public static <REQUEST, RESPONSE> SqlClientAttributesExtractorBuilder<REQUEST, R
5862

5963
private final AttributeKey<String> oldSemconvTableAttribute;
6064
private final boolean statementSanitizationEnabled;
65+
private final boolean captureQueryParameters;
6166

6267
SqlClientAttributesExtractor(
6368
SqlClientAttributesGetter<REQUEST, RESPONSE> getter,
6469
AttributeKey<String> oldSemconvTableAttribute,
65-
boolean statementSanitizationEnabled) {
70+
boolean statementSanitizationEnabled,
71+
boolean captureQueryParameters) {
6672
super(getter);
6773
this.oldSemconvTableAttribute = oldSemconvTableAttribute;
68-
this.statementSanitizationEnabled = statementSanitizationEnabled;
74+
// capturing query parameters disables statement sanitization
75+
this.statementSanitizationEnabled = !captureQueryParameters && statementSanitizationEnabled;
76+
this.captureQueryParameters = captureQueryParameters;
6977
}
7078

7179
@Override
@@ -78,6 +86,9 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
7886
return;
7987
}
8088

89+
Long batchSize = getter.getBatchSize(request);
90+
boolean isBatch = batchSize != null && batchSize > 1;
91+
8192
if (SemconvStability.emitOldDatabaseSemconv()) {
8293
if (rawQueryTexts.size() == 1) { // for backcompat(?)
8394
String rawQueryText = rawQueryTexts.iterator().next();
@@ -95,8 +106,6 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
95106
}
96107

97108
if (SemconvStability.emitStableDatabaseSemconv()) {
98-
Long batchSize = getter.getBatchSize(request);
99-
boolean isBatch = batchSize != null && batchSize > 1;
100109
if (isBatch) {
101110
internalSet(attributes, DB_OPERATION_BATCH_SIZE, batchSize);
102111
}
@@ -127,6 +136,20 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
127136
}
128137
}
129138
}
139+
140+
Map<String, String> queryParameters = getter.getQueryParameters(request);
141+
setQueryParameters(attributes, isBatch, queryParameters);
142+
}
143+
144+
private void setQueryParameters(
145+
AttributesBuilder attributes, boolean isBatch, Map<String, String> queryParameters) {
146+
if (captureQueryParameters && !isBatch && queryParameters != null) {
147+
for (Map.Entry<String, String> entry : queryParameters.entrySet()) {
148+
String key = entry.getKey();
149+
String value = entry.getValue();
150+
internalSet(attributes, DB_QUERY_PARAMETER.getAttributeKey(key), value);
151+
}
152+
}
130153
}
131154

132155
// String.join is not available on android

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ public final class SqlClientAttributesExtractorBuilder<REQUEST, RESPONSE> {
2020
final SqlClientAttributesGetter<REQUEST, RESPONSE> getter;
2121
AttributeKey<String> oldSemconvTableAttribute = DB_SQL_TABLE;
2222
boolean statementSanitizationEnabled = true;
23+
boolean captureQueryParameters = false;
2324

2425
SqlClientAttributesExtractorBuilder(SqlClientAttributesGetter<REQUEST, RESPONSE> getter) {
2526
this.getter = getter;
@@ -48,12 +49,27 @@ public SqlClientAttributesExtractorBuilder<REQUEST, RESPONSE> setStatementSaniti
4849
return this;
4950
}
5051

52+
/**
53+
* Sets whether the query parameters should be captured as span attributes named {@code
54+
* db.query.parameter.<key>}. Enabling this option disables the statement sanitization. Disabled
55+
* by default.
56+
*
57+
* <p>WARNING: captured query parameters may contain sensitive information such as passwords,
58+
* personally identifiable information or protected health info.
59+
*/
60+
@CanIgnoreReturnValue
61+
public SqlClientAttributesExtractorBuilder<REQUEST, RESPONSE> setCaptureQueryParameters(
62+
boolean captureQueryParameters) {
63+
this.captureQueryParameters = captureQueryParameters;
64+
return this;
65+
}
66+
5167
/**
5268
* Returns a new {@link SqlClientAttributesExtractor} with the settings of this {@link
5369
* SqlClientAttributesExtractorBuilder}.
5470
*/
5571
public AttributesExtractor<REQUEST, RESPONSE> build() {
5672
return new SqlClientAttributesExtractor<>(
57-
getter, oldSemconvTableAttribute, statementSanitizationEnabled);
73+
getter, oldSemconvTableAttribute, statementSanitizationEnabled, captureQueryParameters);
5874
}
5975
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
import static java.util.Collections.singleton;
1010

1111
import java.util.Collection;
12+
import java.util.Collections;
13+
import java.util.Map;
1214
import javax.annotation.Nullable;
1315

1416
/**
@@ -66,4 +68,9 @@ default Collection<String> getRawQueryTexts(REQUEST request) {
6668
default Long getBatchSize(REQUEST request) {
6769
return null;
6870
}
71+
72+
// TODO: make this required to implement
73+
default Map<String, String> getQueryParameters(REQUEST request) {
74+
return Collections.emptyMap();
75+
}
6976
}

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package io.opentelemetry.instrumentation.api.incubator.semconv.db;
77

88
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
9+
import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_QUERY_PARAMETER;
910
import static java.util.Collections.emptySet;
1011
import static java.util.Collections.singleton;
1112
import static org.assertj.core.api.Assertions.entry;
@@ -62,6 +63,14 @@ public Long getBatchSize(Map<String, Object> map) {
6263
return read(map, "db.operation.batch.size", Long.class);
6364
}
6465

66+
@SuppressWarnings("unchecked")
67+
@Override
68+
public Map<String, String> getQueryParameters(Map<String, Object> map) {
69+
Map<String, String> parameters =
70+
(Map<String, String>) read(map, "db.query.parameter", Map.class);
71+
return parameters != null ? parameters : Collections.emptyMap();
72+
}
73+
6574
protected String read(Map<String, Object> map, String key) {
6675
return read(map, key, String.class);
6776
}
@@ -387,4 +396,74 @@ void shouldIgnoreBatchSizeOne() {
387396

388397
assertThat(endAttributes.build().isEmpty()).isTrue();
389398
}
399+
400+
@Test
401+
void shouldExtractQueryParameters() {
402+
// given
403+
Map<String, Object> request = new HashMap<>();
404+
request.put("db.name", "potatoes");
405+
// a query with prepared parameters and parameters to sanitize
406+
request.put(
407+
"db.statement",
408+
"SELECT col FROM table WHERE field1=? AND field2='A' AND field3=? AND field4=2");
409+
// a prepared parameters map
410+
Map<String, String> parameterMap = new HashMap<>();
411+
parameterMap.put("0", "'a'");
412+
parameterMap.put("1", "1");
413+
request.put("db.query.parameter", parameterMap);
414+
415+
Context context = Context.root();
416+
417+
AttributesExtractor<Map<String, Object>, Void> underTest =
418+
SqlClientAttributesExtractor.builder(new TestAttributesGetter())
419+
.setCaptureQueryParameters(true)
420+
.build();
421+
422+
// when
423+
AttributesBuilder startAttributes = Attributes.builder();
424+
underTest.onStart(startAttributes, context, request);
425+
426+
AttributesBuilder endAttributes = Attributes.builder();
427+
underTest.onEnd(endAttributes, context, request, null, null);
428+
429+
String prefix = DB_QUERY_PARAMETER.getAttributeKey("").getKey();
430+
Attributes queryParameterAttributes =
431+
startAttributes.removeIf(attribute -> !attribute.getKey().startsWith(prefix)).build();
432+
433+
// then
434+
assertThat(queryParameterAttributes)
435+
.containsOnly(
436+
entry(DB_QUERY_PARAMETER.getAttributeKey("0"), "'a'"),
437+
entry(DB_QUERY_PARAMETER.getAttributeKey("1"), "1"));
438+
439+
assertThat(endAttributes.build().isEmpty()).isTrue();
440+
}
441+
442+
@Test
443+
void shouldNotExtractQueryParametersForBatch() {
444+
// given
445+
Map<String, Object> request = new HashMap<>();
446+
request.put("db.name", "potatoes");
447+
request.put("db.statements", singleton("INSERT INTO potato VALUES(?)"));
448+
request.put("db.operation.batch.size", 2L);
449+
request.put("db.query.parameter", Collections.singletonMap("0", "1"));
450+
451+
Context context = Context.root();
452+
453+
AttributesExtractor<Map<String, Object>, Void> underTest =
454+
SqlClientAttributesExtractor.builder(new TestMultiAttributesGetter())
455+
.setCaptureQueryParameters(true)
456+
.build();
457+
458+
// when
459+
AttributesBuilder startAttributes = Attributes.builder();
460+
underTest.onStart(startAttributes, context, request);
461+
462+
AttributesBuilder endAttributes = Attributes.builder();
463+
underTest.onEnd(endAttributes, context, request, null, null);
464+
465+
// then
466+
assertThat(startAttributes.build()).doesNotContainKey(DB_QUERY_PARAMETER.getAttributeKey("0"));
467+
assertThat(endAttributes.build().isEmpty()).isTrue();
468+
}
390469
}

‎instrumentation/jdbc/README.md‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
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.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.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. |

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,15 @@ tasks {
6565
test {
6666
filter {
6767
excludeTestsMatching("SlickTest")
68+
excludeTestsMatching("PreparedStatementParametersTest")
6869
}
6970
jvmArgs("-Dotel.instrumentation.jdbc-datasource.enabled=true")
7071
}
7172

7273
val testStableSemconv by registering(Test::class) {
7374
filter {
7475
excludeTestsMatching("SlickTest")
76+
excludeTestsMatching("PreparedStatementParametersTest")
7577
}
7678
jvmArgs("-Dotel.instrumentation.jdbc-datasource.enabled=true")
7779
jvmArgs("-Dotel.semconv-stability.opt-in=database")
@@ -85,10 +87,18 @@ tasks {
8587
jvmArgs("-Dotel.semconv-stability.opt-in=database")
8688
}
8789

90+
val testCaptureParameters by registering(Test::class) {
91+
filter {
92+
includeTestsMatching("PreparedStatementParametersTest")
93+
}
94+
jvmArgs("-Dotel.instrumentation.jdbc.capture-query-parameters=true")
95+
}
96+
8897
check {
8998
dependsOn(testSlick)
9099
dependsOn(testStableSemconv)
91100
dependsOn(testSlickStableSemconv)
101+
dependsOn(testCaptureParameters)
92102
}
93103
}
94104

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,18 @@ public final class JdbcSingletons {
2525
private static final Instrumenter<DbRequest, Void> TRANSACTION_INSTRUMENTER;
2626
public static final Instrumenter<DataSource, DbInfo> DATASOURCE_INSTRUMENTER =
2727
createDataSourceInstrumenter(GlobalOpenTelemetry.get(), true);
28+
public static final boolean CAPTURE_QUERY_PARAMETERS;
2829

2930
static {
3031
JdbcNetworkAttributesGetter netAttributesGetter = new JdbcNetworkAttributesGetter();
3132
AttributesExtractor<DbRequest, Void> peerServiceExtractor =
3233
PeerServiceAttributesExtractor.create(
3334
netAttributesGetter, AgentCommonConfig.get().getPeerServiceResolver());
3435

36+
CAPTURE_QUERY_PARAMETERS =
37+
AgentInstrumentationConfig.get()
38+
.getBoolean("otel.instrumentation.jdbc.capture-query-parameters", false);
39+
3540
STATEMENT_INSTRUMENTER =
3641
JdbcInstrumenterFactory.createStatementInstrumenter(
3742
GlobalOpenTelemetry.get(),
@@ -40,7 +45,8 @@ public final class JdbcSingletons {
4045
AgentInstrumentationConfig.get()
4146
.getBoolean(
4247
"otel.instrumentation.jdbc.statement-sanitizer.enabled",
43-
AgentCommonConfig.get().isStatementSanitizationEnabled()));
48+
AgentCommonConfig.get().isStatementSanitizationEnabled()),
49+
CAPTURE_QUERY_PARAMETERS);
4450

4551
TRANSACTION_INSTRUMENTER =
4652
JdbcInstrumenterFactory.createTransactionInstrumenter(

0 commit comments

Comments
 (0)