Skip to content

Commit 19615e6

Browse files
carteruucarteruu
authored
Fix NPE in PostgreSQL plugin when executing statements prepared with generated keys (#840)
1 parent 42586bf commit 19615e6

3 files changed

Lines changed: 126 additions & 2 deletions

File tree

‎CHANGES.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ Release Notes.
55
9.9.0
66
------------------
77

8+
* Fix the `NullPointerException` thrown by the `postgresql-8.x` plugin when executing statements prepared with
9+
`Statement.RETURN_GENERATED_KEYS` or column names (apache/skywalking#14138).
810

911
All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/267?closed=1)
1012

‎apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptor.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@ public class PreparedStatementExecuteMethodsInterceptor implements InstanceMetho
4343
public final void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments,
4444
Class<?>[] argumentsTypes, MethodInterceptResult result) {
4545
StatementEnhanceInfos cacheObject = (StatementEnhanceInfos) objInst.getSkyWalkingDynamicField();
46+
/*
47+
* The statement created by PgConnection#prepareStatement(String, String[]) has no dynamic field. It is
48+
* wrapped and traced by SWPreparedStatement, see JDBCPrepareStatementWithStringArrayInterceptor.
49+
*/
50+
if (cacheObject == null || cacheObject.getConnectionInfo() == null) {
51+
return;
52+
}
4653
ConnectionInfo connectInfo = cacheObject.getConnectionInfo();
4754
AbstractSpan span = ContextManager.createExitSpan(
4855
buildOperationName(connectInfo, method.getName(), cacheObject.getStatementName()), connectInfo
@@ -68,7 +75,7 @@ public final void beforeMethod(EnhancedInstance objInst, Method method, Object[]
6875
public final Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments,
6976
Class<?>[] argumentsTypes, Object ret) {
7077
StatementEnhanceInfos cacheObject = (StatementEnhanceInfos) objInst.getSkyWalkingDynamicField();
71-
if (cacheObject.getConnectionInfo() != null) {
78+
if (cacheObject != null && cacheObject.getConnectionInfo() != null) {
7279
ContextManager.stopSpan();
7380
}
7481
return ret;
@@ -78,7 +85,7 @@ public final Object afterMethod(EnhancedInstance objInst, Method method, Object[
7885
public final void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
7986
Class<?>[] argumentsTypes, Throwable t) {
8087
StatementEnhanceInfos cacheObject = (StatementEnhanceInfos) objInst.getSkyWalkingDynamicField();
81-
if (cacheObject.getConnectionInfo() != null) {
88+
if (cacheObject != null && cacheObject.getConnectionInfo() != null) {
8289
ContextManager.activeSpan().log(t);
8390
}
8491
}

‎apm-sniffer/apm-sdk-plugin/postgresql-8.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/jdbc/postgresql/PreparedStatementExecuteMethodsInterceptorTest.java‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package org.apache.skywalking.apm.plugin.jdbc.postgresql;
20+
21+
import static org.hamcrest.CoreMatchers.is;
22+
import static org.junit.Assert.assertThat;
23+
import static org.mockito.Mockito.when;
24+
import java.lang.reflect.Method;
25+
import java.sql.SQLException;
26+
import org.apache.skywalking.apm.agent.core.context.trace.AbstractTracingSpan;
27+
import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer;
28+
import org.apache.skywalking.apm.agent.core.context.trace.TraceSegment;
29+
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
30+
import org.apache.skywalking.apm.agent.test.helper.SegmentHelper;
31+
import org.apache.skywalking.apm.agent.test.tools.AgentServiceRule;
32+
import org.apache.skywalking.apm.agent.test.tools.SegmentStorage;
33+
import org.apache.skywalking.apm.agent.test.tools.SegmentStoragePoint;
34+
import org.apache.skywalking.apm.agent.test.tools.SpanAssert;
35+
import org.apache.skywalking.apm.agent.test.tools.TracingSegmentRunner;
36+
import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
37+
import org.apache.skywalking.apm.plugin.jdbc.JDBCPluginConfig;
38+
import org.apache.skywalking.apm.plugin.jdbc.define.StatementEnhanceInfos;
39+
import org.apache.skywalking.apm.plugin.jdbc.trace.ConnectionInfo;
40+
import org.junit.Before;
41+
import org.junit.Rule;
42+
import org.junit.Test;
43+
import org.junit.runner.RunWith;
44+
import org.mockito.Mock;
45+
import org.mockito.junit.MockitoJUnit;
46+
import org.mockito.junit.MockitoRule;
47+
48+
@RunWith(TracingSegmentRunner.class)
49+
public class PreparedStatementExecuteMethodsInterceptorTest {
50+
51+
private static final String SQL = "INSERT INTO test (name) VALUES (?)";
52+
53+
@SegmentStoragePoint
54+
private SegmentStorage segmentStorage;
55+
56+
@Rule
57+
public AgentServiceRule serviceRule = new AgentServiceRule();
58+
@Rule
59+
public MockitoRule rule = MockitoJUnit.rule();
60+
61+
private PreparedStatementExecuteMethodsInterceptor interceptor;
62+
63+
@Mock
64+
private ConnectionInfo connectionInfo;
65+
@Mock
66+
private EnhancedInstance objectInstance;
67+
@Mock
68+
private Method method;
69+
70+
@Before
71+
public void setUp() {
72+
JDBCPluginConfig.Plugin.JDBC.SQL_BODY_MAX_LENGTH = 2048;
73+
interceptor = new PreparedStatementExecuteMethodsInterceptor();
74+
}
75+
76+
@Test
77+
public void testCreateDatabaseSpan() throws Throwable {
78+
when(objectInstance.getSkyWalkingDynamicField()).thenReturn(
79+
new StatementEnhanceInfos(connectionInfo, SQL, "PreparedStatement"));
80+
when(method.getName()).thenReturn("executeUpdate");
81+
when(connectionInfo.getComponent()).thenReturn(ComponentsDefine.POSTGRESQL_DRIVER);
82+
when(connectionInfo.getDBType()).thenReturn("PostgreSQL");
83+
when(connectionInfo.getDatabaseName()).thenReturn("test");
84+
when(connectionInfo.getDatabasePeer()).thenReturn("localhost:5432");
85+
86+
interceptor.beforeMethod(objectInstance, method, new Object[0], null, null);
87+
interceptor.afterMethod(objectInstance, method, new Object[0], null, null);
88+
89+
assertThat(segmentStorage.getTraceSegments().size(), is(1));
90+
TraceSegment segment = segmentStorage.getTraceSegments().get(0);
91+
assertThat(SegmentHelper.getSpans(segment).size(), is(1));
92+
AbstractTracingSpan span = SegmentHelper.getSpans(segment).get(0);
93+
SpanAssert.assertLayer(span, SpanLayer.DB);
94+
assertThat(span.getOperationName(), is("PostgreSQL/JDBC/PreparedStatement/executeUpdate"));
95+
SpanAssert.assertTag(span, 0, "PostgreSQL");
96+
SpanAssert.assertTag(span, 1, "test");
97+
SpanAssert.assertTag(span, 2, SQL);
98+
}
99+
100+
/**
101+
* A statement created by {@code PgConnection#prepareStatement(String, String[])}, which is also reached through
102+
* {@code prepareStatement(String, int)} with {@code RETURN_GENERATED_KEYS}, is wrapped by {@code SWPreparedStatement}
103+
* without setting the dynamic field, so the interceptor must skip it instead of throwing NPE.
104+
*/
105+
@Test
106+
public void testSkipStatementWithoutEnhanceInfos() throws Throwable {
107+
when(objectInstance.getSkyWalkingDynamicField()).thenReturn(null);
108+
109+
interceptor.beforeMethod(objectInstance, method, new Object[0], null, null);
110+
interceptor.handleMethodException(objectInstance, method, new Object[0], null, new SQLException());
111+
interceptor.afterMethod(objectInstance, method, new Object[0], null, null);
112+
113+
assertThat(segmentStorage.getTraceSegments().size(), is(0));
114+
}
115+
}

0 commit comments

Comments
 (0)