Skip to content

Commit 343f3be

Browse files
dngyukmdngyukm
committed
fix: Use AOP proxy for DataSource telemetry to preserve bean type
1 parent 8604da3 commit 343f3be

File tree

4 files changed

+187
-16
lines changed
  • instrumentation/spring/spring-boot-autoconfigure/src
  • smoke-tests-otel-starter

4 files changed

+187
-16
lines changed

‎instrumentation/spring/spring-boot-autoconfigure/src/main/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/DataSourcePostProcessor.java

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@
1111
import io.opentelemetry.instrumentation.spring.autoconfigure.internal.properties.InstrumentationConfigUtil;
1212
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
1313
import javax.sql.DataSource;
14+
import org.aopalliance.intercept.MethodInterceptor;
15+
import org.springframework.aop.framework.ProxyFactory;
1416
import org.springframework.aop.scope.ScopedProxyUtils;
17+
import org.springframework.aop.support.AopUtils;
1518
import org.springframework.beans.factory.ObjectProvider;
1619
import org.springframework.beans.factory.config.BeanPostProcessor;
1720
import org.springframework.core.Ordered;
@@ -50,22 +53,32 @@ public Object postProcessAfterInitialization(Object bean, String beanName) {
5053
&& !isRoutingDatasource(bean)
5154
&& !ScopedProxyUtils.isScopedTarget(beanName)) {
5255
DataSource dataSource = (DataSource) bean;
53-
return JdbcTelemetry.builder(openTelemetryProvider.getObject())
54-
.setStatementSanitizationEnabled(
55-
InstrumentationConfigUtil.isStatementSanitizationEnabled(
56-
configPropertiesProvider.getObject(),
57-
"otel.instrumentation.jdbc.statement-sanitizer.enabled"))
58-
.setCaptureQueryParameters(
59-
configPropertiesProvider
60-
.getObject()
61-
.getBoolean(
62-
"otel.instrumentation.jdbc.experimental.capture-query-parameters", false))
63-
.setTransactionInstrumenterEnabled(
64-
configPropertiesProvider
65-
.getObject()
66-
.getBoolean("otel.instrumentation.jdbc.experimental.transaction.enabled", false))
67-
.build()
68-
.wrap(dataSource);
56+
DataSource wrapped =
57+
JdbcTelemetry.builder(openTelemetryProvider.getObject())
58+
.setStatementSanitizationEnabled(
59+
InstrumentationConfigUtil.isStatementSanitizationEnabled(
60+
configPropertiesProvider.getObject(),
61+
"otel.instrumentation.jdbc.statement-sanitizer.enabled"))
62+
.setCaptureQueryParameters(
63+
configPropertiesProvider
64+
.getObject()
65+
.getBoolean(
66+
"otel.instrumentation.jdbc.experimental.capture-query-parameters", false))
67+
.setTransactionInstrumenterEnabled(
68+
configPropertiesProvider
69+
.getObject()
70+
.getBoolean(
71+
"otel.instrumentation.jdbc.experimental.transaction.enabled", false))
72+
.build()
73+
.wrap(dataSource);
74+
ProxyFactory proxyFactory = new ProxyFactory(new Class<?>[] {DataSource.class});
75+
proxyFactory.setTarget(bean);
76+
proxyFactory.addAdvice(
77+
(MethodInterceptor)
78+
invocation ->
79+
AopUtils.invokeJoinpointUsingReflection(
80+
wrapped, invocation.getMethod(), invocation.getArguments()));
81+
return proxyFactory.getProxy();
6982
}
7083
return bean;
7184
}

‎instrumentation/spring/spring-boot-autoconfigure/src/test/java/io/opentelemetry/instrumentation/spring/autoconfigure/internal/instrumentation/jdbc/DataSourcePostProcessorTest.java

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
/*
2+
* Copyright The OpenTelemetry Authors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package io.opentelemetry.instrumentation.spring.autoconfigure.internal.instrumentation.jdbc;
7+
8+
import static org.assertj.core.api.Assertions.assertThat;
9+
import static org.mockito.Mockito.mock;
10+
import static org.mockito.Mockito.when;
11+
12+
import io.opentelemetry.api.OpenTelemetry;
13+
import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
14+
import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
15+
import java.io.PrintWriter;
16+
import java.sql.Connection;
17+
import java.sql.SQLException;
18+
import java.sql.SQLFeatureNotSupportedException;
19+
import java.util.Collections;
20+
import java.util.logging.Logger;
21+
import javax.sql.DataSource;
22+
import org.junit.jupiter.api.DisplayName;
23+
import org.junit.jupiter.api.Test;
24+
import org.springframework.aop.framework.AopProxyUtils;
25+
import org.springframework.beans.factory.config.BeanPostProcessor;
26+
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
27+
28+
class DataSourcePostProcessorTest {
29+
30+
private static final DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
31+
32+
static {
33+
beanFactory.registerSingleton("openTelemetry", OpenTelemetry.noop());
34+
beanFactory.registerSingleton(
35+
"configProperties", DefaultConfigProperties.createFromMap(Collections.emptyMap()));
36+
}
37+
38+
@Test
39+
@DisplayName("when processed bean is NOT of type DataSource should return Object unchanged")
40+
void returnsObject() {
41+
BeanPostProcessor underTest =
42+
new DataSourcePostProcessor(
43+
beanFactory.getBeanProvider(OpenTelemetry.class),
44+
beanFactory.getBeanProvider(ConfigProperties.class));
45+
46+
Object nonDataSource = new Object();
47+
assertThat(underTest.postProcessAfterInitialization(nonDataSource, "testObject"))
48+
.isSameAs(nonDataSource);
49+
}
50+
51+
@Test
52+
@DisplayName("when processed bean is of type DataSource should return DataSource proxy")
53+
void returnsDataSourceProxy() {
54+
BeanPostProcessor underTest =
55+
new DataSourcePostProcessor(
56+
beanFactory.getBeanProvider(OpenTelemetry.class),
57+
beanFactory.getBeanProvider(ConfigProperties.class));
58+
59+
DataSource originalDataSource = new TestDataSource();
60+
61+
Object result = underTest.postProcessAfterInitialization(originalDataSource, "testDataSource");
62+
63+
assertThat(result).isInstanceOf(DataSource.class);
64+
assertThat(result).isNotSameAs(originalDataSource);
65+
66+
Object target = AopProxyUtils.getSingletonTarget(result);
67+
assertThat(target).isSameAs(originalDataSource);
68+
}
69+
70+
@Test
71+
@DisplayName("when processed bean is scoped proxy target should return unchanged")
72+
void returnsScopedProxyTargetUnchanged() {
73+
BeanPostProcessor underTest =
74+
new DataSourcePostProcessor(
75+
beanFactory.getBeanProvider(OpenTelemetry.class),
76+
beanFactory.getBeanProvider(ConfigProperties.class));
77+
78+
DataSource dataSource = new TestDataSource();
79+
String scopedTargetBeanName = "scopedTarget.testDataSource";
80+
81+
Object result = underTest.postProcessAfterInitialization(dataSource, scopedTargetBeanName);
82+
83+
assertThat(result).isSameAs(dataSource);
84+
}
85+
86+
@Test
87+
@DisplayName("proxy should delegate method calls to wrapped telemetry DataSource")
88+
void proxyDelegatesMethodCalls() throws SQLException {
89+
BeanPostProcessor underTest =
90+
new DataSourcePostProcessor(
91+
beanFactory.getBeanProvider(OpenTelemetry.class),
92+
beanFactory.getBeanProvider(ConfigProperties.class));
93+
94+
DataSource originalDataSource = new TestDataSource();
95+
96+
Object result = underTest.postProcessAfterInitialization(originalDataSource, "testDataSource");
97+
98+
DataSource proxiedDataSource = (DataSource) result;
99+
Connection connection = proxiedDataSource.getConnection();
100+
101+
assertThat(connection).isNotNull();
102+
}
103+
104+
private static class TestDataSource implements DataSource {
105+
@Override
106+
public Connection getConnection() throws SQLException {
107+
Connection mockConnection = mock(Connection.class);
108+
when(mockConnection.getMetaData()).thenReturn(mock(java.sql.DatabaseMetaData.class));
109+
return mockConnection;
110+
}
111+
112+
@Override
113+
public Connection getConnection(String username, String password) throws SQLException {
114+
return getConnection();
115+
}
116+
117+
@Override
118+
public PrintWriter getLogWriter() throws SQLException {
119+
return null;
120+
}
121+
122+
@Override
123+
public void setLogWriter(PrintWriter out) throws SQLException {}
124+
125+
@Override
126+
public void setLoginTimeout(int seconds) throws SQLException {}
127+
128+
@Override
129+
public int getLoginTimeout() throws SQLException {
130+
return 0;
131+
}
132+
133+
@Override
134+
public Logger getParentLogger() throws SQLFeatureNotSupportedException {
135+
throw new SQLFeatureNotSupportedException();
136+
}
137+
138+
@Override
139+
public <T> T unwrap(Class<T> iface) throws SQLException {
140+
throw new SQLException("Not supported");
141+
}
142+
143+
@Override
144+
public boolean isWrapperFor(Class<?> iface) throws SQLException {
145+
return false;
146+
}
147+
}
148+
}

‎smoke-tests-otel-starter/spring-boot-3.2/src/main/java/io/opentelemetry/spring/smoketest/RuntimeHints.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,10 @@ public void registerHints(
2727
hint -> {
2828
hint.withMembers(MemberCategory.INVOKE_DECLARED_CONSTRUCTORS);
2929
});
30+
31+
// Register proxy hints for DataSource AOP proxy used by DataSourcePostProcessor
32+
hints
33+
.proxies()
34+
.registerJdkProxy(TypeReference.of("javax.sql.DataSource"));
3035
}
3136
}

‎smoke-tests-otel-starter/spring-boot-3/src/main/java/io/opentelemetry/spring/smoketest/RuntimeHints.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,10 @@ public void registerHints(
2727
hint -> {
2828
hint.withMembers(MemberCategory.INVOKE_DECLARED_CONSTRUCTORS);
2929
});
30+
31+
// Register proxy hints for DataSource AOP proxy used by DataSourcePostProcessor
32+
hints
33+
.proxies()
34+
.registerJdkProxy(TypeReference.of("javax.sql.DataSource"));
3035
}
3136
}

0 commit comments

Comments
 (0)