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

Integrate with sofa-lookout for support metrics 2.0. #127

Merged
merged 9 commits into from
May 22, 2018

Conversation

NeGnail
Copy link
Contributor

@NeGnail NeGnail commented May 17, 2018

Motivation:

Add the lookout module, report the information to lookout.

Modification:

Add lookout module.
A LookoutSubscriber was declared to monitor bolt thread pool status and call information.
Add lookout-api dependency.

Result:

If the "lookout. collect. disable" is false. The call information will be collected and reported to lookout.

NeGnail added 2 commits May 17, 2018 12:19
Adjust the order of events and record elapsed time when async invoke.…
@NeGnail NeGnail mentioned this pull request May 17, 2018
@codecov-io
Copy link

codecov-io commented May 17, 2018

Codecov Report

Merging #127 into master will increase coverage by 0.69%.
The diff coverage is 81.65%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #127      +/-   ##
============================================
+ Coverage     65.74%   66.44%   +0.69%     
- Complexity      685      756      +71     
============================================
  Files           272      280       +8     
  Lines         11626    11904     +278     
  Branches       1961     1981      +20     
============================================
+ Hits           7644     7910     +266     
+ Misses         3015     3006       -9     
- Partials        967      988      +21
Impacted Files Coverage Δ Complexity Δ
...in/java/com/alipay/sofa/rpc/common/RpcOptions.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...va/com/alipay/sofa/rpc/server/bolt/BoltServer.java 75.58% <100%> (+0.58%) 23 <0> (+1) ⬆️
...alipay/sofa/rpc/model/RpcAbstractLookoutModel.java 100% <100%> (ø) 15 <15> (?)
.../com/alipay/sofa/rpc/event/ServerStartedEvent.java 100% <100%> (ø) 0 <0> (?)
...java/com/alipay/sofa/rpc/module/LookoutModule.java 53.33% <53.33%> (ø) 3 <3> (?)
...java/com/alipay/sofa/rpc/lookout/RpcLookoutId.java 76.74% <76.74%> (ø) 9 <9> (?)
...n/java/com/alipay/sofa/rpc/lookout/RpcLookout.java 77.08% <77.08%> (ø) 14 <14> (?)
...m/alipay/sofa/rpc/model/RpcServerLookoutModel.java 80% <80%> (ø) 3 <3> (?)
...a/com/alipay/sofa/rpc/event/LookoutSubscriber.java 87.17% <87.17%> (ø) 14 <14> (?)
...m/alipay/sofa/rpc/model/RpcClientLookoutModel.java 90.9% <90.9%> (ø) 7 <7> (?)
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 109c1e9...77b0fe9. Read the comment docs.


@Override
public boolean needLoad() {
return true;
Copy link
Member

Choose a reason for hiding this comment

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

这里应该加上判断,存在 Lookout的类才加载。

static {
LookoutSubscriber.setLookoutCollectDisable(true);
}

Copy link
Member

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.

让之前的rpc调用不收集metrics信息,避免在lookout的用例初始化lookout的registry之前对registry进行了初始化。如果提前初始化了就会导致registry是一个noop。

Copy link
Member

Choose a reason for hiding this comment

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

这段逻辑挪到 LookoutSubscriber 里去吧。 If(RpcRunningState.isUnitTestMode())

/**
*
* @author <a href="mailto:lw111072@antfin.com">LiWei.Liengen</a>
* @version $Id: BoltServerStartedEvent.java, v 0.1 2018年04月24日 下午10:57 LiWei.Liengen Exp $
Copy link
Member

Choose a reason for hiding this comment

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

你自己的名字拼音是不是错了?
@version 这行删掉吧。

<artifactId>lookout-api</artifactId>
</dependency>
</dependencies>
</project>
Copy link
Member

Choose a reason for hiding this comment

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

<build></build> 从别的 module里复制过来。


private final RpcLookoutId rpcLookoutId = new RpcLookoutId();

private static final String EMPTY_STRING = "";
Copy link
Member

Choose a reason for hiding this comment

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

这个在 StringUtils 里已经有了

* @author <a href="mailto:lw111072@antfin.com">LiWei.Liangen</a>
* @version $Id: LookoutService.java, v 0.1 2018年05月10日 下午10:56 LiWei.Liangen Exp $
*/
public interface LookoutService {
Copy link
Member

Choose a reason for hiding this comment

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

这几个用例依赖第三方服务端吗?如果不依赖,挪到 test-integration 模块下去。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这几个用例依赖zk

<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<skipTests>${dependency.test.skip}</skipTests>
Copy link
Member

Choose a reason for hiding this comment

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

${skipTests}

static {
LookoutSubscriber.setLookoutCollectDisable(true);
}

Copy link
Member

Choose a reason for hiding this comment

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

这段逻辑挪到 LookoutSubscriber 里去吧。 If(RpcRunningState.isUnitTestMode())

@ujjboy ujjboy added this to the 5.4.0 milestone May 18, 2018
public void uninstall() {
if (subscriber != null) {
EventBus.unRegister(ClientEndInvokeEvent.class, subscriber);
EventBus.unRegister(ClientAsyncReceiveEvent.class, subscriber);
Copy link
Member

@ujjboy ujjboy May 21, 2018

Choose a reason for hiding this comment

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

ClientAsyncReceiveEvent 这个事件不需要了吧。

public void install() {
subscriber = new LookoutSubscriber();
EventBus.register(ClientEndInvokeEvent.class, subscriber);
EventBus.register(ClientAsyncReceiveEvent.class, subscriber);
Copy link
Member

@ujjboy ujjboy May 21, 2018

Choose a reason for hiding this comment

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

ClientAsyncReceiveEvent 这个事件不需要了吧。

@ujjboy ujjboy added the extension Integrate with 3rd component label May 22, 2018
@ujjboy ujjboy merged commit 00d62c1 into sofastack:master May 22, 2018
@ujjboy ujjboy changed the title Metrics lookout Integrate with sofa-lookout for support metrics 2.0. May 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension Integrate with 3rd component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants