Skip to content

impl(bigtable): add outstanding_rpcs client schema metric - #16360

Merged
scotthart merged 3 commits into
googleapis:mainfrom
scotthart:bigtable_client_schema_metrics_1
Aug 20, 2026
Merged

impl(bigtable): add outstanding_rpcs client schema metric#16360
scotthart merged 3 commits into
googleapis:mainfrom
scotthart:bigtable_client_schema_metrics_1

Conversation

@scotthart

Copy link
Copy Markdown
Member

This PR adds the first client schema metric and the event (StubSelection) it records telemetry on.

@scotthart
scotthart requested a review from a team as a code owner August 20, 2026 16:14
@product-auto-label product-auto-label Bot added the api: bigtable Issues related to the Bigtable API. label Aug 20, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces client schema metrics for Google Cloud Bigtable, specifically tracking outstanding RPCs using OpenTelemetry. It adds the client_schema_metrics implementation, headers, and unit tests, while integrating stub selection tracking into the OperationContext and Metric classes. Additionally, it refactors some loops in OperationContext to use C++17 structured bindings. Feedback on the pull request focuses on performance optimizations and code simplicity, such as returning char const* instead of std::string from static helper functions, simplifying label filtering to avoid complex abstractions, preventing unconditional fallback allocations in by_name calls, and using std::string_view in metadata processing to avoid unnecessary heap allocations.

Comment thread google/cloud/bigtable/internal/client_schema_metrics.cc Outdated
Comment thread google/cloud/bigtable/internal/client_schema_metrics.cc Outdated
Comment thread google/cloud/bigtable/internal/client_schema_metrics.cc Outdated
Comment thread google/cloud/bigtable/internal/operation_context.cc

@colinmoy colinmoy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like adding the new StubSelection virtual method broke the bazel-otel-abi-2-pr check, so the ABI dump baselines need to be regenerated

@scotthart

Copy link
Copy Markdown
Member Author

Looks like adding the new StubSelection virtual method broke the bazel-otel-abi-2-pr check, so the ABI dump baselines need to be regenerated

So OpenTelemetry has both an ABI 1 and an ABI 2 (we may be able to stop supporting ABI 1 some day). The issue here was not all the methods in a testing Mock were overridden in ABI 2. I will likely write another PR that relocates the OTel mocks to testing_util to help future us not run into this again.

@scotthart
scotthart enabled auto-merge (squash) August 20, 2026 18:33
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.50000% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.23%. Comparing base (ca5075d) to head (6b2eca2).

Files with missing lines Patch % Lines
...e/cloud/bigtable/internal/client_schema_metrics.cc 85.39% 13 Missing ⚠️
...ud/bigtable/internal/client_schema_metrics_test.cc 87.62% 12 Missing ⚠️
...oogle/cloud/bigtable/internal/operation_context.cc 54.54% 5 Missing ⚠️
google/cloud/bigtable/internal/metrics.h 0.00% 2 Missing ⚠️
...le/cloud/bigtable/internal/client_schema_metrics.h 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16360      +/-   ##
==========================================
- Coverage   92.24%   92.23%   -0.02%     
==========================================
  Files        2234     2237       +3     
  Lines      210059   210251     +192     
==========================================
+ Hits       193774   193930     +156     
- Misses      16285    16321      +36     

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

@scotthart
scotthart merged commit 31a0d9f into googleapis:main Aug 20, 2026
66 of 67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the Bigtable API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants