WIP: Populate direct path eligibility and client startup time. - #4

… of *bigtable.Client

Extracts the three debug hooks Handler needed off *bigtable.Client
into a small local interface. *bigtable.Client already implements
SessionDebug / ChannelDebug / ConfigDebug with the exact signatures,
so it satisfies the interface transparently — existing callsites
(cmd/sessionz-demo/main.go, docs) compile unchanged.

Why: the planned public *bigtable.SessionClient (CLAUDE.md immediate
queue #4) will expose the same three methods. Taking an interface now
lets a caller pass either client to Handler without a signature change
later. Same amount of code, one entry point instead of two.

The isNilDebugProviders helper handles both untyped nil and the
typed-nil-interface trap ((*bigtable.Client)(nil) wrapped in a non-nil
interface) so callers that pass a possibly-nil client don't hit an NPE
inside the debug-provider methods.

Verified: go build ./..., go vet ./..., go test ./debugview/ -count=1.

…hook ordering)

Adds two tests to session_test.go closing coverage gaps flagged by the
spec test-coverage audit:

TestInvoke_ConcurrentSecondFailsWithMultiplexViolation (SESSION_SPEC #2)
- Puts session in StateReady via makeActive, pins activeRPC.Store with a
  placeholder vrpcImpl, then calls Invoke. Asserts (a) error wraps
  ErrSessionNotActive, (b) ClassifyErr(err).State == StateUncommitted
  (retryable — attempt never hit wire), (c) error message contains
  'multiPlexingLimit=1' for operator grep.
- Exercises the previously-unreached CAS-fail branch at
  session_vrpc.go:107-109. Prior tests only asserted the multiPlexingLimit
  constant value, never the CAS-fail runtime path.

TestHooks_FiredInSpecOrder (SESSION_SPEC #4)
- Records OnStart / OnActive / OnClosing / OnClose invocations into a
  mutex-guarded []string, drives Start -> OpenSession response ->
  ForceClose, asserts exact order ['OnStart', 'OnActive', 'OnClosing',
  'OnClose'] via reflect.DeepEqual.
- Prior hookCounts fixture recorded only 3 of 4 hooks (no OnClosing) and
  only checked counts, so a regression that swapped OnClosing and OnClose
  would slip through. ForceClose exercises both the explicit
  notifyClosing call at session_lifecycle.go:77 and the safety-net call
  inside notifyClosed at :106.

Both tests pass; full internal/transport suite passes.

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.

…ant rows, dedicated reviewer slide

Slide 2 — invariant table:
- Trimmed SESSION_SPEC #6 example to just the Ready→Closing state transition (drop the in-flight-vRPC / 30s grace details that belonged in the full spec, not the deck cell).
- Rewrote SESSION_POOL_SPEC #2 to name both production picker strategies (LeastInFlight for fewest active vRPCs, LeastLatency using e2eEwma with Java parity) instead of only calling out the e2eEwma vs transportEwma distinction.
- Strengthened SESSION_COMPONENT_SPEC B2 example to explain layer direction + rationale + why it is compiler-enforced (Go import-cycle detector) with the grep as belt-and-braces, not the primary check.
- Removed the 'concrete failure mode' callout that duplicated the same GOAWAY example.

Slide 3 — Part C excerpt: dropped the In-flight vRPC slot row (already implied by the surrounding spec discussion, and the row was redundant given the SESSION_SPEC #2 coverage on slide 2).

Slide 4 — big-feature example replaced:
- Was: unified debugview/ package fold.
- Now: tcpz page (per-connection TCP_INFO for every conn-pool connection). Two-sentence prompt + why the specs made it survivable (B3 forced a fourth Handler arg for TCPStats, POOL #4 kept getsockopt syscalls outside pool locks, B10 kept the sub-1500 PMTU flag on the DTO producer). Six-commit iteration story preserved.

Slide 5 — new dedicated slide 'How the reviewer verifies specs and runs tests':
- Extracted the reviewer flow that used to sit at the bottom of slide 4.
- Adds report-format example table (PASS / VIOLATION / AMBIGUOUS with example rows).
- Adds the smoke-gate command: go test ./internal/transport/ -count=1 -short -timeout=90s.
- Mentions the two new spec-enforcing tests (TestInvoke_ConcurrentSecondFailsWithMultiplexViolation, TestHooks_FiredInSpecOrder) as concrete coverage examples.

Slide counter updated 4 → 5; nav shortcuts (1–5) still work.

…ines ordering

Two Java-parity + SESSION_SPEC fixes surfaced by the 2026-07-17 review
pass on feat/bigtable-sessionz-debug.

L5 — PeakEwma seed constants were defined but never applied. afeHandle
constructed its transportEwma and e2eEwma via NewPeakEwma(afePeakEwmaTau),
so new AFEs returned Value() = 0. LeastLatencyAfePicker's min-cost scan
pinned traffic to the newest AFE until it accumulated ≥1 OK sample,
violating SESSION_POOL_SPEC #2's Java-parity clause "New AFEs don't win
by looking free-cost."

- peak_emwa.go: add NewPeakEwmaSeeded(tau, seed). Sets value = float64(seed)
  and leaves lastUpdate zero so Update's IsZero branch replaces the seed
  with the first real sample — matches Java's SessionList seed-until-
  first-sample semantics.
- session_list.go: wire afeTransportEwmaSeed (500µs) / afeE2eEwmaSeed (1ms)
  at afeHandle construction.
- session_list_test.go, session_pool_afe_test.go: update
  RecordVRpcOutcome_SkipsNonOK and Pool_LeastLatency_IgnoresFailingAFELatency
  to assert Value() == afeE2eEwmaSeed (seed unchanged) instead of == 0
  after non-OK records. The OK-gate invariant they check is the same;
  only the untouched-value baseline moved from 0 to the seed.

L2 — OnStart hook fired AFTER go readLoop / go heartBeatLoop spawned in
Start(). On a fast handshake, readLoop could Recv OpenSessionResponse and
fire onActive before onStart returned — SESSION_SPEC #4 hook-order
violation (onStart → onActive → onClosing → onClose). SessionPoolImpl.OnStart
is a no-op today so no observable impact, but the fix defends future
non-no-op OnStart consumers by construction.

- session_lifecycle.go: move s.hooks.onStart(ctx) to before the readLoop
  and heartBeatLoop spawn in Start(). OnStart takes ctx and has no
  dependency on readLoop, so the reorder is safe.

Passes go test ./internal/session/ ./internal/transport/ ./debugview/
-count=1 -race -short. Behavioral + boundary spec reviews both PASS.

…construction

Java-parity port of SessionPoolImpl.java:424-448. Deletes the
Session→SessionHandle back-ref (Session.poolHandle) whose nil-ness
was abused as a "already handled" signal, and wires per-session
SessionHooks closures at createSession time that capture *SessionHandle
directly. The pool never walks back through Session to find the handle.

Shape changes:
- SessionHandle gains three atomic.Bool one-shot dedup flags:
  activated (defensive onActive re-entry gate) and
  closingRecorded / closeRecorded (Pool.Close Phase1↔Phase2 short-
  circuit; NOT a substitute for SESSION_SPEC #4 exactly-once).
  Loses the onSlotDrained field (moves onto Session).
- Session loses poolHandle atomic.Pointer[SessionHandle]. Gains
  slotDrainedFn func() with a lowercase setSlotDrainedCallback setter.
  notifySlotDrained now calls the field directly (no atomic Load, no
  pointer chase). Single-writer-at-construction: setter called from
  createSession before Session.Start spawns any goroutine.
- SessionPoolImpl.startingSessions retyped from map[*Session]bool to
  map[*SessionHandle]struct{}.
- SessionPoolImpl.createSession mints the handle before NewSession,
  builds hooks closures capturing sh, installs the slot-drained
  closure via setSlotDrainedCallback, backfills sh.session /
  sh.createdAt in two statements immediately after.
- SessionPoolImpl.{OnActive,OnClosing,OnClose} renamed to lowercase
  onActive/onClosing/onClose taking *SessionHandle. onActive CAS-
  gates on sh.activated. onClosing / onClose CAS-gate on the paired
  dedup flag; short-circuit when Pool.Close Phase-1 already ran the
  bookkeeping.
- Pool.Close Phase-1 flips both dedup flags on each snapshot handle
  in order: recordLifetime → recordSessionClose → flip flags →
  sl.OnSessionClosed. Replaces the prior sh.session.poolHandle.Store(nil)
  trick, expressing the dedup as an actual dedup flag instead of
  abusing a back-ref's nullability.

Test migration:
- injectActiveSession / injectActiveOnAfe rewired to the new closure-
  hooks shape. The six test sites that used to do s.poolHandle.Store(sh)
  by hand become no-ops (or move to s.setSlotDrainedCallback(...) for
  the drain-signal tests in session_vrpc_test.go).
- session_pool_lifecycle_test callers use lowercase p.onActive(sh) /
  p.onClosing(sh) / p.onClose(sh, err). The poolHandle assertions
  in TestOnClosing_DropsFromReadyCountAndRecordsLifetime and
  TestOnClosing_StartingSessionIsNoOp were replaced (the former
  drops the check outright; the latter now asserts sh.closingRecorded
  stays unset for a starting-only handle).

Spec updates (paired with code, both reviewers green):
- SESSION_SPEC.md #2 / #10: slot-drained wire re-described; poolHandle
  dropped from the atomic-state list; slotDrainedFn documented as
  single-writer-at-construction.
- SESSION_POOL_SPEC.md #6 transition table (NotRegistered→Idle and
  InFlight→Idle rows) point at Session.slotDrainedFn and lowercase
  SessionPoolImpl.onActive.
- SESSION_COMPONENT_SPEC.md B7 whitelist swaps poolHandle for
  slotDrainedFn; Part C ownership matrix updates the slot-drained
  row (install site now createSession, callback location now Session);
  new Part C row for the three atomic.Bool flags with explicit scope
  disclaimer.

Behavior shift worth flagging (not a spec violation):
- The "pool closed before session became active" race in onActive
  now runs onClosing's recordLifetime path once (createdAt is set
  at createSession, so time.Since is meaningful). Old back-ref
  path skipped it because poolHandle was never stored. Adds one
  lifetimes-ring entry per race case; no spec pins lifetimes-ring
  semantics for this race so treated as improvement, not regression.

Pre-existing race unchanged by this diff:
- TestHeartBeatLoop_ForceClosesOnMissedHeartbeat leaks a heartBeatLoop
  goroutine that races with TestSessionTracer_MetricsRoundTrip's
  global metric init. Reproduces on the base branch under -race
  -count>1; not caused by this refactor.

Five nits from the igor-reviewer-agent review of PR googleapis#20215:

- handleGoAway's per-GOAWAY drain goroutine now derives its 30s timeout
  from s.stream.Context() instead of context.Background(). Pool teardown
  cancels the stream ctx, which now unblocks the drain wait cleanly
  instead of hanging up to 30s on a detached background ctx.
- handleOpenSession: doc comment now spells out that the response body
  is intentionally not consumed (PeerInfo comes from the stream Header,
  not the reply payload). Documents the '_ *spb.OpenSessionResponse'
  signature so a future reader doesn't wonder if fields were missed.
- peerInfoExtracter → extractPeerInfo. Mutator, not a noun; and
  "extracter" is a non-standard spelling. Rename in impl + all test
  callers.
- notifyClosing docstring adds the OnActive-skip caveat: if a session
  tears down while still Starting (Close/ForceClose before OpenSession
  completes), OnActive never runs but OnClosing/OnClose still do.
  SESSION_SPEC #4 orders hooks that fire, not hooks that don't.
- readLoop gains a deferred panic-recover that records a debug tag and
  ForceCloses with REASON_ERROR + the panic value in the description.
  Avoids losing the readLoop goroutine silently if
  handleSessionResponse panics on a malformed frame. New tag constant
  tagSessionReadLoopPanic in debug_tracer.go.

Five nits from the igor-reviewer-agent review of PR googleapis#20215:

- handleGoAway's per-GOAWAY drain goroutine now derives its 30s timeout
  from s.stream.Context() instead of context.Background(). Pool teardown
  cancels the stream ctx, which now unblocks the drain wait cleanly
  instead of hanging up to 30s on a detached background ctx.
- handleOpenSession: doc comment now spells out that the response body
  is intentionally not consumed (PeerInfo comes from the stream Header,
  not the reply payload). Documents the '_ *spb.OpenSessionResponse'
  signature so a future reader doesn't wonder if fields were missed.
- peerInfoExtracter → extractPeerInfo. Mutator, not a noun; and
  "extracter" is a non-standard spelling. Rename in impl + all test
  callers.
- notifyClosing docstring adds the OnActive-skip caveat: if a session
  tears down while still Starting (Close/ForceClose before OpenSession
  completes), OnActive never runs but OnClosing/OnClose still do.
  SESSION_SPEC #4 orders hooks that fire, not hooks that don't.
- readLoop gains a deferred panic-recover that records a debug tag and
  ForceCloses with REASON_ERROR + the panic value in the description.
  Avoids losing the readLoop goroutine silently if
  handleSessionResponse panics on a malformed frame. New tag constant
  tagSessionReadLoopPanic in debug_tracer.go.

Five nits from the igor-reviewer-agent review of PR googleapis#20215:

- handleGoAway's per-GOAWAY drain goroutine now derives its 30s timeout
  from s.stream.Context() instead of context.Background(). Pool teardown
  cancels the stream ctx, which now unblocks the drain wait cleanly
  instead of hanging up to 30s on a detached background ctx.
- handleOpenSession: doc comment now spells out that the response body
  is intentionally not consumed (PeerInfo comes from the stream Header,
  not the reply payload). Documents the '_ *spb.OpenSessionResponse'
  signature so a future reader doesn't wonder if fields were missed.
- peerInfoExtracter → extractPeerInfo. Mutator, not a noun; and
  "extracter" is a non-standard spelling. Rename in impl + all test
  callers.
- notifyClosing docstring adds the OnActive-skip caveat: if a session
  tears down while still Starting (Close/ForceClose before OpenSession
  completes), OnActive never runs but OnClosing/OnClose still do.
  SESSION_SPEC #4 orders hooks that fire, not hooks that don't.
- readLoop gains a deferred panic-recover that records a debug tag and
  ForceCloses with REASON_ERROR + the panic value in the description.
  Avoids losing the readLoop goroutine silently if
  handleSessionResponse panics on a malformed frame. New tag constant
  tagSessionReadLoopPanic in debug_tracer.go.

Add two observation-only debug tags fired from stampAttempt when the
session-path attempt's InvokeResult lacks a real ClusterInformation:

  - TagSessionAttemptNilClusterInfo — ClusterInfo is nil (transport
    failure with no server response, OR server response omitted
    ClusterInformation entirely).
  - TagSessionAttemptEmptyClusterID — ClusterInformation is present
    but ClusterId is empty (server contract violation per
    CLIENT_SIDE_METRICS_SPEC #1).

Both are the input conditions for the reported symptom: session-path
attempts get labeled cluster_id=<unspecified> in downstream metrics
because stampAttempt has nothing to stamp AND session path passes
nil header/trailer MDs to RecordAttemptCompletion, so
ExtractLocation returns the sentinel defaultCluster. Dashboards that
filter attempt_latencies2 on real cluster then hide the affected
attempts while connectivity_error_count (labeled <unspecified>)
still surfaces them, producing the "no errors in latencies but
UNAVAILABLE on connectivity" mismatch.

Land as observation-only first — confirm frequency correlates with
the reported incidents before landing any labeling-fix (candidates:
propagate currOp.lastClusterID from stampAttempt, or broaden the
connectivity-error gate to trust transportType populated).

Also introduces a typed DebugTag string alias + exports RecordDebugTag
so packages under bigtable/internal can fire from their own layer
without drifting off the tag catalog on a raw string. Session→transport
is the allowed direction per Part B; debug-tag registry ownership
stays with the transport package.

Reviewers: session-reviewer PASS on METRICS #1/#2 + POOL #4;
session-component-review PASS on B1/B2/B6/B9 + Part C ClusterInfo
ownership row; igor asked for typed DebugTag which is applied.

The server rejects OpenMaterializedView with
`INVALID_ARGUMENT: Permission is not supported` when the field is left
UNSET. MV is read-only, so PERMISSION_READ is the only valid value on
the request. Without this, every session-path MV open fails; a burst
of opens trips the pool's consecutive-failure breaker within ~1 s.

Also fold the SESSION_CLIENT_SPEC.md #4 text nudge that spec-reviewers
flagged: the MV row + follow-up paragraph both said MV had "no
Permission" while the proto and now the code do carry it as read-only.

Five nits from the igor-reviewer-agent review of PR googleapis#20215:

- handleGoAway's per-GOAWAY drain goroutine now derives its 30s timeout
  from s.stream.Context() instead of context.Background(). Pool teardown
  cancels the stream ctx, which now unblocks the drain wait cleanly
  instead of hanging up to 30s on a detached background ctx.
- handleOpenSession: doc comment now spells out that the response body
  is intentionally not consumed (PeerInfo comes from the stream Header,
  not the reply payload). Documents the '_ *spb.OpenSessionResponse'
  signature so a future reader doesn't wonder if fields were missed.
- peerInfoExtracter → extractPeerInfo. Mutator, not a noun; and
  "extracter" is a non-standard spelling. Rename in impl + all test
  callers.
- notifyClosing docstring adds the OnActive-skip caveat: if a session
  tears down while still Starting (Close/ForceClose before OpenSession
  completes), OnActive never runs but OnClosing/OnClose still do.
  SESSION_SPEC #4 orders hooks that fire, not hooks that don't.
- readLoop gains a deferred panic-recover that records a debug tag and
  ForceCloses with REASON_ERROR + the panic value in the description.
  Avoids losing the readLoop goroutine silently if
  handleSessionResponse panics on a malformed frame. New tag constant
  tagSessionReadLoopPanic in debug_tracer.go.

Callers wiring bigtable/debugview shouldn't have to construct + attach
TCPStats separately just to see /debug/tcpz/. Fold the setup into
NewClientWithConfig: when config.EnableClientDebug is true, auto-
construct a *TCPStats, append tcpStats.ClientOption() to the dial-
option chain (applied to both the classic and session channel pools,
both of which dial from the same `o` slice), and store on
c.tcpStats. Expose via new Client.TCPStats() accessor.

Caller before:
  configs := bigtable.ClientConfig{
      EnableSessionPool: true,
      EnableClientDebug: true,
  }
  tcpStats := bigtable.NewTCPStats()
  opts := []option.ClientOption{
      option.WithEndpoint(...),
      tcpStats.ClientOption(),  // manual attach
  }
  client, _ := bigtable.NewClientWithConfig(ctx, p, i, configs, opts...)
  mux.Handle("/debug/", http.StripPrefix("/debug", debugview.Handler(client, tcpStats)))

Caller after:
  configs := bigtable.ClientConfig{
      EnableSessionPool: true,
      EnableClientDebug: true,   // one flag drives both session
                                 // snapshots + tcpz collector
  }
  client, _ := bigtable.NewClientWithConfig(ctx, p, i, configs,
      option.WithEndpoint(...))
  mux.Handle("/debug/", http.StripPrefix("/debug", debugview.Handler(client, client.TCPStats())))

Non-goals:
  - Callers who need TCPStats for reasons other than debugview can still
    bring their own via bigtable.NewTCPStats() + tcpStats.ClientOption(),
    but they should NOT combine that with EnableClientDebug=true —
    grpc.WithContextDialer is last-write-wins, so the caller-supplied
    collector would be silently overridden and register nothing. Doc
    comment on Client.TCPStats() spells this out.
  - No new Close/teardown call needed on TCPStats — ConnRegistry is a
    passive snapshot-time getsockopt reader, owns no goroutines or fds
    beyond the map that GCs with the Client.

Reviewers cleared: session-reviewer (POOL #4 hot-path non-blocking
preserved — TCPStats.ClientOption is a WithContextDialer that fires
once per dial, zero per-RPC cost), session-component-review (layer-6
→ layer-6 wiring; no boundary crossed), igor-reviewer (naming +
lifecycle fine after wording fix).