Resttemplate plugin #117

glmapper · 2018-11-12T03:01:30Z

Motivation:

Provide the RestTemplate plugin

Result:

Fixes #89

sync code from upstream

coveralls · 2018-11-12T07:39:45Z

Pull Request Test Coverage Report for Build 350

160 of 202 (79.21%) changed or added relevant lines in 11 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+0.7%) to 64.168%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
sofa-tracer-plugins/sofa-tracer-resttmplate-plugin/src/main/java/com/sofa/alipay/tracer/plugins/rest/RestTemplateRequestCarrier.java	5	6	83.33%
sofa-tracer-plugins/sofa-tracer-resttmplate-plugin/src/main/java/com/sofa/alipay/tracer/plugins/rest/SofaTracerRestTemplateBuilder.java	19	20	95.0%
tracer-samples/tracer-sample-with-resttemplate/src/main/java/com/alipay/sofa/tracer/examples/rest/RestTemplateDemoApplication.java	0	3	0.0%
sofa-tracer-plugins/sofa-tracer-resttmplate-plugin/src/main/java/com/sofa/alipay/tracer/plugins/rest/RestTemplateStatJsonReporter.java	41	46	89.13%
sofa-tracer-plugins/sofa-tracer-resttmplate-plugin/src/main/java/com/sofa/alipay/tracer/plugins/rest/interceptor/RestTemplateInterceptor.java	37	45	82.22%
tracer-samples/tracer-sample-with-resttemplate/src/main/java/com/alipay/sofa/tracer/examples/rest/controller/SampleController.java	0	24	0.0%

Files with Coverage Reduction	New Missed Lines	%
tracer-core/src/main/java/com/alipay/common/tracer/core/reporter/digest/DiskReporterImpl.java	1	84.62%

Totals
Change from base Build 332:	0.7%
Covered Lines:	2808
Relevant Lines:	4376

💛 - Coveralls

# Conflicts: # pom.xml # sofa-tracer-plugins/sofa-tracer-datasource-plugin/pom.xml # sofa-tracer-plugins/sofa-tracer-httpclient-plugin/pom.xml # sofa-tracer-plugins/sofa-tracer-springmvc-plugin/pom.xml # tracer-all/pom.xml # tracer-core/pom.xml # tracer-extensions/pom.xml # tracer-samples/pom.xml # tracer-samples/tracer-sample-with-h2/pom.xml # tracer-samples/tracer-sample-with-httpclient/pom.xml # tracer-samples/tracer-sample-with-slf4j/pom.xml # tracer-samples/tracer-sample-with-sofarpc/pom.xml # tracer-samples/tracer-sample-with-springmvc/pom.xml # tracer-samples/tracer-sample-with-zipkin/pom.xml # tracer-sofa-boot-plugin/pom.xml # tracer-sofa-boot-starter/pom.xml # tracer-test/core-test/pom.xml # tracer-test/log4j-test/pom.xml # tracer-test/log4j2-test/pom.xml # tracer-test/logback-test/pom.xml

…a-tracer into resttemplate-plugin

QilongZhang · 2018-11-19T11:22:21Z

@glmapper
SofaTracerRestTemplateBuilder 尽量不要让用户直接使用我们的 API 写代码，这样侵入性太强了。在 Spring Boot 环境下，参考下这个 https://docs.spring.io/spring-boot/docs/1.5.15.RELEASE/reference/htmlsingle/#boot-features-restclient-customization 应该是可以通过依赖注入的形式。

tracer-samples/tracer-sample-with-resttemplate/README.md

QilongZhang · 2018-11-20T02:05:18Z

tracer-samples/tracer-sample-with-resttemplate/README.md

+<dependency>
+    <groupId>com.alipay.sofa</groupId>
+    <artifactId>tracer-sofa-boot-starter</artifactId>
+    <version>2.3.0</version>


前面提示引入parent，这里的version不用配置了。

QilongZhang · 2018-11-20T02:08:10Z

...-plugin/src/main/java/com/sofa/alipay/tracer/plugins/rest/RestTemplateDigestJsonEncoder.java

+/**
+ * RestTemplateDigestJsonEncoder
+ * @author: guolei.sgl
+ * @since: 18/10/15


统一风格，@ since 后面不接日期，更换成版本号。

QilongZhang · 2018-11-20T04:11:54Z

...n/src/main/java/com/sofa/alipay/tracer/plugins/rest/interceptor/RestTemplateInterceptor.java

+        try {
+            return response = execution.execute(request, body);
+        } catch (Exception e) {
+            throw new RuntimeException(e);


这里的异常没必要包一层，直接抛出来。在 finally 中没有打印出现异常的请求，只过滤了 reponse != null 条件，而对于出现异常的请求 response = null。

@QilongZhang 这里将异常向上抛出是不想在catch块中在做一次return，并且这个返回值也不清楚返回什么，所以以异常的方式交给程序本身来处理或者在上层处理。

如果 response = null ，这种情况没有必要将信息输出到tracer日志中，因为 response = null 这种情况本身就没有什么意义，拿不到任何有用的数据。当然如果是异常导致的，catch 块中将会捕获帮助我们定位

我的意思是直接抛出来，不是包一层RuntimeException, 这个重载的方法默认是抛出IOException的

这里我的理解有点偏差，应该将 IOException 直接抛出

…a-tracer into resttemplate-plugin

QilongZhang · 2018-11-20T08:46:02Z

...ava/com/sofa/alipay/tracer/plugins/rest/interceptor/AsyncRestTemplateRequestInterceptor.java

+                sofaTracerSpan));
+            return result;
+        } catch (RuntimeException e) {
+            restTemplateTracer.clientReceiveTagFinish(sofaTracerSpan, String.valueOf(500));


这里 catch 的异常类型，把 RuntimeException 换成 Throwable

@QilongZhang 这里在外层还是抛出去吧，因为这里落的日志不会太详细。这里如果把异常吞掉的话不利于排查问题

@glmapper 不是说吞掉.

try { ListenableFuture<ClientHttpResponse> result = execution.executeAsync(request, body); result.addCallback(new SofaTraceListenableFutureCallback(restTemplateTracer, sofaTracerSpan)); return result; } finally { restTemplateTracer.clientReceiveTagFinish(sofaTracerSpan, String.valueOf(500)); }

@QilongZhang 这样应该是不行的，如果在finally打statueCode是String.valueOf(500)的的log，是否就意味着当前这个处理就是失败的 ?

QilongZhang · 2018-11-20T11:40:16Z

pom.xml

@@ -5,11 +5,11 @@
    <parent>
        <groupId>com.alipay.sofa</groupId>
        <artifactId>sofaboot-dependencies</artifactId>
-        <version>2.5.2</version>
+        <version>2.5.1</version>


这个版本号为什么降下来了？ 2.5.2 里面修复tracer日志配置问题

之前是从2.4.9 升上来的，2.3.0 会依赖 sofaboot 2.5.2以上版本

嗯，所以升级到最新版的2.5.2吧。

这个 PR 里面统一进行了版本的升级 Upgrade to 2.3.0

QilongZhang · 2018-11-20T11:45:57Z

...va/com/alipay/sofa/tracer/boot/resttemplate/configuration/RestTemplateAutoConfiguration.java

+@ConditionalOnWebApplication
+public class RestTemplateAutoConfiguration {
+    @Bean
+    public RestTemplate restTemplateCustomizer() {


这里加个 Condition，判断Template插件是否引入，否则会报错？

不应该使用
ConditionalOnMissintBean
而是使用
@ConditionalOnClass(name = "com.sofa.alipay.tracer.plugins.rest.interceptor.RestTemplateInterceptor")

QilongZhang · 2018-11-20T11:46:28Z

...rc/main/java/com/alipay/sofa/tracer/boot/resttemplate/properties/RestTemplateProperties.java

+ * @since: v2.3.0
+ **/
+@ConfigurationProperties("com.alipay.sofa.tracer.resttemplate")
+public class RestTemplateProperties {


这个类为什么是空的？

这里补充一个enable，如果com.alipay.sofa.tracer.resttemplate.enable设置成false，将不会收集tracer日志

这样写默认值是 false, 即引入依赖情况下，注入的方式默认也是失败的？

QilongZhang · 2018-11-21T00:36:34Z

...rc/main/java/com/alipay/sofa/tracer/boot/resttemplate/properties/RestTemplateProperties.java

+public class RestTemplateProperties {
+
+    private boolean enable = true;
+


注入的场景补充下测试用例吧，默认值换成 Spring Boot 标准的写法， @ConditionalOnProperty(prefix = "com.alipay.sofa.tracer.resttemplate", name = "enabled", matchIfMissing = true)

另外建议考虑这个配置是否有必要存在，这个配置值只是让注入无效，并不是不打印日志。另外默认引入插件的方式不太友好，可以从这个版本就可以改了。

@QilongZhang 加上这个配置com.alipay.sofa.tracer.resttemplate.enabled本意是因为如果用户引入了 tracer-sofa-boot-starter 依赖，当他在使用：

@Autowired RestTemplate restTemplate;

这种方式获取 RestTemplate 实例并且不想打印tracer日志，那么可以通过该配置项来进行控制。但是考虑到后面我们的插件不会被默认带出去，所以这里就没有必要在使用该配置项。因为在插件默认被带出去的前提下，即使用户将此值配置成false，依然可以通过API的方式来进行tracer日志打印。

glmapper and others added 5 commits October 10, 2018 15:42

Merge pull request #4 from alipay/master

9aa096b

sync code from upstream

add rest template plugin

f1d2bbc

Merge branch 'master-alipay'

3a897b4

reback commit

460c9e9

add readme

aee6415

glmapper self-assigned this Nov 12, 2018

glmapper added the enhancement New feature or request label Nov 12, 2018

glmapper added this to the 2.3.0 milestone Nov 12, 2018

卫恒 and others added 3 commits November 12, 2018 14:14

Trigger ci

0aec2b7

upgrade-to-2.3.0

8834099

Merge branch 'master' into resttemplate-plugin

81f9270

glmapper requested review from QilongZhang, guanchao-yang and ujjboy November 12, 2018 07:38

glmapper mentioned this pull request Nov 12, 2018

Add rest template plugin #93

Closed

glmapper removed the request for review from guanchao-yang November 14, 2018 10:22

卫恒 and others added 4 commits November 19, 2018 14:45

upgrade version to 2.3.0

3d5109c

Merge branch 'resttemplate-plugin' of https://github.com/glmapper/sof…

790b1e9

…a-tracer into resttemplate-plugin

Merge branch 'master' into resttemplate-plugin

d05fe95

QilongZhang reviewed Nov 20, 2018

View reviewed changes

tracer-samples/tracer-sample-with-resttemplate/README.md Outdated Show resolved Hide resolved

QilongZhang reviewed Nov 20, 2018

View reviewed changes

卫恒 added 4 commits November 20, 2018 14:35

补充自动注入方式构建Resttemplate&补充readme

29ae802

Merge branch 'resttemplate-plugin' of https://github.com/glmapper/sof…

e8c843c

…a-tracer into resttemplate-plugin

拦截器中的异常直接抛出

7fe160b

优化测试用例时间

43af3c0

QilongZhang reviewed Nov 20, 2018

View reviewed changes

卫恒 added 2 commits November 20, 2018 17:09

修复异常规范问题

d3686c7

fix since & rebuild

125334e

QilongZhang reviewed Nov 20, 2018

View reviewed changes

卫恒 added 5 commits November 20, 2018 20:01

fix cr

89b916d

format code

cff609c

fix cr & fix conflict

ccebd65

readme 中代码块高亮

ebc0244

rebuild ci

2f5dcf1

QilongZhang reviewed Nov 21, 2018

View reviewed changes

delete resttemplate properties

cb3e5bb

QilongZhang approved these changes Nov 21, 2018

View reviewed changes

QilongZhang merged commit 8347b0f into sofastack:master Nov 21, 2018

glmapper deleted the resttemplate-plugin branch February 20, 2021 07:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resttemplate plugin #117

Resttemplate plugin #117

glmapper commented Nov 12, 2018

coveralls commented Nov 12, 2018 •

edited

Loading

QilongZhang commented Nov 19, 2018

QilongZhang Nov 20, 2018

QilongZhang Nov 20, 2018 •

edited

Loading

QilongZhang Nov 20, 2018

glmapper Nov 20, 2018

QilongZhang Nov 20, 2018

glmapper Nov 20, 2018 •

edited

Loading

QilongZhang Nov 20, 2018

glmapper Nov 20, 2018

QilongZhang Nov 20, 2018

glmapper Nov 20, 2018

QilongZhang Nov 20, 2018

glmapper Nov 20, 2018

QilongZhang Nov 20, 2018

glmapper Nov 20, 2018

QilongZhang Nov 20, 2018

QilongZhang Nov 20, 2018

QilongZhang Nov 20, 2018

glmapper Nov 20, 2018

QilongZhang Nov 20, 2018

QilongZhang Nov 21, 2018

glmapper Nov 21, 2018 •

edited

Loading

		public class RestTemplateProperties {

		private boolean enable = true;

Resttemplate plugin #117

Resttemplate plugin #117

Conversation

glmapper commented Nov 12, 2018

Motivation:

Result:

coveralls commented Nov 12, 2018 • edited Loading

Pull Request Test Coverage Report for Build 350

💛 - Coveralls

QilongZhang commented Nov 19, 2018

Choose a reason for hiding this comment

QilongZhang Nov 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glmapper Nov 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glmapper Nov 21, 2018 • edited Loading

Choose a reason for hiding this comment

coveralls commented Nov 12, 2018 •

edited

Loading

QilongZhang Nov 20, 2018 •

edited

Loading

glmapper Nov 20, 2018 •

edited

Loading

glmapper Nov 21, 2018 •

edited

Loading