From 56a1bc4935a3080f5b5688f4143d1009940b14ba Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Mon, 29 Jun 2026 17:59:26 -0400 Subject: [PATCH 1/3] impl(bigtable): make MetricServiceConnection more shareable --- .../bigtable/internal/data_connection_impl.cc | 20 +++++++++++++++++-- .../bigtable/internal/data_connection_impl.h | 3 +++ .../internal/operation_context_factory.cc | 6 ++++-- .../internal/operation_context_factory.h | 10 +++++----- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/google/cloud/bigtable/internal/data_connection_impl.cc b/google/cloud/bigtable/internal/data_connection_impl.cc index 81ac2fb5d3c68..0fbd63c1780ab 100644 --- a/google/cloud/bigtable/internal/data_connection_impl.cc +++ b/google/cloud/bigtable/internal/data_connection_impl.cc @@ -31,6 +31,8 @@ #include "google/cloud/bigtable/result_source_interface.h" #include "google/cloud/bigtable/retry_policy.h" #include "google/cloud/background_threads.h" +#include "google/cloud/common_options.h" +#include "google/cloud/credentials.h" #include "google/cloud/grpc_options.h" #include "google/cloud/idempotency.h" #include "google/cloud/internal/algorithm.h" @@ -39,6 +41,7 @@ #include "google/cloud/internal/random.h" #include "google/cloud/internal/retry_loop.h" #include "google/cloud/internal/streaming_read_rpc.h" +#include "google/cloud/universe_domain_options.h" #include #include @@ -217,6 +220,17 @@ bigtable::Row TransformReadModifyWriteRowResponse( return bigtable::Row(std::move(*row.mutable_key()), std::move(cells)); } +#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(); + return options; +} +#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS + DataConnectionImpl::DataConnectionImpl( std::unique_ptr background, std::unique_ptr stub_manager, @@ -227,6 +241,8 @@ DataConnectionImpl::DataConnectionImpl( 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. @@ -234,8 +250,8 @@ DataConnectionImpl::DataConnectionImpl( std::string client_uid = internal::Sample(gen, 16, "abcdefghijklmnopqrstuvwxyz0123456789"); operation_context_factory_ = - std::make_unique(std::move(client_uid), - options_); + std::make_unique( + std::move(client_uid), metric_service_connection_, options_); } else { operation_context_factory_ = std::make_unique(); diff --git a/google/cloud/bigtable/internal/data_connection_impl.h b/google/cloud/bigtable/internal/data_connection_impl.h index 7722ae2755a0e..2cdbeb8920e6a 100644 --- a/google/cloud/bigtable/internal/data_connection_impl.h +++ b/google/cloud/bigtable/internal/data_connection_impl.h @@ -25,6 +25,7 @@ #include "google/cloud/bigtable/prepared_query.h" #include "google/cloud/bigtable/result_source_interface.h" #include "google/cloud/background_threads.h" +#include "google/cloud/monitoring/v3/metric_connection.h" #include "google/cloud/options.h" #include "google/cloud/status_or.h" #include "google/cloud/version.h" @@ -142,6 +143,8 @@ class DataConnectionImpl : public bigtable::DataConnection { std::unique_ptr background_; std::unique_ptr stub_manager_; + std::shared_ptr<::google::cloud::monitoring_v3::MetricServiceConnection> + metric_service_connection_; std::unique_ptr operation_context_factory_; std::shared_ptr limiter_; Options options_; diff --git a/google/cloud/bigtable/internal/operation_context_factory.cc b/google/cloud/bigtable/internal/operation_context_factory.cc index 6337dcac4113b..bd020586a8f95 100644 --- a/google/cloud/bigtable/internal/operation_context_factory.cc +++ b/google/cloud/bigtable/internal/operation_context_factory.cc @@ -158,9 +158,11 @@ MetricsOperationContextFactory::MetricsOperationContextFactory( } MetricsOperationContextFactory::MetricsOperationContextFactory( - std::string client_uid, Options options) + std::string client_uid, + std::shared_ptr conn, + Options options) : MetricsOperationContextFactory( - std::move(client_uid), nullptr, + std::move(client_uid), std::move(conn), std::make_shared(), std::move(options)) {} MetricsOperationContextFactory::MetricsOperationContextFactory( diff --git a/google/cloud/bigtable/internal/operation_context_factory.h b/google/cloud/bigtable/internal/operation_context_factory.h index e1ca4ea5c3407..c8d77b4f7c6b0 100644 --- a/google/cloud/bigtable/internal/operation_context_factory.h +++ b/google/cloud/bigtable/internal/operation_context_factory.h @@ -88,16 +88,16 @@ class SimpleOperationContextFactory : public OperationContextFactory { class MetricsOperationContextFactory : public OperationContextFactory { public: - explicit MetricsOperationContextFactory(std::string client_uid, - Options options = {}); + MetricsOperationContextFactory( + std::string client_uid, + std::shared_ptr conn, + Options options = {}); // Used for injecting a MockMetricsServiceConnection for testing. MetricsOperationContextFactory( std::string client_uid, std::shared_ptr conn, - std::shared_ptr clock = - std::make_shared(), - Options options = {}); + std::shared_ptr clock, Options options = {}); // This constructs an instance only suitable for testing. The provided metric // is copied into every RPC metric vector, preventing normal Metric From 44fd71ec1ea3c13481ae964d152cd877ee777081 Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Tue, 30 Jun 2026 09:43:15 -0400 Subject: [PATCH 2/3] address comments --- .../bigtable/internal/data_connection_impl.cc | 24 +++++++++---------- .../bigtable/internal/data_connection_impl.h | 4 ++++ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/google/cloud/bigtable/internal/data_connection_impl.cc b/google/cloud/bigtable/internal/data_connection_impl.cc index 0fbd63c1780ab..374dc5c9fb06c 100644 --- a/google/cloud/bigtable/internal/data_connection_impl.cc +++ b/google/cloud/bigtable/internal/data_connection_impl.cc @@ -198,6 +198,17 @@ std::string_view InstanceNameFromTableName(std::string_view table_name) { 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(); + return options; +} +#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS + } // namespace bigtable::Row TransformReadModifyWriteRowResponse( @@ -220,17 +231,6 @@ bigtable::Row TransformReadModifyWriteRowResponse( return bigtable::Row(std::move(*row.mutable_key()), std::move(cells)); } -#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(); - return options; -} -#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS - DataConnectionImpl::DataConnectionImpl( std::unique_ptr background, std::unique_ptr stub_manager, @@ -259,7 +259,7 @@ DataConnectionImpl::DataConnectionImpl( #else operation_context_factory_ = std::make_unique(); -#endif +#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS } DataConnectionImpl::DataConnectionImpl( diff --git a/google/cloud/bigtable/internal/data_connection_impl.h b/google/cloud/bigtable/internal/data_connection_impl.h index 2cdbeb8920e6a..10a2e9da540db 100644 --- a/google/cloud/bigtable/internal/data_connection_impl.h +++ b/google/cloud/bigtable/internal/data_connection_impl.h @@ -25,7 +25,9 @@ #include "google/cloud/bigtable/prepared_query.h" #include "google/cloud/bigtable/result_source_interface.h" #include "google/cloud/background_threads.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/options.h" #include "google/cloud/status_or.h" #include "google/cloud/version.h" @@ -143,8 +145,10 @@ class DataConnectionImpl : public bigtable::DataConnection { std::unique_ptr background_; std::unique_ptr stub_manager_; +#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS std::shared_ptr<::google::cloud::monitoring_v3::MetricServiceConnection> metric_service_connection_; +#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS std::unique_ptr operation_context_factory_; std::shared_ptr limiter_; Options options_; From 160e8a1df904d29a7d1ae19cc1813e1171709719 Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Tue, 30 Jun 2026 10:51:28 -0400 Subject: [PATCH 3/3] use forward declaration to fix tests and minimize ifdef proliferation --- google/cloud/bigtable/internal/data_connection_impl.cc | 3 +++ google/cloud/bigtable/internal/data_connection_impl.h | 10 +++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/google/cloud/bigtable/internal/data_connection_impl.cc b/google/cloud/bigtable/internal/data_connection_impl.cc index 374dc5c9fb06c..2c2d44d2d6a89 100644 --- a/google/cloud/bigtable/internal/data_connection_impl.cc +++ b/google/cloud/bigtable/internal/data_connection_impl.cc @@ -41,6 +41,9 @@ #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 diff --git a/google/cloud/bigtable/internal/data_connection_impl.h b/google/cloud/bigtable/internal/data_connection_impl.h index 10a2e9da540db..a859d5c938ace 100644 --- a/google/cloud/bigtable/internal/data_connection_impl.h +++ b/google/cloud/bigtable/internal/data_connection_impl.h @@ -25,9 +25,6 @@ #include "google/cloud/bigtable/prepared_query.h" #include "google/cloud/bigtable/result_source_interface.h" #include "google/cloud/background_threads.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/options.h" #include "google/cloud/status_or.h" #include "google/cloud/version.h" @@ -36,6 +33,11 @@ namespace google { namespace cloud { +namespace monitoring_v3 { +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN +class MetricServiceConnection; +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END +} // namespace monitoring_v3 namespace bigtable_internal { GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN @@ -145,10 +147,8 @@ class DataConnectionImpl : public bigtable::DataConnection { std::unique_ptr background_; std::unique_ptr stub_manager_; -#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS std::shared_ptr<::google::cloud::monitoring_v3::MetricServiceConnection> metric_service_connection_; -#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS std::unique_ptr operation_context_factory_; std::shared_ptr limiter_; Options options_;