Visitar URL original
feat(storage): log effective gRPC transport and channel connection readiness by kalragauri · Pull Request #16478 · googleapis/google-cloud-cpp · GitHub
Skip to content

feat(storage): log effective gRPC transport and channel connection readiness - #16478

Merged
kalragauri merged 5 commits into
googleapis:mainfrom
kalragauri:fix/gci-otel
Sep 29, 2026
Merged

kalragauri merged 5 commits into
googleapis:mainfrom
kalragauri:fix/gci-otel

Conversation

@kalragauri

@kalragauri kalragauri commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

When DirectPath over Interconnect is enabled, applications cannot easily verify whether it took effect. Options such as EndpointOption or UniverseDomainOption silently override the feature, and there is no built-in signal indicating how long the initial channel takes to connect. This is a follow-up to #16408.

To resolve this, this PR adds google/cloud/storage/internal/grpc/channel_telemetry.{h,cc} to introduce the following functionality:

  • Transport Classification: TransportType and DetectTransportType() categorize the effective endpoint as CloudPath, DirectPath, or DirectPathInterconnect.
  • Configuration Logging: LogChannelConfiguration(), invoked during CreateDecoratedStubs(), logs an INFO message detailing the transport type. It emits a WARNING if DirectPathXdsOverInterconnectOption is configured but not requested by the effective endpoint.
  • Connection Telemetry: StartChannelTelemetry(), invoked during CreateStorageStub(), tracks AsyncWaitConnectionReady() on the initial channel to log connection duration. Because connection attempts are already initiated at this point via GrpcChannelRefresh, this tracking is purely observational and does not alter behavior.

Design Considerations:

  • Channel Selection: Telemetry is tracked only for channel 0. Since certain VM environments spawn hundreds of channels, this approach aligns with the performance trade-offs noted in GrpcChannelRefresh::Refresh().
  • Log Severity for Failures: Connection readiness failures are logged at INFO rather than WARNING. A completion queue shutting down prior to connection completes with kDeadlineExceeded, which cannot be reliably distinguished from a true connection timeout. Logging WARNING in these cases would cause false positives in short-lived operations, so WARNING is reserved strictly for clear configuration mismatches.

Note: The google-c2p resolver might still fall back to CloudPath internally. Because this internal fallback cannot be detected by the client library, DetectTransportType() strictly reflects the requested transport type.

@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label Sep 23, 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 introduces gRPC channel telemetry to track and log the transport type (CloudPath, DirectPath, or DirectPathInterconnect) and connection latency for Google Cloud Storage clients. It adds the channel_telemetry helper, integrates it into the storage stub factory, and includes comprehensive unit tests. The review feedback highlights style guide violations in channel_telemetry.cc where auto is incorrectly used to deduce primitive/scalar types for kC2pPrefix, kC2pExperimentalPrefix, and kForceXds instead of using explicit types.

Comment thread google/cloud/storage/internal/grpc/channel_telemetry.cc Outdated
Comment thread google/cloud/storage/internal/grpc/channel_telemetry.cc Outdated
@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.81275% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.33%. Comparing base (1403c7a) to head (c1af5ce).

Files with missing lines Patch % Lines
...e/cloud/storage/internal/grpc/channel_telemetry.cc 95.78% 4 Missing ⚠️
...ud/storage/internal/grpc/channel_telemetry_test.cc 96.94% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16478      +/-   ##
==========================================
- Coverage   92.35%   92.33%   -0.02%     
==========================================
  Files        2258     2260       +2     
  Lines      216448   216699     +251     
==========================================
+ Hits       199904   200093     +189     
- Misses      16544    16606      +62     

☔ 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.

Comment thread google/cloud/storage/internal/grpc/channel_telemetry.cc
Comment thread google/cloud/storage/internal/grpc/channel_telemetry.cc
Comment thread google/cloud/storage/internal/grpc/channel_telemetry_test.cc
Comment thread google/cloud/storage/internal/grpc/channel_telemetry.cc
Comment thread google/cloud/storage/internal/storage_stub_factory.cc Outdated
@kalragauri
kalragauri enabled auto-merge (squash) September 29, 2026 05:12
@kalragauri
kalragauri merged commit bf9d54f into googleapis:main Sep 29, 2026
69 checks passed
@kalragauri
kalragauri deleted the fix/gci-otel branch October 6, 2026 06:10

This branch was successfully deployed

1 active deployment
false — c1af5cef Deployed Sep 29, 2026 by kalragauri via Save PR ref #12118
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.

3 participants