Skip to content

fix(bigtable): preserve resource labels on batcher metrics - #18712

Open
AliAbouelazm wants to merge 1 commit into
googleapis:mainfrom
AliAbouelazm:fix/bigtable-batcher-metric-labels
Open

AliAbouelazm wants to merge 1 commit into
googleapis:mainfrom
AliAbouelazm:fix/bigtable-batcher-metric-labels

Conversation

@AliAbouelazm

Copy link
Copy Markdown

Fixes #18546.

The mutations batcher creates operation metrics directly through the metrics controller, bypassing the resource labels supplied by the table or authorized view. The missing project label causes the exporter to drop these metrics and log a warning on each export.

Create each batch's metric through the target's existing _create_operation helper. Metrics still start before waiting for flow control, so operation and throttling latency measurements keep their existing timing. Regenerate the synchronous implementation and tests with CrossSync.

Regression coverage includes tables and authorized views, default and explicit app profiles, multiple batches, and flow-control timing in both synchronous and asynchronous clients. The label regression fails before the fix in all eight configurations.

Validation:

  • Full package unit tests on Python 3.12 with upb: 5,506 passed, 8 skipped.
  • Full package unit tests on Python 3.14 with both Python and upb protobuf implementations: 5,506 passed, 8 skipped per run.
  • Python 3.14 mypy: all 46 source files pass.
  • Ruff import and format checks, Flake8, and package metadata checks pass.
  • Independent offline validation confirms that the real mutation and export pipeline produces correctly labeled operation, attempt, and throttling metrics. All service calls are mocked.

The existing aggregate coverage gate remains below 99% on both untouched main and this branch: 98.330% before and 98.345% after. Every added executable line is covered.

Live Bigtable system tests were not run.

Create batcher operation metrics through the target helper so project,
instance, table, and app-profile labels reach the metrics exporter. Keep
metric creation before flow-control waiting to preserve latency timing.

Add sync and async regression tests for tables, authorized views, app
profiles, multiple batches, and flow-control timing. Regenerate sync code
and tests with CrossSync.

Fixes googleapis#18546

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request refactors the mutations batcher (both async and sync autogenerated versions) to pass the target object instead of the metrics_controller to add_to_flow_with_metrics. This allows the batcher to delegate operation creation directly to the target via _create_operation, enabling better tracking of resource labels (such as project, instance, table, and app profile IDs) on client-side metrics. Additionally, unit tests have been added to verify metric resource labels and flow throttling behavior. There are no review comments to address, and I have no further feedback to provide.

@AliAbouelazm
AliAbouelazm marked this pull request as ready for review October 16, 2026 07:49
@AliAbouelazm
AliAbouelazm requested a review from a team as a code owner October 16, 2026 07:49
@parthea parthea self-assigned this Oct 16, 2026
@parthea parthea added kokoro:force-run Add this label to force Kokoro to re-run the tests. kokoro:run Add this label to force Kokoro to re-run the tests. labels Oct 16, 2026
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Oct 16, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bigtable: mutations_batcher() causes "Missing resource_project attribute for metric …" warnings every 60s

3 participants