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.

Sign up for GitHub

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

Jump to bottom

support reactor #228

Closed

shengxiang205 wants to merge 21 commits into sofastack:3.x from shengxiang205:3.x

shengxiang205 commented May 29, 2019

Motivation:

add sofa tracer reactor plugin
add tracer sample with webflux

Modification:

add reactor transaction to keep global sofa tracer span
and keep and restore global sofa tracer span

Result:

Fixes #227

shengxiang205 added 2 commits

May 29, 2019 18:31


          1. add sofa tracer reactor plugin

c8131eb

2. add sofa examples webflux


          1. reformat sample code

67ef7e3

shengxiang205 changed the title ~~support webflux~~ support reactor

glmapper requested review from glmapper and ujjboy

May 29, 2019 11:54

glmapper added the enhancement label

glmapper assigned shengxiang205

glmapper requested a review from guanchao-yang

May 30, 2019 02:01

shengxiang205 added 6 commits

May 31, 2019 11:35


          1. move reactor to com.alipay.common.tracer.core.reactor

a0b516d

2. add flux test case and fix bug with flux operation


          1. make compile work

6261df2


          1 optimize import

610bd70


          1. fix first span bug in webflux

8d7132a

2. support spring webflux via spring aop


          seperate webflux plugin from spring mvc


          1. move webflux and spring mvc together, ^_^

cca398d

glmapper requested changes

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

pom.xml Outdated Show resolved Hide resolved

sofa-tracer-plugins/sofa-tracer-webflux-plugin/pom.xml Outdated

+                      <dependency>
+                          <groupId>com.alipay.sofa</groupId>
+                          <artifactId>sofa-tracer-springmvc-plugin</artifactId>

Contributor

glmapper Jun 17, 2019

这里感觉不用依赖 springmvc 的插件，部分重复代码冗余一份，既然分开两个插件，就应该相互独立开

Author

shengxiang205 Jun 20, 2019

复用度很高, 抽取一个sofa-tracer-web-base-plugin ?

...lux-plugin/src/main/java/com/alipay/sofa/tracer/plugins/webflux/WebfluxSofaTracerFilter.java Show resolved Hide resolved

tracer-core/pom.xml Outdated Show resolved Hide resolved

...r-core/src/main/java/com/alipay/common/tracer/core/reactor/SofaTracerReactorTransformer.java Outdated Show resolved Hide resolved

tracer-core/src/main/java/com/alipay/common/tracer/core/reactor/SofaTracerSpanContainer.java Outdated Show resolved Hide resolved

tracer-core/src/test/java/com/alipay/common/tracer/core/reactor/base/ReactorTracer.java Show resolved Hide resolved

tracer-core/src/test/java/com/alipay/common/tracer/core/reactor/base/ReactorTracer.java Show resolved Hide resolved

tracer-samples/tracer-sample-with-springwebflux/pom.xml Outdated Show resolved Hide resolved

shengxiang205 and others added 4 commits

June 20, 2019 21:48


          resolve review problems

d19fb90


          change to hooks

35bd8a1


          using hooks to deal with reactor

4cc7b05


          rename comments

e16c207

glmapper requested changes

View reviewed changes

...m/alipay/sofa/tracer/boot/springmvc/configuration/OpenTracingSpringMvcAutoConfiguration.java Outdated

+                      @ConditionalOnMissingBean
+                      static public HookRegisteringBeanDefinitionRegistryPostProcessor hookRegisteringBeanDefinitionRegistryPostProcessor(ConfigurableApplicationContext context) {
+                          return new HookRegisteringBeanDefinitionRegistryPostProcessor(context);
+                      }
                   }

Contributor

glmapper Jul 1, 2019

HookRegisteringBeanDefinitionRegistryPostProcessor 的作用域应该不仅仅是在WebfluxSofaTracerFilterConfiguration 中，应该对于 reactor 组件生效，然后通过例如 com.alipay.sofa.tracer.reactor.enabled 来控制是否启用 reactor 组件的埋点

...sofa/tracer/boot/springmvc/processor/HookRegisteringBeanDefinitionRegistryPostProcessor.java Outdated

+              import com.alipay.common.tracer.core.reactor.SofaTracerReactorSubscriber;
+              import org.apache.commons.logging.Log;
+              import org.apache.commons.logging.LogFactory;

Contributor

glmapper Jul 1, 2019

无用的引入可以移除掉

...sofa/tracer/boot/springmvc/processor/HookRegisteringBeanDefinitionRegistryPostProcessor.java Outdated

+              import org.apache.commons.logging.LogFactory;
+              import org.reactivestreams.Publisher;
+              import org.springframework.beans.BeansException;
+              import org.springframework.beans.factory.BeanFactory;

Contributor

glmapper Jul 1, 2019

移除没有用到的引用

...sofa/tracer/boot/springmvc/processor/HookRegisteringBeanDefinitionRegistryPostProcessor.java Outdated

+              import org.springframework.context.ConfigurableApplicationContext;
+              import reactor.core.CoreSubscriber;
+              import reactor.core.Fuseable;
+              import reactor.core.Scannable;

Contributor

glmapper Jul 1, 2019

同上

...sofa/tracer/boot/springmvc/processor/HookRegisteringBeanDefinitionRegistryPostProcessor.java Outdated

+              import reactor.core.publisher.Hooks;
+              import reactor.core.publisher.Mono;
+              import reactor.core.publisher.Operators;
+              import reactor.util.context.Context;

Contributor

glmapper Jul 1, 2019

同上

...sofa/tracer/boot/springmvc/processor/HookRegisteringBeanDefinitionRegistryPostProcessor.java Outdated

+              import reactor.core.publisher.Operators;
+              import reactor.util.context.Context;
+              import java.util.function.BooleanSupplier;

Contributor

glmapper Jul 1, 2019

同上

...sofa/tracer/boot/springmvc/processor/HookRegisteringBeanDefinitionRegistryPostProcessor.java Outdated

+                      Hooks.resetOnEachOperator(SOFA_TRACE_REACTOR_KEY);
+                  }
+                  private final ConfigurableApplicationContext context;

Contributor

glmapper Jul 1, 2019

context 这里貌似没有什么具体的作用，是否可以考虑移除掉

...m/alipay/sofa/tracer/boot/springmvc/configuration/OpenTracingSpringMvcAutoConfiguration.java Outdated

               import org.springframework.context.annotation.Bean;
               import org.springframework.context.annotation.Configuration;
               import org.springframework.core.Ordered;
               import org.springframework.core.annotation.Order;
               import org.springframework.web.server.WebFilter;
+              import reactor.core.publisher.Hooks;

Contributor

glmapper Jul 1, 2019

无用引用

卫恒 and others added 6 commits

July 1, 2019 14:41


          some optimized

16c9936


          Merge pull request #1 from glmapper/glmapper-reactor-fix

9e22bb4

some optimized


          fix Duplicate json data bug

9ed96da


          remove remote app


          make reactor auto configuration work

dfa2416


          code format

4ec1f4d

Member

ujjboy commented Jul 3, 2019

Hi, @shengxiang205 can you split this PR into two smaller PR? One is about the feature, and another is about code format.

guanchao-yang requested changes

View reviewed changes

...ugin/src/main/java/com/alipay/sofa/tracer/plugins/webflux/SpringWebfluxJsonStatReporter.java Outdated

+              public class SpringWebfluxJsonStatReporter extends SpringWebfluxStatReporter {
+                  /***
+                   * 输出拼接器

Member

guanchao-yang Jul 4, 2019

新的代码注释中不要添加中文，建议都以 English 描述

...ugin/src/main/java/com/alipay/sofa/tracer/plugins/webflux/SpringWebfluxJsonStatReporter.java Outdated

+                          // 这里强制刷一次
+                          appender.flush();
+                      } catch (Throwable t) {
+                          SelfLog.error("统计日志<" + statTracerName + ">输出异常", t);

Member

guanchao-yang Jul 4, 2019

换为 English 日志输出，还有注释也换为 English 吧。

...x-plugin/src/main/java/com/alipay/sofa/tracer/plugins/webflux/SpringWebfluxStatReporter.java

+                          // 这里强制刷一次
+                          appender.flush();
+                      } catch (Throwable t) {
+                          SelfLog.error("统计日志<" + statTracerName + ">输出异常", t);

Member

guanchao-yang Jul 4, 2019

English

tracer-sofa-boot-starter/pom.xml

@@ @@ -29,6 +29,10 @@ @@
                           <groupId>com.alipay.sofa</groupId>
                           <artifactId>sofa-tracer-springmvc-plugin</artifactId>
                       </dependency>
+                      <dependency>
+                          <groupId>com.alipay.sofa</groupId>
+                          <artifactId>sofa-tracer-webflux-plugin</artifactId>

Member

guanchao-yang Jul 4, 2019

另外，sofa-tracer-webflux-plugin 模块下需要补充一个 readme.md描述对该插件的使用方式，可以参考下其他插件的编写方式哈。 @glmapper 正式发布后，使用文档也记得放到 RELEASE Notes 里面。

Contributor

glmapper Jul 4, 2019

另外，sofa-tracer-webflux-plugin 模块下需要补充一个 readme.md描述对该插件的使用方式，可以参考下其他插件的编写方式哈。 @glmapper 正式发布后，使用文档也记得放到 RELEASE Notes 里面。

文档部分随着发布报告一起同步，@shengxiang205 可以参考 https://github.com/glmapper/tarcer-test-samples 这个工程将 README 补充下

...-plugin/src/main/java/com/alipay/sofa/tracer/plugins/zipkin/adapter/ZipkinV2SpanAdapter.java Outdated

@@ @@ -43,6 +44,7 @@ @@
                   /**
                    * convent sofaTracerSpan model to zipKinSpan model
+                   *

Member

guanchao-yang Jul 4, 2019

这次怎么这么多代码格式化哈？之前的代码格式化的插件是有什么问题吗？如果有的话，格式化也建议单独一个 PR 哈。

卫恒 added 2 commits

July 4, 2019 16:10


          comment en

4646dc7


          reback code format and fix cn comment

81e7f61


          reback code formar

9c7e751

Contributor

glmapper commented Jul 4, 2019

@ujjboy @guanchao-yang all requested changes has been solved

Contributor

glmapper commented Jul 31, 2019

@shengxiang205 本周计划先发布 2.4.1-SNAPSHOT 版本，这个 PR 你看下还是否有修改或者优化

Contributor

glmapper commented Aug 29, 2022

这个 PR 暂时先关闭，历史问题，暂无驱动

glmapper closed this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels