Added JDBC db.query.parameter span attributes

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

@@ -12,7 +12,9 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
 import io.opentelemetry.instrumentation.api.internal.SemconvStability;
+import io.opentelemetry.semconv.AttributeKeyTemplate;
 import java.util.Collection;
+import java.util.Map;
 
 /**
  * Extractor of <a
@@ -38,6 +40,8 @@ public final class SqlClientAttributesExtractor<REQUEST, RESPONSE>
       AttributeKey.stringKey("db.collection.name");
   private static final AttributeKey<Long> DB_OPERATION_BATCH_SIZE =
       AttributeKey.longKey("db.operation.batch.size");
+  private static final AttributeKeyTemplate<String> DB_QUERY_PARAMETER =
+      AttributeKeyTemplate.stringKeyTemplate("db.query.parameter");
 
   /** Creates the SQL client attributes extractor with default configuration. */
   public static <REQUEST, RESPONSE> AttributesExtractor<REQUEST, RESPONSE> create(
@@ -58,26 +62,35 @@ public static <REQUEST, RESPONSE> SqlClientAttributesExtractorBuilder<REQUEST, R
 
   private final AttributeKey<String> oldSemconvTableAttribute;
   private final boolean statementSanitizationEnabled;
+  private final boolean captureQueryParameters;
 
   SqlClientAttributesExtractor(
       SqlClientAttributesGetter<REQUEST, RESPONSE> getter,
       AttributeKey<String> oldSemconvTableAttribute,
-      boolean statementSanitizationEnabled) {
+      boolean statementSanitizationEnabled,
+      boolean captureQueryParameters) {
     super(getter);
     this.oldSemconvTableAttribute = oldSemconvTableAttribute;
-    this.statementSanitizationEnabled = statementSanitizationEnabled;
+    // captureQueryParameters disables statementSanitizationEnabled
+    this.statementSanitizationEnabled = !captureQueryParameters && statementSanitizationEnabled;
+    this.captureQueryParameters = captureQueryParameters;
   }
 
   @Override
+  @SuppressWarnings("AlreadyChecked")
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
     super.onStart(attributes, parentContext, request);
 
     Collection<String> rawQueryTexts = getter.getRawQueryTexts(request);
+    Map<String, String> preparedStatementParameters = getter.getQueryParameters(request);
 
     if (rawQueryTexts.isEmpty()) {
       return;
     }
 
+    Long batchSize = getter.getBatchSize(request);
+    boolean isBatch = batchSize != null && batchSize > 1;
+
     if (SemconvStability.emitOldDatabaseSemconv()) {
       if (rawQueryTexts.size() == 1) { // for backcompat(?)
         String rawQueryText = rawQueryTexts.iterator().next();
@@ -91,12 +104,11 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
         if (!SQL_CALL.equals(operation)) {
           internalSet(attributes, oldSemconvTableAttribute, sanitizedStatement.getMainIdentifier());
         }
+        setQueryParameters(attributes, isBatch, preparedStatementParameters);
       }
     }
 
     if (SemconvStability.emitStableDatabaseSemconv()) {
-      Long batchSize = getter.getBatchSize(request);
-      boolean isBatch = batchSize != null && batchSize > 1;
       if (isBatch) {
         internalSet(attributes, DB_OPERATION_BATCH_SIZE, batchSize);
       }
@@ -112,6 +124,7 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
         if (!SQL_CALL.equals(operation)) {
           internalSet(attributes, DB_COLLECTION_NAME, sanitizedStatement.getMainIdentifier());
         }
+        setQueryParameters(attributes, isBatch, preparedStatementParameters);
       } else {
         MultiQuery multiQuery =
             MultiQuery.analyze(getter.getRawQueryTexts(request), statementSanitizationEnabled);
@@ -129,6 +142,19 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
     }
   }
 
+  private void setQueryParameters(
+      AttributesBuilder attributes,
+      boolean isBatch,
+      Map<String, String> preparedStatementParameters) {
+    if (captureQueryParameters && !isBatch && preparedStatementParameters != null) {
+      for (Map.Entry<String, String> entry : preparedStatementParameters.entrySet()) {
+        String key = entry.getKey();
+        String value = entry.getValue();
+        internalSet(attributes, DB_QUERY_PARAMETER.getAttributeKey(key), value);
+      }
+    }
+  }
+
   // String.join is not available on android
   private static String join(String delimiter, Collection<String> collection) {
     StringBuilder builder = new StringBuilder();

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

@@ -20,6 +20,7 @@ public final class SqlClientAttributesExtractorBuilder<REQUEST, RESPONSE> {
   final SqlClientAttributesGetter<REQUEST, RESPONSE> getter;
   AttributeKey<String> oldSemconvTableAttribute = DB_SQL_TABLE;
   boolean statementSanitizationEnabled = true;
+  boolean captureQueryParameters = false;
 
   SqlClientAttributesExtractorBuilder(SqlClientAttributesGetter<REQUEST, RESPONSE> getter) {
     this.getter = getter;
@@ -45,6 +46,28 @@ public SqlClientAttributesExtractorBuilder<REQUEST, RESPONSE> setTableAttribute(
   public SqlClientAttributesExtractorBuilder<REQUEST, RESPONSE> setStatementSanitizationEnabled(
       boolean statementSanitizationEnabled) {
     this.statementSanitizationEnabled = statementSanitizationEnabled;
+
+    if (statementSanitizationEnabled) {
+      this.captureQueryParameters = false;
+    }
+
+    return this;
+  }
+
+  /**
+   * Sets whether the {@code db.query.parameter.<key>} attributes extracted by the constructed
+   * {@link SqlClientAttributesExtractor} should be opted-in. If set to {@code true}, all parameters
+   * from {@code PreparedStatement} will be exposed as attributes. Disabled by default.
+   */
+  @CanIgnoreReturnValue
+  public SqlClientAttributesExtractorBuilder<REQUEST, RESPONSE> setCaptureQueryParameters(
+      boolean captureQueryParameters) {
+    this.captureQueryParameters = captureQueryParameters;
+
+    if (captureQueryParameters) {
+      this.statementSanitizationEnabled = false;
+    }
+
     return this;
   }
 
@@ -54,6 +77,6 @@ public SqlClientAttributesExtractorBuilder<REQUEST, RESPONSE> setStatementSaniti
    */
   public AttributesExtractor<REQUEST, RESPONSE> build() {
     return new SqlClientAttributesExtractor<>(
-        getter, oldSemconvTableAttribute, statementSanitizationEnabled);
+        getter, oldSemconvTableAttribute, statementSanitizationEnabled, captureQueryParameters);
   }
 }

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

@@ -9,6 +9,7 @@
 import static java.util.Collections.singleton;
 
 import java.util.Collection;
+import java.util.Map;
 import javax.annotation.Nullable;
 
 /**
@@ -66,4 +67,10 @@ default Collection<String> getRawQueryTexts(REQUEST request) {
   default Long getBatchSize(REQUEST request) {
     return null;
   }
+
+  // TODO: make this required to implement
+  @Nullable
+  default Map<String, String> getQueryParameters(REQUEST request) {
+    return null;
+  }
 }

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

@@ -6,6 +6,7 @@
 package io.opentelemetry.instrumentation.api.incubator.semconv.db;
 
 import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat;
+import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_QUERY_PARAMETER;
 import static java.util.Collections.emptySet;
 import static java.util.Collections.singleton;
 import static org.assertj.core.api.Assertions.entry;
@@ -22,6 +23,7 @@
 import java.util.HashMap;
 import java.util.Map;
 import org.junit.jupiter.api.Test;
+import org.spockframework.util.Nullable;
 
 @SuppressWarnings("deprecation") // using deprecated semconv
 class SqlClientAttributesExtractorTest {
@@ -62,6 +64,23 @@ public Long getBatchSize(Map<String, Object> map) {
       return read(map, "db.operation.batch.size", Long.class);
     }
 
+    @SuppressWarnings("EmptyCatch")
+    @Nullable
+    @Override
+    public Map<String, String> getQueryParameters(Map<String, Object> map) {
+      String parameterString = read(map, "db.query.parameter");
+
+      if (parameterString == null) {
+        return null;
+      }
+
+      Map<String, String> parameters = new HashMap<>();
+      for (String s : parameterString.split(";")) {
+        parameters.put(Integer.toString(parameters.size()), s);
+      }
+      return parameters;
+    }
+
     protected String read(Map<String, Object> map, String key) {
       return read(map, key, String.class);
     }
@@ -387,4 +406,91 @@ void shouldIgnoreBatchSizeOne() {
 
     assertThat(endAttributes.build().isEmpty()).isTrue();
   }
+
+  @Test
+  void shouldExtractQueryParameters() {
+    // given
+    Map<String, Object> request = new HashMap<>();
+    request.put("db.name", "potatoes");
+    // a query with prepared parameters and parameters to sanitize
+    request.put(
+        "db.statement",
+        "SELECT col FROM table WHERE field1=? AND field2='A' AND field3=? AND field4=2");
+    // a prepared parameters map
+    request.put("db.query.parameter", "'a';1");
+
+    Context context = Context.root();
+
+    AttributesExtractor<Map<String, Object>, Void> underTest =
+        SqlClientAttributesExtractor.builder(new TestAttributesGetter())
+            .setCaptureQueryParameters(true)
+            .build();
+
+    // when
+    AttributesBuilder startAttributes = Attributes.builder();
+    underTest.onStart(startAttributes, context, request);
+
+    AttributesBuilder endAttributes = Attributes.builder();
+    underTest.onEnd(endAttributes, context, request, null, null);
+
+    String prefix = DB_QUERY_PARAMETER.getAttributeKey("").getKey();
+    Attributes queryParameterAttributes =
+        startAttributes.removeIf(attribute -> !attribute.getKey().startsWith(prefix)).build();
+
+    // then
+    if (SemconvStability.emitStableDatabaseSemconv() && SemconvStability.emitOldDatabaseSemconv()) {
+      assertThat(queryParameterAttributes)
+          .containsOnly(
+              entry(DB_QUERY_PARAMETER.getAttributeKey("0"), "'a'"),
+              entry(DB_QUERY_PARAMETER.getAttributeKey("1"), "1"));
+    } else if (SemconvStability.emitOldDatabaseSemconv()) {
+      assertThat(queryParameterAttributes)
+          .containsOnly(
+              entry(DB_QUERY_PARAMETER.getAttributeKey("0"), "'a'"),
+              entry(DB_QUERY_PARAMETER.getAttributeKey("1"), "1"));
+    } else if (SemconvStability.emitStableDatabaseSemconv()) {
+      assertThat(queryParameterAttributes)
+          .containsOnly(
+              entry(DB_QUERY_PARAMETER.getAttributeKey("0"), "'a'"),
+              entry(DB_QUERY_PARAMETER.getAttributeKey("1"), "1"));
+    }
+
+    assertThat(endAttributes.build().isEmpty()).isTrue();
+  }
+
+  @Test
+  void shouldNotExtractQueryParametersForBatch() {
+    // given
+    Map<String, Object> request = new HashMap<>();
+    request.put("db.name", "potatoes");
+    request.put("db.statements", singleton("INSERT INTO potato VALUES(?)"));
+    request.put("db.operation.batch.size", 1L);
+    request.put("db.query.parameter", "1");
+
+    Context context = Context.root();
+
+    AttributesExtractor<Map<String, Object>, Void> underTest =
+        SqlClientAttributesExtractor.create(new TestMultiAttributesGetter());
+
+    // when
+    AttributesBuilder startAttributes = Attributes.builder();
+    underTest.onStart(startAttributes, context, request);
+
+    AttributesBuilder endAttributes = Attributes.builder();
+    underTest.onEnd(endAttributes, context, request, null, null);
+
+    // then
+    if (SemconvStability.emitStableDatabaseSemconv() && SemconvStability.emitOldDatabaseSemconv()) {
+      assertThat(startAttributes.build())
+          .doesNotContainKey(DB_QUERY_PARAMETER.getAttributeKey("0"));
+    } else if (SemconvStability.emitOldDatabaseSemconv()) {
+      assertThat(startAttributes.build())
+          .doesNotContainKey(DB_QUERY_PARAMETER.getAttributeKey("0"));
+    } else if (SemconvStability.emitStableDatabaseSemconv()) {
+      assertThat(startAttributes.build())
+          .doesNotContainKey(DB_QUERY_PARAMETER.getAttributeKey("0"));
+    }
+
+    assertThat(endAttributes.build().isEmpty()).isTrue();
+  }
 }

instrumentation/jdbc/README.md

@@ -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.capture-query-parameters`    | Boolean | `false` | Enables the attribute db.query.parameter.\<key\>.<br/><br/>WARNING: captured query parameters may contain sensitive information such as passwords, personally identifiable information or protected health info. Exposing such info may result in substantial fines and penalties or criminal liability. Consult your peers, superiors and a legal counsel before enabling this option.<br/><br/>This option will disable otel.instrumentation.jdbc.statement-sanitizer |

instrumentation/jdbc/javaagent/build.gradle.kts

@@ -65,13 +65,15 @@ tasks {
   test {
     filter {
       excludeTestsMatching("SlickTest")
+      excludeTestsMatching("PreparedStatementParametersTest")
     }
     jvmArgs("-Dotel.instrumentation.jdbc-datasource.enabled=true")
   }
 
   val testStableSemconv by registering(Test::class) {
     filter {
       excludeTestsMatching("SlickTest")
+      excludeTestsMatching("PreparedStatementParametersTest")
     }
     jvmArgs("-Dotel.instrumentation.jdbc-datasource.enabled=true")
     jvmArgs("-Dotel.semconv-stability.opt-in=database")
@@ -85,9 +87,18 @@ tasks {
     jvmArgs("-Dotel.semconv-stability.opt-in=database")
   }
 
+  val testCaptureParameters by registering(Test::class) {
+    filter {
+      includeTestsMatching("PreparedStatementParametersTest")
+    }
+    jvmArgs("-Dotel.instrumentation.jdbc-datasource.enabled=true")
+    jvmArgs("-Dotel.instrumentation.jdbc.capture-query-parameters=true")
+  }
+
   check {
     dependsOn(testSlick)
     dependsOn(testStableSemconv)
     dependsOn(testSlickStableSemconv)
+    dependsOn(testCaptureParameters)
   }
 }

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

@@ -46,6 +46,10 @@ public final class JdbcSingletons {
                             .getBoolean(
                                 "otel.instrumentation.jdbc.statement-sanitizer.enabled",
                                 AgentCommonConfig.get().isStatementSanitizationEnabled()))
+                    .setCaptureQueryParameters(
+                        AgentInstrumentationConfig.get()
+                            .getBoolean(
+                                "otel.instrumentation.jdbc.capture-query-parameters", false))
                     .build())
             .addAttributesExtractor(ServerAttributesExtractor.create(netAttributesGetter))
             .addAttributesExtractor(