Closed
Description
I find the getConnection()
method is slower than expected.
So I check the source code:
public Object getConnection(final ProceedingJoinPoint pjp) throws Throwable {
Connection conn = (Connection) pjp.proceed();
String url = conn.getMetaData().getURL();
String user = conn.getMetaData().getUserName();
String dbType;
boolean withActiveSpanOnly = false;
try {
dbType = url.split(":")[1];
} catch (Throwable t) {
throw new IllegalArgumentException(
"Invalid JDBC URL. Expected to find the database type after the first ':'. URL: " + url);
}
return new TracingConnection(conn, dbType, user, withActiveSpanOnly);
}
For this line: String user = conn.getMetaData().getUserName();
mysql-connector-java
will execute SELECT USER()
every time.
Activity
dozer47528 commentedon Dec 11, 2018
Actually, I think it's hard for
opentracing-spring-cloud-jdbc-starter
to fix this issue.opentracing-spring-cloud-jdbc-starter
should follow the standard of JDBC, this behavior implemented bymysql-connector-java
.I resolved this issue by add this argument to mysql jdbc url:
After changing
useHostsInPrivileges
tofalse
,mysql-connector-java
can get the username from memory directly.wuyupengwoaini commentedon Jan 9, 2019
I think the userName is not necessary at all,because normally application accesses the database by a fixed user. And then we can replace the userName by database ip and port because this is helpful to draw topology.
so the opentracing-jdbc project should support more info about database such as database ip and port
In design level, I think opentracing-jdbc is not flexiable beacuse of it's construction method.So i suggest we need to improve the opentracing-jdbc firstly and then add database ip and port to acted as tag k-v.
wuyupengwoaini commentedon Jan 9, 2019
@jpkrohling
wuyupengwoaini commentedon Jan 14, 2019
Now,TracingConnnection receieve a ConnectionInfo Object containing all the database information(instance,ip,port,type,username).So username is not required.And then we can reslove the database information by parse the jdbc url.For example the skywalking is doing so.
https://github.com/apache/incubator-skywalking/blob/master/apm-sniffer/apm-sdk-plugin/jdbc-commons/src/main/java/org/apache/skywalking/apm/plugin/jdbc/connectionurl/parser/ConnectionURLParser.java
wuyupengwoaini commentedon Jan 14, 2019
@dozer47528 why close this issue,i think we need a pr to solve this problem
wuyupengwoaini commentedon Jan 14, 2019
@pavolloffay what do you think about my suggestion?
pavolloffay commentedon Jan 16, 2019
@wuyupengwoaini would you like to contribute a fix with your solution?
wuyupengwoaini commentedon Jan 17, 2019
Ok, i'll try to fix it within a few days
wuyupengwoaini commentedon Feb 13, 2019
@pavolloffay please help me to review the pr