wu-sheng commented on a change in pull request #5109:
URL: https://github.com/apache/skywalking/pull/5109#discussion_r456133788



##########
File path: 
apm-sniffer/apm-sdk-plugin/mysql-common/src/main/java/org/apache/skywalking/apm/plugin/jdbc/mysql/PreparedStatementExecuteMethodsInterceptor.java
##########
@@ -40,15 +40,14 @@
     public final void beforeMethod(EnhancedInstance objInst, Method method, 
Object[] allArguments,
                                    Class<?>[] argumentsTypes, 
MethodInterceptResult result) {
         StatementEnhanceInfos cacheObject = (StatementEnhanceInfos) 
objInst.getSkyWalkingDynamicField();
-        ConnectionInfo connectInfo = cacheObject.getConnectionInfo();
         /**
          * For avoid NPE. In this particular case, Execute sql inside the 
{@link com.mysql.jdbc.ConnectionImpl} constructor,
          * before the interceptor sets the connectionInfo.
-         *
+         * Fixed https://github.com/apache/skywalking/issues/4965. When 
invoking prepareCall, cacheObject is null. Because it will determine 
procedures's parameter types by querying the table of proc in mysql.

Review comment:
       Don't add the issue link, unless it is a very special thing. 

##########
File path: 
apm-sniffer/apm-sdk-plugin/mysql-common/src/main/java/org/apache/skywalking/apm/plugin/jdbc/mysql/PreparedStatementExecuteMethodsInterceptor.java
##########
@@ -40,15 +40,14 @@
     public final void beforeMethod(EnhancedInstance objInst, Method method, 
Object[] allArguments,
                                    Class<?>[] argumentsTypes, 
MethodInterceptResult result) {
         StatementEnhanceInfos cacheObject = (StatementEnhanceInfos) 
objInst.getSkyWalkingDynamicField();
-        ConnectionInfo connectInfo = cacheObject.getConnectionInfo();
         /**
          * For avoid NPE. In this particular case, Execute sql inside the 
{@link com.mysql.jdbc.ConnectionImpl} constructor,
          * before the interceptor sets the connectionInfo.
-         *
+         * Fixed https://github.com/apache/skywalking/issues/4965. When 
invoking prepareCall, cacheObject is null. Because it will determine 
procedures's parameter types by querying the table of proc in mysql.

Review comment:
       You should forward the ConnectionInfo from Connection when 
`#prepareCall` called.

##########
File path: 
apm-sniffer/apm-sdk-plugin/mysql-common/src/main/java/org/apache/skywalking/apm/plugin/jdbc/mysql/PreparedStatementExecuteMethodsInterceptor.java
##########
@@ -40,15 +40,14 @@
     public final void beforeMethod(EnhancedInstance objInst, Method method, 
Object[] allArguments,
                                    Class<?>[] argumentsTypes, 
MethodInterceptResult result) {
         StatementEnhanceInfos cacheObject = (StatementEnhanceInfos) 
objInst.getSkyWalkingDynamicField();
-        ConnectionInfo connectInfo = cacheObject.getConnectionInfo();
         /**
          * For avoid NPE. In this particular case, Execute sql inside the 
{@link com.mysql.jdbc.ConnectionImpl} constructor,
          * before the interceptor sets the connectionInfo.
-         *
+         * Fixed https://github.com/apache/skywalking/issues/4965. When 
invoking prepareCall, cacheObject is null. Because it will determine 
procedures's parameter types by querying the table of proc in mysql.

Review comment:
       About `#prepareCall`, why `cacheObject` should be null? I prefer you 
should fix this NPE by having the correct ConnectionInfo rather than skipping 
it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to