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 2435fbc2ba90..9357fe0abbdc 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 @@ -10,8 +10,14 @@ import io.opentelemetry.instrumentation.jdbc.datasource.JdbcTelemetry; import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.InstrumentationConfigUtil; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; import javax.sql.DataSource; +import org.aopalliance.intercept.MethodInterceptor; +import org.aopalliance.intercept.MethodInvocation; +import org.springframework.aop.framework.ProxyFactory; import org.springframework.aop.scope.ScopedProxyUtils; +import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.config.BeanPostProcessor; import org.springframework.core.Ordered; @@ -50,22 +56,57 @@ public Object postProcessAfterInitialization(Object bean, String beanName) { && !isRoutingDatasource(bean) && !ScopedProxyUtils.isScopedTarget(beanName)) { DataSource dataSource = (DataSource) bean; - return JdbcTelemetry.builder(openTelemetryProvider.getObject()) - .setStatementSanitizationEnabled( - InstrumentationConfigUtil.isStatementSanitizationEnabled( - configPropertiesProvider.getObject(), - "otel.instrumentation.jdbc.statement-sanitizer.enabled")) - .setCaptureQueryParameters( - configPropertiesProvider - .getObject() - .getBoolean( - "otel.instrumentation.jdbc.experimental.capture-query-parameters", false)) - .setTransactionInstrumenterEnabled( - configPropertiesProvider - .getObject() - .getBoolean("otel.instrumentation.jdbc.experimental.transaction.enabled", false)) - .build() - .wrap(dataSource); + + // Wrap the original DataSource with OpenTelemetry instrumentation + DataSource wrapped = + JdbcTelemetry.builder(openTelemetryProvider.getObject()) + .setStatementSanitizationEnabled( + InstrumentationConfigUtil.isStatementSanitizationEnabled( + configPropertiesProvider.getObject(), + "otel.instrumentation.jdbc.statement-sanitizer.enabled")) + .setCaptureQueryParameters( + configPropertiesProvider + .getObject() + .getBoolean( + "otel.instrumentation.jdbc.experimental.capture-query-parameters", false)) + .setTransactionInstrumenterEnabled( + configPropertiesProvider + .getObject() + .getBoolean( + "otel.instrumentation.jdbc.experimental.transaction.enabled", false)) + .build() + .wrap(dataSource); + + /** + * Spring Boot's configuration binding and rebinding mechanisms (such as those triggered by + * Nacos configuration refresh) may attempt to reconstruct beans using their concrete class + * constructors. If a custom DataSource implementation (such as OpenTelemetryDataSource) is + * returned directly, Spring may not find a suitable constructor during rebinding, resulting + * in errors like "ExistingValue must be an instance of com.zaxxer.hikari.HikariDataSource". + * + *

To prevent this, we create a JDK dynamic proxy implementing only the DataSource + * interface. The proxy delegates all method calls to the wrapped (instrumented) DataSource. + * This approach "hides" the actual implementation class and ensures that Spring interacts + * only with the DataSource interface, avoiding issues related to constructor resolution or + * type casting during bean rebinding. + */ + ProxyFactory proxyFactory = new ProxyFactory(DataSource.class); + // Set the original bean as the target (important for AOP and bean lifecycle) + proxyFactory.setTarget(bean); + // Delegate all method calls to the wrapped, instrumented DataSource + proxyFactory.addAdvice( + new MethodInterceptor() { + @Nullable + @Override + public Object invoke(@Nonnull MethodInvocation invocation) throws Throwable { + return AopUtils.invokeJoinpointUsingReflection( + wrapped, invocation.getMethod(), invocation.getArguments()); + } + }); + + // Return the proxy instead of the instrumented DataSource instance + // This ensures proper interaction with Spring's bean lifecycle and rebinding + return proxyFactory.getProxy(); } return bean; } diff --git a/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/DataSourcePostProcessorContextTest.java b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/DataSourcePostProcessorContextTest.java new file mode 100644 index 000000000000..05d555178256 --- /dev/null +++ b/instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/DataSourcePostProcessorContextTest.java @@ -0,0 +1,68 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.instrumentation.spring.autoconfigure.internal.instrumentation.jdbc; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.*; + +import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; +import java.sql.Connection; +import javax.sql.DataSource; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.ObjectProvider; + +class DataSourcePostProcessorTest { + + @Test + void shouldWrapDataSourceWithProxy() throws Exception { + // Set up mocks + DataSource original = mock(DataSource.class); + Connection mockConn = mock(Connection.class); + when(original.getConnection()).thenReturn(mockConn); + + // Create mocks for OpenTelemetry and ConfigProperties + OpenTelemetry openTelemetry = mock(OpenTelemetry.class); + ConfigProperties configProperties = mock(ConfigProperties.class); + when(configProperties.getBoolean(anyString(), anyBoolean())).thenReturn(false); + + // Mock JdbcTelemetry builder chain to return a mock DataSource as 'wrapped' + JdbcTelemetry.Builder builder = mock(JdbcTelemetry.Builder.class, RETURNS_SELF); + JdbcTelemetry telemetry = mock(JdbcTelemetry.class); + DataSource wrapped = mock(DataSource.class); + when(wrapped.getConnection()).thenReturn(mockConn); + when(telemetry.wrap(any(DataSource.class))).thenReturn(wrapped); + when(builder.build()).thenReturn(telemetry); + + // Mock the static builder method (requires mockito-inline or PowerMockito) + try (var mocked = Mockito.mockStatic(JdbcTelemetry.class)) { + mocked.when(() -> JdbcTelemetry.builder(any())).thenReturn(builder); + + ObjectProvider openTelemetryProvider = () -> openTelemetry; + ObjectProvider configPropertiesProvider = () -> configProperties; + + DataSourcePostProcessor postProcessor = + new DataSourcePostProcessor(openTelemetryProvider, configPropertiesProvider); + + // Act + Object processed = postProcessor.postProcessAfterInitialization(original, "myDataSource"); + + // Assert + assertThat(processed).isInstanceOf(DataSource.class); + assertThat(processed).isNotSameAs(original); + + // The proxy should delegate calls to the wrapped DataSource + DataSource proxied = (DataSource) processed; + Connection proxiedConnection = proxied.getConnection(); + assertThat(proxiedConnection).isEqualTo(mockConn); + + // The original DataSource should NOT be called directly by the proxy + verify(original, never()).getConnection(); + // The wrapped DataSource should be called + verify(wrapped, times(1)).getConnection(); + } + } +}