make statement sanitizer configurable for spring boot (#11350)

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
open-telemetry · May 17, 2024 · d37c739 · d37c739
1 parent f52d29a
commit d37c739
Show file tree

Hide file tree

Showing 20 changed files with 188 additions and 57 deletions.
diff --git a/instrumentation/spring/spring-boot-autoconfigure/build.gradle.kts b/instrumentation/spring/spring-boot-autoconfigure/build.gradle.kts
@@ -71,6 +71,8 @@ dependencies {
   }
   testImplementation("javax.servlet:javax.servlet-api:3.1.0")
   testImplementation("jakarta.servlet:jakarta.servlet-api:5.0.0")
+  testRuntimeOnly("com.h2database:h2:1.4.197")
+  testRuntimeOnly("io.r2dbc:r2dbc-h2:1.0.0.RELEASE")
 
   testImplementation(project(":testing-common"))
   testImplementation("io.opentelemetry:opentelemetry-sdk")

diff --git a/...ry/instrumentation/spring/autoconfigure/instrumentation/jdbc/DataSourcePostProcessor.java b/...ry/instrumentation/spring/autoconfigure/instrumentation/jdbc/DataSourcePostProcessor.java
@@ -8,6 +8,8 @@
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.instrumentation.jdbc.datasource.JdbcTelemetry;
+import io.opentelemetry.instrumentation.spring.autoconfigure.internal.InstrumentationConfigUtil;
+import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import javax.sql.DataSource;
 import org.springframework.aop.scope.ScopedProxyUtils;
 import org.springframework.beans.factory.ObjectProvider;
@@ -17,9 +19,13 @@
 final class DataSourcePostProcessor implements BeanPostProcessor, Ordered {
 
   private final ObjectProvider<OpenTelemetry> openTelemetryProvider;
+  private final ObjectProvider<ConfigProperties> configPropertiesProvider;
 
-  DataSourcePostProcessor(ObjectProvider<OpenTelemetry> openTelemetryProvider) {
+  DataSourcePostProcessor(
+      ObjectProvider<OpenTelemetry> openTelemetryProvider,
+      ObjectProvider<ConfigProperties> configPropertiesProvider) {
     this.openTelemetryProvider = openTelemetryProvider;
+    this.configPropertiesProvider = configPropertiesProvider;
   }
 
   @CanIgnoreReturnValue
@@ -28,7 +34,13 @@ public Object postProcessAfterInitialization(Object bean, String beanName) {
     // Exclude scoped proxy beans to avoid double wrapping
     if (bean instanceof DataSource && !ScopedProxyUtils.isScopedTarget(beanName)) {
       DataSource dataSource = (DataSource) bean;
-      return JdbcTelemetry.create(openTelemetryProvider.getObject()).wrap(dataSource);
+      return JdbcTelemetry.builder(openTelemetryProvider.getObject())
+          .setStatementSanitizationEnabled(
+              InstrumentationConfigUtil.isStatementSanitizationEnabled(
+                  configPropertiesProvider.getObject(),
+                  "otel.instrumentation.jdbc.statement-sanitizer.enabled"))
+          .build()
+          .wrap(dataSource);
     }
     return bean;
   }

diff --git a/...ation/spring/autoconfigure/instrumentation/jdbc/JdbcInstrumentationAutoConfiguration.java b/...ation/spring/autoconfigure/instrumentation/jdbc/JdbcInstrumentationAutoConfiguration.java
@@ -7,6 +7,7 @@
 
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.ConditionalOnEnabledInstrumentation;
+import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import javax.sql.DataSource;
 import org.springframework.beans.factory.ObjectProvider;
 import org.springframework.boot.autoconfigure.AutoConfiguration;
@@ -27,7 +28,8 @@ public JdbcInstrumentationAutoConfiguration() {}
   @Bean
   // static to avoid "is not eligible for getting processed by all BeanPostProcessors" warning
   static DataSourcePostProcessor dataSourcePostProcessor(
-      ObjectProvider<OpenTelemetry> openTelemetryProvider) {
-    return new DataSourcePostProcessor(openTelemetryProvider);
+      ObjectProvider<OpenTelemetry> openTelemetryProvider,
+      ObjectProvider<ConfigProperties> configPropertiesProvider) {
+    return new DataSourcePostProcessor(openTelemetryProvider, configPropertiesProvider);
   }
 }
diff --git a/...ring/autoconfigure/instrumentation/mongo/MongoClientInstrumentationAutoConfiguration.java b/...ring/autoconfigure/instrumentation/mongo/MongoClientInstrumentationAutoConfiguration.java
@@ -8,6 +8,7 @@
 import com.mongodb.MongoClientSettings;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.instrumentation.mongo.v3_1.MongoTelemetry;
+import io.opentelemetry.instrumentation.spring.autoconfigure.internal.InstrumentationConfigUtil;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.SdkEnabled;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
@@ -32,8 +33,8 @@ public MongoClientSettingsBuilderCustomizer customizer(
         builder.addCommandListener(
             MongoTelemetry.builder(openTelemetry)
                 .setStatementSanitizationEnabled(
-                    config.getBoolean(
-                        "otel.instrumentation.mongo.statement-sanitizer.enabled", true))
+                    InstrumentationConfigUtil.isStatementSanitizationEnabled(
+                        config, "otel.instrumentation.mongo.statement-sanitizer.enabled"))
                 .build()
                 .newCommandListener());
   }

diff --git a/...ntation/r2dbc/R2dbcAutoConfiguration.java → ...2dbcInstrumentationAutoConfiguration.java b/...ntation/r2dbc/R2dbcAutoConfiguration.java → ...2dbcInstrumentationAutoConfiguration.java
@@ -7,9 +7,9 @@
 
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.instrumentation.spring.autoconfigure.internal.SdkEnabled;
+import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import io.r2dbc.spi.ConnectionFactory;
 import org.springframework.beans.factory.ObjectProvider;
-import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
@@ -22,16 +22,15 @@
 @ConditionalOnProperty(name = "otel.instrumentation.r2dbc.enabled", matchIfMissing = true)
 @Conditional(SdkEnabled.class)
 @Configuration(proxyBeanMethods = false)
-public class R2dbcAutoConfiguration {
+public class R2dbcInstrumentationAutoConfiguration {
 
-  public R2dbcAutoConfiguration() {}
+  public R2dbcInstrumentationAutoConfiguration() {}
 
   @Bean
   // static to avoid "is not eligible for getting processed by all BeanPostProcessors" warning
   static R2dbcInstrumentingPostProcessor r2dbcInstrumentingPostProcessor(
       ObjectProvider<OpenTelemetry> openTelemetryProvider,
-      @Value("${otel.instrumentation.common.db-statement-sanitizer.enabled:true}")
-          boolean statementSanitizationEnabled) {
-    return new R2dbcInstrumentingPostProcessor(openTelemetryProvider, statementSanitizationEnabled);
+      ObjectProvider<ConfigProperties> configPropertiesProvider) {
+    return new R2dbcInstrumentingPostProcessor(openTelemetryProvider, configPropertiesProvider);
   }
 }
diff --git a/...mentation/spring/autoconfigure/instrumentation/r2dbc/R2dbcInstrumentingPostProcessor.java b/...mentation/spring/autoconfigure/instrumentation/r2dbc/R2dbcInstrumentingPostProcessor.java
@@ -7,6 +7,8 @@
 
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.instrumentation.r2dbc.v1_0.R2dbcTelemetry;
+import io.opentelemetry.instrumentation.spring.autoconfigure.internal.InstrumentationConfigUtil;
+import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import io.r2dbc.spi.ConnectionFactory;
 import io.r2dbc.spi.ConnectionFactoryOptions;
 import org.springframework.aop.scope.ScopedProxyUtils;
@@ -17,20 +19,24 @@
 class R2dbcInstrumentingPostProcessor implements BeanPostProcessor {
 
   private final ObjectProvider<OpenTelemetry> openTelemetryProvider;
-  private final boolean statementSanitizationEnabled;
+  private final ObjectProvider<ConfigProperties> configPropertiesProvider;
 
   R2dbcInstrumentingPostProcessor(
-      ObjectProvider<OpenTelemetry> openTelemetryProvider, boolean statementSanitizationEnabled) {
+      ObjectProvider<OpenTelemetry> openTelemetryProvider,
+      ObjectProvider<ConfigProperties> configPropertiesProvider) {
     this.openTelemetryProvider = openTelemetryProvider;
-    this.statementSanitizationEnabled = statementSanitizationEnabled;
+    this.configPropertiesProvider = configPropertiesProvider;
   }
 
   @Override
   public Object postProcessAfterInitialization(Object bean, String beanName) {
     if (bean instanceof ConnectionFactory && !ScopedProxyUtils.isScopedTarget(beanName)) {
       ConnectionFactory connectionFactory = (ConnectionFactory) bean;
       return R2dbcTelemetry.builder(openTelemetryProvider.getObject())
-          .setStatementSanitizationEnabled(statementSanitizationEnabled)
+          .setStatementSanitizationEnabled(
+              InstrumentationConfigUtil.isStatementSanitizationEnabled(
+                  configPropertiesProvider.getObject(),
+                  "otel.instrumentation.r2dbc.statement-sanitizer.enabled"))
           .build()
           .wrapConnectionFactory(connectionFactory, getConnectionFactoryOptions(connectionFactory));
     }

diff --git a/...pentelemetry/instrumentation/spring/autoconfigure/internal/InstrumentationConfigUtil.java b/...pentelemetry/instrumentation/spring/autoconfigure/internal/InstrumentationConfigUtil.java
@@ -0,0 +1,21 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.spring.autoconfigure.internal;
+
+import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
+
+/**
+ * This class is internal and is hence not for public use. Its APIs are unstable and can change at
+ * any time.
+ */
+public class InstrumentationConfigUtil {
+  private InstrumentationConfigUtil() {}
+
+  public static boolean isStatementSanitizationEnabled(ConfigProperties config, String key) {
+    return config.getBoolean(
+        key, config.getBoolean("otel.instrumentation.common.db-statement-sanitizer.enabled", true));
+  }
+}
diff --git a/...t-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/...t-autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json
@@ -276,7 +276,19 @@
     {
       "name": "otel.instrumentation.common.db-statement-sanitizer.enabled",
       "type": "java.lang.Boolean",
-      "description": "Enables the DB statement sanitization for R2DBC.",
+      "description": "Enables the DB statement sanitization.",
+      "defaultValue": true
+    },
+    {
+      "name": "otel.instrumentation.jdbc.enabled",
+      "type": "java.lang.Boolean",
+      "description": "Enable the JDBC instrumentation.",
+      "defaultValue": true
+    },
+    {
+      "name": "otel.instrumentation.jdbc.statement-sanitizer.enabled",
+      "type": "java.lang.Boolean",
+      "description": "Enables the DB statement sanitization.",
       "defaultValue": true
     },
     {
@@ -359,7 +371,13 @@
     {
       "name": "otel.instrumentation.r2dbc.enabled",
       "type": "java.lang.Boolean",
-      "description": "Enable the R2DBC (reactive JDBC) instrumentation. Also see <code>otel.instrumentation.common.db-statement-sanitizer.enabled</code>.",
+      "description": "Enable the R2DBC (reactive JDBC) instrumentation.",
+      "defaultValue": true
+    },
+    {
+      "name": "otel.instrumentation.r2dbc.statement-sanitizer.enabled",
+      "type": "java.lang.Boolean",
+      "description": "Enables the DB statement sanitization.",
       "defaultValue": true
     },
     {

diff --git a/...rumentation/spring/spring-boot-autoconfigure/src/main/resources/META-INF/spring.factories b/...rumentation/spring/spring-boot-autoconfigure/src/main/resources/META-INF/spring.factories
@@ -6,7 +6,7 @@ io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.mongo.Mong
 io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.logging.OpenTelemetryAppenderAutoConfiguration,\
 io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.jdbc.JdbcInstrumentationAutoConfiguration,\
 io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.micrometer.MicrometerBridgeAutoConfiguration,\
-io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.r2dbc.R2dbcAutoConfiguration,\
+io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.r2dbc.R2dbcInstrumentationAutoConfiguration,\
 io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.web.SpringWebInstrumentationAutoConfiguration,\
 io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.webflux.SpringWebfluxInstrumentationAutoConfiguration,\
 io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.webmvc.SpringWebMvc5InstrumentationAutoConfiguration

diff --git a/...esources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports b/...esources/META-INF/spring/org.springframework.boot.autoconfigure.AutoConfiguration.imports
@@ -5,7 +5,7 @@ io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.mongo.Mong
 io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.logging.OpenTelemetryAppenderAutoConfiguration
 io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.jdbc.JdbcInstrumentationAutoConfiguration
 io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.micrometer.MicrometerBridgeAutoConfiguration
-io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.r2dbc.R2dbcAutoConfiguration
+io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.r2dbc.R2dbcInstrumentationAutoConfiguration
 io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.web.SpringWebInstrumentationAutoConfiguration
 io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.webflux.SpringWebfluxInstrumentationAutoConfiguration
 io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.webmvc.SpringWebMvc6InstrumentationAutoConfiguration
diff --git a/...spring/autoconfigure/instrumentation/r2dbc/R2DbcInstrumentationAutoConfigurationTest.java b/...spring/autoconfigure/instrumentation/r2dbc/R2DbcInstrumentationAutoConfigurationTest.java
@@ -0,0 +1,63 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.spring.autoconfigure.instrumentation.r2dbc;
+
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension;
+import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
+import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
+import io.opentelemetry.semconv.incubating.DbIncubatingAttributes;
+import java.util.Collections;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.springframework.boot.autoconfigure.AutoConfigurations;
+import org.springframework.boot.autoconfigure.r2dbc.R2dbcAutoConfiguration;
+import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+import org.springframework.r2dbc.core.DatabaseClient;
+
+class R2DbcInstrumentationAutoConfigurationTest {
+
+  @RegisterExtension
+  static final LibraryInstrumentationExtension testing = LibraryInstrumentationExtension.create();
+
+  private final ApplicationContextRunner runner =
+      new ApplicationContextRunner()
+          .withBean(
+              ConfigProperties.class,
+              () -> DefaultConfigProperties.createFromMap(Collections.emptyMap()))
+          .withConfiguration(
+              AutoConfigurations.of(
+                  R2dbcInstrumentationAutoConfiguration.class, R2dbcAutoConfiguration.class))
+          .withBean("openTelemetry", OpenTelemetry.class, testing::getOpenTelemetry);
+
+  @Test
+  void statementSanitizerEnabledByDefault() {
+    runner.run(
+        context -> {
+          DatabaseClient client = context.getBean(DatabaseClient.class);
+          client
+              .sql(
+                  "CREATE TABLE IF NOT EXISTS player(id INT NOT NULL AUTO_INCREMENT, name VARCHAR(255), age INT, PRIMARY KEY (id))")
+              .fetch()
+              .all()
+              .blockLast();
+          client.sql("SELECT * FROM player WHERE id = 1").fetch().all().blockLast();
+          testing.waitAndAssertTraces(
+              trace ->
+                  trace.hasSpansSatisfyingExactly(
+                      span ->
+                          span.hasAttribute(
+                              DbIncubatingAttributes.DB_STATEMENT,
+                              "CREATE TABLE IF NOT EXISTS player(id INT NOT NULL AUTO_INCREMENT, name VARCHAR(?), age INT, PRIMARY KEY (id))")),
+              trace ->
+                  trace.hasSpansSatisfyingExactly(
+                      span ->
+                          span.hasAttribute(
+                              DbIncubatingAttributes.DB_STATEMENT,
+                              "SELECT * FROM player WHERE id = ?")));
+        });
+  }
+}
diff --git a/...test/java/io/opentelemetry/spring/smoketest/GraalVmNativeKafkaSpringStarterSmokeTest.java b/...test/java/io/opentelemetry/spring/smoketest/GraalVmNativeKafkaSpringStarterSmokeTest.java
@@ -28,5 +28,5 @@
     },
     webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
 @EnabledInNativeImage // see JvmMongodbSpringStarterSmokeTest for the JVM test
-@RequiresDockerComposeEnvVariable
+@RequiresDockerCompose
 public class GraalVmNativeKafkaSpringStarterSmokeTest extends AbstractKafkaSpringStarterSmokeTest {}
diff --git a/...st/java/io/opentelemetry/spring/smoketest/GraalVmNativeMongodbSpringStarterSmokeTest.java b/...st/java/io/opentelemetry/spring/smoketest/GraalVmNativeMongodbSpringStarterSmokeTest.java
@@ -24,6 +24,6 @@
     classes = {OtelSpringStarterSmokeTestApplication.class, SpringSmokeOtelConfiguration.class},
     webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
 @EnabledInNativeImage // see JvmMongodbSpringStarterSmokeTest for the JVM test
-@EnabledInGithubActions
+@RequiresDockerCompose
 public class GraalVmNativeMongodbSpringStarterSmokeTest
     extends AbstractMongodbSpringStarterSmokeTest {}
diff --git a/...3/src/test/java/io/opentelemetry/spring/smoketest/OtelSpringStarterDisabledSmokeTest.java b/...3/src/test/java/io/opentelemetry/spring/smoketest/OtelSpringStarterDisabledSmokeTest.java
@@ -16,8 +16,7 @@
     },
     properties = {"otel.sdk.disabled=true"})
 @DisabledInNativeImage // Without this the native tests in the OtelSpringStarterSmokeTest class will
-// fail with org.h2.jdbc.JdbcSQLSyntaxErrorException: Table "TEST_TABLE"
-// already exists
+// fail with org.h2.jdbc.JdbcSQLSyntaxErrorException: Table "CUSTOMER" already exists
 class OtelSpringStarterDisabledSmokeTest {
 
   @Test

diff --git a/...n/src/main/java/io/opentelemetry/spring/smoketest/AbstractOtelSpringStarterSmokeTest.java b/...n/src/main/java/io/opentelemetry/spring/smoketest/AbstractOtelSpringStarterSmokeTest.java
@@ -60,6 +60,7 @@ class AbstractOtelSpringStarterSmokeTest extends AbstractSpringStarterSmokeTest
   @Autowired private OtlpExporterProperties otlpExporterProperties;
   @Autowired private RestTemplateBuilder restTemplateBuilder;
   @LocalServerPort private int port;
+  @Autowired private JdbcTemplate jdbcTemplate;
 
   @Configuration(proxyBeanMethods = false)
   static class TestConfiguration {
@@ -69,7 +70,8 @@ static class TestConfiguration {
     public void loadData() {
       jdbcTemplate
           .getObject()
-          .execute("create table test_table (id bigint not null, primary key (id))");
+          .execute(
+              "create table customer (id bigint not null, name varchar not null, primary key (id))");
     }
 
     @Bean
@@ -128,7 +130,7 @@ void shouldSendTelemetry() {
                         .hasKind(SpanKind.CLIENT)
                         .hasAttribute(
                             DbIncubatingAttributes.DB_STATEMENT,
-                            "create table test_table (id bigint not null, primary key (id))")),
+                            "create table customer (id bigint not null, name varchar not null, primary key (id))")),
         traceAssert ->
             traceAssert.hasSpansSatisfyingExactly(
                 clientSpan ->
@@ -171,6 +173,30 @@ void shouldSendTelemetry() {
             CodeIncubatingAttributes.CODE_NAMESPACE, "org.springframework.boot.StartupInfoLogger");
   }
 
+  @Test
+  void databaseQuery() {
+    testing.clearAllExportedData();
+
+    testing.runWithSpan(
+        "server",
+        () -> {
+          jdbcTemplate.query(
+              "select name from customer where id = 1", (rs, rowNum) -> rs.getString("name"));
+        });
+
+    // 1 is not replaced by ?, otel.instrumentation.common.db-statement-sanitizer.enabled=false
+    testing.waitAndAssertTraces(
+        traceAssert ->
+            traceAssert.hasSpansSatisfyingExactly(
+                span -> span.hasName("server"),
+                span -> span.satisfies(s -> assertThat(s.getName()).endsWith(".getConnection")),
+                span ->
+                    span.hasKind(SpanKind.CLIENT)
+                        .hasAttribute(
+                            DbIncubatingAttributes.DB_STATEMENT,
+                            "select name from customer where id = 1")));
+  }
+
   @Test
   void restTemplate() {
     testing.clearAllExportedData();

diff --git a/smoke-tests-otel-starter/spring-boot-common/src/main/resources/application.yaml b/smoke-tests-otel-starter/spring-boot-common/src/main/resources/application.yaml
@@ -1,5 +1,8 @@
 otel:
   instrumentation:
+    common:
+      db-statement-sanitizer:
+        enabled: false
     kafka:
       experimental-span-attributes: true
     logback-appender: