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

Document why XdsTestClient.check_channel_successful_calls is not using calls_succeeded #13

Open
sergiitk opened this issue Dec 12, 2023 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@sergiitk
Copy link
Member

sergiitk commented Dec 12, 2023

Context: grpc/grpc#35280 (comment)

Not sure why we didn't notice calls_succeeded in the first place.
I tried the oldest builds of xds test clients of each language, and all of them correctly set this field.

See #13 (comment).

cc @yashykt

@sergiitk sergiitk added the bug Something isn't working label Dec 12, 2023
@sergiitk
Copy link
Member Author

In addition, there's no issue in outputting it too.

diff --git a/framework/rpc/grpc_channelz.py b/framework/rpc/grpc_channelz.py
index 59e1478..0972803 100644
--- a/framework/rpc/grpc_channelz.py
+++ b/framework/rpc/grpc_channelz.py
@@ -110,3 +110,4 @@ class ChannelzServiceClient(framework.rpc.grpc.GrpcClientHelper):
         result += (
-            f" call_started={channel.data.calls_started}"
+            f" calls_succeeded={channel.data.calls_succeeded}"
+            + f" calls_started={channel.data.calls_started}"
             + f" calls_failed={channel.data.calls_failed}"

I1211 16:37:10.451982 140704577574656 client_app.py:320] [psm-grpc-client-5cdf484df6-nd4x7] Detected successful calls to xDS control plane trafficdirector.googleapis.com:443, channel: <Channel channel_id=4 target=trafficdirector.googleapis.com:443 calls_succeeded=0 calls_started=2 calls_failed=0 state=READY>

@sergiitk
Copy link
Member Author

OK, I remember now. Because it's a bidi stream, calls_succeeded won't be updated until the RPC is finished. Completely forgot about it. We should explain it better in a comment to check_channel_successful_calls.

@sergiitk sergiitk changed the title XdsTestClient.check_channel_successful_calls should be using calls_succeeded Document why XdsTestClient.check_channel_successful_calls is not using calls_succeeded Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants