You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/04/12 12:40:02 UTC
[GitHub] [skywalking-java] xu1009 opened a new pull request, #154: fix mysql5.x npe
xu1009 opened a new pull request, #154:
URL: https://github.com/apache/skywalking-java/pull/154
### Fix <https://github.com/apache/skywalking/issues/8860>
- [x] Add a unit test to verify that the fix works.
- [x] Explain briefly why the bug exists and how to fix it.
- [x] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<8860>.
- [x] Update the [`CHANGES` log](https://github.com/apache/skywalking-java/blob/main/CHANGES.md).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #154: Fix mysql5.x NPE
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #154:
URL: https://github.com/apache/skywalking-java/pull/154#discussion_r848394729
##########
CHANGES.md:
##########
@@ -37,6 +37,7 @@ Release Notes.
* Fix the bug that maybe causing memory leak and repeated traceId when use gateway-2.1.x-plugin or gateway-3.x-plugin.
* Fix Grpc 1.x plugin could leak context due to gRPC cancelled.
* Add JDK ThreadPoolExecutor Plugin.
+* Fix mysql-5.x plugins NPE.
Review Comment:
```suggestion
* Support default database(not set through JDBC URL) in mysql-5.x plugin.
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] xu1009 commented on a diff in pull request #154: Fix mysql5.x NPE
Posted by GitBox <gi...@apache.org>.
xu1009 commented on code in PR #154:
URL: https://github.com/apache/skywalking-java/pull/154#discussion_r848393257
##########
apm-sniffer/apm-sdk-plugin/mysql-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/mysql/v5/ConnectionCreate5xInterceptor.java:
##########
@@ -42,7 +43,8 @@ public void beforeMethod(Class clazz, Method method, Object[] allArguments, Clas
public Object afterMethod(Class clazz, Method method, Object[] allArguments, Class<?>[] parameterTypes,
Object ret) {
if (ret instanceof EnhancedInstance) {
- ConnectionInfo connectionInfo = ConnectionCache.get(allArguments[0].toString(), allArguments[1].toString(), allArguments[3].toString());
+ String database = Objects.isNull(allArguments[3]) ? "" : allArguments[3].toString();
Review Comment:
jdbc url like
jdbc:mysql://ip:port?zeroDateTimeBehavior=convertToNull&useUnicode=true&characterEncoding=UTF8&connectTimeout=10000&socketTimeout=20000&useSSL=false&useTimezone=true&serverTimezone=GMT%2B8
the database not assign in jdbcurl, this can get null databse
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #154: Fix mysql5.x NPE
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #154:
URL: https://github.com/apache/skywalking-java/pull/154#discussion_r848390714
##########
apm-sniffer/apm-sdk-plugin/mysql-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/mysql/v5/ConnectionCreate5xInterceptor.java:
##########
@@ -42,7 +43,8 @@ public void beforeMethod(Class clazz, Method method, Object[] allArguments, Clas
public Object afterMethod(Class clazz, Method method, Object[] allArguments, Class<?>[] parameterTypes,
Object ret) {
if (ret instanceof EnhancedInstance) {
- ConnectionInfo connectionInfo = ConnectionCache.get(allArguments[0].toString(), allArguments[1].toString(), allArguments[3].toString());
+ String database = Objects.isNull(allArguments[3]) ? "" : allArguments[3].toString();
Review Comment:
URL could be NULL? Because? Users forgot to set?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng commented on a diff in pull request #154: Support default database(not set through JDBC URL) in mysql-5.x plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #154:
URL: https://github.com/apache/skywalking-java/pull/154#discussion_r848395885
##########
apm-sniffer/apm-sdk-plugin/mysql-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/mysql/v5/ConnectionCreate5xInterceptor.java:
##########
@@ -42,7 +43,8 @@ public void beforeMethod(Class clazz, Method method, Object[] allArguments, Clas
public Object afterMethod(Class clazz, Method method, Object[] allArguments, Class<?>[] parameterTypes,
Object ret) {
if (ret instanceof EnhancedInstance) {
- ConnectionInfo connectionInfo = ConnectionCache.get(allArguments[0].toString(), allArguments[1].toString(), allArguments[3].toString());
+ String database = Objects.isNull(allArguments[3]) ? "" : allArguments[3].toString();
Review Comment:
Good to me.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] xu1009 commented on a diff in pull request #154: Fix mysql5.x NPE
Posted by GitBox <gi...@apache.org>.
xu1009 commented on code in PR #154:
URL: https://github.com/apache/skywalking-java/pull/154#discussion_r848394217
##########
apm-sniffer/apm-sdk-plugin/mysql-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/mysql/v5/ConnectionCreate5xInterceptor.java:
##########
@@ -42,7 +43,8 @@ public void beforeMethod(Class clazz, Method method, Object[] allArguments, Clas
public Object afterMethod(Class clazz, Method method, Object[] allArguments, Class<?>[] parameterTypes,
Object ret) {
if (ret instanceof EnhancedInstance) {
- ConnectionInfo connectionInfo = ConnectionCache.get(allArguments[0].toString(), allArguments[1].toString(), allArguments[3].toString());
+ String database = Objects.isNull(allArguments[3]) ? "" : allArguments[3].toString();
Review Comment:
we specified the database in sql not in jdbcurl, like selct * from database.table
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-java] wu-sheng merged pull request #154: Support default database(not set through JDBC URL) in mysql-5.x plugin
Posted by GitBox <gi...@apache.org>.
wu-sheng merged PR #154:
URL: https://github.com/apache/skywalking-java/pull/154
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org