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

[#933] fix: incorrect metric grpc_server_connection_number #934

Merged
merged 1 commit into from
Jun 8, 2023

Conversation

xianjingfeng
Copy link
Member

What changes were proposed in this pull request?

Fix incorrect metric grpc_server_connection_number

Why are the changes needed?

Fix: #933

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing UT

@xianjingfeng xianjingfeng requested a review from zuston June 7, 2023 03:17
@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2023

Codecov Report

Merging #934 (18c07b8) into master (cb22510) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master     #934      +/-   ##
============================================
- Coverage     55.34%   55.32%   -0.02%     
+ Complexity     2273     2272       -1     
============================================
  Files           344      344              
  Lines         16982    16983       +1     
  Branches       1349     1349              
============================================
- Hits           9398     9396       -2     
- Misses         7038     7041       +3     
  Partials        546      546              
Impacted Files Coverage Δ
...le/common/rpc/MonitoringServerTransportFilter.java 33.33% <0.00%> (-4.17%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -37,7 +37,9 @@ public Attributes transportReady(Attributes transportAttrs) {
}

public void transportTerminated(Attributes transportAttrs) {
grpcMetrics.decGauge(GRPC_SERVER_CONNECTION_NUMBER_KEY);
if (transportAttrs != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

in which case, the transportAttrs is null?

Copy link
Member Author

Choose a reason for hiding this comment

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

When handshake fails.
grpc/grpc-java#7625 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

We can use telnet to trigger the problem. Just close it when telnet connect to rpc port.

@@ -37,7 +37,9 @@ public Attributes transportReady(Attributes transportAttrs) {
}

public void transportTerminated(Attributes transportAttrs) {
grpcMetrics.decGauge(GRPC_SERVER_CONNECTION_NUMBER_KEY);
if (transportAttrs != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Do you check when the transportAttrs will be null?

@xianjingfeng xianjingfeng merged commit b6a725b into apache:master Jun 8, 2023
54 checks passed
@xianjingfeng
Copy link
Member Author

Merged, thanks all.

@jerqi
Copy link
Contributor

jerqi commented Jun 9, 2023

Merged, thanks all.

@xianjingfeng Should we merge this pr to branch 0.7?

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.

[Bug] Incorrect metric grpc_server_connection_number
5 participants