Repository navigation
fix(storage): do not report DirectPath when gRPC falls back to DNS - #16568
kalragauri wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the gRPC channel telemetry to distinguish between the requested transport type and the actual transport type selected by gRPC at runtime. It introduces helper functions to classify load balancing policies and determine the selected transport, updating the logging and telemetry callbacks accordingly. Feedback is provided to optimize the retrieval of the channel ready outcome in StartChannelTelemetry by moving the value out of the future to avoid an unnecessary copy of non-fundamental types.
e9b1397 to
b67b845
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16568 +/- ##
==========================================
+ Coverage 92.37% 92.39% +0.01%
==========================================
Files 2265 2265
Lines 219470 219566 +96
==========================================
+ Hits 202745 202877 +132
+ Misses 16725 16689 -36 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Currently,
transport_typeon thechannel_creation_latencymetric and in the channel-ready log line is inferred from the configured endpoint viaDetectTransportType(), so it reports whatever the endpoint asked for. At runtime thegoogle-c2presolver can fall back to DNS, for example on hosts that gRPC does not detect as GCE VMs, and gRPC < 1.85.0 ignores theforce-xdsquery parameter. Channels that were in fact using DNS (CloudPath) were therefore labeledDirectPathorDirectPathInterconnect.ChannelReadyWaiternow reads the channel's load-balancing policy viagrpc::Channel::GetLoadBalancingPolicyName()when the wait for the first channel ends, and once the channel is readySelectedTransportType()corrects the requested transport:pick_firstmeansgoogle-c2pfell back to DNS, and the label becomesCloudPath. It is gRPC's default policy, and therefore what a DNS result produces for this library.DirectPathInterconnectbecomesDirectPath, becauseforce-xdsis ignored.Only two policy names are interpreted:
pick_firstas DNS, andxds_cluster_manager_experimental, the top-level policy of every xDS resolver result (gRFC A31), as xDS. Any other name, including an empty one, is treated as unknown: the label stays at the requested transport (still adjusted for an ignoredforce-xds) and, for DirectPath requests, the ready line says so.Both the
channel_creation_latencymetric callback andLogChannelReadyreceive the corrected value. Thetransport_typelabel name and its values (CloudPath,DirectPath,DirectPathInterconnect) are unchanged.The log lines now distinguish the requested transport from the corrected one:
LogChannelConfigurationlogsrequested_transport_type=(renamed fromtransport_type=) and addsgrpc_version=to the startupINFOline.LogChannelReadylogstransport_type=,requested_transport_type=,lb_policy=, andelapsed_ms=when the channel becomes ready, with a note explaining a DNS fallback, an ignoredforce-xds, or an unrecognized LB policy.