Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make statement sanitizer configurable for spring boot #11350

Merged
merged 11 commits into from
May 17, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
{
Expand Down Expand Up @@ -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
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -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 = ?")));
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,5 @@
},
webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
@EnabledInNativeImage // see JvmMongodbSpringStarterSmokeTest for the JVM test
@RequiresDockerComposeEnvVariable
@RequiresDockerCompose
public class GraalVmNativeKafkaSpringStarterSmokeTest extends AbstractKafkaSpringStarterSmokeTest {}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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 ->
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
otel:
instrumentation:
common:
db-statement-sanitizer:
enabled: false
trask marked this conversation as resolved.
Show resolved Hide resolved
kafka:
experimental-span-attributes: true
logback-appender:
Expand Down
Loading
Loading