feat(bigtable): rename session pool display to <resource-id>-<PERM> by sushanb · Pull Request #20248 · googleapis/google-cloud-go

sushanb · 2026-07-28T21:47:38Z

Summary

Reformat SessionPoolImpl.poolName — the string that is:

stamped as the OTel session_name metric label (via WithSessionPoolName → sessionTracer.setPoolName), and
rendered as the pool identity in the sessionz debug view.

Old format: \"TablePool-1 [READ]\" — three concatenated concepts (proto session type, monotonic id, permission bracket) that convey nothing an operator debugging a specific table would recognise.

New format: \"<resource-id>-<PERM>\"

Resource kind	Old label	New label
standard table	`TablePool-1 [READ]`	`my-table-READ`
authorized view	`AuthorizedViewPool-1 [READ]`	`my-table/my-view-READ`
materialized view	`MaterializedViewPool-1 [READ]`	`my-mat-view-READ`

The numeric pool id lives on SessionPoolImpl.poolID for the sessionz ↔ channelz reverse link and gets baked into per-session log names via createSession — cardinality of session_name stays bounded by (resource × permission).

AV disambiguation

For authorized views the table qualifier is preserved as <table>/<view> so two AVs with the same view id on different tables produce distinct session_name timeseries. Otherwise they would silently aggregate on the label — a metric-integrity concern flagged by both session-reviewer and igor-reviewer in early review. / is disjoint from Bigtable resource-id grammar ([-_.a-zA-Z0-9]) so the compound decomposes unambiguously.

Files touched

bigtable/internal/session/client.go — new poolKey.displayName() + poolKey.displayResource() helpers; getOrCreateSessionPool uses them instead of fmt.Sprintf(\"%sPool-%d\", ...) + \" [\" + label + \"]\".
bigtable/internal/session/client_test.go — new TestPoolKey_DisplayName covers table/AV/MV + read/write + AV-collision + malformed-input fallback pins.

Test plan

go test ./bigtable/internal/session/ ./bigtable/internal/transport/ -count=1 -race -short -timeout=180s
New TestPoolKey_DisplayName covers 12 cases including the AV-collision guard and 4 malformed-input fallback pins.

Reviewers

Three subreviewers cleared: session-reviewer (METRICS #3 pool-scoped bounded cardinality + sole-writer discipline preserved), session-component-review (Part B / Part C boundaries unchanged), igor-reviewer (metric-integrity framing correct; naming + fallback pins fine).

Follow-ups (not in this PR)

Same change on sessionz-debug (working branch); will port after this lands.
Java parity check: match Java SessionPoolInfo.name format if it diverges.

The `SessionPoolImpl.poolName` string is stamped as the OTel `session_name` metric label (via `WithSessionPoolName` → `sessionTracer.setPoolName`) and rendered as the pool identity in sessionz. Today it reads "TablePool-1 [READ]" — three concatenated concepts (proto session type, monotonic id, permission bracket) that convey nothing an operator debugging a specific table would recognise. Change the format to "<resource-id>-<PERM>": table:<id> → "<id>-<PERM>" e.g. "my-table-READ" av:<t>:<v> → "<t>/<v>-<PERM>" e.g. "my-table/my-view-READ" mv:<v> → "<v>-<PERM>" e.g. "my-view-READ" For authorized views the table qualifier is preserved as "<table>/<view>" so two AVs with the same view id on different tables produce distinct `session_name` timeseries — otherwise they would silently aggregate on the label. `/` is disjoint from Bigtable resource-id grammar ([-_.a-zA-Z0-9]) so the compound decomposes unambiguously. The numeric pool id lives on `SessionPoolImpl.poolID` for the sessionz ↔ channelz reverse link and gets baked into per-session log names via `createSession` — cardinality of `session_name` stays bounded by (resource × permission). Two file changes: - `internal/session/client.go`: new `poolKey.displayName()` + `poolKey.displayResource()` helpers; `getOrCreateSessionPool` uses them instead of the old sprintf. - `internal/session/client_test.go`: new `TestPoolKey_DisplayName` covers table/AV/MV + read/write + AV-collision + malformed-input fallback pins. **Breaking change for `session_name` label consumers.** Dashboard and alert regexes that filter on the old `Pool-<N>` / `[READ]` / `[WRITE]` substrings will stop matching. The new alphabet includes `/` for AV pools; downstream Prom/OTel exporters and dashboard regexes should tolerate it. Coordinate with metrics/dashboard owners before rolling out. Reviewers cleared: session-reviewer (METRICS #3 pool-scoped bounded cardinality; sole-writer discipline preserved), session-component-review (Part B/C boundaries unchanged), igor-reviewer (metric-integrity framing correct; naming + fallback pins fine).

gemini-code-assist

Code Review

This pull request introduces a human-readable pool identity formatting mechanism (displayName and displayResource on poolKey) to be used as the OTel session_name metric label and in sessionz, along with comprehensive unit tests. The reviewer suggests using strings.Cut instead of strings.Replace when parsing authorized views to improve robustness against malformed inputs and prevent potential metric label collisions.

gemini-code-assist · 2026-07-28T21:48:46Z

+	case strings.HasPrefix(r, "av:"):
+		// "av:<table>:<view>" → "<table>/<view>" so the label
+		// disambiguates AVs with the same view id on different tables.
+		rest := strings.TrimPrefix(r, "av:")
+		return strings.Replace(rest, ":", "/", 1)


Using strings.Cut is more idiomatic and robust than strings.Replace with a limit of 1. Additionally, if the resource string is malformed and does not contain a colon (e.g., "av:my-view"), strings.Replace would return "my-view", which could collide with a standard table named "my-view". Using strings.Cut allows us to safely detect this case and fall back to returning the original string r to prevent metric label collisions and preserve the exact string representation.

case strings.HasPrefix(r, "av:"): // "av:<table>:<view>" → "<table>/<view>" so the label // disambiguates AVs with the same view id on different tables. rest := strings.TrimPrefix(r, "av:") before, after, found := strings.Cut(rest, ":") if !found { return r } return before + "/" + after

References

When refactoring or optimizing metric labels, ensure that the exact string representation is preserved to prevent breaking downstream dashboards, queries, and alerts.

Field-rename only inside internal/session/client.go. The `resource` identifier holds prefixed encodings ("table:<id>", "av:<t>:<v>", "mv:<v>") that are the pool's unique map key; naming it `resourceName` matches what the field actually holds and is symmetric with the sessionz-debug version (72471f3 on feat/bigtable-sessionz-debug). Zero behavioral change: displayName / displayResource / less all read the same value; the pool map keying / dedup semantics are unchanged.

sushanb · 2026-07-28T22:01:18Z

+// remains on SessionPoolImpl (as poolID) and gets baked into per-session
+// log names via createSession — `session_name` label cardinality stays
+// bounded by (resource × permission).
+func (k poolKey) displayName() string {


just use displayName. no need to use displayResource

Done in 496782df95. Folded displayResource() into displayName() — the switch now rewrites r in place and the single return r + "-" + k.perm.display() at the bottom stamps the permission suffix. Zero behavioral change; all 12 cases in TestPoolKey_DisplayName still pass. Same fold applied to feat/bigtable-sessionz-debug at 91b23dcb27 so the two branches stay symmetric.

Per author review comment: `displayResource()` had exactly one caller (`displayName()`), so the split added no value. Fold into a single switch that rewrites `r` in place and returns `r + "-" + perm.display()`. Zero behavioral change — same three prefix cases, same fallthrough on unknown, same output for every input pinned by TestPoolKey_DisplayName.

Per PR googleapis#20248 review: fold the single-caller helper into displayName. Zero behavioral change; TestPoolKey_DisplayName still covers all 12 cases.

🤖 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 28, 2026 21:47

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

gemini-code-assist Bot reviewed Jul 28, 2026

View reviewed changes

sushanb commented Jul 28, 2026

View reviewed changes

mutianf approved these changes Jul 29, 2026

View reviewed changes

sushanb merged commit 35e146e into googleapis:main Jul 29, 2026
19 checks passed

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

chore(main): release bigtable 1.52.0 #20207

Merged

Navigation Menu

Uh oh!

feat(bigtable): rename session pool display to <resource-id>-<PERM> - #20248

feat(bigtable): rename session pool display to <resource-id>-<PERM>#20248
sushanb merged 3 commits into
googleapis:mainfrom
sushanb:feat/bigtable-sessiontracer-poolname

sushanb commented Jul 28, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot 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 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

AV disambiguation

Files touched

Test plan

Reviewers

Follow-ups (not in this PR)

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

sushanb commented Jul 28, 2026 •

edited

Loading