impl(bigtable): enable directpath probe and cloudpath fallback

[scotthart]

No description provided.

[gemini-code-assist]

Code Review

This pull request refactors DataConnection creation to support DirectPath fallback and diagnostics, introduces helper functions for connection components, updates default timeouts, and adds integration tests for direct access compatibility metrics. The code review feedback highlights critical issues where completion queue threads are blocked by calling f.get() inside asynchronous lambdas, which can lead to thread starvation. Additionally, there are numerous style guide violations regarding the improper use of auto for obscured return types and primitive types, as well as an opportunity to avoid an unnecessary heap allocation in MetricsOperationContextFactory.

[gemini-code-assist]

Calling f.get() inside a RunAsync lambda blocks a completion queue thread. Completion queue threads are typically used for asynchronous event loops (like gRPC), and blocking them can lead to thread starvation or deadlocks. Instead, use .then() to attach a non-blocking continuation to the future.

    cp_future.then([](future<std::unique_ptr<bigtable_internal::StubManager>> f) { (void)f.get(); });

[gemini-code-assist]

Calling f.get() inside a RunAsync lambda blocks a completion queue thread. Completion queue threads are typically used for asynchronous event loops (like gRPC), and blocking them can lead to thread starvation or deadlocks. Instead, use .then() to attach a non-blocking continuation to the future.

  dp_future.then([](future<std::unique_ptr<bigtable_internal::StubManager>> f) { (void)f.get(); });

[gemini-code-assist]

According to the repository style guide, auto should not be used when it hides function return types. Please use the explicit type std::shared_ptr<MutateRowsLimiter> instead.

  std::shared_ptr<MutateRowsLimiter> limiter =
      bigtable_internal::MakeMutateRowsLimiter(background->cq(), options);

References

1. Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. ^(link)

[gemini-code-assist]

According to the repository style guide (Type Deduction section), auto should not be used when it hides function return types. Please use the explicit type std::shared_ptr<monitoring_v3::MetricServiceConnection> instead.

    std::shared_ptr<monitoring_v3::MetricServiceConnection> const
        metric_service_connection =
            monitoring_v3::MakeMetricServiceConnection(
                bigtable::internal::MetricsExporterConnectionOptions(options));

References

1. Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. ^(link)

[gemini-code-assist]

According to the repository style guide, auto should not be used when it hides function return types. Please use the explicit type std::unique_ptr<OperationContextFactory> instead.

  std::unique_ptr<OperationContextFactory> operation_context_factory =
      MakeOperationContextFactory(options, instances);

References

1. Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. ^(link)

[gemini-code-assist]

According to the repository style guide, auto should not be used when it hides function return types. Please use the explicit type bigtable_internal::DataConnectionComponents instead.

  bigtable_internal::DataConnectionComponents components =
      bigtable_internal::MakeDataConnectionComponents(std::move(options), {},
                                                      __func__);

References

1. Reject Obscured Domain & Return Types: Flag and reject auto when it hides StatusOr, domain objects, protobuf messages/fields, or function return types. ^(link)

[gemini-code-assist]

According to the repository style guide, auto should not be used for primitive or scalar types (including pointer types like char const*). Please use char const* explicitly.

constexpr char const* DefaultDirectPathDataEndpoint() {
  return "google-c2p:///bigtable.googleapis.com";
}

References

1. Disallow auto for Primitives: Require explicit numeric and scalar types (std::size_t, std::int64_t, bool, etc.) rather than deducing them from literals. ^(link)

[gemini-code-assist]

According to the repository style guide, auto should not be used for primitive or scalar types (including pointer types like char const*). Please use char const* explicitly.

constexpr char const* DefaultDirectPathAuthority() {
  return "bigtable.googleapis.com";
}

References

1. Disallow auto for Primitives: Require explicit numeric and scalar types (std::size_t, std::int64_t, bool, etc.) rather than deducing them from literals. ^(link)

[gemini-code-assist]

According to the repository style guide, auto should not be used for primitive or scalar types (including pointer types like char const*). Please use char const* explicitly.

constexpr char const* DefaultDirectPathMetadataHost() { return "169.254.169.254"; }

References

1. Disallow auto for Primitives: Require explicit numeric and scalar types (std::size_t, std::int64_t, bool, etc.) rather than deducing them from literals. ^(link)

[gemini-code-assist]

Creating a std::shared_ptr via std::make_shared just to call clone on it and immediately discard it introduces an unnecessary heap allocation. Since DirectAccessCompatibility can be constructed on the stack, you can allocate it on the stack instead.

  DirectAccessCompatibility metric(kBigtableMetricNamePath, provider_);
  direct_access_compatibility_ = std::shared_ptr<DirectAccessCompatibility>(
      static_cast<DirectAccessCompatibility*>(
          metric.clone(client_resource_labels_).release()));

[codecov]

Codecov Report

❌ Patch coverage is 50.84746% with 87 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.24%. Comparing base (5c916cf) to head (a14deab).

Files with missing lines	Patch %	Lines
...d/bigtable/tests/observability_integration_test.cc	7.50%	74 Missing ⚠️
google/cloud/bigtable/data_connection.cc	83.33%	13 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #16383      +/-   ##
==========================================
- Coverage   92.28%   92.24%   -0.04%     
==========================================
  Files        2245     2246       +1     
  Lines      211685   211847     +162     
==========================================
+ Hits       195349   195419      +70     
- Misses      16336    16428      +92     

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