Skip to content

fix(storage): do not report DirectPath when gRPC falls back to DNS - #16568

Open
kalragauri wants to merge 3 commits into
googleapis:mainfrom
kalragauri:fix/grpc-warning
Open

kalragauri wants to merge 3 commits into
googleapis:mainfrom
kalragauri:fix/grpc-warning

Conversation

@kalragauri

Copy link
Copy Markdown
Contributor

Currently, transport_type on the channel_creation_latency metric and in the channel-ready log line is inferred from the configured endpoint via DetectTransportType(), so it reports whatever the endpoint asked for. At runtime the google-c2p resolver can fall back to DNS, for example on hosts that gRPC does not detect as GCE VMs, and gRPC < 1.85.0 ignores the force-xds query parameter. Channels that were in fact using DNS (CloudPath) were therefore labeled DirectPath or DirectPathInterconnect.

ChannelReadyWaiter now reads the channel's load-balancing policy via grpc::Channel::GetLoadBalancingPolicyName() when the wait for the first channel ends, and once the channel is ready SelectedTransportType() corrects the requested transport:

  • pick_first means google-c2p fell back to DNS, and the label becomes CloudPath. It is gRPC's default policy, and therefore what a DNS result produces for this library.
  • Otherwise, on gRPC < 1.85.0, DirectPathInterconnect becomes DirectPath, because force-xds is ignored.

Only two policy names are interpreted: pick_first as DNS, and xds_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 ignored force-xds) and, for DirectPath requests, the ready line says so.

Both the channel_creation_latency metric callback and LogChannelReady receive the corrected value. The transport_type label name and its values (CloudPath, DirectPath, DirectPathInterconnect) are unchanged.

The log lines now distinguish the requested transport from the corrected one:

  • LogChannelConfiguration logs requested_transport_type= (renamed from transport_type=) and adds grpc_version= to the startup INFO line.
  • LogChannelReady logs transport_type=, requested_transport_type=, lb_policy=, and elapsed_ms= when the channel becomes ready, with a note explaining a DNS fallback, an ignored force-xds, or an unrecognized LB policy.

@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Oct 15, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment thread google/cloud/storage/internal/grpc/channel_telemetry.cc Outdated
@kalragauri
kalragauri marked this pull request as ready for review October 15, 2026 07:23
@kalragauri
kalragauri requested review from a team as code owners October 15, 2026 07:23
@kalragauri
kalragauri requested a review from v-pratap October 15, 2026 07:24
@codecov

codecov Bot commented Oct 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.12230% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.39%. Comparing base (7af76ee) to head (0b8f5b1).

Files with missing lines Patch % Lines
...e/cloud/storage/internal/grpc/channel_telemetry.cc 96.66% 2 Missing ⚠️
...ud/storage/internal/grpc/channel_telemetry_test.cc 97.46% 2 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch was successfully deployed

1 active deployment
false — 0b8f5b1f Deployed Oct 15, 2026 by kalragauri via Save PR ref #12457
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant