diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractor.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractor.java index 733a8a2eae..ca60e051f4 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractor.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractor.java @@ -12,7 +12,9 @@ import io.opentelemetry.api.common.AttributesBuilder; 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 AttributeKey.stringKey("db.collection.name"); private static final AttributeKey DB_OPERATION_BATCH_SIZE = AttributeKey.longKey("db.operation.batch.size"); + private static final AttributeKeyTemplate DB_QUERY_PARAMETER = + AttributeKeyTemplate.stringKeyTemplate("db.query.parameter"); /** Creates the SQL client attributes extractor with default configuration. */ public static AttributesExtractor create( @@ -58,14 +62,18 @@ public final class SqlClientAttributesExtractor private final AttributeKey oldSemconvTableAttribute; private final boolean statementSanitizationEnabled; + private final boolean captureQueryParameters; SqlClientAttributesExtractor( SqlClientAttributesGetter getter, AttributeKey oldSemconvTableAttribute, - boolean statementSanitizationEnabled) { + boolean statementSanitizationEnabled, + boolean captureQueryParameters) { super(getter); this.oldSemconvTableAttribute = oldSemconvTableAttribute; - this.statementSanitizationEnabled = statementSanitizationEnabled; + // capturing query parameters disables statement sanitization + this.statementSanitizationEnabled = !captureQueryParameters && statementSanitizationEnabled; + this.captureQueryParameters = captureQueryParameters; } @Override @@ -78,6 +86,9 @@ public final class SqlClientAttributesExtractor 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(); @@ -95,8 +106,6 @@ public final class SqlClientAttributesExtractor } if (SemconvStability.emitStableDatabaseSemconv()) { - Long batchSize = getter.getBatchSize(request); - boolean isBatch = batchSize != null && batchSize > 1; if (isBatch) { internalSet(attributes, DB_OPERATION_BATCH_SIZE, batchSize); } @@ -127,6 +136,20 @@ public final class SqlClientAttributesExtractor } } } + + Map queryParameters = getter.getQueryParameters(request); + setQueryParameters(attributes, isBatch, queryParameters); + } + + private void setQueryParameters( + AttributesBuilder attributes, boolean isBatch, Map queryParameters) { + if (captureQueryParameters && !isBatch && queryParameters != null) { + for (Map.Entry entry : queryParameters.entrySet()) { + String key = entry.getKey(); + String value = entry.getValue(); + internalSet(attributes, DB_QUERY_PARAMETER.getAttributeKey(key), value); + } + } } // String.join is not available on android diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractorBuilder.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractorBuilder.java index 43cbc63062..7eefed9cf8 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractorBuilder.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractorBuilder.java @@ -20,6 +20,7 @@ public final class SqlClientAttributesExtractorBuilder { final SqlClientAttributesGetter getter; AttributeKey oldSemconvTableAttribute = DB_SQL_TABLE; boolean statementSanitizationEnabled = true; + boolean captureQueryParameters = false; SqlClientAttributesExtractorBuilder(SqlClientAttributesGetter getter) { this.getter = getter; @@ -48,12 +49,27 @@ public final class SqlClientAttributesExtractorBuilder { return this; } + /** + * Sets whether the query parameters should be captured as span attributes named {@code + * db.query.parameter.}. Enabling this option disables the statement sanitization. Disabled + * by default. + * + *

WARNING: captured query parameters may contain sensitive information such as passwords, + * personally identifiable information or protected health info. + */ + @CanIgnoreReturnValue + public SqlClientAttributesExtractorBuilder setCaptureQueryParameters( + boolean captureQueryParameters) { + this.captureQueryParameters = captureQueryParameters; + return this; + } + /** * Returns a new {@link SqlClientAttributesExtractor} with the settings of this {@link * SqlClientAttributesExtractorBuilder}. */ public AttributesExtractor build() { return new SqlClientAttributesExtractor<>( - getter, oldSemconvTableAttribute, statementSanitizationEnabled); + getter, oldSemconvTableAttribute, statementSanitizationEnabled, captureQueryParameters); } } diff --git a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesGetter.java b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesGetter.java index 1b795363e7..bdf64e638a 100644 --- a/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesGetter.java +++ b/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesGetter.java @@ -9,6 +9,8 @@ import static java.util.Collections.emptySet; import static java.util.Collections.singleton; import java.util.Collection; +import java.util.Collections; +import java.util.Map; import javax.annotation.Nullable; /** @@ -66,4 +68,9 @@ public interface SqlClientAttributesGetter default Long getBatchSize(REQUEST request) { return null; } + + // TODO: make this required to implement + default Map getQueryParameters(REQUEST request) { + return Collections.emptyMap(); + } } diff --git a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractorTest.java b/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractorTest.java index d3cd7dc032..a8ae1c8062 100644 --- a/instrumentation-api-incubator/src/test/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractorTest.java +++ b/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; @@ -62,6 +63,14 @@ class SqlClientAttributesExtractorTest { return read(map, "db.operation.batch.size", Long.class); } + @SuppressWarnings("unchecked") + @Override + public Map getQueryParameters(Map map) { + Map parameters = + (Map) read(map, "db.query.parameter", Map.class); + return parameters != null ? parameters : Collections.emptyMap(); + } + protected String read(Map map, String key) { return read(map, key, String.class); } @@ -387,4 +396,74 @@ class SqlClientAttributesExtractorTest { assertThat(endAttributes.build().isEmpty()).isTrue(); } + + @Test + void shouldExtractQueryParameters() { + // given + Map 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 + Map parameterMap = new HashMap<>(); + parameterMap.put("0", "'a'"); + parameterMap.put("1", "1"); + request.put("db.query.parameter", parameterMap); + + Context context = Context.root(); + + AttributesExtractor, 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 + 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 request = new HashMap<>(); + request.put("db.name", "potatoes"); + request.put("db.statements", singleton("INSERT INTO potato VALUES(?)")); + request.put("db.operation.batch.size", 2L); + request.put("db.query.parameter", Collections.singletonMap("0", "1")); + + Context context = Context.root(); + + AttributesExtractor, Void> underTest = + SqlClientAttributesExtractor.builder(new TestMultiAttributesGetter()) + .setCaptureQueryParameters(true) + .build(); + + // when + AttributesBuilder startAttributes = Attributes.builder(); + underTest.onStart(startAttributes, context, request); + + AttributesBuilder endAttributes = Attributes.builder(); + underTest.onEnd(endAttributes, context, request, null, null); + + // then + assertThat(startAttributes.build()).doesNotContainKey(DB_QUERY_PARAMETER.getAttributeKey("0")); + assertThat(endAttributes.build().isEmpty()).isTrue(); + } } diff --git a/instrumentation/jdbc/README.md b/instrumentation/jdbc/README.md index 720aaefbd5..fd048534bb 100644 --- a/instrumentation/jdbc/README.md +++ b/instrumentation/jdbc/README.md @@ -1,6 +1,7 @@ # 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.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.capture-query-parameters` | Boolean | `false` | Enable the capture of query parameters as span attributes. Enabling this option disables the statement sanitization.

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. | diff --git a/instrumentation/jdbc/javaagent/build.gradle.kts b/instrumentation/jdbc/javaagent/build.gradle.kts index 10720fb42a..07e99b8d11 100644 --- a/instrumentation/jdbc/javaagent/build.gradle.kts +++ b/instrumentation/jdbc/javaagent/build.gradle.kts @@ -65,6 +65,7 @@ tasks { test { filter { excludeTestsMatching("SlickTest") + excludeTestsMatching("PreparedStatementParametersTest") } jvmArgs("-Dotel.instrumentation.jdbc-datasource.enabled=true") } @@ -72,6 +73,7 @@ tasks { 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,10 +87,18 @@ tasks { jvmArgs("-Dotel.semconv-stability.opt-in=database") } + val testCaptureParameters by registering(Test::class) { + filter { + includeTestsMatching("PreparedStatementParametersTest") + } + jvmArgs("-Dotel.instrumentation.jdbc.capture-query-parameters=true") + } + check { dependsOn(testSlick) dependsOn(testStableSemconv) dependsOn(testSlickStableSemconv) + dependsOn(testCaptureParameters) } } diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java index 1b474d833d..bff92fc9a5 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java @@ -25,6 +25,7 @@ public final class JdbcSingletons { private static final Instrumenter TRANSACTION_INSTRUMENTER; public static final Instrumenter DATASOURCE_INSTRUMENTER = createDataSourceInstrumenter(GlobalOpenTelemetry.get(), true); + public static final boolean CAPTURE_QUERY_PARAMETERS; static { JdbcNetworkAttributesGetter netAttributesGetter = new JdbcNetworkAttributesGetter(); @@ -32,6 +33,10 @@ public final class JdbcSingletons { PeerServiceAttributesExtractor.create( netAttributesGetter, AgentCommonConfig.get().getPeerServiceResolver()); + CAPTURE_QUERY_PARAMETERS = + AgentInstrumentationConfig.get() + .getBoolean("otel.instrumentation.jdbc.capture-query-parameters", false); + STATEMENT_INSTRUMENTER = JdbcInstrumenterFactory.createStatementInstrumenter( GlobalOpenTelemetry.get(), @@ -40,7 +45,8 @@ public final class JdbcSingletons { AgentInstrumentationConfig.get() .getBoolean( "otel.instrumentation.jdbc.statement-sanitizer.enabled", - AgentCommonConfig.get().isStatementSanitizationEnabled())); + AgentCommonConfig.get().isStatementSanitizationEnabled()), + CAPTURE_QUERY_PARAMETERS); TRANSACTION_INSTRUMENTER = JdbcInstrumenterFactory.createTransactionInstrumenter( diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java index a822ab9807..38fad89aa0 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java @@ -8,12 +8,14 @@ package io.opentelemetry.javaagent.instrumentation.jdbc; import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface; +import static io.opentelemetry.javaagent.instrumentation.jdbc.JdbcSingletons.CAPTURE_QUERY_PARAMETERS; import static io.opentelemetry.javaagent.instrumentation.jdbc.JdbcSingletons.statementInstrumenter; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.namedOneOf; import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import static net.bytebuddy.matcher.ElementMatchers.takesNoArguments; @@ -24,8 +26,15 @@ import io.opentelemetry.instrumentation.jdbc.internal.JdbcData; import io.opentelemetry.javaagent.bootstrap.CallDepth; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import java.net.URL; +import java.sql.Date; import java.sql.PreparedStatement; +import java.sql.RowId; import java.sql.Statement; +import java.sql.Time; +import java.sql.Timestamp; +import java.util.Calendar; +import java.util.Map; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -53,6 +62,36 @@ public class PreparedStatementInstrumentation implements TypeInstrumentation { transformer.applyAdviceToMethod( named("addBatch").and(takesNoArguments()).and(isPublic()), PreparedStatementInstrumentation.class.getName() + "$AddBatchAdvice"); + transformer.applyAdviceToMethod( + namedOneOf( + "setBoolean", + "setShort", + "setInt", + "setLong", + "setFloat", + "setDouble", + "setBigDecimal", + "setString", + "setDate", + "setTime", + "setTimestamp", + "setURL", + "setRowId", + "setNString") + .and(takesArgument(0, int.class)) + .and(takesArguments(2)) + .and(isPublic()), + PreparedStatementInstrumentation.class.getName() + "$SetParameter2Advice"); + transformer.applyAdviceToMethod( + namedOneOf("setDate", "setTime", "setTimestamp") + .and(takesArgument(0, int.class)) + .and(takesArgument(2, Calendar.class)) + .and(takesArguments(3)) + .and(isPublic()), + PreparedStatementInstrumentation.class.getName() + "$SetTimeParameter3Advice"); + transformer.applyAdviceToMethod( + named("clearParameters").and(takesNoArguments()).and(isPublic()), + PreparedStatementInstrumentation.class.getName() + "$ClearParametersAdvice"); } @SuppressWarnings("unused") @@ -84,7 +123,8 @@ public class PreparedStatementInstrumentation implements TypeInstrumentation { } Context parentContext = currentContext(); - request = DbRequest.create(statement); + Map parameters = JdbcData.getParameters(statement); + request = DbRequest.create(statement, parameters); if (request == null || !statementInstrumenter().shouldStart(parentContext, request)) { return; @@ -120,4 +160,68 @@ public class PreparedStatementInstrumentation implements TypeInstrumentation { JdbcData.addPreparedStatementBatch(statement); } } + + @SuppressWarnings("unused") + public static class SetParameter2Advice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void onExit( + @Advice.This PreparedStatement statement, + @Advice.Argument(0) int index, + @Advice.Argument(1) Object value) { + if (!CAPTURE_QUERY_PARAMETERS) { + return; + } + + String str = null; + + if (value instanceof Boolean + // Short, Int, Long, Float, Double, BigDecimal + || value instanceof Number + || value instanceof String + || value instanceof Date + || value instanceof Time + || value instanceof Timestamp + || value instanceof URL + || value instanceof RowId) { + str = value.toString(); + } + + if (str != null) { + JdbcData.addParameter(statement, Integer.toString(index - 1), str); + } + } + } + + @SuppressWarnings("unused") + public static class SetTimeParameter3Advice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void onExit( + @Advice.This PreparedStatement statement, + @Advice.Argument(0) int index, + @Advice.Argument(1) Object value, + @Advice.Argument(2) Calendar calendar) { + if (!CAPTURE_QUERY_PARAMETERS) { + return; + } + + String str = null; + + if (value instanceof Date || value instanceof Time || value instanceof Timestamp) { + str = value.toString(); + } + + if (str != null) { + JdbcData.addParameter(statement, Integer.toString(index - 1), str); + } + } + } + + @SuppressWarnings("unused") + public static class ClearParametersAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void clearBatch(@Advice.This PreparedStatement statement) { + JdbcData.clearParameters(statement); + } + } } diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java index 93cf1711ea..81b252b87f 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java @@ -25,6 +25,7 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import java.sql.PreparedStatement; import java.sql.Statement; +import java.util.Map; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -157,12 +158,13 @@ public class StatementInstrumentation implements TypeInstrumentation { Context parentContext = currentContext(); if (statement instanceof PreparedStatement) { - Long batchSize = JdbcData.getPreparedStatementBatchSize((PreparedStatement) statement); String sql = JdbcData.preparedStatement.get((PreparedStatement) statement); if (sql == null) { return; } - request = DbRequest.create(statement, sql, batchSize); + Long batchSize = JdbcData.getPreparedStatementBatchSize((PreparedStatement) statement); + Map parameters = JdbcData.getParameters((PreparedStatement) statement); + request = DbRequest.create(statement, sql, batchSize, parameters); } else { JdbcData.StatementBatchInfo batchInfo = JdbcData.getStatementBatchInfo(statement); if (batchInfo == null) { diff --git a/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/PreparedStatementParametersTest.java b/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/PreparedStatementParametersTest.java new file mode 100644 index 0000000000..fc5db502d8 --- /dev/null +++ b/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/test/PreparedStatementParametersTest.java @@ -0,0 +1,556 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jdbc.test; + +import static io.opentelemetry.instrumentation.api.internal.SemconvStability.emitStableDatabaseSemconv; +import static io.opentelemetry.instrumentation.testing.junit.db.SemconvStabilityUtil.maybeStable; +import static io.opentelemetry.instrumentation.testing.junit.db.SemconvStabilityUtil.maybeStableDbSystemName; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_CONNECTION_STRING; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_NAME; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_QUERY_PARAMETER; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SQL_TABLE; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_STATEMENT; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SYSTEM; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_USER; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; +import io.opentelemetry.api.trace.SpanKind; +import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension; +import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; +import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; +import java.math.BigDecimal; +import java.sql.Connection; +import java.sql.Date; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Time; +import java.sql.Timestamp; +import java.util.Calendar; +import java.util.Locale; +import java.util.Map; +import java.util.Properties; +import java.util.stream.Stream; +import org.apache.derby.jdbc.EmbeddedDriver; +import org.hsqldb.jdbc.JDBCDriver; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +@SuppressWarnings("deprecation") // using deprecated semconv +class PreparedStatementParametersTest { + + @RegisterExtension static final AutoCleanupExtension cleanup = AutoCleanupExtension.create(); + + @RegisterExtension + static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); + + private static final String dbName = "jdbcUnitTest"; + private static final String dbNameLower = dbName.toLowerCase(Locale.ROOT); + + private static final Map jdbcUrls = + ImmutableMap.of( + "h2", "jdbc:h2:mem:" + dbName, + "derby", "jdbc:derby:memory:" + dbName, + "hsqldb", "jdbc:hsqldb:mem:" + dbName); + private static final Map jdbcUserNames = Maps.newHashMap(); + private static final Properties connectionProps = new Properties(); + + static { + jdbcUserNames.put("derby", "APP"); + jdbcUserNames.put("h2", null); + jdbcUserNames.put("hsqldb", "SA"); + + connectionProps.put("databaseName", "someDb"); + connectionProps.put("OPEN_NEW", "true"); // So H2 doesn't complain about username/password. + connectionProps.put("create", "true"); + } + + static Stream preparedStatementStream() throws SQLException { + return Stream.of( + Arguments.of( + "h2", + new org.h2.Driver().connect(jdbcUrls.get("h2"), null), + null, + "SELECT 3, ?", + "SELECT 3, ?", + "SELECT " + dbNameLower, + "h2:mem:", + null), + Arguments.of( + "derby", + new EmbeddedDriver().connect(jdbcUrls.get("derby"), connectionProps), + "APP", + "SELECT 3 FROM SYSIBM.SYSDUMMY1 WHERE IBMREQD=? OR 1=1", + "SELECT 3 FROM SYSIBM.SYSDUMMY1 WHERE IBMREQD=? OR 1=1", + "SELECT SYSIBM.SYSDUMMY1", + "derby:memory:", + "SYSIBM.SYSDUMMY1"), + Arguments.of( + "hsqldb", + new JDBCDriver().connect(jdbcUrls.get("hsqldb"), null), + "SA", + "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS WHERE USER_NAME=? OR 1=1", + "SELECT 3 FROM INFORMATION_SCHEMA.SYSTEM_USERS WHERE USER_NAME=? OR 1=1", + "SELECT INFORMATION_SCHEMA.SYSTEM_USERS", + "hsqldb:mem:", + "INFORMATION_SCHEMA.SYSTEM_USERS")); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testBooleanPreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + test( + system, + connection, + username, + query, + sanitizedQuery, + spanName, + url, + table, + statement -> statement.setBoolean(1, true), + "true"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testShortPreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + test( + system, + connection, + username, + query, + sanitizedQuery, + spanName, + url, + table, + statement -> statement.setShort(1, (short) 0), + "0"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testIntPreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + test( + system, + connection, + username, + query, + sanitizedQuery, + spanName, + url, + table, + statement -> statement.setInt(1, 0), + "0"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testLongPreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + test( + system, + connection, + username, + query, + sanitizedQuery, + spanName, + url, + table, + statement -> statement.setLong(1, 0), + "0"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testFloatPreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + test( + system, + connection, + username, + query, + sanitizedQuery, + spanName, + url, + table, + statement -> statement.setFloat(1, 0.1f), + "0.1"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testDoublePreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + test( + system, + connection, + username, + query, + sanitizedQuery, + spanName, + url, + table, + statement -> statement.setDouble(1, 0.1), + "0.1"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testBigDecimalPreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + test( + system, + connection, + username, + query, + sanitizedQuery, + spanName, + url, + table, + statement -> statement.setBigDecimal(1, BigDecimal.ZERO), + "0"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testStringPreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + test( + system, + connection, + username, + query, + sanitizedQuery, + spanName, + url, + table, + statement -> statement.setString(1, "S"), + "S"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testDate2PreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + String updatedQuery = query.replace("USER_NAME=?", "CURDATE()=?"); + String updatedQuerySanitized = sanitizedQuery.replace("USER_NAME=?", "CURDATE()=?"); + + test( + system, + connection, + username, + updatedQuery, + updatedQuerySanitized, + spanName, + url, + table, + statement -> statement.setDate(1, Date.valueOf("2000-01-01")), + "2000-01-01"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testDate3PreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + String updatedQuery = query.replace("USER_NAME=?", "CURDATE()=?"); + String updatedQuerySanitized = sanitizedQuery.replace("USER_NAME=?", "CURDATE()=?"); + + test( + system, + connection, + username, + updatedQuery, + updatedQuerySanitized, + spanName, + url, + table, + statement -> statement.setDate(1, Date.valueOf("2000-01-01"), Calendar.getInstance()), + "2000-01-01"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testTime2PreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + String updatedQuery = query.replace("USER_NAME=?", "CURTIME()=?"); + String updatedQuerySanitized = sanitizedQuery.replace("USER_NAME=?", "CURTIME()=?"); + + test( + system, + connection, + username, + updatedQuery, + updatedQuerySanitized, + spanName, + url, + table, + statement -> statement.setTime(1, Time.valueOf("00:00:00")), + "00:00:00"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testTime3PreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + String updatedQuery = query.replace("USER_NAME=?", "CURTIME()=?"); + String updatedQuerySanitized = sanitizedQuery.replace("USER_NAME=?", "CURTIME()=?"); + + test( + system, + connection, + username, + updatedQuery, + updatedQuerySanitized, + spanName, + url, + table, + statement -> statement.setTime(1, Time.valueOf("00:00:00"), Calendar.getInstance()), + "00:00:00"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testTimestamp2PreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + String updatedQuery = query.replace("USER_NAME=?", "NOW()=?"); + String updatedQuerySanitized = sanitizedQuery.replace("USER_NAME=?", "NOW()=?"); + + test( + system, + connection, + username, + updatedQuery, + updatedQuerySanitized, + spanName, + url, + table, + statement -> statement.setTimestamp(1, Timestamp.valueOf("2000-01-01 00:00:00")), + "2000-01-01 00:00:00.0"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testTimestamp3PreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + String updatedQuery = query.replace("USER_NAME=?", "NOW()=?"); + String updatedQuerySanitized = sanitizedQuery.replace("USER_NAME=?", "NOW()=?"); + + test( + system, + connection, + username, + updatedQuery, + updatedQuerySanitized, + spanName, + url, + table, + statement -> + statement.setTimestamp( + 1, Timestamp.valueOf("2000-01-01 00:00:00"), Calendar.getInstance()), + "2000-01-01 00:00:00.0"); + } + + @ParameterizedTest + @MethodSource("preparedStatementStream") + void testNstringPreparedStatementParameter( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table) + throws SQLException { + Assumptions.assumeFalse(system.equalsIgnoreCase("derby")); + + test( + system, + connection, + username, + query, + sanitizedQuery, + spanName, + url, + table, + statement -> statement.setNString(1, "S"), + "S"); + } + + private static void test( + String system, + Connection connection, + String username, + String query, + String sanitizedQuery, + String spanName, + String url, + String table, + ThrowingConsumer setParameter, + String expectedParameterValue) + throws SQLException { + PreparedStatement statement = connection.prepareStatement(query); + cleanup.deferCleanup(statement); + + ResultSet resultSet = + testing.runWithSpan( + "parent", + () -> { + setParameter.accept(statement); + statement.execute(); + return statement.getResultSet(); + }); + + resultSet.next(); + assertThat(resultSet.getInt(1)).isEqualTo(3); + + testing.waitAndAssertTraces( + trace -> + trace.hasSpansSatisfyingExactly( + span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(), + span -> + span.hasName(spanName) + .hasKind(SpanKind.CLIENT) + .hasParent(trace.getSpan(0)) + .hasAttributesSatisfyingExactly( + equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(system)), + equalTo(maybeStable(DB_NAME), dbNameLower), + equalTo(DB_USER, emitStableDatabaseSemconv() ? null : username), + equalTo(DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : url), + equalTo(maybeStable(DB_STATEMENT), sanitizedQuery), + equalTo(maybeStable(DB_OPERATION), "SELECT"), + equalTo(maybeStable(DB_SQL_TABLE), table), + equalTo( + DB_QUERY_PARAMETER.getAttributeKey("0"), expectedParameterValue)))); + } + + public interface ThrowingConsumer { + void accept(T t) throws E; + } +} diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java index 73f0e2f714..ae3b9e0860 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java @@ -244,10 +244,12 @@ public final class OpenTelemetryDriver implements Driver { Instrumenter statementInstrumenter = JdbcInstrumenterFactory.createStatementInstrumenter(openTelemetry); + boolean captureQueryParameters = JdbcInstrumenterFactory.captureQueryParameters(); Instrumenter transactionInstrumenter = JdbcInstrumenterFactory.createTransactionInstrumenter(openTelemetry); + return OpenTelemetryConnection.create( - connection, dbInfo, statementInstrumenter, transactionInstrumenter); + connection, dbInfo, statementInstrumenter, transactionInstrumenter, captureQueryParameters); } @Override diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetry.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetry.java index dd86484ca6..a37aa0b2e6 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetry.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetry.java @@ -27,14 +27,17 @@ public final class JdbcTelemetry { private final Instrumenter dataSourceInstrumenter; private final Instrumenter statementInstrumenter; private final Instrumenter transactionInstrumenter; + private final boolean captureQueryParameters; JdbcTelemetry( Instrumenter dataSourceInstrumenter, Instrumenter statementInstrumenter, - Instrumenter transactionInstrumenter) { + Instrumenter transactionInstrumenter, + boolean captureQueryParameters) { this.dataSourceInstrumenter = dataSourceInstrumenter; this.statementInstrumenter = statementInstrumenter; this.transactionInstrumenter = transactionInstrumenter; + this.captureQueryParameters = captureQueryParameters; } public DataSource wrap(DataSource dataSource) { @@ -42,6 +45,7 @@ public final class JdbcTelemetry { dataSource, this.dataSourceInstrumenter, this.statementInstrumenter, - this.transactionInstrumenter); + this.transactionInstrumenter, + this.captureQueryParameters); } } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryBuilder.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryBuilder.java index bdb173928f..564773bfd7 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryBuilder.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/JdbcTelemetryBuilder.java @@ -7,7 +7,11 @@ package io.opentelemetry.instrumentation.jdbc.datasource; import com.google.errorprone.annotations.CanIgnoreReturnValue; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.jdbc.internal.DbRequest; import io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory; +import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; +import javax.sql.DataSource; /** A builder of {@link JdbcTelemetry}. */ public final class JdbcTelemetryBuilder { @@ -17,6 +21,7 @@ public final class JdbcTelemetryBuilder { private boolean statementInstrumenterEnabled = true; private boolean statementSanitizationEnabled = true; private boolean transactionInstrumenterEnabled = false; + private boolean captureQueryParameters = false; JdbcTelemetryBuilder(OpenTelemetry openTelemetry) { this.openTelemetry = openTelemetry; @@ -50,14 +55,38 @@ public final class JdbcTelemetryBuilder { return this; } + /** + * Configures whether parameters are captured for JDBC Statements. Enabling this option disables + * the statement sanitization. Disabled by default. + * + *

WARNING: captured query parameters may contain sensitive information such as passwords, + * personally identifiable information or protected health info. + */ + @CanIgnoreReturnValue + public JdbcTelemetryBuilder setCaptureQueryParameters(boolean enabled) { + this.captureQueryParameters = enabled; + return this; + } + /** Returns a new {@link JdbcTelemetry} with the settings of this {@link JdbcTelemetryBuilder}. */ public JdbcTelemetry build() { - return new JdbcTelemetry( + Instrumenter dataSourceInstrumenter = JdbcInstrumenterFactory.createDataSourceInstrumenter( - openTelemetry, dataSourceInstrumenterEnabled), + openTelemetry, dataSourceInstrumenterEnabled); + Instrumenter statementInstrumenter = JdbcInstrumenterFactory.createStatementInstrumenter( - openTelemetry, statementInstrumenterEnabled, statementSanitizationEnabled), + openTelemetry, + statementInstrumenterEnabled, + statementSanitizationEnabled, + captureQueryParameters); + Instrumenter transactionInstrumenter = JdbcInstrumenterFactory.createTransactionInstrumenter( - openTelemetry, transactionInstrumenterEnabled)); + openTelemetry, transactionInstrumenterEnabled); + + return new JdbcTelemetry( + dataSourceInstrumenter, + statementInstrumenter, + transactionInstrumenter, + captureQueryParameters); } } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java index 021897e02a..08e1708040 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/datasource/OpenTelemetryDataSource.java @@ -50,6 +50,7 @@ public class OpenTelemetryDataSource implements DataSource, AutoCloseable { private final Instrumenter statementInstrumenter; private final Instrumenter transactionInstrumenter; private volatile DbInfo cachedDbInfo; + private final boolean captureQueryParameters; /** * Create a OpenTelemetry DataSource wrapping another DataSource. @@ -74,6 +75,7 @@ public class OpenTelemetryDataSource implements DataSource, AutoCloseable { this.dataSourceInstrumenter = createDataSourceInstrumenter(openTelemetry, true); this.statementInstrumenter = createStatementInstrumenter(openTelemetry); this.transactionInstrumenter = createTransactionInstrumenter(openTelemetry, false); + this.captureQueryParameters = false; } /** @@ -87,11 +89,13 @@ public class OpenTelemetryDataSource implements DataSource, AutoCloseable { DataSource delegate, Instrumenter dataSourceInstrumenter, Instrumenter statementInstrumenter, - Instrumenter transactionInstrumenter) { + Instrumenter transactionInstrumenter, + boolean captureQueryParameters) { this.delegate = delegate; this.dataSourceInstrumenter = dataSourceInstrumenter; this.statementInstrumenter = statementInstrumenter; this.transactionInstrumenter = transactionInstrumenter; + this.captureQueryParameters = captureQueryParameters; } @Override @@ -99,7 +103,7 @@ public class OpenTelemetryDataSource implements DataSource, AutoCloseable { Connection connection = wrapCall(delegate::getConnection); DbInfo dbInfo = getDbInfo(connection); return OpenTelemetryConnection.create( - connection, dbInfo, statementInstrumenter, transactionInstrumenter); + connection, dbInfo, statementInstrumenter, transactionInstrumenter, captureQueryParameters); } @Override @@ -107,7 +111,7 @@ public class OpenTelemetryDataSource implements DataSource, AutoCloseable { Connection connection = wrapCall(() -> delegate.getConnection(username, password)); DbInfo dbInfo = getDbInfo(connection); return OpenTelemetryConnection.create( - connection, dbInfo, statementInstrumenter, transactionInstrumenter); + connection, dbInfo, statementInstrumenter, transactionInstrumenter, captureQueryParameters); } @Override diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/DbRequest.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/DbRequest.java index cb16d91542..3cf7ecb860 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/DbRequest.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/DbRequest.java @@ -7,6 +7,7 @@ package io.opentelemetry.instrumentation.jdbc.internal; import static io.opentelemetry.instrumentation.jdbc.internal.JdbcUtils.connectionFromStatement; import static io.opentelemetry.instrumentation.jdbc.internal.JdbcUtils.extractDbInfo; +import static java.util.Collections.emptyMap; import com.google.auto.value.AutoValue; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; @@ -15,6 +16,7 @@ import java.sql.PreparedStatement; import java.sql.Statement; import java.util.Collection; import java.util.Collections; +import java.util.Map; import javax.annotation.Nullable; /** @@ -25,23 +27,38 @@ import javax.annotation.Nullable; public abstract class DbRequest { @Nullable - public static DbRequest create(PreparedStatement statement) { - return create(statement, JdbcData.preparedStatement.get(statement)); + public static DbRequest create( + PreparedStatement statement, Map preparedStatementParameters) { + return create( + statement, JdbcData.preparedStatement.get(statement), preparedStatementParameters); } @Nullable public static DbRequest create(Statement statement, String dbStatementString) { - return create(statement, dbStatementString, null); + return create(statement, dbStatementString, null, emptyMap()); } @Nullable - public static DbRequest create(Statement statement, String dbStatementString, Long batchSize) { + private static DbRequest create( + Statement statement, + String dbStatementString, + Map preparedStatementParameters) { + return create(statement, dbStatementString, null, preparedStatementParameters); + } + + @Nullable + public static DbRequest create( + Statement statement, + String dbStatementString, + Long batchSize, + Map preparedStatementParameters) { Connection connection = connectionFromStatement(statement); if (connection == null) { return null; } - return create(extractDbInfo(connection), dbStatementString, batchSize); + return create( + extractDbInfo(connection), dbStatementString, batchSize, preparedStatementParameters); } public static DbRequest create( @@ -51,24 +68,38 @@ public abstract class DbRequest { return null; } - return create(extractDbInfo(connection), queryTexts, batchSize); + return create(extractDbInfo(connection), queryTexts, batchSize, emptyMap()); } public static DbRequest create(DbInfo dbInfo, String queryText) { - return create(dbInfo, queryText, null); - } - - public static DbRequest create(DbInfo dbInfo, String queryText, Long batchSize) { - return create(dbInfo, Collections.singletonList(queryText), batchSize); - } - - public static DbRequest create(DbInfo dbInfo, Collection queryTexts, Long batchSize) { - return new AutoValue_DbRequest(dbInfo, queryTexts, batchSize, null); + return create(dbInfo, queryText, null, emptyMap()); } public static DbRequest create( - DbInfo dbInfo, Collection queryTexts, Long batchSize, String operation) { - return new AutoValue_DbRequest(dbInfo, queryTexts, batchSize, operation); + DbInfo dbInfo, + String queryText, + Long batchSize, + Map preparedStatementParameters) { + return create( + dbInfo, Collections.singletonList(queryText), batchSize, preparedStatementParameters); + } + + public static DbRequest create( + DbInfo dbInfo, + Collection queryTexts, + Long batchSize, + Map preparedStatementParameters) { + return create(dbInfo, queryTexts, batchSize, null, preparedStatementParameters); + } + + private static DbRequest create( + DbInfo dbInfo, + Collection queryTexts, + Long batchSize, + String operation, + Map preparedStatementParameters) { + return new AutoValue_DbRequest( + dbInfo, queryTexts, batchSize, operation, preparedStatementParameters); } @Nullable @@ -82,7 +113,7 @@ public abstract class DbRequest { } public static DbRequest createTransaction(DbInfo dbInfo, String operation) { - return create(dbInfo, Collections.emptyList(), null, operation); + return create(dbInfo, Collections.emptyList(), null, operation, emptyMap()); } public abstract DbInfo getDbInfo(); @@ -95,4 +126,6 @@ public abstract class DbRequest { // used for transaction instrumentation @Nullable public abstract String getOperation(); + + public abstract Map getPreparedStatementParameters(); } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcAttributesGetter.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcAttributesGetter.java index f01d836ab7..0792c1db7a 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcAttributesGetter.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcAttributesGetter.java @@ -9,6 +9,7 @@ import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlClientAttrib import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; import java.sql.SQLException; import java.util.Collection; +import java.util.Map; import javax.annotation.Nullable; final class JdbcAttributesGetter implements SqlClientAttributesGetter { @@ -58,4 +59,9 @@ final class JdbcAttributesGetter implements SqlClientAttributesGetter getQueryParameters(DbRequest request) { + return request.getPreparedStatementParameters(); + } } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcData.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcData.java index 04f58860a5..c21e3ef762 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcData.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcData.java @@ -13,6 +13,8 @@ import java.sql.PreparedStatement; import java.sql.Statement; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.WeakHashMap; @@ -35,6 +37,8 @@ public final class JdbcData { private static final VirtualField preparedStatementBatch = VirtualField.find(PreparedStatement.class, PreparedStatementBatchInfo.class); + private static final VirtualField> parameters = + VirtualField.find(PreparedStatement.class, Map.class); private JdbcData() {} @@ -103,9 +107,32 @@ public final class JdbcData { PreparedStatement prepared = (PreparedStatement) statement; preparedStatement.set(prepared, null); preparedStatementBatch.set(prepared, null); + parameters.set(prepared, null); } } + public static Map getParameters(PreparedStatement statement) { + Map parametersMap = parameters.get(statement); + return parametersMap != null ? parametersMap : Collections.emptyMap(); + } + + public static void addParameter(PreparedStatement statement, String key, String value) { + if (value == null) { + return; + } + + Map parametersMap = parameters.get(statement); + if (parametersMap == null) { + parametersMap = new HashMap<>(); + parameters.set(statement, parametersMap); + } + parametersMap.put(key, value); + } + + public static void clearParameters(PreparedStatement statement) { + parameters.set(statement, null); + } + /** * This class is internal and is hence not for public use. Its APIs are unstable and can change at * any time. diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java index 626e62452f..a81dfde199 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java @@ -32,26 +32,42 @@ public final class JdbcInstrumenterFactory { private static final JdbcNetworkAttributesGetter netAttributesGetter = new JdbcNetworkAttributesGetter(); - public static Instrumenter createStatementInstrumenter( - OpenTelemetry openTelemetry) { - return createStatementInstrumenter( - openTelemetry, - true, - ConfigPropertiesUtil.getBoolean( - "otel.instrumentation.common.db-statement-sanitizer.enabled", true)); + public static boolean captureQueryParameters() { + return ConfigPropertiesUtil.getBoolean( + "otel.instrumentation.jdbc.capture-query-parameters", false); } public static Instrumenter createStatementInstrumenter( - OpenTelemetry openTelemetry, boolean enabled, boolean statementSanitizationEnabled) { + OpenTelemetry openTelemetry) { + return createStatementInstrumenter(openTelemetry, captureQueryParameters()); + } + + static Instrumenter createStatementInstrumenter( + OpenTelemetry openTelemetry, boolean captureQueryParameters) { return createStatementInstrumenter( - openTelemetry, emptyList(), enabled, statementSanitizationEnabled); + openTelemetry, + emptyList(), + true, + ConfigPropertiesUtil.getBoolean( + "otel.instrumentation.common.db-statement-sanitizer.enabled", true), + captureQueryParameters); + } + + public static Instrumenter createStatementInstrumenter( + OpenTelemetry openTelemetry, + boolean enabled, + boolean statementSanitizationEnabled, + boolean captureQueryParameters) { + return createStatementInstrumenter( + openTelemetry, emptyList(), enabled, statementSanitizationEnabled, captureQueryParameters); } public static Instrumenter createStatementInstrumenter( OpenTelemetry openTelemetry, List> extractors, boolean enabled, - boolean statementSanitizationEnabled) { + boolean statementSanitizationEnabled, + boolean captureQueryParameters) { return Instrumenter.builder( openTelemetry, INSTRUMENTATION_NAME, @@ -59,6 +75,7 @@ public final class JdbcInstrumenterFactory { .addAttributesExtractor( SqlClientAttributesExtractor.builder(dbAttributesGetter) .setStatementSanitizationEnabled(statementSanitizationEnabled) + .setCaptureQueryParameters(captureQueryParameters) .build()) .addAttributesExtractor(ServerAttributesExtractor.create(netAttributesGetter)) .addAttributesExtractors(extractors) diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryCallableStatement.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryCallableStatement.java index f2c25b172d..7d6b1eab09 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryCallableStatement.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryCallableStatement.java @@ -51,8 +51,9 @@ class OpenTelemetryCallableStatement OpenTelemetryConnection connection, DbInfo dbInfo, String query, - Instrumenter instrumenter) { - super(delegate, connection, dbInfo, query, instrumenter); + Instrumenter instrumenter, + boolean captureQueryParameters) { + super(delegate, connection, dbInfo, query, instrumenter, captureQueryParameters); } @Override diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java index 4874ed4a6f..66964f866d 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnection.java @@ -55,16 +55,19 @@ public class OpenTelemetryConnection implements Connection { private final DbInfo dbInfo; protected final Instrumenter statementInstrumenter; protected final Instrumenter transactionInstrumenter; + private final boolean captureQueryParameters; protected OpenTelemetryConnection( Connection delegate, DbInfo dbInfo, Instrumenter statementInstrumenter, - Instrumenter transactionInstrumenter) { + Instrumenter transactionInstrumenter, + boolean captureQueryParameters) { this.delegate = delegate; this.dbInfo = dbInfo; this.statementInstrumenter = statementInstrumenter; this.transactionInstrumenter = transactionInstrumenter; + this.captureQueryParameters = captureQueryParameters; } // visible for testing @@ -81,13 +84,14 @@ public class OpenTelemetryConnection implements Connection { Connection delegate, DbInfo dbInfo, Instrumenter statementInstrumenter, - Instrumenter transactionInstrumenter) { + Instrumenter transactionInstrumenter, + boolean captureQueryParameters) { if (hasJdbc43) { return new OpenTelemetryConnectionJdbc43( - delegate, dbInfo, statementInstrumenter, transactionInstrumenter); + delegate, dbInfo, statementInstrumenter, transactionInstrumenter, captureQueryParameters); } return new OpenTelemetryConnection( - delegate, dbInfo, statementInstrumenter, transactionInstrumenter); + delegate, dbInfo, statementInstrumenter, transactionInstrumenter, captureQueryParameters); } @Override @@ -115,7 +119,7 @@ public class OpenTelemetryConnection implements Connection { public PreparedStatement prepareStatement(String sql) throws SQLException { PreparedStatement statement = delegate.prepareStatement(sql); return new OpenTelemetryPreparedStatement<>( - statement, this, dbInfo, sql, statementInstrumenter); + statement, this, dbInfo, sql, statementInstrumenter, captureQueryParameters); } @Override @@ -124,7 +128,7 @@ public class OpenTelemetryConnection implements Connection { PreparedStatement statement = delegate.prepareStatement(sql, resultSetType, resultSetConcurrency); return new OpenTelemetryPreparedStatement<>( - statement, this, dbInfo, sql, statementInstrumenter); + statement, this, dbInfo, sql, statementInstrumenter, captureQueryParameters); } @Override @@ -134,35 +138,35 @@ public class OpenTelemetryConnection implements Connection { PreparedStatement statement = delegate.prepareStatement(sql, resultSetType, resultSetConcurrency, resultSetHoldability); return new OpenTelemetryPreparedStatement<>( - statement, this, dbInfo, sql, statementInstrumenter); + statement, this, dbInfo, sql, statementInstrumenter, captureQueryParameters); } @Override public PreparedStatement prepareStatement(String sql, int autoGeneratedKeys) throws SQLException { PreparedStatement statement = delegate.prepareStatement(sql, autoGeneratedKeys); return new OpenTelemetryPreparedStatement<>( - statement, this, dbInfo, sql, statementInstrumenter); + statement, this, dbInfo, sql, statementInstrumenter, captureQueryParameters); } @Override public PreparedStatement prepareStatement(String sql, int[] columnIndexes) throws SQLException { PreparedStatement statement = delegate.prepareStatement(sql, columnIndexes); return new OpenTelemetryPreparedStatement<>( - statement, this, dbInfo, sql, statementInstrumenter); + statement, this, dbInfo, sql, statementInstrumenter, captureQueryParameters); } @Override public PreparedStatement prepareStatement(String sql, String[] columnNames) throws SQLException { PreparedStatement statement = delegate.prepareStatement(sql, columnNames); return new OpenTelemetryPreparedStatement<>( - statement, this, dbInfo, sql, statementInstrumenter); + statement, this, dbInfo, sql, statementInstrumenter, captureQueryParameters); } @Override public CallableStatement prepareCall(String sql) throws SQLException { CallableStatement statement = delegate.prepareCall(sql); return new OpenTelemetryCallableStatement<>( - statement, this, dbInfo, sql, statementInstrumenter); + statement, this, dbInfo, sql, statementInstrumenter, captureQueryParameters); } @Override @@ -170,7 +174,7 @@ public class OpenTelemetryConnection implements Connection { throws SQLException { CallableStatement statement = delegate.prepareCall(sql, resultSetType, resultSetConcurrency); return new OpenTelemetryCallableStatement<>( - statement, this, dbInfo, sql, statementInstrumenter); + statement, this, dbInfo, sql, statementInstrumenter, captureQueryParameters); } @Override @@ -180,7 +184,7 @@ public class OpenTelemetryConnection implements Connection { CallableStatement statement = delegate.prepareCall(sql, resultSetType, resultSetConcurrency, resultSetHoldability); return new OpenTelemetryCallableStatement<>( - statement, this, dbInfo, sql, statementInstrumenter); + statement, this, dbInfo, sql, statementInstrumenter, captureQueryParameters); } @Override @@ -408,8 +412,10 @@ public class OpenTelemetryConnection implements Connection { Connection delegate, DbInfo dbInfo, Instrumenter statementInstrumenter, - Instrumenter transactionInstrumenter) { - super(delegate, dbInfo, statementInstrumenter, transactionInstrumenter); + Instrumenter transactionInstrumenter, + boolean captureQueryParameters) { + super( + delegate, dbInfo, statementInstrumenter, transactionInstrumenter, captureQueryParameters); } @SuppressWarnings("Since15") diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java index eaabc1685c..a5b4e7f652 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java @@ -43,18 +43,31 @@ import java.sql.SQLXML; import java.sql.Time; import java.sql.Timestamp; import java.util.Calendar; +import java.util.HashMap; +import java.util.Map; @SuppressWarnings("OverloadMethodsDeclarationOrder") class OpenTelemetryPreparedStatement extends OpenTelemetryStatement implements PreparedStatement { + private final boolean captureQueryParameters; + private final Map parameters; public OpenTelemetryPreparedStatement( S delegate, OpenTelemetryConnection connection, DbInfo dbInfo, String query, - Instrumenter instrumenter) { + Instrumenter instrumenter, + boolean captureQueryParameters) { super(delegate, connection, dbInfo, query, instrumenter); + this.captureQueryParameters = captureQueryParameters; + this.parameters = new HashMap<>(); + } + + private void putParameter(int index, Object value) { + if (this.captureQueryParameters && value != null) { + parameters.put(Integer.toString(index - 1), value.toString()); + } } @Override @@ -87,46 +100,55 @@ class OpenTelemetryPreparedStatement extends OpenTe @Override public void setBoolean(int parameterIndex, boolean x) throws SQLException { delegate.setBoolean(parameterIndex, x); + putParameter(parameterIndex, String.valueOf(x)); } @Override public void setByte(int parameterIndex, byte x) throws SQLException { delegate.setByte(parameterIndex, x); + putParameter(parameterIndex, String.valueOf(x)); } @Override public void setShort(int parameterIndex, short x) throws SQLException { delegate.setShort(parameterIndex, x); + putParameter(parameterIndex, String.valueOf(x)); } @Override public void setInt(int parameterIndex, int x) throws SQLException { delegate.setInt(parameterIndex, x); + putParameter(parameterIndex, String.valueOf(x)); } @Override public void setLong(int parameterIndex, long x) throws SQLException { delegate.setLong(parameterIndex, x); + putParameter(parameterIndex, String.valueOf(x)); } @Override public void setFloat(int parameterIndex, float x) throws SQLException { delegate.setFloat(parameterIndex, x); + putParameter(parameterIndex, String.valueOf(x)); } @Override public void setDouble(int parameterIndex, double x) throws SQLException { delegate.setDouble(parameterIndex, x); + putParameter(parameterIndex, String.valueOf(x)); } @Override public void setBigDecimal(int parameterIndex, BigDecimal x) throws SQLException { delegate.setBigDecimal(parameterIndex, x); + putParameter(parameterIndex, x); } @Override public void setString(int parameterIndex, String x) throws SQLException { delegate.setString(parameterIndex, x); + putParameter(parameterIndex, x); } @Override @@ -138,35 +160,41 @@ class OpenTelemetryPreparedStatement extends OpenTe @Override public void setDate(int parameterIndex, Date x) throws SQLException { delegate.setDate(parameterIndex, x); + putParameter(parameterIndex, x); } @SuppressWarnings("UngroupedOverloads") @Override public void setDate(int parameterIndex, Date x, Calendar cal) throws SQLException { delegate.setDate(parameterIndex, x, cal); + putParameter(parameterIndex, x); } @SuppressWarnings("UngroupedOverloads") @Override public void setTime(int parameterIndex, Time x) throws SQLException { delegate.setTime(parameterIndex, x); + putParameter(parameterIndex, x); } @Override public void setTime(int parameterIndex, Time x, Calendar cal) throws SQLException { delegate.setTime(parameterIndex, x, cal); + putParameter(parameterIndex, x); } @SuppressWarnings("UngroupedOverloads") @Override public void setTimestamp(int parameterIndex, Timestamp x) throws SQLException { delegate.setTimestamp(parameterIndex, x); + putParameter(parameterIndex, x); } @SuppressWarnings("UngroupedOverloads") @Override public void setTimestamp(int parameterIndex, Timestamp x, Calendar cal) throws SQLException { delegate.setTimestamp(parameterIndex, x, cal); + putParameter(parameterIndex, x); } @SuppressWarnings("UngroupedOverloads") @@ -307,6 +335,7 @@ class OpenTelemetryPreparedStatement extends OpenTe @Override public void setURL(int parameterIndex, URL x) throws SQLException { delegate.setURL(parameterIndex, x); + putParameter(parameterIndex, x); } @Override @@ -317,11 +346,13 @@ class OpenTelemetryPreparedStatement extends OpenTe @Override public void setRowId(int parameterIndex, RowId x) throws SQLException { delegate.setRowId(parameterIndex, x); + putParameter(parameterIndex, x); } @Override public void setNString(int parameterIndex, String value) throws SQLException { delegate.setNString(parameterIndex, value); + putParameter(parameterIndex, value); } @SuppressWarnings("UngroupedOverloads") @@ -361,6 +392,7 @@ class OpenTelemetryPreparedStatement extends OpenTe @Override public void clearParameters() throws SQLException { delegate.clearParameters(); + parameters.clear(); } @Override @@ -368,8 +400,15 @@ class OpenTelemetryPreparedStatement extends OpenTe return wrapBatchCall(delegate::executeBatch); } + @Override + protected T wrapCall(String sql, ThrowingSupplier callable) + throws E { + DbRequest request = DbRequest.create(dbInfo, sql, null, parameters); + return wrapCall(request, callable); + } + private T wrapBatchCall(ThrowingSupplier callable) throws E { - DbRequest request = DbRequest.create(dbInfo, query, batchSize); + DbRequest request = DbRequest.create(dbInfo, query, batchSize, parameters); return wrapCall(request, callable); } diff --git a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java index 78e4c782eb..759f2d7192 100644 --- a/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java +++ b/instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java @@ -30,6 +30,7 @@ import java.sql.SQLException; import java.sql.SQLWarning; import java.sql.Statement; import java.util.ArrayList; +import java.util.Collections; import java.util.List; class OpenTelemetryStatement implements Statement { @@ -385,7 +386,7 @@ class OpenTelemetryStatement implements Statement { } private T wrapBatchCall(ThrowingSupplier callable) throws E { - DbRequest request = DbRequest.create(dbInfo, batchCommands, batchSize); + DbRequest request = DbRequest.create(dbInfo, batchCommands, batchSize, Collections.emptyMap()); return wrapCall(request, callable); } } diff --git a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java index 836525ff2e..eb30c43c79 100644 --- a/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java +++ b/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java @@ -16,6 +16,7 @@ import static io.opentelemetry.semconv.ServerAttributes.SERVER_PORT; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_CONNECTION_STRING; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_NAME; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_OPERATION; +import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_QUERY_PARAMETER; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SQL_TABLE; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_STATEMENT; import static io.opentelemetry.semconv.incubating.DbIncubatingAttributes.DB_SYSTEM; @@ -30,10 +31,22 @@ import io.opentelemetry.instrumentation.jdbc.TestConnection; import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo; import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension; +import io.opentelemetry.sdk.testing.assertj.AttributeAssertion; +import java.math.BigDecimal; +import java.net.MalformedURLException; +import java.net.URI; +import java.sql.Date; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; +import java.sql.Time; +import java.sql.Timestamp; +import java.util.Arrays; +import java.util.Calendar; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -221,6 +234,73 @@ class OpenTelemetryConnectionTest { connection.close(); } + // https://github.com/open-telemetry/semantic-conventions/pull/2093 + @SuppressWarnings("deprecation") + @Test + void testVerifyPrepareStatementParameters() throws SQLException, MalformedURLException { + OpenTelemetry openTelemetry = testing.getOpenTelemetry(); + Instrumenter instrumenter = + createStatementInstrumenter(testing.getOpenTelemetry(), true); + Instrumenter transactionInstrumenter = + createTransactionInstrumenter(openTelemetry, false); + DbInfo dbInfo = getDbInfo(); + OpenTelemetryConnection connection = + new OpenTelemetryConnection( + new TestConnection(), dbInfo, instrumenter, transactionInstrumenter, true); + String query = "SELECT * FROM users WHERE id=? AND age=3"; + String sanitized = "SELECT * FROM users WHERE id=? AND age=3"; + PreparedStatement statement = connection.prepareStatement(query); + // doesn't need to match the number of placeholders in this context + statement.setBoolean(1, true); + statement.setShort(2, (short) 1); + statement.setInt(3, 2); + statement.setLong(4, 3); + statement.setFloat(5, 4); + statement.setDouble(6, 5.5); + statement.setBigDecimal(7, BigDecimal.valueOf(6)); + statement.setString(8, "S"); + statement.setDate(9, Date.valueOf("2000-01-01")); + statement.setDate(10, Date.valueOf("2000-01-02"), Calendar.getInstance()); + statement.setTime(11, Time.valueOf("00:00:00")); + statement.setTime(12, Time.valueOf("00:00:01"), Calendar.getInstance()); + statement.setTimestamp(13, Timestamp.valueOf("2000-01-01 00:00:00")); + statement.setTimestamp(14, Timestamp.valueOf("2000-01-01 00:00:01"), Calendar.getInstance()); + statement.setURL(15, URI.create("http://localhost:8080").toURL()); + statement.setNString(16, "S"); + + testing.runWithSpan( + "parent", + () -> { + ResultSet resultSet = statement.executeQuery(); + assertThat(resultSet).isInstanceOf(OpenTelemetryResultSet.class); + assertThat(resultSet.getStatement()).isEqualTo(statement); + }); + + jdbcTraceAssertion( + dbInfo, + sanitized, + "SELECT", + equalTo(DB_QUERY_PARAMETER.getAttributeKey("0"), "true"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("1"), "1"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("2"), "2"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("3"), "3"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("4"), "4.0"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("5"), "5.5"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("6"), "6"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("7"), "S"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("8"), "2000-01-01"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("9"), "2000-01-02"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("10"), "00:00:00"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("11"), "00:00:01"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("12"), "2000-01-01 00:00:00.0"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("13"), "2000-01-01 00:00:01.0"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("14"), "http://localhost:8080"), + equalTo(DB_QUERY_PARAMETER.getAttributeKey("15"), "S")); + + statement.close(); + connection.close(); + } + private static DbInfo getDbInfo() { return DbInfo.builder() .system("my_system") @@ -239,7 +319,22 @@ class OpenTelemetryConnectionTest { } @SuppressWarnings("deprecation") // old semconv - private static void jdbcTraceAssertion(DbInfo dbInfo, String query, String operation) { + private static void jdbcTraceAssertion( + DbInfo dbInfo, String query, String operation, AttributeAssertion... assertions) { + List baseAttributeAssertions = + Arrays.asList( + equalTo(maybeStable(DB_SYSTEM), maybeStableDbSystemName(dbInfo.getSystem())), + equalTo(maybeStable(DB_NAME), dbInfo.getName()), + equalTo(DB_USER, emitStableDatabaseSemconv() ? null : dbInfo.getUser()), + equalTo( + DB_CONNECTION_STRING, emitStableDatabaseSemconv() ? null : dbInfo.getShortUrl()), + equalTo(maybeStable(DB_STATEMENT), query), + equalTo(maybeStable(DB_OPERATION), operation), + equalTo(maybeStable(DB_SQL_TABLE), "users"), + equalTo(SERVER_ADDRESS, dbInfo.getHost()), + equalTo(SERVER_PORT, dbInfo.getPort())); + + List additionAttributeAssertions = Arrays.asList(assertions); testing.waitAndAssertTraces( trace -> trace.hasSpansSatisfyingExactly( @@ -249,19 +344,10 @@ class OpenTelemetryConnectionTest { .hasKind(SpanKind.CLIENT) .hasParent(trace.getSpan(0)) .hasAttributesSatisfyingExactly( - equalTo( - maybeStable(DB_SYSTEM), - maybeStableDbSystemName(dbInfo.getSystem())), - equalTo(maybeStable(DB_NAME), dbInfo.getName()), - equalTo(DB_USER, emitStableDatabaseSemconv() ? null : dbInfo.getUser()), - equalTo( - DB_CONNECTION_STRING, - emitStableDatabaseSemconv() ? null : dbInfo.getShortUrl()), - equalTo(maybeStable(DB_STATEMENT), query), - equalTo(maybeStable(DB_OPERATION), operation), - equalTo(maybeStable(DB_SQL_TABLE), "users"), - equalTo(SERVER_ADDRESS, dbInfo.getHost()), - equalTo(SERVER_PORT, dbInfo.getPort())))); + Stream.concat( + baseAttributeAssertions.stream(), + additionAttributeAssertions.stream()) + .collect(Collectors.toList())))); } @SuppressWarnings("deprecation") // old semconv @@ -299,6 +385,6 @@ class OpenTelemetryConnectionTest { createTransactionInstrumenter(openTelemetry, true); DbInfo dbInfo = getDbInfo(); return new OpenTelemetryConnection( - new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter); + new TestConnection(), dbInfo, statementInstrumenter, transactionInstrumenter, false); } } diff --git a/instrumentation/jdbc/metadata.yaml b/instrumentation/jdbc/metadata.yaml index e0bf435530..7dd9f28340 100644 --- a/instrumentation/jdbc/metadata.yaml +++ b/instrumentation/jdbc/metadata.yaml @@ -22,6 +22,14 @@ configurations: description: Used to specify a mapping from host names or IP addresses to peer services. type: map default: "" + - name: otel.instrumentation.jdbc.capture-query-parameters + description: > + Sets whether the query parameters should be captured as span attributes named + db.query.parameter.<key>. Enabling this option disables the statement + sanitization.

WARNING: captured query parameters may contain sensitive information such as + passwords, personally identifiable information or protected health info. + type: boolean + default: false - name: otel.instrumentation.jdbc-datasource.enabled description: Enables instrumentation of JDBC datasource connections. type: boolean diff --git a/instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/TestPreparedStatement.java b/instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/TestPreparedStatement.java index 9721c5580f..8eb3806d03 100644 --- a/instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/TestPreparedStatement.java +++ b/instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/TestPreparedStatement.java @@ -26,12 +26,16 @@ import java.sql.SQLXML; import java.sql.Time; import java.sql.Timestamp; import java.util.Calendar; +import java.util.HashMap; +import java.util.Map; class TestPreparedStatement extends TestStatement implements PreparedStatement { private boolean hasResultSet = true; + Map parameters; TestPreparedStatement(Connection connection) { super(connection); + this.parameters = new HashMap<>(); } @Override @@ -147,7 +151,9 @@ class TestPreparedStatement extends TestStatement implements PreparedStatement { public void setFloat(int parameterIndex, float x) throws SQLException {} @Override - public void setInt(int parameterIndex, int x) throws SQLException {} + public void setInt(int parameterIndex, int x) throws SQLException { + parameters.put(Integer.toString(parameterIndex), Integer.toString(x)); + } @Override public void setLong(int parameterIndex, long x) throws SQLException {} diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/DataSourcePostProcessor.java b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/DataSourcePostProcessor.java index 22c437605d..c3d93c1ed4 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/DataSourcePostProcessor.java +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/DataSourcePostProcessor.java @@ -55,6 +55,10 @@ final class DataSourcePostProcessor implements BeanPostProcessor, Ordered { InstrumentationConfigUtil.isStatementSanitizationEnabled( configPropertiesProvider.getObject(), "otel.instrumentation.jdbc.statement-sanitizer.enabled")) + .setCaptureQueryParameters( + configPropertiesProvider + .getObject() + .getBoolean("otel.instrumentation.jdbc.capture-query-parameters", false)) .setTransactionInstrumenterEnabled( configPropertiesProvider .getObject() diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/instrumentation/spring/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json index b4f7b9d918..1bf97200cc 100644 --- a/instrumentation/spring/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/instrumentation/spring/spring-boot-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -351,6 +351,12 @@ "description": "Enables the DB statement sanitization.", "defaultValue": true }, + { + "name": "otel.instrumentation.jdbc.capture-query-parameters", + "type": "java.lang.Boolean", + "description": "Sets whether the query parameters should be captured as span attributes named db.query.parameter.<key>. Enabling this option disables the statement sanitization.

WARNING: captured query parameters may contain sensitive information such as passwords, personally identifiable information or protected health info.", + "defaultValue": false + }, { "name": "otel.instrumentation.jdbc.experimental.transaction.enabled", "type": "java.lang.Boolean",