Skip to content

fix(bigtable): use instance_name instead of name in GetClientConfig - #20465

Merged
sushanb merged 5 commits into
googleapis:mainfrom
sushanb:fix/bigtable-getclientconfig-request-params
Sep 3, 2026
Merged

sushanb merged 5 commits into
googleapis:mainfrom
sushanb:fix/bigtable-getclientconfig-request-params

Conversation

@sushanb

@sushanb sushanb commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • The header key was also wrong: the proto field is instance_name, not name.
  • Fix bigtable/internal/session/client.go to build the header the same way perResourceMetadata already does for every other session RPC, and lift the construction into a buildConfigMetadata helper so the shape is unit-testable.

Test plan

  • go test ./internal/session/ -run TestBuildConfigMetadata -count=1 passes.
  • go test ./internal/session/ ./internal/transport/ -count=1 -short passes.
  • New unit test pins the exact header string (instance_name=projects%2Fp%2Finstances%2Fi&app_profile_id=profile+with+spaces) so regressions land in CI.

…ation

The x-goog-request-params header on the session client's config poll used
`name=<instance>` and omitted app_profile_id entirely. Java's
Util.composeMetadata + ClientConfigurationManager pair the proto field
name (instance_name) with app_profile_id so the RLS router keys on both;
without this, the server can route a session-client config poll to a
different backend than the same client's data-path RPCs, which is a real
divergence when app profiles map to different single-cluster routings.

Fix the header to instance_name=<esc>&app_profile_id=<esc>, matching
what perResourceMetadata already sends for every other session RPC, and
lift the construction into a buildConfigMetadata helper with a unit
test that pins the shape.
@sushanb
sushanb requested review from a team as code owners September 3, 2026 04:54
@product-auto-label product-auto-label Bot added the api: bigtable Issues related to the Bigtable API. label Sep 3, 2026

@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 metadata construction for GetClientConfiguration RPCs into a helper function buildConfigMetadata to align with Java parity by including both instance_name and app_profile_id in the request parameters, and adds a corresponding unit test. The feedback suggests conditionally appending app_profile_id only when it is non-empty to avoid potential routing issues with empty values, and recommends refactoring the unit test into a table-driven test to cover both scenarios.

Comment thread bigtable/internal/session/client.go
Comment thread bigtable/internal/session/client_test.go
@sushanb sushanb changed the title fix(bigtable): send instance_name+app_profile_id on GetClientConfiguration use the correct annotations for GetClientConfig Sep 3, 2026
@sushanb sushanb changed the title use the correct annotations for GetClientConfig use instance_name instead of name in GetClientConfig Sep 3, 2026
…ssion_configuration

A successful poll can still leave the client with no session_configuration
— e.g. an instance/profile whose control-plane config never landed — and
today that silently falls back to defaults with no operator-visible signal.
Fire client_config_session_configuration_null on the debug-tag counter
whenever resp.SessionConfiguration is nil on a successful poll so ops can
grep it apart from failed polls (which already emit
client_config_poll_failed).
…_load=0

Companion signal to client_config_session_configuration_null: fires
client_config_session_load_zero when a successful poll returns a
populated SessionClientConfiguration whose session_load is 0 (explicit
"route 0% through session, 100% through classic"). Legitimate operating
point, but worth surfacing so operators can spot instances/app-profiles
effectively opted out of the session data path. Mutually exclusive with
the null-config tag.
… null session_configuration"

This reverts commit f14f59a.
@sushanb sushanb changed the title use instance_name instead of name in GetClientConfig fix(bigtable): use instance_name instead of name in GetClientConfig Sep 3, 2026
@sushanb
sushanb merged commit 375f198 into googleapis:main Sep 3, 2026
19 checks passed
machi-genggeng pushed a commit to machi-genggeng/google-cloud-go that referenced this pull request Sep 3, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.55.0](googleapis/google-cloud-go@bigtable/v1.54.0...bigtable/v1.55.0)
(2026-09-03)


### Features

* **bigtable/bttest:** Honor the table's timestamp granularity
([googleapis#20456](googleapis#20456))
([0054866](googleapis@0054866))
* **bigtable:** Debug tags for GetClientConfiguration
session_configuration state
([googleapis#20472](googleapis#20472))
([8e57e4b](googleapis@8e57e4b))


### Bug Fixes

* **bigtable:** Use instance_name instead of name in GetClientConfig
([googleapis#20465](googleapis#20465))
([375f198](googleapis@375f198))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
sushanb pushed a commit that referenced this pull request Sep 3, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.55.0](bigtable/v1.54.0...bigtable/v1.55.0)
(2026-09-03)


### Features

* **bigtable/bttest:** Honor the table's timestamp granularity
([#20456](#20456))
([0054866](0054866))
* **bigtable:** Debug tags for GetClientConfiguration
session_configuration state
([#20472](#20472))
([8e57e4b](8e57e4b))


### Bug Fixes

* **bigtable:** Use instance_name instead of name in GetClientConfig
([#20465](#20465))
([375f198](375f198))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
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