READY feat(bigtable): add metrics exporter in the client to export outstandingrpcs and perconnectionerror count - #3

…ctionerror count (#13510)

lift and shift of sushanb#3
```
curl -L -O https://github.com/sushanb/google-cloud-go/commit/b9d20984c514b4632f7c613e4d6ab58b1776a939.patch
git apply b9d2098.patch
```

…itted, formalize retry-signal provenance

CLIENT_SIDE_METRICS_SPEC.md
- New invariant #2: per-attempt latency labels on the session path (client_blocking_latencies from SentAt, server_latencies from Stats.BackendLatency, transport_type/*) MUST be sourced only from InvokeResult and stamped in exactly one place — sessionTable.stampAttempt. Corrects a prior wrong claim that InvokeResult.TransportLatency was not a client-side metric.
- New invariant #3: session-tracer-hosted OTel histograms (session.durations, session.open_latencies, session.uptime, transport_latencies) — registered once by InitializeSessionMetrics, emitted only by sessionTracer, session_name label is pool-scoped (bounded) not Session.LogName (unbounded).
- Scope line updated to reference session_tracer.go and session_pool.go recording sites; invariant count bumped to 3.

SESSION_SPEC.md
- Invariant #6: deprecate the LastRpcIdAdmitted retry oracle. Proto field is still sent by the server but the Go transport does not read it (git grep returns zero hits in internal/transport/). Section heading trimmed; deferred-work callout notes any reintroduction requires paired code+spec+test change with a Java-parity check.
- Invariant #9: new 'Retry-signal provenance' table splitting server-only inputs (RetryInfo, narrow always-retry code set, x-goog-cbt-cookie-routing-cookie) from client-only inputs (AttemptState tag, idempotency check, 3-attempt cap, deadline-fit, zero-value fail-closed). Absorbs several redundant bullets from the prior 'Additional invariants' list.
- Added 'How to verify' pointer to CLAUDE.md smoke-gate for consistency (2 of 5 specs updated; remaining specs to follow).

SESSION_CLIENT_SPEC.md
- Added 'How to verify' pointer.

SESSION_COMPONENT_SPEC.md
- New Part C row for session-tracer OTel histograms: sessionTracer sole writer, registered once by InitializeSessionMetrics, transport_latencies recorded only from session_pool.go:650 under the positive-delta gate, session_name must be pool-scoped not LogName.

Restructures the spec-driven review deck from a 5-slide feature tour
into a 4-slide 'why we moved from one-shot prompting to specs' story:

- Slide 1 — one-shot prompting definition + the three PRs that shipped
  this way (googleapis#19987 DirectAccessChecker, googleapis#20027 ChannelPrimer, googleapis#20099
  client-side-metrics decouple). Common shape: extract one implicitly-
  unary abstraction into an interface.
- Slide 2 — Jetstream is 30k+ LOC; one-shot breaks. Introduces the five
  spec files with verified invariant counts (10 / 4 / 5 / 3 / 12+PartC)
  and one illustrative rule per spec.
- Slide 3 — SESSION_COMPONENT_SPEC.md tour: Part A (7-layer descriptive
  map), Part B (12 boundary MUST-rules with grep patterns), Part C
  (ownership matrix excerpts).
- Slide 4 — three prompt sizes with real examples from this branch:
  (1) simple refactor — activeVRPC/casActiveVRPC accessor extraction;
  (2) logic addition — adaptive session-creation throttler + how the
  spec invariants (POOL #5, CLIENT #3, B6) chain together;
  (3) big feature — unified debugview/ (7 z-pages behind one Handler)
  and how B3, POOL #4, B10 crystallized during that refactor.
  Plus reviewer-agent flow and PASS/VIOLATION/AMBIGUOUS semantics.

CSS and navigation unchanged. Counter reflects 4 slides. Companion
specs-deck.md not updated in this commit — will follow.

…anchor

Addresses mutianf ask on PR googleapis#20117 ("just keep one variable"): the tracer
was carrying two timestamps (startTime, openedAt) but Java's
SessionTracerImpl only needs one — its `uptime` stopwatch is started in
onStart() and drives session.open_latencies, session.durations, and
session.uptime uniformly, with `state == State.Ready` as the boolean
"ready" label.

Changes:
- session_tracer.go: drop `openedAt time.Time` and its lock-guarded
  `openedAtSnapshot()` accessor. Add `opened bool` (protected by the
  same mu) as the ready flag, flipped in recordOpen. recordClose and
  sampleUptime now always anchor at msSince(startTime), matching
  Java's uptime.elapsed() semantics. `ready` label sourced from
  snap.opened.
- session_debug.go: rename exported `Session.OpenedAt()` →
  `Session.StartedAt()` (lock-free — startTime is immutable after
  newSessionTracer).
- session_lifecycle.go, session_pool.go: two callers updated. Their
  event fields (`age` in the close event, `SessionAge` in the slow-vRPC
  event) now count from construction instead of Ready-transition — for
  a healthy session that's a sub-100ms shift, and for a pre-open death
  the field is now populated instead of zero-valued.
- CLIENT_SIDE_METRICS_SPEC.md: refresh invariant #3's metric roster
  for session.durations and session.uptime; note the Java-parity
  rationale.

Semantic shift worth flagging: `session.durations` for a ready session
now includes the open-handshake window (~sub-100ms). Java records the
same value. `session.uptime` is unaffected in practice (pool's
sampleActiveUptimes iterator already gated on StateReady).

session-reviewer (behavioral, 4 specs) and session-component-review
(boundaries, SESSION_COMPONENT_SPEC Part B/C) both came back CLEAN
after the spec update. Full transport suite passes.

…ason

Addresses mutianf feedback on googleapis#20190.

- Remove peerInfo/mu from sessionTracer. PeerInfo is set-once on
  Session (SESSION_SPEC #3, atomic.Pointer, sole writer in
  handleOpenSession), so mirroring it on the tracer under a second
  mutex was pointless duplication. record*/sample* now take
  peerInfo *spb.PeerInfo as a parameter; callers pass
  Session.PeerInfo() at the call site.
- Introduce peerInfoLabels(*PeerInfo) helper: nil -> ("unknown", ""),
  populated -> (TransportTypeName, ApplicationFrontendSubzone).
  Replaces tracerSnapshot / setPeerInfo / snapshot().
- opened demoted to atomic.Bool. Tracer holds no lock now.
- Doc tightened: closingReason is always non-empty; Session
  synthesizes a fallback if the close path forgot to set one,
  mirroring java-bigtable's SessionImpl.notifyTerminalClose guard
  (SessionImpl.java:782-793).
- Tests: SnapshotUnknownTransportOnNilPeer + SnapshotAfterPeerInfoSet
  collapsed into TestPeerInfoLabels (Nil / SubzoneSet subtests).
  Nil-histograms no-op smoke test keeps "" for closingReason with an
  inline note explaining the always-non-empty contract is a
  caller-side invariant, not tracer-enforced.

…StartMaintenance

Follow-up to bf49fd5. The single-pass driver on SessionPoolImpl does
more than scaling — it samples the time-series ring, samples every active
session's uptime, sweeps stuck WaitServerClose sessions, and only THEN
runs the sizer and (if it asked for growth) spawns createSession
goroutines. "Scaling" was misleading for the first three of those; the
new name reflects that this is a general-purpose maintenance driver
whose scaling decision is one of four things it does.

  PerformScaling            → Maintain
  StartBackgroundScaling    → StartMaintenance

Pure rename — no behavior change. Touches the SessionPool interface
(session/lazy_pool.go), sessionClient.getOrCreatePool bootstrap
(session/client.go), the definition + adjacent docs (session_pool_scaling.go,
session_pool_lifecycle.go), the internal callers in CheckoutSession +
onClosing (session_pool.go, session_pool_lifecycle.go), and the test
function names/comments.

Verified against SESSION_POOL_SPEC #5 (scaling gated + passive shrink),
CLIENT_SIDE_METRICS_SPEC #3 (sampleActiveUptimes sole-writer), and the
SESSION_COMPONENT_SPEC boundary/ownership rules — all pass. Two spec
prose spots still call the loop "pool scaling loop"; deferred to a
paired spec update.

…pollingLoop

pollingLoop's inner m.pollsWG.Add(1) before poll() (line ~465) could
fire after Close's m.pollsWG.Wait() had started, panicking with
"sync: WaitGroup misuse: Add called concurrently with Wait" when the
counter was momentarily at 0. Compounded by pollingLoop's own
goroutine not being tracked in pollsWG at all — Close's Wait never
awaited the loop itself.

Self-track pollingLoop in pollsWG: Add(1) before `go m.pollingLoop()`
in Start, defer Done() at the top of the loop. This keeps the counter
≥ 1 for the loop's lifetime, so the inner per-poll Add(1) can never
race Wait's counter-hits-0 wake. Also guarantees Close waits for the
loop itself to exit, not just its child polls.

Found by the golang-concurrency skill audit (MED #2 + MED #3 folded
into one fix per the audit's own recommendation).

Addresses igor pre-PR audit item #3: newSessionHandle was defined in
session_list.go with production visibility but every caller lives in
_test.go (session_snapshot_test, session_pool_lifecycle_test,
session_list_test, session_pool_afe_test, session_pool_test). Production
code (createSession at session_pool_scaling.go) mints handles via
struct literal so the hot path doesn't take a function-call frame.

Move the function to session_list_test.go — same package, so all _test.go
callers keep working via test-time package scope. Docstring now names
the production mint site and the reason it's shared here.

No behavior change; tests still pass.

1. Phase-1 of Pool.Close now CAS-gates recordLifetime and
   recordSessionClose. Previously a racing onClosing (GOAWAY /
   heartbeat trip) that landed between p.sl.AllHandles() and Phase-1's
   snapshot loop already recorded lifetime under its own CAS at
   onClosing:286; Phase-1's unconditional Store made the follow-up
   recordLifetime call fire a second time.

2. onActive's late "pool closed before session became active" branch
   spawned an untracked ForceClose goroutine. If Close's Phase-5
   (p.spawns.Wait) ran before that goroutine, the ForceClose →
   onClose → recordSessionClose → noteAbnormalCloseIfAny chain
   touched pool metric state after Close returned. Now tracked via
   p.spawns.Add(1) + defer p.spawns.Done().

3. isAbnormalCloseReason didn't whitelist "User", the label
   closeReasonLabel produces for CLOSE_SESSION_REASON_USER — which is
   what Pool.Close's Phase-2 stamps on every session. On a healthy
   N-session pool, Pool.Close was therefore tripping the
   consecutive-failure circuit breaker on every teardown AND draining
   any late-arriving waiters with ErrConsecutiveFailures instead of
   the new ErrPoolClosed sentinel. Add "User" to the whitelist. Note:
   "PoolClose" (also flagged in igor's audit) turned out NOT to reach
   isAbnormalCloseReason — it's a recordSessionClose fallback label,
   not a setCloseReason value.

4. Phase-1 loop's nil-guard now hoists to `continue` so
   p.sl.OnSessionClosed(sh) doesn't run on nil handles.
   Matches the sibling shape in sampleActiveUptimes /
   sweepStuckSessions in the same file.

Regression tests: TestPoolClose_OnClosingBeforePhase1_NoDoubleLifetime
covers #1; TestConsecutiveFailures_UserReasonNotAbnormal covers #3.

… source

Move the transport-overhead subtraction from the pool layer into the
Session layer so the semantic is canonical at the producer, not
re-derived by each consumer.

Changes:
* InvokeResult.WireLatency (raw round-trip Send→Recv) added; renamed
  to E2ELatency mid-review per igor-reviewer's Java-parity ask
  (SessionList.updateLatency in Java takes (e2eLatency, backendLatency),
  and the picker docstring already leans on "e2eEwma" terminology).
* InvokeResult.TransportLatency now = E2ELatency − BackendLatency
  (guarded > 0 for clock-skew protection). Zero when Stats is missing,
  the call errored pre-Recv, or the subtraction was non-positive.
* Session.processResult is the sole computation site.
* SessionPoolImpl.Invoke drops the local subtraction; consumes the
  pre-computed value directly. Cleaner hot path + eliminates the
  duplication risk (only one place doing the subtraction).
* SlowVRpcEvent gains an E2ELatency field so the slow-vRPC forensics
  view keeps the raw round-trip alongside the overhead-only value
  (per igor-reviewer nit — losing the raw round-trip on the slow row
  is a step backwards for post-hoc debugging).

Tests:
* New TestInvoke_E2EAndTransportLatency (three subtests):
  - nil Stats → E2ELatency > 0, TransportLatency = 0.
  - backend > wire (clock skew) → E2ELatency > 0, TransportLatency = 0.
  - backend < wire → TransportLatency = E2ELatency − backend.
  Locks the semantic swap against future refactors.

Spec updates (paired with the code move):
* CLIENT_SIDE_METRICS_SPEC.md #2: TransportLatency description now
  reflects E2ELatency + processResult as the computation site.
* CLIENT_SIDE_METRICS_SPEC.md #3: transport_latencies sole-writer
  contract now references the source-side gate.
* SESSION_COMPONENT_SPEC.md Part C transport_latencies row: pointer
  to the positive-delta gate now points at session_vrpc.go
  processResult (was session_pool.go:646-647).

Reformat SessionPoolImpl.poolName — the string stamped as the OTel
`session_name` metric label (via WithSessionPoolName →
sessionTracer.setPoolName) and rendered as pool identity in sessionz.

Old: "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: "<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"

AV format preserves the table qualifier 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. `/` 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 per-session log names via createSession —
`session_name` cardinality stays bounded by (resource × permission).

Two file changes:
  - internal/session/client.go: rename poolKey.resource → poolKey.resourceName
    for clarity; new poolKey.displayName() + poolKey.displayResource()
    helpers; getOrCreateSessionPool uses them instead of the old sprintf.
  - internal/session/client_test.go: new TestPoolKey_DisplayName covers
    12 cases (table/AV/MV + read/write + AV-collision + malformed-input
    fallback pins).

BREAKING 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 sole-writer +
bounded-cardinality preserved), session-component-review (Part B/C
boundaries unchanged), igor-reviewer.

…oogleapis#20248)

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