Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Datasource support #58

Merged
merged 24 commits into from
Oct 8, 2018
Merged

Conversation

QilongZhang
Copy link
Contributor

#53

@QilongZhang QilongZhang added the enhancement New feature or request label Aug 27, 2018
@QilongZhang QilongZhang added this to the 2.2.0 milestone Aug 27, 2018
@coveralls
Copy link

coveralls commented Aug 27, 2018

Pull Request Test Coverage Report for Build 186

  • 379 of 538 (70.45%) changed or added relevant lines in 26 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.8%) to 64.428%

Changes Missing Coverage Covered Lines Changed/Added Lines %
sofa-tracer-plugins/sofa-tracer-datasource-plugin/src/main/java/com/alipay/sofa/tracer/plugins/datasource/DBType.java 31 32 96.88%
sofa-tracer-plugins/sofa-tracer-datasource-plugin/src/main/java/com/alipay/sofa/tracer/plugins/datasource/tracer/DataSourceTracerKeys.java 0 1 0.0%
sofa-tracer-plugins/sofa-tracer-datasource-plugin/src/main/java/com/alipay/sofa/tracer/plugins/datasource/tracer/DataSourceTracerState.java 14 15 93.33%
sofa-tracer-plugins/sofa-tracer-datasource-plugin/src/main/java/com/alipay/sofa/tracer/plugins/datasource/Prop.java 7 9 77.78%
sofa-tracer-plugins/sofa-tracer-datasource-plugin/src/main/java/com/alipay/sofa/tracer/plugins/datasource/MethodRegistry.java 81 84 96.43%
sofa-tracer-plugins/sofa-tracer-datasource-plugin/src/main/java/com/alipay/sofa/tracer/plugins/datasource/tracer/Endpoint.java 6 9 66.67%
sofa-tracer-plugins/sofa-tracer-datasource-plugin/src/main/java/com/alipay/sofa/tracer/plugins/datasource/tracer/StatementTracerInterceptor.java 8 11 72.73%
sofa-tracer-plugins/sofa-tracer-datasource-plugin/src/main/java/com/alipay/sofa/tracer/plugins/datasource/utils/SqlUtils.java 5 8 62.5%
tracer-samples/tracer-sample-with-h2/src/main/java/com/alipay/sofa/tracer/examples/datasource/DemoApplication.java 0 3 0.0%
sofa-tracer-plugins/sofa-tracer-datasource-plugin/src/main/java/com/alipay/sofa/tracer/plugins/datasource/Invocation.java 7 11 63.64%
Files with Coverage Reduction New Missed Lines %
tracer-sofa-boot-starter/src/main/java/com/alipay/sofa/tracer/boot/springmvc/configuration/OpenTracingSpringMvcAutoConfiguration.java 1 92.86%
tracer-core/src/main/java/com/alipay/common/tracer/core/appender/self/TracerDaemon.java 2 66.67%
Totals Coverage Status
Change from base Build 171: 0.8%
Covered Lines: 2619
Relevant Lines: 4065

💛 - Coveralls

System.out.println("finally interceptor");
}
return reVal;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sample 我们补充一个 https://www.sqlite.org/index.html sqllite 或者 http://www.h2database.com/html/main.html H2DB 的实例吧,这样场景更完整些

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已补充

int result = st
.executeUpdate("insert into mars (id, gmt_create, gmt_modified, username) \n"
+ "values (100, now(), now(), 'neo')");
Assert.assertTrue("数据检查", result == 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这么 comment 就不出现中文了。

if (closed) {
throw new IllegalStateException("BasePreparedStatement has been closed");
}
if (initialized) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QilongZhang

if (initialized) {
            return;
        }
        realPreparedStatement = doPrepareStatement(sql);
        for (PreparedParameter preparedParameter : preparedParameters) {
            preparedParameter.invoke(realPreparedStatement);
        }
        initialized = true;
  • 这里在并发时,会有重复初始化的风险

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

理论上不会出现并发

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QilongZhang 这里多个线程同时调用这个实例方法就会有并发初始化的问题。 @Xiaoshu-Yan 关注哈。

<groupId>com.alipay.sofa</groupId>
<artifactId>tracer-sofa-boot-starter</artifactId>
</dependency>
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

H2 客户端的依赖;还有启动方式也在这里加一下吧

该样例工程使用 H2 内存数据库,并暴露 /create 用于新建一张数据表,用户访问 /create 即可执行创建数据库表 SQL 语句:
```sql
DROP TABLE IF EXISTS TEST;
CREATE TABLE TEST(ID INT PRIMARY KEY, NAME VARCHAR(255));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

光这个 SQL 语句,用户拿到还是很蒙圈的。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个仅仅是为了演示该条sql语句的tracer记录

@guanchao-yang
Copy link
Member

@QilongZhang 和同步之前的代码冲突了。

if (closed) {
throw new IllegalStateException("BasePreparedStatement has been closed");
}
if (initialized) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QilongZhang 这里多个线程同时调用这个实例方法就会有并发初始化的问题。 @Xiaoshu-Yan 关注哈。


import java.util.List;

import static com.alipay.common.tracer.core.configuration.SofaTracerConfiguration.TRACER_APPNAME_KEY;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already import SofaTracerConfiguration , No need import static field of this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ujjboy it need. I try to delete it , but compile error.

applicationName);
}
String applicationName = environment.getProperty(TRACER_APPNAME_KEY);
Assert.isTrue(!StringUtils.isBlank(applicationName), TRACER_APPNAME_KEY
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TRACER_APPNAME_KEY change to SofaTracerConfiguration.TRACER_APPNAME_KEY

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok


@Test
public void testDataSource() {
Assert.isTrue(dataSource instanceof HikariDataSource);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to org.junit.Assert#assertTrue()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed


@Test
public void testDataSource() {
Assert.isTrue(dataSource instanceof SmartDataSource);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to org.junit.Assert#assertTrue()


import com.alipay.sofa.tracer.boot.datasource.processor.DataSourceBeanFactoryPostProcessor;
import org.junit.Test;
import org.springframework.util.Assert;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to org.junit.Assert#assertTrue()

createDataSourceProxy(beanFactory, beanName, dataSource, getTomcatJdbcUrlKey());
} else if (DataSourceUtils.isHikariDataSource(dataSource.getBeanClassName())) {
createDataSourceProxy(beanFactory, beanName, dataSource, getHikariJdbcUrlKey());
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems getXXKey() can move to DataSourceUtils, too. The code will more simple, like

createDataSourceProxy(beanFactory, beanName, dataSource, getJdbcUrlKeyByDataSource());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it has been moved to DataSourceUtils.

sofaTracerSpan.setTag(DataSourceTracerKeys.CONNECTION_ESTABLISH_COST,
(Long) getStateValue(DataSourceTracerKeys.CONNECTION_ESTABLISH_COST));
} else {
sofaTracerSpan.setStartTime((Long) getStateValue(DataSourceTracerKeys.START_TIME));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里应该是当前时间

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YanXs 已改成当前时间

* @author qilong.zql
* @since 2.2.0
*/
public abstract class BasePreparedStatement implements PreparedStatement {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个类可以去掉,只是trace的话不会修改sql

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Xiaoshu-Yan 保留? 主站会使用sql服务端埋点,可以直接依赖开源版扩展。

* @author qilong.zql
* @since 2.2.0
*/
public interface SecuritySpec {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个也删掉吧,没啥用

return originalSql;
}

class PreparedAbstractStatementInterceptorChainImpl extends AbstractStatementInterceptorChain {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个类名去掉Abstract改成 PreparedStatementInterceptorChainImpl

}

class AbstractStatementInterceptorChainImpl extends AbstractStatementInterceptorChain {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个类名也改一下吧 StatementInterceptorChainImpl

}

public static String resolveDatabaseFromUrl(String url) {
Assert.isTrue(!StringUtils.isBlank(url), "Jdbc url must not be empty!");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

考虑一下其他类型数据库,比如
oracle(jdbc:oracle:thin:@localhost:1521:sid)
sqlserver(jdbc:sqlserver://localhost:1433;databaseName=AdventureWorks)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Xiaoshu-Yan 已修改,1、支持 SQLServer 2、支持 oracle SID 格式

@Xiaoshu-Yan
Copy link

@QilongZhang 修改一下前面review

@QilongZhang QilongZhang merged commit 418e9af into sofastack:master Oct 8, 2018
@QilongZhang QilongZhang deleted the datasource_support branch November 16, 2018 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants