From ebdc2044f4ea929bec9570abcaf585bde6b40b46 Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Wed, 29 Jul 2026 14:13:34 -0400 Subject: [PATCH 1/4] impl(bigtable): add grpc metrics exporter --- google/cloud/bigtable/BUILD.bazel | 7 +- google/cloud/bigtable/CMakeLists.txt | 20 +- .../bigtable/bigtable_client_unit_tests.bzl | 1 + .../bigtable/google_cloud_cpp_bigtable.bzl | 2 + .../internal/grpc_metrics_exporter.cc | 377 ++++++++++++++++ .../bigtable/internal/grpc_metrics_exporter.h | 140 ++++++ .../internal/grpc_metrics_exporter_test.cc | 420 ++++++++++++++++++ 7 files changed, 965 insertions(+), 2 deletions(-) create mode 100644 google/cloud/bigtable/internal/grpc_metrics_exporter.cc create mode 100644 google/cloud/bigtable/internal/grpc_metrics_exporter.h create mode 100644 google/cloud/bigtable/internal/grpc_metrics_exporter_test.cc diff --git a/google/cloud/bigtable/BUILD.bazel b/google/cloud/bigtable/BUILD.bazel index ba0cf3e9189d5..d0898645e708f 100644 --- a/google/cloud/bigtable/BUILD.bazel +++ b/google/cloud/bigtable/BUILD.bazel @@ -43,7 +43,10 @@ cc_library( srcs = google_cloud_cpp_bigtable_srcs, hdrs = google_cloud_cpp_bigtable_hdrs, local_defines = select({ - ":metrics_enabled": ["GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS"], + ":metrics_enabled": [ + "GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS", + "GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS", + ], "//conditions:default": [], }), visibility = [ @@ -65,6 +68,7 @@ cc_library( ] + select({ ":metrics_enabled": [ "//:opentelemetry", + "@grpc//:grpcpp_otel_plugin", "@opentelemetry-cpp//api", "@opentelemetry-cpp//sdk/src/metrics", ], @@ -121,6 +125,7 @@ cc_library( local_defines = select({ ":metrics_enabled": [ "GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS", + "GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS", "GOOGLE_CLOUD_CPP_BIGTABLE_QUERY_PLAN_REFRESH_ASSERT", ], "//conditions:default": ["GOOGLE_CLOUD_CPP_BIGTABLE_QUERY_PLAN_REFRESH_ASSERT"], diff --git a/google/cloud/bigtable/CMakeLists.txt b/google/cloud/bigtable/CMakeLists.txt index 2be88e34d3e0c..47fc356283896 100644 --- a/google/cloud/bigtable/CMakeLists.txt +++ b/google/cloud/bigtable/CMakeLists.txt @@ -190,6 +190,8 @@ add_library( internal/endpoint_options.h internal/google_bytes_traits.cc internal/google_bytes_traits.h + internal/grpc_metrics_exporter.cc + internal/grpc_metrics_exporter.h internal/logging_result_set_reader.cc internal/logging_result_set_reader.h internal/metrics.cc @@ -285,7 +287,17 @@ target_compile_definitions(google_cloud_cpp_bigtable PRIVATE GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS) target_link_libraries(google_cloud_cpp_bigtable PUBLIC google-cloud-cpp::opentelemetry) -set(EXTRA_MODULES "google_cloud_cpp_opentelemetry" "opentelemetry_metrics") +if (TARGET gRPC::grpcpp_otel_plugin) + target_compile_definitions( + google_cloud_cpp_bigtable + PRIVATE GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS) + target_link_libraries(google_cloud_cpp_bigtable + PUBLIC gRPC::grpcpp_otel_plugin) + set(EXTRA_MODULES "google_cloud_cpp_opentelemetry" "grpcpp_otel_plugin" + "opentelemetry_metrics") +else () + set(EXTRA_MODULES "google_cloud_cpp_opentelemetry" "opentelemetry_metrics") +endif () google_cloud_cpp_add_common_options(google_cloud_cpp_bigtable) target_include_directories( google_cloud_cpp_bigtable @@ -460,6 +472,7 @@ if (BUILD_TESTING) internal/defaults_test.cc internal/dynamic_channel_pool_test.cc internal/google_bytes_traits_test.cc + internal/grpc_metrics_exporter_test.cc internal/logging_result_set_reader_test.cc internal/metrics_test.cc internal/mutate_rows_limiter_test.cc @@ -528,6 +541,11 @@ if (BUILD_TESTING) target_compile_definitions( ${target} PRIVATE GOOGLE_CLOUD_CPP_BIGTABLE_WITH_OTEL_METRICS) endif () + if (TARGET gRPC::grpcpp_otel_plugin) + target_compile_definitions( + ${target} + PRIVATE GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS) + endif () google_cloud_cpp_add_common_options(${target}) add_test(NAME ${target} COMMAND ${target}) endforeach () diff --git a/google/cloud/bigtable/bigtable_client_unit_tests.bzl b/google/cloud/bigtable/bigtable_client_unit_tests.bzl index 99c7af16cbaba..acb6542878348 100644 --- a/google/cloud/bigtable/bigtable_client_unit_tests.bzl +++ b/google/cloud/bigtable/bigtable_client_unit_tests.bzl @@ -55,6 +55,7 @@ bigtable_client_unit_tests = [ "internal/defaults_test.cc", "internal/dynamic_channel_pool_test.cc", "internal/google_bytes_traits_test.cc", + "internal/grpc_metrics_exporter_test.cc", "internal/logging_result_set_reader_test.cc", "internal/metrics_test.cc", "internal/mutate_rows_limiter_test.cc", diff --git a/google/cloud/bigtable/google_cloud_cpp_bigtable.bzl b/google/cloud/bigtable/google_cloud_cpp_bigtable.bzl index 3548270b00fa9..054d50d102fe4 100644 --- a/google/cloud/bigtable/google_cloud_cpp_bigtable.bzl +++ b/google/cloud/bigtable/google_cloud_cpp_bigtable.bzl @@ -94,6 +94,7 @@ google_cloud_cpp_bigtable_hdrs = [ "internal/dynamic_channel_pool.h", "internal/endpoint_options.h", "internal/google_bytes_traits.h", + "internal/grpc_metrics_exporter.h", "internal/logging_result_set_reader.h", "internal/metrics.h", "internal/mutate_rows_limiter.h", @@ -206,6 +207,7 @@ google_cloud_cpp_bigtable_srcs = [ "internal/default_row_reader.cc", "internal/defaults.cc", "internal/google_bytes_traits.cc", + "internal/grpc_metrics_exporter.cc", "internal/logging_result_set_reader.cc", "internal/metrics.cc", "internal/mutate_rows_limiter.cc", diff --git a/google/cloud/bigtable/internal/grpc_metrics_exporter.cc b/google/cloud/bigtable/internal/grpc_metrics_exporter.cc new file mode 100644 index 0000000000000..e0e86ee484348 --- /dev/null +++ b/google/cloud/bigtable/internal/grpc_metrics_exporter.cc @@ -0,0 +1,377 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "google/cloud/bigtable/internal/grpc_metrics_exporter.h" +#include "google/cloud/common_options.h" +#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS +#include "google/cloud/bigtable/internal/data_connection_impl.h" +#include "google/cloud/bigtable/options.h" +#include "google/cloud/bigtable/version.h" +#include "google/cloud/opentelemetry/internal/monitoring_exporter.h" +#include "google/cloud/opentelemetry/resource_detector.h" +#include "google/cloud/grpc_options.h" +#include "google/cloud/internal/algorithm.h" +#include "google/cloud/log.h" +#include "absl/strings/match.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" +#include +#include +#include +#include +#include +#include +#if OPENTELEMETRY_VERSION_MAJOR > 1 || \ + (OPENTELEMETRY_VERSION_MAJOR == 1 && OPENTELEMETRY_VERSION_MINOR >= 10) +#include +#include +#include +#include +#include +#endif // OPENTELEMETRY_VERSION_MAJOR > 1 || (OPENTELEMETRY_VERSION_MAJOR == 1 + // && OPENTELEMETRY_VERSION_MINOR >= 10) +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS + +namespace google { +namespace cloud { +namespace bigtable_internal { +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN + +GrpcMetricsExporterRegistry& GrpcMetricsExporterRegistry::Singleton() { + static auto* registry = new GrpcMetricsExporterRegistry; + return *registry; +} + +std::shared_ptr GrpcMetricsExporterRegistry::GetOrCreate( + std::shared_ptr const& conn, + Options const& options, std::string const& client_uid) { + std::unique_lock lk(mu_); + auto authority = options.get(); + auto it = exporters_.find(authority); + if (it != exporters_.end()) { + if (auto exporter = it->second.lock()) { + return exporter; + } + } + auto exporter = + std::make_shared(conn, options, client_uid); + exporters_[std::move(authority)] = exporter; + return exporter; +} + +void GrpcMetricsExporterRegistry::Unregister(std::string const& authority) { + std::unique_lock lk(mu_); + auto it = exporters_.find(authority); + if (it != exporters_.end() && it->second.expired()) { + exporters_.erase(it); + } +} + +void GrpcMetricsExporterRegistry::Clear() { + std::unique_lock lk(mu_); + exporters_.clear(); +} + +#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS + +std::vector MakeLatencyHistogramBoundaries() { + static auto const kBoundaries = [] { + using dseconds = std::chrono::duration>; + std::vector boundaries; + auto boundary = std::chrono::milliseconds(0); + auto increment = std::chrono::milliseconds(2); + for (int i = 0; i != 50; ++i) { + boundaries.push_back( + std::chrono::duration_cast(boundary).count()); + boundary += increment; + } + increment = std::chrono::milliseconds(10); + for (int i = 0; i != 150 && boundary <= std::chrono::minutes(5); ++i) { + boundaries.push_back( + std::chrono::duration_cast(boundary).count()); + if (i != 0 && i % 10 == 0) increment *= 2; + boundary += increment; + } + return boundaries; + }(); + return kBoundaries; +} + +namespace { + +void AddHistogramView(opentelemetry::sdk::metrics::MeterProvider& provider, + std::vector boundaries, std::string const& name, + std::string const& unit) { + auto constexpr kGrpcMeterName = "grpc-c++"; + auto constexpr kGrpcSchema = ""; + + auto histogram_aggregation_config = std::make_unique< + opentelemetry::sdk::metrics::HistogramAggregationConfig>(); + histogram_aggregation_config->boundaries_ = std::move(boundaries); + auto aggregation_config = + std::shared_ptr( + std::move(histogram_aggregation_config)); + + auto description = absl::StrCat("A view of ", name, + " with histogram boundaries more appropriate " + "for Google Cloud Bigtable RPCs"); + +#if OPENTELEMETRY_VERSION_MAJOR > 1 || \ + (OPENTELEMETRY_VERSION_MAJOR == 1 && OPENTELEMETRY_VERSION_MINOR >= 23) + (void)unit; + provider.AddView( + opentelemetry::sdk::metrics::InstrumentSelectorFactory::Create( + opentelemetry::sdk::metrics::InstrumentType::kHistogram, name, unit), + opentelemetry::sdk::metrics::MeterSelectorFactory::Create( + kGrpcMeterName, grpc::Version(), kGrpcSchema), + opentelemetry::sdk::metrics::ViewFactory::Create( + name, std::move(description), + opentelemetry::sdk::metrics::AggregationType::kHistogram, + std::move(aggregation_config))); +#elif OPENTELEMETRY_VERSION_MAJOR > 1 || \ + (OPENTELEMETRY_VERSION_MAJOR == 1 && OPENTELEMETRY_VERSION_MINOR >= 10) + provider.AddView( + opentelemetry::sdk::metrics::InstrumentSelectorFactory::Create( + opentelemetry::sdk::metrics::InstrumentType::kHistogram, name, unit), + opentelemetry::sdk::metrics::MeterSelectorFactory::Create( + kGrpcMeterName, grpc::Version(), kGrpcSchema), + opentelemetry::sdk::metrics::ViewFactory::Create( + name, std::move(description), unit, + opentelemetry::sdk::metrics::AggregationType::kHistogram, + std::move(aggregation_config))); +#else // OPENTELEMETRY_VERSION_MAJOR > 1 || (OPENTELEMETRY_VERSION_MAJOR == 1 + // && OPENTELEMETRY_VERSION_MINOR >= 10) + (void)unit; + provider.AddView( + std::make_unique( + opentelemetry::sdk::metrics::InstrumentType::kHistogram, name), + std::make_unique( + kGrpcMeterName, grpc::Version(), kGrpcSchema), + std::make_unique( + name, std::move(description), + opentelemetry::sdk::metrics::AggregationType::kHistogram, + std::move(aggregation_config))); +#endif // OPENTELEMETRY_VERSION_MAJOR > 1 || (OPENTELEMETRY_VERSION_MAJOR == 1 + // && OPENTELEMETRY_VERSION_MINOR >= 23) +} + +} // namespace + +std::shared_ptr MakeGrpcMeterProvider( + std::unique_ptr exporter, + opentelemetry::sdk::metrics::PeriodicExportingMetricReaderOptions + reader_options) { +#if OPENTELEMETRY_VERSION_MAJOR > 1 || \ + (OPENTELEMETRY_VERSION_MAJOR == 1 && OPENTELEMETRY_VERSION_MINOR >= 10) + auto provider = opentelemetry::sdk::metrics::MeterProviderFactory::Create( + std::make_unique(), + opentelemetry::sdk::resource::Resource::Create({})); +#else // OPENTELEMETRY_VERSION_MAJOR > 1 || (OPENTELEMETRY_VERSION_MAJOR == 1 + // && OPENTELEMETRY_VERSION_MINOR >= 10) + auto provider = std::make_unique( + std::make_unique(), + opentelemetry::sdk::resource::Resource::Create({})); +#endif // OPENTELEMETRY_VERSION_MAJOR > 1 || (OPENTELEMETRY_VERSION_MAJOR == 1 + // && OPENTELEMETRY_VERSION_MINOR >= 10) + auto* p = + static_cast(provider.get()); + AddHistogramView(*p, MakeLatencyHistogramBoundaries(), + "grpc.client.attempt.duration", "s"); + +#if OPENTELEMETRY_VERSION_MAJOR > 1 || OPENTELEMETRY_VERSION_MINOR >= 10 + p->AddMetricReader( + opentelemetry::sdk::metrics::PeriodicExportingMetricReaderFactory::Create( + std::move(exporter), std::move(reader_options))); +#else // OPENTELEMETRY_VERSION_MAJOR > 1 || OPENTELEMETRY_VERSION_MINOR >= 10 + p->AddMetricReader( + std::make_unique< + opentelemetry::sdk::metrics::PeriodicExportingMetricReader>( + std::move(exporter), std::move(reader_options))); +#endif // OPENTELEMETRY_VERSION_MAJOR > 1 || OPENTELEMETRY_VERSION_MINOR >= 10 + + return std::shared_ptr( + std::move(provider)); +} + +MonitoredResourceResult MakeMonitoredResource( + opentelemetry::sdk::metrics::PointDataAttributes const& pda, + opentelemetry::sdk::resource::Resource const& detected_resource, + Options const& options, std::string const& client_uid) { + namespace sc = ::opentelemetry::semconv; + google::api::MonitoredResource resource; + resource.set_type("bigtable.googleapis.com/Client"); + auto& labels = *resource.mutable_labels(); + auto const& attributes = pda.attributes.GetAttributes(); + auto get_attr = [&](std::string const& key) { + auto it = attributes.find(key); + if (it == attributes.end()) return std::string{}; + return opentelemetry::nostd::get(it->second); + }; + auto const& detected_attributes = detected_resource.GetAttributes(); + auto by_name = [&](std::string const& name, std::string default_value = {}) { + auto const l = detected_attributes.find(name); + if (l == detected_attributes.end()) return default_value; + return opentelemetry::nostd::get(l->second); + }; + + auto project_id = get_attr("project_id"); + // Fall back to resolving the project ID from `InstanceChannelAffinityOption` + // if the static `ProjectIdOption` is not set on the client (which is common + // in client configurations with instance routing / channel affinity). + if (project_id.empty() && + options.has()) { + auto const& instances = + options.get(); + if (!instances.empty()) { + project_id = instances[0].project_id(); + } + } + if (project_id.empty()) { + project_id = by_name(sc::cloud::kCloudAccountId); + } + + labels["project_id"] = project_id; + labels["instance"] = get_attr("instance"); + labels["app_profile"] = options.get(); + labels["client_name"] = "cpp.Bigtable/" + bigtable::version_string(); + labels["uuid"] = client_uid; + + auto client_project = by_name(sc::cloud::kCloudAccountId); + if (client_project.empty()) { + client_project = project_id; + } + if (!client_project.empty()) { + labels["client_project"] = client_project; + } + labels["location"] = by_name(sc::cloud::kCloudAvailabilityZone, + by_name(sc::cloud::kCloudRegion, "global")); + labels["cloud_platform"] = by_name(sc::cloud::kCloudPlatform, "unknown"); + labels["host_id"] = by_name("faas.id", by_name(sc::host::kHostId, "unknown")); + auto hostname = by_name(sc::host::kHostName); + if (!hostname.empty()) { + labels["hostname"] = hostname; + } + + return MonitoredResourceResult{std::move(project_id), std::move(resource)}; +} + +GrpcMetricsExporter::GrpcMetricsExporter( + std::shared_ptr const& conn, + Options const& options, std::string const& client_uid) + : authority_(options.get()) { + auto detector = otel::MakeResourceDetector(); + auto detected_resource = detector->Detect(); + + auto dynamic_resource_fn = + [options, client_uid, detected_resource = std::move(detected_resource)]( + opentelemetry::sdk::metrics::PointDataAttributes const& pda) { + auto res = + MakeMonitoredResource(pda, detected_resource, options, client_uid); + return std::make_pair(std::move(res.project_id), + std::move(res.resource)); + }; + + std::set excluded_labels{"project_id", "instance"}; + auto resource_filter_fn = + [excluded_labels = std::move(excluded_labels)](std::string const& key) { + return internal::Contains(excluded_labels, key); + }; + + auto exporter = otel_internal::MakeMonitoringExporter( + dynamic_resource_fn, resource_filter_fn, conn, options); + + auto reader_options = + opentelemetry::sdk::metrics::PeriodicExportingMetricReaderOptions{}; + // otel::PeriodicExportingMetricReader enforces that export_timeout_millis < + // export_interval_millis. + reader_options.export_interval_millis = + options.get(); + reader_options.export_timeout_millis = + (std::min)(std::chrono::milliseconds(std::chrono::seconds(30)), + reader_options.export_interval_millis / 2); + + provider_ = + MakeGrpcMeterProvider(std::move(exporter), std::move(reader_options)); + + auto const metrics = std::vector{ + absl::string_view{"grpc.client.attempt.duration"}, + absl::string_view{"grpc.lb.rls.default_target_picks"}, + absl::string_view{"grpc.lb.rls.target_picks"}, + absl::string_view{"grpc.lb.rls.failed_picks"}, + absl::string_view{"grpc.xds_client.server_failure"}, + absl::string_view{"grpc.xds_client.resource_updates_invalid"}, + absl::string_view{"grpc.subchannel.disconnections"}, + absl::string_view{"grpc.subchannel.connection_attempts_succeeded"}, + absl::string_view{"grpc.subchannel.connection_attempts_failed"}, + absl::string_view{"grpc.subchannel.open_connections"}, + }; + auto scope_filter = + [authority = authority_]( + grpc::OpenTelemetryPluginBuilder::ChannelScope const& scope) { + GCP_LOG(INFO) << "GrpcMetricsExporter: scope filter checking target=" + << scope.target() + << " default_authority=" << scope.default_authority() + << " vs expected authority=" << authority; + return scope.default_authority() == authority; + }; + auto status = + grpc::OpenTelemetryPluginBuilder() + .SetMeterProvider(provider_) + .EnableMetrics(metrics) + .SetGenericMethodAttributeFilter([](absl::string_view target) { + return absl::StartsWith(target, "google.bigtable.v2"); + }) + .SetChannelScopeFilter(std::move(scope_filter)) + .BuildAndRegisterGlobal(); + if (!status.ok()) { + GCP_LOG(ERROR) << "Cannot register provider status=" << status.ToString(); + } +} + +GrpcMetricsExporter::~GrpcMetricsExporter() { + if (provider_) { + auto* p = static_cast( + provider_.get()); + p->Shutdown(); + } + GrpcMetricsExporterRegistry::Singleton().Unregister(authority_); +} + +#else // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS + +GrpcMetricsExporter::GrpcMetricsExporter( + std::shared_ptr const&, + Options const&, std::string const&) {} + +GrpcMetricsExporter::~GrpcMetricsExporter() = default; + +#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS + +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END +} // namespace bigtable_internal +} // namespace cloud +} // namespace google diff --git a/google/cloud/bigtable/internal/grpc_metrics_exporter.h b/google/cloud/bigtable/internal/grpc_metrics_exporter.h new file mode 100644 index 0000000000000..13173207bc855 --- /dev/null +++ b/google/cloud/bigtable/internal/grpc_metrics_exporter.h @@ -0,0 +1,140 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_BIGTABLE_INTERNAL_GRPC_METRICS_EXPORTER_H +#define GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_BIGTABLE_INTERNAL_GRPC_METRICS_EXPORTER_H + +#include "google/cloud/options.h" +#include "google/cloud/version.h" +#include +#include +#include +#include + +#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS +#include "google/cloud/monitoring/v3/metric_connection.h" +#include "google/api/monitored_resource.pb.h" +#include +#include +#include +#include +#include +#include +#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS + +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 + +class GrpcMetricsExporter; + +/** + * Thread-safe registry managing the lifetime and deduplication of + * `GrpcMetricsExporter` instances. + * + * gRPC OpenTelemetry plugin registration (`BuildAndRegisterGlobal`) is + * process-global per target channel authority. If an exporter's lifetime were + * bound to an individual `DataConnectionImpl` without sharing, closing a single + * connection would destroy the `MeterProvider` and call `Shutdown()`, + * prematurely halting metric export globally for other active connections in + * the same process. + * + * Conversely, keeping static `std::shared_ptr` singletons permanently violates + * Google C++ Style Guide rules against static non-trivially destructible + * objects and prevents `MeterProvider::Shutdown()` from ever flushing metrics + * upon connection shutdown. + * + * `GrpcMetricsExporterRegistry` resolves both requirements: + * 1. **Deduplication / Sharing**: Tracks active `GrpcMetricsExporter` instances + * via `std::weak_ptr` keyed by channel authority. Multiple connection + * instances targeting the same authority share ownership of the same + * exporter `std::shared_ptr`. + * 2. **Clean Shutdown**: When all `DataConnectionImpl` instances sharing an + * exporter are closed, the last `std::shared_ptr` is destroyed, invoking + * `~GrpcMetricsExporter()` which cleanly calls `MeterProvider::Shutdown()` + * and unregisters the authority from the registry. + */ +class GrpcMetricsExporterRegistry { + public: + static GrpcMetricsExporterRegistry& Singleton(); + + std::shared_ptr GetOrCreate( + std::shared_ptr const& conn, + Options const& options, std::string const& client_uid); + + void Unregister(std::string const& authority); + + void Clear(); + + private: + GrpcMetricsExporterRegistry() = default; + + std::map> exporters_; + std::mutex mu_; +}; + +#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS +struct MonitoredResourceResult { + std::string project_id; + google::api::MonitoredResource resource; +}; + +MonitoredResourceResult MakeMonitoredResource( + opentelemetry::sdk::metrics::PointDataAttributes const& pda, + opentelemetry::sdk::resource::Resource const& detected_resource, + Options const& options, std::string const& client_uid); + +inline MonitoredResourceResult MakeMonitoredResource( + opentelemetry::sdk::metrics::PointDataAttributes const& pda, + Options const& options, std::string const& client_uid) { + return MakeMonitoredResource( + pda, opentelemetry::sdk::resource::Resource::Create({}), options, + client_uid); +} + +std::vector MakeLatencyHistogramBoundaries(); + +std::shared_ptr MakeGrpcMeterProvider( + std::unique_ptr exporter, + opentelemetry::sdk::metrics::PeriodicExportingMetricReaderOptions + reader_options); +#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS + +class GrpcMetricsExporter { + public: + GrpcMetricsExporter( + std::shared_ptr const& conn, + Options const& options, std::string const& client_uid); + ~GrpcMetricsExporter(); + + private: +#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS + std::string authority_; + std::shared_ptr provider_; +#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS +}; + +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END +} // namespace bigtable_internal +} // namespace cloud +} // namespace google + +#endif // GOOGLE_CLOUD_CPP_GOOGLE_CLOUD_BIGTABLE_INTERNAL_GRPC_METRICS_EXPORTER_H diff --git a/google/cloud/bigtable/internal/grpc_metrics_exporter_test.cc b/google/cloud/bigtable/internal/grpc_metrics_exporter_test.cc new file mode 100644 index 0000000000000..ce1ba5750a863 --- /dev/null +++ b/google/cloud/bigtable/internal/grpc_metrics_exporter_test.cc @@ -0,0 +1,420 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS + +#include "google/cloud/bigtable/internal/grpc_metrics_exporter.h" +#include "google/cloud/bigtable/options.h" +#include "google/cloud/bigtable/version.h" +#include "google/cloud/grpc_options.h" +#include "google/cloud/options.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace google { +namespace cloud { +namespace bigtable_internal { +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN +namespace { + +using ::testing::AllOf; +using ::testing::AtLeast; +using ::testing::Contains; +using ::testing::ElementsAre; +using ::testing::ElementsAreArray; +using ::testing::Eq; +using ::testing::Ge; +using ::testing::IsEmpty; +using ::testing::Le; +using ::testing::Not; +using ::testing::ResultOf; +using ::testing::Return; +using ::testing::SizeIs; +using ::testing::VariantWith; + +class MockPushMetricExporter + : public opentelemetry::sdk::metrics::PushMetricExporter { + public: + // NOLINTBEGIN(bugprone-exception-escape) + MOCK_METHOD(opentelemetry::sdk::common::ExportResult, Export, + (opentelemetry::sdk::metrics::ResourceMetrics const&), + (noexcept, override)); + + MOCK_METHOD(opentelemetry::sdk::metrics::AggregationTemporality, + GetAggregationTemporality, + (opentelemetry::sdk::metrics::InstrumentType), + (const, noexcept, override)); + + MOCK_METHOD(bool, ForceFlush, (std::chrono::microseconds), + (noexcept, override)); + MOCK_METHOD(bool, Shutdown, (std::chrono::microseconds), + (noexcept, override)); + // NOLINTEND(bugprone-exception-escape) +}; + +auto constexpr kDurationMetric = "grpc.client.attempt.duration"; + +auto MatchesLatencyBoundaries() { + return ResultOf( + "boundaries are latency boundaries", + [](opentelemetry::sdk::metrics::HistogramPointData const& hpd) { + return hpd.boundaries_; + }, + ElementsAreArray(MakeLatencyHistogramBoundaries())); +} + +auto ExpectedLatencyHistogram() { + return ResultOf( + "data is histogram with right boundaries", + [](opentelemetry::sdk::metrics::PointDataAttributes const& pda) { + return pda.point_data; + }, + VariantWith( + MatchesLatencyBoundaries())); +} + +template +auto WithPointData(Matcher&& matcher) { + return ResultOf( + "instrument descriptor name", + [](opentelemetry::sdk::metrics::MetricData const& md) { + return md.point_data_attr_; + }, + std::forward(matcher)); +} + +template +auto WithMetricsData(Matcher&& matcher) { + return ResultOf( + "metric_data_", + [](opentelemetry::sdk::metrics::ScopeMetrics const& sm) { + return sm.metric_data_; + }, + std::forward(matcher)); +} + +auto MatchesInstrumentName(std::string name) { + return ResultOf( + "instrument descriptor name", + [](opentelemetry::sdk::metrics::MetricData const& md) { + return md.instrument_descriptor.name_; + }, + std::move(name)); +} + +auto MetricDataEmpty() { + return ResultOf( + "scope_metric_data_", + [](opentelemetry::sdk::metrics::ResourceMetrics const& data) { + return data.scope_metric_data_; + }, + IsEmpty()); +} + +auto MetricDataNotEmpty() { + return ResultOf( + "scope_metric_data_", + [](opentelemetry::sdk::metrics::ResourceMetrics const& data) { + return data.scope_metric_data_; + }, + Not(IsEmpty())); +} + +auto constexpr kExportInterval = std::chrono::milliseconds(50); +auto constexpr kExportTimeout = std::chrono::milliseconds(25); + +auto TestReaderOptions() { + opentelemetry::sdk::metrics::PeriodicExportingMetricReaderOptions + reader_options; + reader_options.export_interval_millis = kExportInterval; + reader_options.export_timeout_millis = kExportTimeout; + return reader_options; +} + +class DummyMetricServiceConnection + : public monitoring_v3::MetricServiceConnection { + public: + ~DummyMetricServiceConnection() override = default; +}; + +TEST(GrpcMetricsExporterRegistryTest, GetOrCreateAndLifecycle) { + auto& registry = GrpcMetricsExporterRegistry::Singleton(); + registry.Clear(); + + auto conn = std::make_shared(); + + Options options1; + options1.set("test-authority-1"); + Options options2; + options2.set("test-authority-2"); + + auto exporter1_a = registry.GetOrCreate(conn, options1, "client-1"); + ASSERT_NE(exporter1_a, nullptr); + + // Second request for same authority returns the existing shared instance + auto exporter1_b = registry.GetOrCreate(conn, options1, "client-2"); + EXPECT_EQ(exporter1_a, exporter1_b); + + // Different authority gets a different instance + auto exporter2 = registry.GetOrCreate(conn, options2, "client-3"); + ASSERT_NE(exporter2, nullptr); + EXPECT_NE(exporter1_a, exporter2); + + // When all references to authority 1 are dropped, destructor runs and + // authority is unregistered + exporter1_a.reset(); + exporter1_b.reset(); + + // A new request for authority 1 creates a new instance + auto exporter1_c = registry.GetOrCreate(conn, options1, "client-4"); + ASSERT_NE(exporter1_c, nullptr); + EXPECT_NE(exporter1_c, exporter2); +} + +TEST(GrpcMetricsExporterTest, MakeMonitoredResource) { + Options options; + options.set("test-app-profile"); + std::string const client_uid = "test-client-uid"; + + opentelemetry::sdk::metrics::PointDataAttributes pda; + pda.attributes = opentelemetry::sdk::metrics::PointAttributes({ + {"project_id", "test-project"}, + {"instance", "test-instance"}, + }); + + auto result = MakeMonitoredResource(pda, options, client_uid); + EXPECT_THAT(result.project_id, Eq("test-project")); + + auto const& resource = result.resource; + EXPECT_THAT(resource.type(), Eq("bigtable.googleapis.com/Client")); + + auto const& labels = resource.labels(); + EXPECT_THAT(labels.at("project_id"), Eq("test-project")); + EXPECT_THAT(labels.at("client_project"), Eq("test-project")); + EXPECT_THAT(labels.at("instance"), Eq("test-instance")); + EXPECT_THAT(labels.at("app_profile"), Eq("test-app-profile")); + EXPECT_THAT(labels.at("client_name"), + Eq("cpp.Bigtable/" + bigtable::version_string())); + EXPECT_THAT(labels.at("uuid"), Eq("test-client-uid")); + EXPECT_THAT(labels.at("location"), Eq("global")); + EXPECT_THAT(labels.at("cloud_platform"), Eq("unknown")); + EXPECT_THAT(labels.at("host_id"), Eq("unknown")); +} + +TEST(GrpcMetricsExporterTest, MakeMonitoredResourceMissingAttributes) { + Options options; + std::string const client_uid = "test-client-uid"; + + opentelemetry::sdk::metrics::PointDataAttributes pda; + pda.attributes = opentelemetry::sdk::metrics::PointAttributes({}); + + auto result = MakeMonitoredResource(pda, options, client_uid); + EXPECT_THAT(result.project_id, Eq("")); + + auto const& resource = result.resource; + EXPECT_THAT(resource.type(), Eq("bigtable.googleapis.com/Client")); + + auto const& labels = resource.labels(); + EXPECT_THAT(labels.at("project_id"), Eq("")); + EXPECT_THAT(labels.at("instance"), Eq("")); + EXPECT_THAT(labels.at("app_profile"), Eq("")); + EXPECT_THAT(labels.at("client_name"), + Eq("cpp.Bigtable/" + bigtable::version_string())); + EXPECT_THAT(labels.at("uuid"), Eq("test-client-uid")); + EXPECT_THAT(labels.at("location"), Eq("global")); + EXPECT_THAT(labels.at("cloud_platform"), Eq("unknown")); + EXPECT_THAT(labels.at("host_id"), Eq("unknown")); +} + +TEST(GrpcMetricsExporterTest, MakeMonitoredResourcePartialAttributes) { + Options options; + std::string const client_uid = "test-client-uid"; + + opentelemetry::sdk::metrics::PointDataAttributes pda; + pda.attributes = opentelemetry::sdk::metrics::PointAttributes({ + {"project_id", "test-project"}, + }); + + auto result = MakeMonitoredResource(pda, options, client_uid); + EXPECT_THAT(result.project_id, Eq("test-project")); + + auto const& resource = result.resource; + EXPECT_THAT(resource.type(), Eq("bigtable.googleapis.com/Client")); + + auto const& labels = resource.labels(); + EXPECT_THAT(labels.at("project_id"), Eq("test-project")); + EXPECT_THAT(labels.at("client_project"), Eq("test-project")); + EXPECT_THAT(labels.at("instance"), Eq("")); + EXPECT_THAT(labels.at("app_profile"), Eq("")); + EXPECT_THAT(labels.at("client_name"), + Eq("cpp.Bigtable/" + bigtable::version_string())); + EXPECT_THAT(labels.at("uuid"), Eq("test-client-uid")); + EXPECT_THAT(labels.at("location"), Eq("global")); + EXPECT_THAT(labels.at("cloud_platform"), Eq("unknown")); + EXPECT_THAT(labels.at("host_id"), Eq("unknown")); +} + +TEST(GrpcMetricsExporterTest, MakeMonitoredResourceExtraAttributes) { + Options options; + std::string const client_uid = "test-client-uid"; + + opentelemetry::sdk::metrics::PointDataAttributes pda; + pda.attributes = opentelemetry::sdk::metrics::PointAttributes({ + {"project_id", "test-project"}, + {"instance", "test-instance"}, + {"grpc.method", "google.bigtable.v2.Bigtable/ReadRows"}, + {"grpc.status", "OK"}, + }); + + auto result = MakeMonitoredResource(pda, options, client_uid); + EXPECT_THAT(result.project_id, Eq("test-project")); + + auto const& resource = result.resource; + EXPECT_THAT(resource.type(), Eq("bigtable.googleapis.com/Client")); + + auto const& labels = resource.labels(); + EXPECT_THAT(labels.size(), Eq(9)); + EXPECT_THAT(labels.at("project_id"), Eq("test-project")); + EXPECT_THAT(labels.at("client_project"), Eq("test-project")); + EXPECT_THAT(labels.at("instance"), Eq("test-instance")); + EXPECT_THAT(labels.at("app_profile"), Eq("")); + EXPECT_THAT(labels.at("client_name"), + Eq("cpp.Bigtable/" + bigtable::version_string())); + EXPECT_THAT(labels.at("uuid"), Eq("test-client-uid")); + EXPECT_THAT(labels.at("location"), Eq("global")); + EXPECT_THAT(labels.at("cloud_platform"), Eq("unknown")); + EXPECT_THAT(labels.at("host_id"), Eq("unknown")); + EXPECT_THAT(labels.find("grpc.method"), Eq(labels.end())); + EXPECT_THAT(labels.find("grpc.status"), Eq(labels.end())); +} + +TEST(GrpcMetricsExporterTest, MakeMonitoredResourceWithDetectedResource) { + namespace sc = ::opentelemetry::semconv; + Options options; + options.set("test-app-profile"); + std::string const client_uid = "test-client-uid"; + + opentelemetry::sdk::metrics::PointDataAttributes pda; + pda.attributes = opentelemetry::sdk::metrics::PointAttributes({ + {"project_id", "test-project"}, + {"instance", "test-instance"}, + }); + + auto detected_resource = opentelemetry::sdk::resource::Resource::Create({ + {sc::cloud::kCloudAccountId, "detected-vm-project"}, + {sc::cloud::kCloudPlatform, "gcp_compute_engine"}, + {sc::cloud::kCloudAvailabilityZone, "us-central1-a"}, + {sc::host::kHostId, "123456789"}, + {sc::host::kHostName, "test-vm-host"}, + }); + + auto result = + MakeMonitoredResource(pda, detected_resource, options, client_uid); + EXPECT_THAT(result.project_id, Eq("test-project")); + + auto const& resource = result.resource; + EXPECT_THAT(resource.type(), Eq("bigtable.googleapis.com/Client")); + + auto const& labels = resource.labels(); + EXPECT_THAT(labels.at("project_id"), Eq("test-project")); + EXPECT_THAT(labels.at("client_project"), Eq("detected-vm-project")); + EXPECT_THAT(labels.at("instance"), Eq("test-instance")); + EXPECT_THAT(labels.at("app_profile"), Eq("test-app-profile")); + EXPECT_THAT(labels.at("client_name"), + Eq("cpp.Bigtable/" + bigtable::version_string())); + EXPECT_THAT(labels.at("uuid"), Eq("test-client-uid")); + EXPECT_THAT(labels.at("location"), Eq("us-central1-a")); + EXPECT_THAT(labels.at("cloud_platform"), Eq("gcp_compute_engine")); + EXPECT_THAT(labels.at("host_id"), Eq("123456789")); + EXPECT_THAT(labels.at("hostname"), Eq("test-vm-host")); +} + +TEST(GrpcMetricsExporterTest, MakeLatencyHistogramBoundaries) { + auto const boundaries = MakeLatencyHistogramBoundaries(); + ASSERT_THAT(boundaries, Not(IsEmpty())); + ASSERT_THAT(boundaries, SizeIs(Le(200U))); + auto sorted = boundaries; + std::sort(sorted.begin(), sorted.end()); + EXPECT_THAT(boundaries, ElementsAreArray(sorted.begin(), sorted.end())); + std::vector diff; + std::adjacent_difference(boundaries.begin(), boundaries.end(), + std::back_inserter(diff)); + ASSERT_THAT(diff, Not(IsEmpty())); + EXPECT_THAT(*std::min_element(std::next(diff.begin()), diff.end()), + Ge(0.001)); + ASSERT_THAT(boundaries.back(), Le(300)); +} + +TEST(GrpcMetricsExporterTest, ValidateGrpcClientAttemptDuration) { + std::atomic export_count{0}; + auto mock = std::make_unique(); + EXPECT_CALL(*mock, Shutdown).WillOnce(Return(true)); + EXPECT_CALL(*mock, GetAggregationTemporality) + .WillRepeatedly(Return( + opentelemetry::sdk::metrics::AggregationTemporality::kCumulative)); + EXPECT_CALL(*mock, Export(MetricDataEmpty())) + .WillRepeatedly( + Return(opentelemetry::sdk::common::ExportResult::kSuccess)); + EXPECT_CALL(*mock, Export(MetricDataNotEmpty())) + .Times(AtLeast(1)) + .WillRepeatedly( + [&export_count]( + opentelemetry::sdk::metrics::ResourceMetrics const& data) { + EXPECT_THAT( + data.scope_metric_data_, + Contains(WithMetricsData(Contains(AllOf( + MatchesInstrumentName(kDurationMetric), + WithPointData(ElementsAre(ExpectedLatencyHistogram()))))))); + ++export_count; + return opentelemetry::sdk::common::ExportResult::kSuccess; + }); + + { + auto provider = MakeGrpcMeterProvider(std::move(mock), TestReaderOptions()); + auto meter = provider->GetMeter("grpc-c++", grpc::Version()); + auto histogram = meter->CreateDoubleHistogram(kDurationMetric, + "test-only-description", "s"); + for (int i = 0; i != 50 && export_count.load() == 0; ++i) { + histogram->Record(1.0, opentelemetry::context::Context{}); + std::this_thread::sleep_for(kExportInterval); + } + } +} + +} // namespace +GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END +} // namespace bigtable_internal +} // namespace cloud +} // namespace google + +#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS From 92192f1331bdc436a4ab2ffe6fa651d21d53f920 Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Wed, 29 Jul 2026 16:02:55 -0400 Subject: [PATCH 2/4] refactor registry to similar mechanism used in storage --- .../internal/grpc_metrics_exporter.cc | 57 ++++-------------- .../bigtable/internal/grpc_metrics_exporter.h | 60 +++++-------------- .../internal/grpc_metrics_exporter_test.cc | 36 +++-------- 3 files changed, 36 insertions(+), 117 deletions(-) diff --git a/google/cloud/bigtable/internal/grpc_metrics_exporter.cc b/google/cloud/bigtable/internal/grpc_metrics_exporter.cc index e0e86ee484348..0f28846d21f1e 100644 --- a/google/cloud/bigtable/internal/grpc_metrics_exporter.cc +++ b/google/cloud/bigtable/internal/grpc_metrics_exporter.cc @@ -66,34 +66,14 @@ GrpcMetricsExporterRegistry& GrpcMetricsExporterRegistry::Singleton() { return *registry; } -std::shared_ptr GrpcMetricsExporterRegistry::GetOrCreate( - std::shared_ptr const& conn, - Options const& options, std::string const& client_uid) { +bool GrpcMetricsExporterRegistry::Register(std::string authority) { std::unique_lock lk(mu_); - auto authority = options.get(); - auto it = exporters_.find(authority); - if (it != exporters_.end()) { - if (auto exporter = it->second.lock()) { - return exporter; - } - } - auto exporter = - std::make_shared(conn, options, client_uid); - exporters_[std::move(authority)] = exporter; - return exporter; -} - -void GrpcMetricsExporterRegistry::Unregister(std::string const& authority) { - std::unique_lock lk(mu_); - auto it = exporters_.find(authority); - if (it != exporters_.end() && it->second.expired()) { - exporters_.erase(it); - } + return known_authorities_.insert(std::move(authority)).second; } void GrpcMetricsExporterRegistry::Clear() { std::unique_lock lk(mu_); - exporters_.clear(); + known_authorities_.clear(); } #ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS @@ -279,10 +259,12 @@ MonitoredResourceResult MakeMonitoredResource( return MonitoredResourceResult{std::move(project_id), std::move(resource)}; } -GrpcMetricsExporter::GrpcMetricsExporter( +void EnableGrpcMetrics( std::shared_ptr const& conn, - Options const& options, std::string const& client_uid) - : authority_(options.get()) { + Options const& options, std::string const& client_uid) { + auto authority = options.get(); + if (!GrpcMetricsExporterRegistry::Singleton().Register(authority)) return; + auto detector = otel::MakeResourceDetector(); auto detected_resource = detector->Detect(); @@ -314,7 +296,7 @@ GrpcMetricsExporter::GrpcMetricsExporter( (std::min)(std::chrono::milliseconds(std::chrono::seconds(30)), reader_options.export_interval_millis / 2); - provider_ = + auto provider = MakeGrpcMeterProvider(std::move(exporter), std::move(reader_options)); auto const metrics = std::vector{ @@ -330,7 +312,7 @@ GrpcMetricsExporter::GrpcMetricsExporter( absl::string_view{"grpc.subchannel.open_connections"}, }; auto scope_filter = - [authority = authority_]( + [authority = std::move(authority)]( grpc::OpenTelemetryPluginBuilder::ChannelScope const& scope) { GCP_LOG(INFO) << "GrpcMetricsExporter: scope filter checking target=" << scope.target() @@ -340,7 +322,7 @@ GrpcMetricsExporter::GrpcMetricsExporter( }; auto status = grpc::OpenTelemetryPluginBuilder() - .SetMeterProvider(provider_) + .SetMeterProvider(provider) .EnableMetrics(metrics) .SetGenericMethodAttributeFilter([](absl::string_view target) { return absl::StartsWith(target, "google.bigtable.v2"); @@ -352,23 +334,6 @@ GrpcMetricsExporter::GrpcMetricsExporter( } } -GrpcMetricsExporter::~GrpcMetricsExporter() { - if (provider_) { - auto* p = static_cast( - provider_.get()); - p->Shutdown(); - } - GrpcMetricsExporterRegistry::Singleton().Unregister(authority_); -} - -#else // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS - -GrpcMetricsExporter::GrpcMetricsExporter( - std::shared_ptr const&, - Options const&, std::string const&) {} - -GrpcMetricsExporter::~GrpcMetricsExporter() = default; - #endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END diff --git a/google/cloud/bigtable/internal/grpc_metrics_exporter.h b/google/cloud/bigtable/internal/grpc_metrics_exporter.h index 13173207bc855..ca8833b2049fb 100644 --- a/google/cloud/bigtable/internal/grpc_metrics_exporter.h +++ b/google/cloud/bigtable/internal/grpc_metrics_exporter.h @@ -17,9 +17,9 @@ #include "google/cloud/options.h" #include "google/cloud/version.h" -#include #include #include +#include #include #ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS @@ -44,50 +44,28 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END namespace bigtable_internal { GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN -class GrpcMetricsExporter; - /** - * Thread-safe registry managing the lifetime and deduplication of - * `GrpcMetricsExporter` instances. + * Thread-safe registry tracking registered channel authorities for gRPC + * OpenTelemetry metrics. * * gRPC OpenTelemetry plugin registration (`BuildAndRegisterGlobal`) is - * process-global per target channel authority. If an exporter's lifetime were - * bound to an individual `DataConnectionImpl` without sharing, closing a single - * connection would destroy the `MeterProvider` and call `Shutdown()`, - * prematurely halting metric export globally for other active connections in - * the same process. - * - * Conversely, keeping static `std::shared_ptr` singletons permanently violates - * Google C++ Style Guide rules against static non-trivially destructible - * objects and prevents `MeterProvider::Shutdown()` from ever flushing metrics - * upon connection shutdown. - * - * `GrpcMetricsExporterRegistry` resolves both requirements: - * 1. **Deduplication / Sharing**: Tracks active `GrpcMetricsExporter` instances - * via `std::weak_ptr` keyed by channel authority. Multiple connection - * instances targeting the same authority share ownership of the same - * exporter `std::shared_ptr`. - * 2. **Clean Shutdown**: When all `DataConnectionImpl` instances sharing an - * exporter are closed, the last `std::shared_ptr` is destroyed, invoking - * `~GrpcMetricsExporter()` which cleanly calls `MeterProvider::Shutdown()` - * and unregisters the authority from the registry. + * process-global per target channel authority. Once registered, gRPC maintains + * the MeterProvider globally for the lifetime of the process. */ class GrpcMetricsExporterRegistry { public: static GrpcMetricsExporterRegistry& Singleton(); - std::shared_ptr GetOrCreate( - std::shared_ptr const& conn, - Options const& options, std::string const& client_uid); - - void Unregister(std::string const& authority); + // Returns `true` if @p authority is newly registered, `false` if @p authority + // was already registered. + bool Register(std::string authority); void Clear(); private: GrpcMetricsExporterRegistry() = default; - std::map> exporters_; + std::set known_authorities_; std::mutex mu_; }; @@ -116,21 +94,15 @@ std::shared_ptr MakeGrpcMeterProvider( std::unique_ptr exporter, opentelemetry::sdk::metrics::PeriodicExportingMetricReaderOptions reader_options); -#endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS -class GrpcMetricsExporter { - public: - GrpcMetricsExporter( - std::shared_ptr const& conn, - Options const& options, std::string const& client_uid); - ~GrpcMetricsExporter(); - - private: -#ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS - std::string authority_; - std::shared_ptr provider_; +void EnableGrpcMetrics( + std::shared_ptr const& conn, + Options const& options, std::string const& client_uid); +#else +inline void EnableGrpcMetrics( + std::shared_ptr const&, + Options const&, std::string const&) {} #endif // GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS -}; GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace bigtable_internal diff --git a/google/cloud/bigtable/internal/grpc_metrics_exporter_test.cc b/google/cloud/bigtable/internal/grpc_metrics_exporter_test.cc index ce1ba5750a863..2d8c670a62aa1 100644 --- a/google/cloud/bigtable/internal/grpc_metrics_exporter_test.cc +++ b/google/cloud/bigtable/internal/grpc_metrics_exporter_test.cc @@ -167,38 +167,20 @@ class DummyMetricServiceConnection ~DummyMetricServiceConnection() override = default; }; -TEST(GrpcMetricsExporterRegistryTest, GetOrCreateAndLifecycle) { +TEST(GrpcMetricsExporterRegistryTest, RegisterAndDeduplicate) { auto& registry = GrpcMetricsExporterRegistry::Singleton(); registry.Clear(); - auto conn = std::make_shared(); + EXPECT_TRUE(registry.Register("test-authority-1")); + // Second attempt for the same authority returns false + EXPECT_FALSE(registry.Register("test-authority-1")); - Options options1; - options1.set("test-authority-1"); - Options options2; - options2.set("test-authority-2"); + // Different authority returns true + EXPECT_TRUE(registry.Register("test-authority-2")); - auto exporter1_a = registry.GetOrCreate(conn, options1, "client-1"); - ASSERT_NE(exporter1_a, nullptr); - - // Second request for same authority returns the existing shared instance - auto exporter1_b = registry.GetOrCreate(conn, options1, "client-2"); - EXPECT_EQ(exporter1_a, exporter1_b); - - // Different authority gets a different instance - auto exporter2 = registry.GetOrCreate(conn, options2, "client-3"); - ASSERT_NE(exporter2, nullptr); - EXPECT_NE(exporter1_a, exporter2); - - // When all references to authority 1 are dropped, destructor runs and - // authority is unregistered - exporter1_a.reset(); - exporter1_b.reset(); - - // A new request for authority 1 creates a new instance - auto exporter1_c = registry.GetOrCreate(conn, options1, "client-4"); - ASSERT_NE(exporter1_c, nullptr); - EXPECT_NE(exporter1_c, exporter2); + registry.Clear(); + // After Clear(), authority can be registered again + EXPECT_TRUE(registry.Register("test-authority-1")); } TEST(GrpcMetricsExporterTest, MakeMonitoredResource) { From c665869efdabadfa9c1395bc7600ee3429d15a76 Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Wed, 29 Jul 2026 16:13:58 -0400 Subject: [PATCH 3/4] address more review comments --- .../internal/grpc_metrics_exporter.cc | 20 ++++++++++++------- .../bigtable/internal/grpc_metrics_exporter.h | 2 +- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/google/cloud/bigtable/internal/grpc_metrics_exporter.cc b/google/cloud/bigtable/internal/grpc_metrics_exporter.cc index 0f28846d21f1e..6e86b93ad3dc6 100644 --- a/google/cloud/bigtable/internal/grpc_metrics_exporter.cc +++ b/google/cloud/bigtable/internal/grpc_metrics_exporter.cc @@ -78,7 +78,7 @@ void GrpcMetricsExporterRegistry::Clear() { #ifdef GOOGLE_CLOUD_CPP_BIGTABLE_WITH_GRPC_OTEL_METRICS -std::vector MakeLatencyHistogramBoundaries() { +std::vector const& MakeLatencyHistogramBoundaries() { static auto const kBoundaries = [] { using dseconds = std::chrono::duration>; std::vector boundaries; @@ -208,13 +208,19 @@ MonitoredResourceResult MakeMonitoredResource( auto const& attributes = pda.attributes.GetAttributes(); auto get_attr = [&](std::string const& key) { auto it = attributes.find(key); - if (it == attributes.end()) return std::string{}; + if (it == attributes.end() || + !opentelemetry::nostd::holds_alternative(it->second)) { + return std::string{}; + } return opentelemetry::nostd::get(it->second); }; auto const& detected_attributes = detected_resource.GetAttributes(); auto by_name = [&](std::string const& name, std::string default_value = {}) { auto const l = detected_attributes.find(name); - if (l == detected_attributes.end()) return default_value; + if (l == detected_attributes.end() || + !opentelemetry::nostd::holds_alternative(l->second)) { + return default_value; + } return opentelemetry::nostd::get(l->second); }; @@ -314,10 +320,10 @@ void EnableGrpcMetrics( auto scope_filter = [authority = std::move(authority)]( grpc::OpenTelemetryPluginBuilder::ChannelScope const& scope) { - GCP_LOG(INFO) << "GrpcMetricsExporter: scope filter checking target=" - << scope.target() - << " default_authority=" << scope.default_authority() - << " vs expected authority=" << authority; + GCP_LOG(DEBUG) << "GrpcMetricsExporter: scope filter checking target=" + << scope.target() + << " default_authority=" << scope.default_authority() + << " vs expected authority=" << authority; return scope.default_authority() == authority; }; auto status = diff --git a/google/cloud/bigtable/internal/grpc_metrics_exporter.h b/google/cloud/bigtable/internal/grpc_metrics_exporter.h index ca8833b2049fb..8ab268b7c2fb1 100644 --- a/google/cloud/bigtable/internal/grpc_metrics_exporter.h +++ b/google/cloud/bigtable/internal/grpc_metrics_exporter.h @@ -88,7 +88,7 @@ inline MonitoredResourceResult MakeMonitoredResource( client_uid); } -std::vector MakeLatencyHistogramBoundaries(); +std::vector const& MakeLatencyHistogramBoundaries(); std::shared_ptr MakeGrpcMeterProvider( std::unique_ptr exporter, From b3a723f3883b9a3024b558eb4301323a148e0534 Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Wed, 29 Jul 2026 16:32:20 -0400 Subject: [PATCH 4/4] clang tidy fix --- google/cloud/bigtable/internal/grpc_metrics_exporter_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/cloud/bigtable/internal/grpc_metrics_exporter_test.cc b/google/cloud/bigtable/internal/grpc_metrics_exporter_test.cc index 2d8c670a62aa1..a9a9ab2c933cd 100644 --- a/google/cloud/bigtable/internal/grpc_metrics_exporter_test.cc +++ b/google/cloud/bigtable/internal/grpc_metrics_exporter_test.cc @@ -342,7 +342,7 @@ TEST(GrpcMetricsExporterTest, MakeMonitoredResourceWithDetectedResource) { } TEST(GrpcMetricsExporterTest, MakeLatencyHistogramBoundaries) { - auto const boundaries = MakeLatencyHistogramBoundaries(); + auto const& boundaries = MakeLatencyHistogramBoundaries(); ASSERT_THAT(boundaries, Not(IsEmpty())); ASSERT_THAT(boundaries, SizeIs(Le(200U))); auto sorted = boundaries;