-
Notifications
You must be signed in to change notification settings - Fork 26.5k
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
Fix obtaining remote application name on the server side #13811
Fix obtaining remote application name on the server side #13811
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 3.3 #13811 +/- ##
==========================================
+ Coverage 38.91% 38.93% +0.01%
==========================================
Files 1897 1897
Lines 79317 79321 +4
Branches 11529 11530 +1
==========================================
+ Hits 30866 30880 +14
+ Misses 44132 44125 -7
+ Partials 4319 4316 -3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oxsean @EarthChen @icodening PTAL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@@ -119,6 +120,11 @@ protected RpcInvocation onBuildRpcInvocationCompletion(RpcInvocation invocation) | |||
getContext().getServiceDescriptor().getInterfaceName(), | |||
getContext().getMethodName())); | |||
} | |||
String consumerAppKey = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add it in org.apache.dubbo.rpc.protocol.tri.h12.AbstractServerTransportListener#buildRpcInvocation
I think not only grpc need the consumerAppName.
What is the purpose of the change
Brief changelog
Verifying this change
Checklist