feat(bigtable): add getClientConfigDirectAccessChecker for session pools by sushanb · Pull Request #20209 · googleapis/google-cloud-go

sushanb · 2026-07-23T20:33:13Z

Summary

Adds a session-pool sibling to pingAndWarmDirectAccessChecker. Session channel pools do not use PingAndWarm — they warm channels via the OpenSession handshake on each newly-opened stream (see #20208 — NoOpChannelPrimer). Passing a NoOp primer into the classic checker would leave isALTSConn unset, so the ALTS check would always fail for session pools.

getClientConfigDirectAccessChecker runs the same CheckCompatibility flow (dial → probe → ALTS check → success-metric or async investigation), but issues GetClientConfiguration as the probe RPC — the same verb session pools already talk on the wire.

Changes

direct_access_checker.go (modified): extract two shared helpers from pingAndWarmDirectAccessChecker so both checkers can use them without duplication.
- investigateDirectAccessFailure(logger, reportFailure, probeSingle, originalErr) — the GCE-environment precondition walk, now taking a probeSingle callback so each checker plugs in its own RPC-verb probe.
- newAltsProbeChannel(ctx, targetEndpoint) — the ALTS + oauth + authority-override dial used by the single-endpoint investigation probe. Behavior unchanged.
- pingAndWarmDirectAccessChecker.investigateFailure / .probeSingleEndpoint become thin wrappers over the shared helpers. Existing behavior preserved.
direct_access_checker_getclientconfig.go (new): getClientConfigDirectAccessChecker struct, constructor, CheckCompatibility, probeGetClientConfig (the compatibility probe), probeSingleEndpoint (the single-endpoint investigation probe), and recordProbePeer — the ALTS + IP-protocol side-effect helper that mirrors what BigtableConn.Prime does for PingAndWarm.
direct_access_checker_getclientconfig_test.go (new): 7 tests covering interface satisfaction, dialer identity, dial-failure short-circuit, and the four IP/ALTS observation branches of recordProbePeer.

Test plan

`go test ./bigtable/internal/transport/ -run 'GetClientConfigDirectAccess|RecordProbePeer' -count=1 -short` → 7/7 pass.
`go test ./bigtable/internal/transport/ -count=1 -short` (full transport suite, ~200 tests, 42s) → all pass, verifying the pingAndWarm refactor is behavior-preserving.
`go build` / `go vet` / `golint` all clean.
CI green.

Follow-up

A subsequent PR will add a session-pool factory that wires this checker alongside NoOpChannelPrimer (from #20208) and the ClientConfigurationManager polling loop.

Session channel pools do not use PingAndWarm — they warm their channels via the OpenSession handshake on each newly-opened stream (see NoOpChannelPrimer). This makes the classic pingAndWarmDirectAccessChecker a poor fit: passing NoOpChannelPrimer to it would leave isALTSConn unset, so the ALTS check would always fail for session pools. getClientConfigDirectAccessChecker is the session-pool sibling. Same CheckCompatibility flow (dial → probe → ALTS check → success or async investigation), but issues GetClientConfiguration as the probe RPC — the same verb session pools already talk on the wire. Shared plumbing extracted from pingAndWarmDirectAccessChecker: - investigateDirectAccessFailure(logger, reportFailure, probeSingle, originalErr) — package function that walks the GCE-environment precondition ladder and delegates the end-to-end ALTS check to a probeSingle callback. Both checkers reuse it, each plugging in its own RPC-verb probe. - newAltsProbeChannel(ctx, targetEndpoint) — the ALTS + oauth + authority-override dial used by the single-endpoint investigation probe. Behavior unchanged. pingAndWarmDirectAccessChecker.investigateFailure / .probeSingleEndpoint become thin wrappers over the shared helpers; existing behavior preserved (all 200+ transport tests still pass). New tests cover interface satisfaction, dialer identity, dial-failure short-circuit, and the ALTS/IP-protocol observation side effects on BigtableConn.

gemini-code-assist

Code Review

This pull request refactors the Direct Access failure investigation logic to extract a shared helper, and introduces a new getClientConfigDirectAccessChecker that uses GetClientConfiguration as its probe RPC for session-pool channels. The review feedback highlights that probeSingleEndpoint should treat PermissionDenied as a successful probe, consistent with CheckCompatibility, since receiving this status code indicates a successful ALTS handshake.

gemini-code-assist · 2026-07-23T20:35:14Z

+	if err := c.probeGetClientConfig(probeCtx, btc); err != nil {
+		return fmt.Errorf("GetClientConfiguration() failed: %w", err)
+	}


During the failure investigation, if probeGetClientConfig returns a PermissionDenied error, it indicates that the ALTS handshake actually succeeded (since the gRPC status code was received from the server), but the credentials simply lack permission to call GetClientConfiguration. Treating this as a failure causes the investigation to incorrectly report alts_handshake_failed.

We should treat PermissionDenied as a successful probe in probeSingleEndpoint, consistent with how it is handled in CheckCompatibility.

Suggested change

if err := c.probeGetClientConfig(probeCtx, btc); err != nil {

return fmt.Errorf("GetClientConfiguration() failed: %w", err)

}

if err := c.probeGetClientConfig(probeCtx, btc); err != nil && status.Code(err) != codes.PermissionDenied {

return fmt.Errorf("GetClientConfiguration() failed: %w", err)

}

…sion_client Names the file (and struct) by the consumer — session client — rather than the probe verb. Matches the pattern of "this file is for the session pool's DA compatibility check" instead of "this file uses GetClientConfiguration". direct_access_checker_getclientconfig.go → direct_access_check_session_client.go getClientConfigDirectAccessChecker → sessionClientDirectAccessChecker Behavior unchanged.

Adds bigtable/internal/transport/client_configuration_rpc.go — a small wrapper around stub.GetClientConfiguration that centralizes the {instance, appProfile} request shape + metadata attachment. Callers own retry / timeout / metric policy: - ClientConfigurationManager wraps this in an exponential-backoff loop keyed off the current config's MaxRpcRetryCount. - The session DirectAccessChecker calls it once and treats failure as a signal to run the async investigation. Ported verbatim from the sessionz-debug working branch — keeps the GetClientConfiguration call site in one place so future changes (e.g. adding tracing options, standardizing header/trailer capture) land once, not per-consumer.

…urationOnce Fold the manager's inline stub.GetClientConfiguration call onto the shared FetchClientConfigurationOnce helper introduced in the previous commit — keeps the (instance, appProfile, metadata) tuple in one place. Drops the now-unused "google.golang.org/grpc" import + the local GetClientConfigurationRequest construction. Header/trailer capture was already discarded by the manager (it only cares about the response body), so passing the helper's `_, _` return is a wash.

mutianf · 2026-07-28T17:32:07Z

+	defer cancel()
+
+	var p peer.Peer
+	_, err := client.GetClientConfiguration(probeCtx, req, grpc.Peer(&p))


should we reuse the result from this call?

Yes — done in 08e9992. probeGetClientConfig now returns (*ClientConfiguration, error) and the checker stashes the response body in a lazily-set atomic.Pointer, populated on both the compatible path and the PermissionDenied fall-through path. Exposed via a new LastProbeConfig() accessor.

Wire-up into ClientConfigurationManager so it can seed its initial state from LastProbeConfig() and skip its own eager first poll — held for a separate PR since that touches the manager constructor + the SessionClient assembly site. Happy to bundle if you'd prefer it in this PR; just let me know.

Follow-up in 281f259: hoisted LastProbeConfig() onto the DirectAccessChecker interface so future callers can consume it via the interface without a type assertion. pingAndWarm variant returns nil (PingAndWarm's response has no ClientConfiguration body); disabled variant returns nil (never probes); session variant returns the atomic-pointer-backed stash. Sets us up cleanly for the ClientConfigurationManager seed wiring in a follow-up PR.

Addresses mutianf review comment on PR googleapis#20209 at direct_access_check_session_client.go:141 ("should we reuse the result from this call?"). - probeGetClientConfig now returns (*ClientConfiguration, error) instead of just error. - sessionClientDirectAccessChecker stashes the response body via a lazily-set atomic.Pointer[ClientConfiguration], populated on both the compatible path and the PermissionDenied fall-through path (any path where the server actually returned a body). - New LastProbeConfig() accessor exposes it for downstream consumers. Intended follow-up (separate PR): pipe LastProbeConfig() into ClientConfigurationManager construction so the manager can seed its initial state from the probe response and skip its own eager first poll. That wiring touches manager + client construction so I'm holding it here to keep this PR scoped to the checker surface. The internal investigate-failure re-probe still discards the response (uses `_,` explicitly with a comment) — it's a diagnostic-only endpoint-isolated retry, not a source-of-truth for config.

Promote LastProbeConfig() from a concrete method on sessionClientDirectAccessChecker to a first-class method on the DirectAccessChecker interface so callers (e.g. a future ClientConfigurationManager seed) can consume it without a type assertion. - pingAndWarmDirectAccessChecker.LastProbeConfig returns nil — PingAndWarm's response carries no ClientConfiguration body, so there's nothing to reuse. - disabledDirectAccessChecker.LastProbeConfig returns nil — the disabled stub never probes. - sessionClientDirectAccessChecker.LastProbeConfig retains the atomic-pointer-backed accessor from the previous commit. Follow-up (still deferred to a separate PR): pipe checker.LastProbeConfig() into ClientConfigurationManager construction so the manager skips its own eager first poll.

🤖 I have created a release *beep* *boop* --- ## [1.52.0](bigtable/v1.51.0...bigtable/v1.52.0) (2026-08-03) ### Features * **bigtable:** Add AFE picker (Simple / LeastInFlight / LeastLatency) ([#20204](#20204)) ([bcbf714](bcbf714)) * **bigtable:** Add ClientConfig.DisableSession to opt out of session backend ([#20297](#20297)) ([7ee5e44](7ee5e44)) * **bigtable:** Add getClientConfigDirectAccessChecker for session pools ([#20209](#20209)) ([3b8d30a](3b8d30a)) * **bigtable:** Add NoOpChannelPrimer for session channel pools ([#20208](#20208)) ([d055a8a](d055a8a)) * **bigtable:** Add per-AFE sessionList for the two-tier session pool ([#20224](#20224)) ([dbf0c3f](dbf0c3f)) * **bigtable:** Add protoRowToRow conversion helper for TableShim ([#20257](#20257)) ([1297143](1297143)) * **bigtable:** Add Session debug surface (observability fields + methods) ([#20211](#20211)) ([d8d3e16](d8d3e16)) * **bigtable:** Add Session lifecycle (Start, Close, ForceClose, readLoop, heartBeatLoop) ([#20215](#20215)) ([b9e53c6](b9e53c6)) * **bigtable:** Add Session struct + state machine ([#20117](#20117)) ([09acbb3](09acbb3)) * **bigtable:** Add session.Config.EnableDebug to gate sessionz debug state ([#20247](#20247)) ([ce74c31](ce74c31)) * **bigtable:** Add SessionClient + SessionTable + lazyPool ([#20228](#20228)) ([ab2c96c](ab2c96c)) * **bigtable:** Add SessionPoolImpl (two-tier pool + scaling + debug) ([#20225](#20225)) ([683eda8](683eda8)) * **bigtable:** Rename session pool display to <resource-id>-<PERM> ([#20248](#20248)) ([35e146e](35e146e)) * **bigtable:** Route Client.Open()-returned *Table through the Diverter ([#20273](#20273)) ([2b81c7d](2b81c7d)) * **bigtable:** State-based classification for abnormal session close ([#20243](#20243)) ([f2905b7](f2905b7)) * **bigtable:** TableShim fallback to classic on session UNIMPLEMENTED ([#20269](#20269)) ([36540af](36540af)) * **bigtable:** TTL-on-idle cache for per-resource session.TableAPI ([#20263](#20263)) ([00b2a49](00b2a49)) * **bigtable:** Wire Diverter on Client and route Open* via TableShim ([#20256](#20256)) ([b32fbd7](b32fbd7)) ### Bug Fixes * **bigtable:** AFE picker latency signal — subtract poolWait and compute TransportLatency = wire − backend at source ([#20281](#20281)) ([bb8c4d5](bb8c4d5)) * **bigtable:** Guard NewStream OnFinish against grpc-go double-fire ([#20295](#20295)) ([b51da29](b51da29)) * **bigtable:** Real per-resource pool teardown on sessionTable.Close + cache close-race gate ([#20264](#20264)) ([599aea9](599aea9)) * **bigtable:** Session.durations / session.uptime — set explicit histogram bucket boundaries ([#20276](#20276)) ([97eee22](97eee22)) * **bigtable:** SessionTableHandle self-heals across cache eviction ([#20296](#20296)) ([0dd98cd](0dd98cd)) * **bigtable:** Translate ctx errors to gRPC status on session vRPC ([#20299](#20299)) ([0f3b2a5](0f3b2a5)) * **bigtable:** Treat PingAndWarm NotFound as a successful prime ([#20219](#20219)) ([a1557ad](a1557ad)) ### Performance Improvements * **bigtable:** Delete periodic Tick loop; sizing is event-driven ([#20285](#20285)) ([2c096bd](2c096bd)) * **bigtable:** Drop pick_lost_race debug tag from CheckoutSession hot path ([#20280](#20280)) ([bd0e400](bd0e400)) --- 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 requested review from a team as code owners July 23, 2026 20:33

product-auto-label Bot added the api: bigtable Issues related to the Bigtable API. label Jul 23, 2026

gemini-code-assist Bot reviewed Jul 23, 2026

View reviewed changes

sushanb added 3 commits July 23, 2026 20:37

mutianf approved these changes Jul 28, 2026

View reviewed changes

sushanb added 2 commits July 28, 2026 17:38

sushanb merged commit 3b8d30a into googleapis:main Jul 28, 2026
19 checks passed

release-please Bot mentioned this pull request Jul 28, 2026

chore(main): release bigtable 1.52.0 #20207

Merged

Navigation Menu

Uh oh!

feat(bigtable): add getClientConfigDirectAccessChecker for session pools - #20209

feat(bigtable): add getClientConfigDirectAccessChecker for session pools#20209
sushanb merged 6 commits into
googleapis:mainfrom
sushanb:feat/bigtable-getclientconfig-da-checker

sushanb commented Jul 23, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jul 23, 2026

Uh oh!

mutianf Jul 28, 2026

Uh oh!

sushanb Jul 28, 2026

Uh oh!

sushanb Jul 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sushanb commented Jul 23, 2026

Summary

Changes

Test plan

Follow-up

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

mutianf Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

sushanb Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

sushanb Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants