impl(bigtable): export grpc metrics

[scotthart]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces a GrpcMetricsExporter to export gRPC OpenTelemetry metrics for Google Cloud Bigtable, integrating it into the DataConnectionImpl and adding corresponding build configurations and unit tests. The review feedback highlights several key improvements: enforcing the singleton pattern for GrpcMetricsExporterRegistry by making its constructor private, optimizing latency boundary generation via static caching, removing an unused default parameter in an internal namespace lambda to align with the style guide, and resolving a potential use-after-move/evaluation-order issue when constructing MonitoredResourceResult.

[gemini-code-assist]

Since GrpcMetricsExporterRegistry is a singleton accessed via Singleton(), its constructor should be private to prevent external instantiation and enforce the singleton pattern.

class GrpcMetricsExporterRegistry {
 public:
  static GrpcMetricsExporterRegistry& Singleton();

  // Returns true if authority is newly registered, false if it was already
  // registered.
  bool Register(std::string authority);

  void Clear();

 private:
  GrpcMetricsExporterRegistry() = default;

  std::set<std::string> known_authority_;
  std::mutex mu_;
};

[gemini-code-assist]

To avoid re-computation on each function call, cache the result in a static variable. This one-time cost is often preferable to repeated computation, even for seemingly quick operations.

std::vector<double> MakeLatencyHistogramBoundaries() {
  static auto const boundaries = [] {
    using dseconds = std::chrono::duration<double, std::ratio<1>>;
    std::vector<double> 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<dseconds>(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<dseconds>(boundary).count());
      if (i != 0 && i % 10 == 0) increment *= 2;
      boundary += increment;
    }
    return boundaries;
  }();
  return boundaries;
}

References

1. To avoid re-computation on each function call, cache the result in a static variable. This one-time cost is often preferable to repeated computation, even for seemingly quick operations.

[gemini-code-assist]

The default parameter fallback is never used with a non-default value. Removing it simplifies the lambda and adheres to the guideline discouraging default parameters in internal namespaces.

  auto get_attr = [&](std::string const& key) {
    auto it = attributes.find(key);
    if (it == attributes.end()) return std::string{};
    return opentelemetry::nostd::get<std::string>(it->second);
  };

References

1. Discourage default parameters for functions in an internal namespace. ^(link)

[gemini-code-assist]

Accessing labels["project_id"] and then moving resource in the same initializer list can lead to potential use-after-move or unspecified order of evaluation issues. Extracting project_id first and moving it avoids copying and ensures safety.

  auto project_id = get_attr("project_id");
  labels["project_id"] = project_id;
  labels["instance"] = get_attr("instance");
  labels["app_profile"] = options.get<bigtable::AppProfileIdOption>();
  labels["client_name"] = "cpp.Bigtable/" + bigtable::version_string();
  labels["uuid"] = client_uid;
  return MonitoredResourceResult{std::move(project_id), std::move(resource)};

[codecov]

Codecov Report

❌ Patch coverage is 96.45390% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.27%. Comparing base (9130810) to head (2926f28).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
...e/cloud/bigtable/internal/grpc_metrics_exporter.cc	93.39%	7 Missing ⚠️
...ud/bigtable/internal/grpc_metrics_exporter_test.cc	98.22%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #16241      +/-   ##
==========================================
+ Coverage   92.24%   92.27%   +0.02%     
==========================================
  Files        2265     2217      -48     
  Lines      210223   206674    -3549     
==========================================
- Hits       193928   190704    -3224     
+ Misses      16295    15970     -325     

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