From e89f713b4a7ecba8188d4f5d1531b9250a9bd97a Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Fri, 31 Jul 2026 14:41:20 -0400 Subject: [PATCH 1/2] impl(bigtable): add options for enabling directpath and directpath metrics --- google/cloud/bigtable/data_connection.cc | 46 ++++++++++++- google/cloud/bigtable/data_connection_test.cc | 40 ++++++++++++ .../internal/bigtable_stub_factory.cc | 6 +- .../bigtable/internal/data_connection_impl.cc | 64 +------------------ .../bigtable/internal/data_connection_impl.h | 14 +--- .../internal/data_connection_impl_test.cc | 5 +- google/cloud/bigtable/internal/defaults.cc | 34 +++++++--- google/cloud/bigtable/internal/defaults.h | 6 +- .../cloud/bigtable/internal/defaults_test.cc | 39 +++++++++++ google/cloud/bigtable/options.h | 29 +++++++++ .../testing/embedded_server_test_fixture.cc | 5 +- .../testing/table_integration_test.cc | 5 ++ 12 files changed, 203 insertions(+), 90 deletions(-) diff --git a/google/cloud/bigtable/data_connection.cc b/google/cloud/bigtable/data_connection.cc index 6fd19a08706ae..89a7dc91f0c5e 100644 --- a/google/cloud/bigtable/data_connection.cc +++ b/google/cloud/bigtable/data_connection.cc @@ -18,6 +18,7 @@ #include "google/cloud/bigtable/internal/data_connection_impl.h" #include "google/cloud/bigtable/internal/data_tracing_connection.h" #include "google/cloud/bigtable/internal/defaults.h" +#include "google/cloud/bigtable/internal/grpc_metrics_exporter.h" #include "google/cloud/bigtable/internal/mutate_rows_limiter.h" #include "google/cloud/bigtable/internal/partial_result_set_source.h" #include "google/cloud/bigtable/internal/row_reader_impl.h" @@ -29,7 +30,12 @@ #include "google/cloud/grpc_options.h" #include "google/cloud/internal/opentelemetry.h" #include "google/cloud/internal/unified_grpc_credentials.h" +#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS +#include "google/cloud/monitoring/v3/metric_connection.h" +#include "google/cloud/internal/random.h" +#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS #include +#include namespace google { namespace cloud { @@ -194,6 +200,40 @@ std::shared_ptr MakeDataConnection(Options options) { background->cq(), options); auto limiter = bigtable_internal::MakeMutateRowsLimiter(background->cq(), options); + + std::shared_ptr + metric_service_connection; + std::unique_ptr + operation_context_factory; + +#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS + if (options.get()) { + metric_service_connection = monitoring_v3::MakeMetricServiceConnection( + internal::MetricsExporterConnectionOptions(options)); + auto gen = google::cloud::internal::MakeDefaultPRNG(); + std::string client_uid = google::cloud::internal::Sample( + gen, 16, "abcdefghijklmnopqrstuvwxyz0123456789"); +#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS + if (bigtable::internal::IsDirectPath(options) && + options.has() && + options.get() == + experimental::DirectPathMetricsMode::kEnabled) { + bigtable_internal::EnableGrpcMetrics(metric_service_connection, options, + client_uid); + } +#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS + operation_context_factory = + std::make_unique( + std::move(client_uid), metric_service_connection, options); + } else { + operation_context_factory = + std::make_unique(); + } +#else + operation_context_factory = + std::make_unique(); +#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS + std::shared_ptr conn; if (options.has()) { @@ -212,14 +252,16 @@ std::shared_ptr MakeDataConnection(Options options) { std::move(background), std::make_unique( std::move(affinity_stubs), stub_creation_fn), - std::move(limiter), std::move(options)); + std::move(operation_context_factory), std::move(limiter), + std::move(options)); } else { auto stub = bigtable_internal::CreateBigtableStub( std::move(auth), background->cq(), options); conn = std::make_shared( std::move(background), std::make_unique(std::move(stub)), - std::move(limiter), std::move(options)); + std::move(operation_context_factory), std::move(limiter), + std::move(options)); } if (google::cloud::internal::TracingEnabled(conn->options())) { conn = bigtable_internal::MakeDataTracingConnection(std::move(conn)); diff --git a/google/cloud/bigtable/data_connection_test.cc b/google/cloud/bigtable/data_connection_test.cc index db96239c29d1d..ca4c710426ffc 100644 --- a/google/cloud/bigtable/data_connection_test.cc +++ b/google/cloud/bigtable/data_connection_test.cc @@ -109,6 +109,46 @@ TEST(MakeDataConnection, TestingOtelCollectorEnvVar) { auto conn = MakeDataConnection(TestOptions().set(true)); EXPECT_NE(conn, nullptr); } + +TEST(MakeDataConnection, DirectPathMetricsModeOptionDisabled) { + testing_util::ScopedEnvironment env("GOOGLE_CLOUD_CPP_TESTING_OTEL_COLLECTOR", + "1"); + InstanceResource instance_a{Project("my-project"), "instance-a"}; + auto conn = MakeDataConnection( + {instance_a}, TestOptions() + .set(true) + .set( + experimental::DirectPathMetricsMode::kDisabled)); + EXPECT_NE(conn, nullptr); +} + +TEST(MakeDataConnection, DirectPathMetricsModeOptionEnabled) { + testing_util::ScopedEnvironment env("GOOGLE_CLOUD_CPP_TESTING_OTEL_COLLECTOR", + "1"); + InstanceResource instance_a{Project("my-project"), "instance-a"}; + auto conn = MakeDataConnection( + {instance_a}, TestOptions() + .set(true) + .set( + experimental::DirectPathMode::kEnabled) + .set( + experimental::DirectPathMetricsMode::kEnabled)); + EXPECT_NE(conn, nullptr); +} + +TEST(MakeDataConnection, DirectPathModeOptionDisabledNoGrpcMetrics) { + testing_util::ScopedEnvironment env("GOOGLE_CLOUD_CPP_TESTING_OTEL_COLLECTOR", + "1"); + InstanceResource instance_a{Project("my-project"), "instance-a"}; + auto conn = MakeDataConnection( + {instance_a}, TestOptions() + .set(true) + .set( + experimental::DirectPathMode::kDisabled) + .set( + experimental::DirectPathMetricsMode::kEnabled)); + EXPECT_NE(conn, nullptr); +} #endif } // namespace diff --git a/google/cloud/bigtable/internal/bigtable_stub_factory.cc b/google/cloud/bigtable/internal/bigtable_stub_factory.cc index b9b2830616371..f3dfc55e61c6f 100644 --- a/google/cloud/bigtable/internal/bigtable_stub_factory.cc +++ b/google/cloud/bigtable/internal/bigtable_stub_factory.cc @@ -64,8 +64,8 @@ std::string CreateFeaturesMetadata(bool is_direct_path) { return internal::UrlsafeBase64EncodeWithPadding(proto.SerializeAsString()); } -std::string FeaturesMetadata() { - if (bigtable::internal::IsDirectPath()) { +std::string FeaturesMetadata(Options const& options) { + if (bigtable::internal::IsDirectPath(options)) { static auto const* const kDirectPathFeatures = new std::string(CreateFeaturesMetadata(true)); return *kDirectPathFeatures; @@ -84,7 +84,7 @@ std::shared_ptr ApplyCommonDecorators( stub = std::make_shared( std::move(stub), std::multimap{ - {"bigtable-features", FeaturesMetadata()}}, + {"bigtable-features", FeaturesMetadata(options)}}, internal::HandCraftedLibClientHeader()); if (internal::Contains(options.get(), "rpc")) { GCP_LOG(INFO) << "Enabled logging for gRPC calls"; diff --git a/google/cloud/bigtable/internal/data_connection_impl.cc b/google/cloud/bigtable/internal/data_connection_impl.cc index 5a09bf55acaf1..a0da74dd7a9a6 100644 --- a/google/cloud/bigtable/internal/data_connection_impl.cc +++ b/google/cloud/bigtable/internal/data_connection_impl.cc @@ -18,7 +18,7 @@ #include "google/cloud/bigtable/internal/async_row_sampler.h" #include "google/cloud/bigtable/internal/bulk_mutator.h" #include "google/cloud/bigtable/internal/default_row_reader.h" -#include "google/cloud/bigtable/internal/defaults.h" +#include "google/cloud/bigtable/internal/grpc_metrics_exporter.h" #include "google/cloud/bigtable/internal/logging_result_set_reader.h" #include "google/cloud/bigtable/internal/operation_context.h" #include "google/cloud/bigtable/internal/partial_result_set_reader.h" @@ -39,12 +39,8 @@ #include "google/cloud/internal/async_retry_loop.h" #include "google/cloud/internal/getenv.h" #include "google/cloud/internal/make_status.h" -#include "google/cloud/internal/random.h" #include "google/cloud/internal/retry_loop.h" #include "google/cloud/internal/streaming_read_rpc.h" -#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS -#include "google/cloud/monitoring/v3/metric_connection.h" -#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS #include "google/cloud/universe_domain_options.h" #include #include @@ -201,23 +197,6 @@ std::string_view InstanceNameFromTableName(std::string_view table_name) { if (pos == std::string_view::npos) return {}; return table_name.substr(0, pos); } - -#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS -Options MetricsExporterConnectionOptions(Options options) { - // We start with a copy of the client options to preserve credentials and - // universe domain, but we must unset Bigtable-specific endpoints/authorities - // to allow default Monitoring defaults. - options.unset(); - options.unset(); - auto collector = internal::GetEnv("GOOGLE_CLOUD_CPP_TESTING_OTEL_COLLECTOR"); - if (collector.has_value()) { - // Override credentials when using the otel_collector test server. - options.set(MakeInsecureCredentials()); - } - return options; -} -#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS - } // namespace bigtable::Row TransformReadModifyWriteRowResponse( @@ -240,45 +219,6 @@ bigtable::Row TransformReadModifyWriteRowResponse( return bigtable::Row(std::move(*row.mutable_key()), std::move(cells)); } -DataConnectionImpl::DataConnectionImpl( - std::unique_ptr background, - std::unique_ptr stub_manager, - std::shared_ptr limiter, Options options) - : background_(std::move(background)), - stub_manager_(std::move(stub_manager)), - limiter_(std::move(limiter)), - options_(MergeOptions(std::move(options), DataConnection::options())) { -#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS - if (options_.get()) { - metric_service_connection_ = monitoring_v3::MakeMetricServiceConnection( - MetricsExporterConnectionOptions(options_)); - // The client_uid is eventually used in conjunction with other data labels - // to identify metric data points. This pseudorandom string is used to aid - // in disambiguation. - auto gen = internal::MakeDefaultPRNG(); - std::string client_uid = - internal::Sample(gen, 16, "abcdefghijklmnopqrstuvwxyz0123456789"); - operation_context_factory_ = - std::make_unique( - std::move(client_uid), metric_service_connection_, options_); - } else { - operation_context_factory_ = - std::make_unique(); - } -#else - operation_context_factory_ = - std::make_unique(); -#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS -} - -DataConnectionImpl::DataConnectionImpl( - std::unique_ptr background, - std::shared_ptr stub, - std::shared_ptr limiter, Options options) - : DataConnectionImpl(std::move(background), - std::make_unique(std::move(stub)), - std::move(limiter), std::move(options)) {} - DataConnectionImpl::DataConnectionImpl( std::unique_ptr background, std::unique_ptr stub_manager, @@ -290,6 +230,8 @@ DataConnectionImpl::DataConnectionImpl( limiter_(std::move(limiter)), options_(MergeOptions(std::move(options), DataConnection::options())) {} +DataConnectionImpl::~DataConnectionImpl() = default; + DataConnectionImpl::DataConnectionImpl( std::unique_ptr background, std::shared_ptr stub, diff --git a/google/cloud/bigtable/internal/data_connection_impl.h b/google/cloud/bigtable/internal/data_connection_impl.h index a859d5c938ace..82b757582fc1e 100644 --- a/google/cloud/bigtable/internal/data_connection_impl.h +++ b/google/cloud/bigtable/internal/data_connection_impl.h @@ -52,26 +52,14 @@ bigtable::Row TransformReadModifyWriteRowResponse( class DataConnectionImpl : public bigtable::DataConnection { public: - ~DataConnectionImpl() override = default; + ~DataConnectionImpl() override; - DataConnectionImpl(std::unique_ptr background, - std::unique_ptr stub_manager, - std::shared_ptr limiter, - Options options); - - // This constructor is used for testing. DataConnectionImpl( std::unique_ptr background, std::unique_ptr stub_manager, std::unique_ptr operation_context_factory, std::shared_ptr limiter, Options options); - DataConnectionImpl(std::unique_ptr background, - std::shared_ptr stub, - std::shared_ptr limiter, - Options options); - - // This constructor is used for testing. DataConnectionImpl( std::unique_ptr background, std::shared_ptr stub, diff --git a/google/cloud/bigtable/internal/data_connection_impl_test.cc b/google/cloud/bigtable/internal/data_connection_impl_test.cc index 7874bb8e2e5f0..537a29871e968 100644 --- a/google/cloud/bigtable/internal/data_connection_impl_test.cc +++ b/google/cloud/bigtable/internal/data_connection_impl_test.cc @@ -16,6 +16,7 @@ #include "google/cloud/bigtable/data_connection.h" #include "google/cloud/bigtable/internal/crc32c.h" #include "google/cloud/bigtable/internal/defaults.h" +#include "google/cloud/bigtable/internal/grpc_metrics_exporter.h" #include "google/cloud/bigtable/internal/query_plan.h" #ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS #include "google/cloud/bigtable/internal/metrics.h" @@ -267,7 +268,9 @@ std::shared_ptr TestConnection( std::make_shared()) { auto background = internal::MakeBackgroundThreadsFactory()(); return std::make_shared( - std::move(background), std::move(stub), std::move(limiter), Options{}); + std::move(background), std::move(stub), + std::make_unique(), std::move(limiter), + Options{}); } std::shared_ptr TestConnection( diff --git a/google/cloud/bigtable/internal/defaults.cc b/google/cloud/bigtable/internal/defaults.cc index ff71df2383cba..20b862f92daeb 100644 --- a/google/cloud/bigtable/internal/defaults.cc +++ b/google/cloud/bigtable/internal/defaults.cc @@ -126,16 +126,20 @@ int DefaultConnectionPoolSize() { cpu_count * BIGTABLE_CLIENT_DEFAULT_CHANNELS_PER_CPU); } -bool IsDirectPath() { +bool IsDirectPath(Options const& options) { auto const direct_path = - google::cloud::internal::GetEnv("GOOGLE_CLOUD_ENABLE_DIRECT_PATH") - .value_or(""); + google::cloud::internal::GetEnv("GOOGLE_CLOUD_ENABLE_DIRECT_PATH"); // Bigtable specific env var for Direct Path support used by all clients. auto const cbt_direct_path = - google::cloud::internal::GetEnv("CBT_ENABLE_DIRECTPATH").value_or(""); - return absl::c_any_of(absl::StrSplit(direct_path, ','), - [](absl::string_view v) { return v == "bigtable"; }) || - cbt_direct_path == "true"; + google::cloud::internal::GetEnv("CBT_ENABLE_DIRECTPATH"); + if (direct_path.has_value() || cbt_direct_path.has_value()) { + return absl::c_any_of( + absl::StrSplit(direct_path.value_or(""), ','), + [](absl::string_view v) { return v == "bigtable"; }) || + cbt_direct_path.value_or("") == "true"; + } + return options.get() == + experimental::DirectPathMode::kEnabled; } Options HandleUniverseDomain(Options opts) { @@ -184,7 +188,7 @@ Options DefaultOptions(Options opts) { } // Set the specific data endpoints if Direct Path is enabled. - if (IsDirectPath()) { + if (IsDirectPath(opts)) { opts.set<::google::cloud::bigtable_internal::DataEndpointOption>( "google-c2p:///bigtable.googleapis.com") .set("bigtable.googleapis.com"); @@ -316,6 +320,20 @@ Options DefaultTableAdminOptions(Options opts) { opts.get<::google::cloud::bigtable_internal::AdminEndpointOption>()); } +#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS +Options MetricsExporterConnectionOptions(Options options) { + options.unset(); + options.unset(); + auto collector = google::cloud::internal::GetEnv( + "GOOGLE_CLOUD_CPP_TESTING_OTEL_COLLECTOR"); + if (collector.has_value()) { + options.set( + google::cloud::MakeInsecureCredentials()); + } + return options; +} +#endif + } // namespace internal GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace bigtable diff --git a/google/cloud/bigtable/internal/defaults.h b/google/cloud/bigtable/internal/defaults.h index d535d30a05af8..411dda3dd0b54 100644 --- a/google/cloud/bigtable/internal/defaults.h +++ b/google/cloud/bigtable/internal/defaults.h @@ -29,7 +29,7 @@ int DefaultConnectionPoolSize(); /** * Returns true if Direct Path is enabled for Bigtable. */ -bool IsDirectPath(); +bool IsDirectPath(Options const& options); /** * Returns an `Options` with the appropriate defaults for Bigtable. @@ -52,6 +52,10 @@ Options DefaultInstanceAdminOptions(Options opts); Options DefaultTableAdminOptions(Options opts); +#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS +Options MetricsExporterConnectionOptions(Options options); +#endif + } // namespace internal GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace bigtable diff --git a/google/cloud/bigtable/internal/defaults_test.cc b/google/cloud/bigtable/internal/defaults_test.cc index 732e9fcdb7993..1ef15af5a3e9f 100644 --- a/google/cloud/bigtable/internal/defaults_test.cc +++ b/google/cloud/bigtable/internal/defaults_test.cc @@ -579,6 +579,45 @@ TEST(EndpointEnvTest, BigtableDirectPathOverridesUserEndpoints) { EXPECT_EQ("bigtable.googleapis.com", opts.get()); } +TEST(EndpointEnvTest, DirectPathModeOptionEnabled) { + ScopedEnvironment emulator("BIGTABLE_EMULATOR_HOST", std::nullopt); + ScopedEnvironment direct_path("GOOGLE_CLOUD_ENABLE_DIRECT_PATH", + std::nullopt); + ScopedEnvironment cbt_direct_path("CBT_ENABLE_DIRECTPATH", std::nullopt); + + auto opts = Options{}.set( + experimental::DirectPathMode::kEnabled); + EXPECT_TRUE(IsDirectPath(opts)); + opts = DefaultOptions(opts); + EXPECT_EQ("google-c2p:///bigtable.googleapis.com", + opts.get<::google::cloud::bigtable_internal::DataEndpointOption>()); + EXPECT_EQ("bigtable.googleapis.com", opts.get()); +} + +TEST(EndpointEnvTest, DirectPathModeOptionDisabled) { + ScopedEnvironment emulator("BIGTABLE_EMULATOR_HOST", std::nullopt); + ScopedEnvironment direct_path("GOOGLE_CLOUD_ENABLE_DIRECT_PATH", + std::nullopt); + ScopedEnvironment cbt_direct_path("CBT_ENABLE_DIRECTPATH", std::nullopt); + + auto opts = Options{}.set( + experimental::DirectPathMode::kDisabled); + EXPECT_FALSE(IsDirectPath(opts)); + auto default_opts = DefaultDataOptions(opts); + EXPECT_EQ("bigtable.googleapis.com", default_opts.get()); +} + +TEST(EndpointEnvTest, DirectPathEnvVarOverridesDirectPathModeOption) { + ScopedEnvironment emulator("BIGTABLE_EMULATOR_HOST", std::nullopt); + ScopedEnvironment direct_path("GOOGLE_CLOUD_ENABLE_DIRECT_PATH", + std::nullopt); + ScopedEnvironment cbt_direct_path("CBT_ENABLE_DIRECTPATH", "false"); + + auto opts = Options{}.set( + experimental::DirectPathMode::kEnabled); + EXPECT_FALSE(IsDirectPath(opts)); +} + TEST(EndpointEnvTest, EmulatorOverridesCloudDirectPath) { ScopedEnvironment emulator("BIGTABLE_EMULATOR_HOST", "emulator-host:8000"); ScopedEnvironment direct_path("GOOGLE_CLOUD_ENABLE_DIRECT_PATH", "bigtable"); diff --git a/google/cloud/bigtable/options.h b/google/cloud/bigtable/options.h index 92c9b010b1b0c..fcf5bc8762ea8 100644 --- a/google/cloud/bigtable/options.h +++ b/google/cloud/bigtable/options.h @@ -213,6 +213,35 @@ struct DynamicChannelPoolSizingPolicyOption { using Type = DynamicChannelPoolSizingPolicy; }; +enum class DirectPathMode { + kDisabled, // Default. + kEnabled, +}; + +/** + * Option to control whether DirectPath should be used for Bigtable + * connections. + * + * By default, DirectPath is disabled. + */ +struct DirectPathModeOption { + using Type = DirectPathMode; +}; + +enum class DirectPathMetricsMode { + kEnabled, // Default. + kDisabled, +}; + +/** + * Option to control whether DirectPath should emit OpenTelemetry metrics. + * + * By default, DirectPath OpenTelemetry metrics are enabled. + */ +struct DirectPathMetricsModeOption { + using Type = DirectPathMetricsMode; +}; + } // namespace experimental /// The complete list of options accepted by `bigtable::*Client` diff --git a/google/cloud/bigtable/testing/embedded_server_test_fixture.cc b/google/cloud/bigtable/testing/embedded_server_test_fixture.cc index 3d8967ae186e3..10c48063e3224 100644 --- a/google/cloud/bigtable/testing/embedded_server_test_fixture.cc +++ b/google/cloud/bigtable/testing/embedded_server_test_fixture.cc @@ -16,6 +16,7 @@ #include "google/cloud/bigtable/internal/bigtable_metadata_decorator.h" #include "google/cloud/bigtable/internal/bigtable_stub.h" #include "google/cloud/bigtable/internal/data_connection_impl.h" +#include "google/cloud/bigtable/internal/grpc_metrics_exporter.h" #include "google/cloud/bigtable/internal/mutate_rows_limiter.h" #include "google/cloud/bigtable/options.h" #include "google/cloud/bigtable/retry_policy.h" @@ -82,7 +83,9 @@ void EmbeddedServerTestFixture::SetUp() { data_connection_ = std::make_shared( std::make_unique< google::cloud::internal::AutomaticallyCreatedBackgroundThreads>(), - stub, std::make_shared(), + stub, + std::make_unique(), + std::make_shared(), std::move(opts)); table_ = std::make_shared( diff --git a/google/cloud/bigtable/testing/table_integration_test.cc b/google/cloud/bigtable/testing/table_integration_test.cc index ab39945248a93..7b3315924de10 100644 --- a/google/cloud/bigtable/testing/table_integration_test.cc +++ b/google/cloud/bigtable/testing/table_integration_test.cc @@ -124,6 +124,11 @@ void TableAdminTestEnvironment::TearDown() { void TableIntegrationTest::SetUp() { Options options; + // Disable metrics for the setup connection used to clean up/create tables. + // This prevents premature registration of the process-global gRPC telemetry + // plugin with default (60-second) periods, which would otherwise override + // the custom period (5 seconds) requested inside the tests. + options.set(false); if (google::cloud::internal::GetEnv( "GOOGLE_CLOUD_CPP_BIGTABLE_TESTING_CHANNEL_POOL") .value_or("") == "dynamic") { From 9528d3ffe491dd74f53c5528e169bed42e9c62d8 Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Fri, 31 Jul 2026 16:18:47 -0400 Subject: [PATCH 2/2] address review comments --- google/cloud/bigtable/data_connection.cc | 3 ++- google/cloud/bigtable/internal/bigtable_stub_factory.cc | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/google/cloud/bigtable/data_connection.cc b/google/cloud/bigtable/data_connection.cc index 89a7dc91f0c5e..781cffc595f91 100644 --- a/google/cloud/bigtable/data_connection.cc +++ b/google/cloud/bigtable/data_connection.cc @@ -224,7 +224,8 @@ std::shared_ptr MakeDataConnection(Options options) { #endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS operation_context_factory = std::make_unique( - std::move(client_uid), metric_service_connection, options); + std::move(client_uid), std::move(metric_service_connection), + options); } else { operation_context_factory = std::make_unique(); diff --git a/google/cloud/bigtable/internal/bigtable_stub_factory.cc b/google/cloud/bigtable/internal/bigtable_stub_factory.cc index f3dfc55e61c6f..79d38966a4f0b 100644 --- a/google/cloud/bigtable/internal/bigtable_stub_factory.cc +++ b/google/cloud/bigtable/internal/bigtable_stub_factory.cc @@ -64,7 +64,7 @@ std::string CreateFeaturesMetadata(bool is_direct_path) { return internal::UrlsafeBase64EncodeWithPadding(proto.SerializeAsString()); } -std::string FeaturesMetadata(Options const& options) { +std::string const& FeaturesMetadata(Options const& options) { if (bigtable::internal::IsDirectPath(options)) { static auto const* const kDirectPathFeatures = new std::string(CreateFeaturesMetadata(true));