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

RpcContext新增remoteApplicationName字段 #3816

Merged
merged 1 commit into from
Apr 10, 2019

Conversation

manzhizhen
Copy link

@manzhizhen manzhizhen commented Apr 6, 2019

What is the purpose of the change

In the process of using Dubbo, there are many scenarios where the Provider needs to know the applicationName of the Consumer (not just the Consumer IP). For example, I want to output the upstream caller to the log file and want to limit the flow according to the upstream application. Or downgrade, want to use the upstream application for arson drills, etc., so we usually add Filter to achieve (for example, DubboAppContextFilter in Sentinel, which is specifically done on the Consumer side: https://github.com/alibaba/Sentinel/ Blob/6bb4ff21dadde00c9453a5f57d030d55f87b71a8/sentinel-adapter/sentinel-dubbo-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/dubbo/DubboAppContextFilter.java), so I also hope that Dubbo can put this inside Simple things have been done

在我们使用Dubbo的过程中,有很多场景Provider端需要知道Consumer端的applicationName(而不仅仅只需要知道Consumer的IP),比如我想将上游调用方输出到出入口日志中、想根据上游应用来限流或降级、想根据上游应用来进行放火演练等,所以我们通常会自己加Filter来实现(例如Sentinel中的DubboAppContextFilter,就是专门在Consumer端做这个事情:https://github.com/alibaba/Sentinel/blob/6bb4ff21dadde00c9453a5f57d030d55f87b71a8/sentinel-adapter/sentinel-dubbo-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/dubbo/DubboAppContextFilter.java),

所以,我也特别希望Dubbo内部能把这块简单的事情给做了

@codecov-io
Copy link

codecov-io commented Apr 6, 2019

Codecov Report

Merging #3816 into master will decrease coverage by 0.01%.
The diff coverage is 83.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3816      +/-   ##
============================================
- Coverage     63.64%   63.62%   -0.02%     
  Complexity       71       71              
============================================
  Files           704      704              
  Lines         31065    31069       +4     
  Branches       5037     5037              
============================================
- Hits          19772    19769       -3     
- Misses         9024     9028       +4     
- Partials       2269     2272       +3
Impacted Files Coverage Δ Complexity Δ
...ava/org/apache/dubbo/rpc/filter/ContextFilter.java 93.33% <100%> (ø) 0 <0> (ø) ⬇️
...src/main/java/org/apache/dubbo/rpc/RpcContext.java 70% <75%> (+0.11%) 0 <0> (ø) ⬇️
...apache/dubbo/common/config/ConfigurationUtils.java 64% <0%> (-8%) 0% <0%> (ø)
.../exchange/support/header/HeaderExchangeServer.java 66.98% <0%> (-2.84%) 0% <0%> (ø)
...dubbo/remoting/exchange/support/DefaultFuture.java 75.51% <0%> (-0.69%) 0% <0%> (ø)

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 8445349...2eb5bc0. Read the comment docs.

@manzhizhen manzhizhen closed this Apr 7, 2019
@manzhizhen manzhizhen reopened this Apr 7, 2019
Copy link
Member

@htynkn htynkn left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Remote application name only used in tag router now(https://github.com/apache/incubator-dubbo/blob/master/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/tag/TagRouter.java#L237) and I think it make sense to include in context

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants